Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Suggestion: Tupfile.base #132

Closed
ppannuto opened this issue Jul 31, 2013 · 27 comments
Closed

Feature Suggestion: Tupfile.base #132

ppannuto opened this issue Jul 31, 2013 · 27 comments

Comments

@ppannuto
Copy link
Contributor

I've found it a minor weakness / annoyance of tup that the first time I check out a copy of a repo, I have to remember to run tup init before I can tup upd. There's a few challenges with running init automatically, the most obvious of which is knowing where the root of the project directory is (assuming it's the current directory feels risky).

My proposal is the addition of a new file: Tupfile.base (name flexible, maybe Tupefile.root, Tupfile.ini?)

This file would be placed in the root directory of the project to identify it. In addition to identifying the root, I would propose a simple syntax (currently thinking .ini-style syntax) for expressing things such as default variants for the project.

In my current mental model, this file is parsed if and only if tup cannot find a .tup directory. More specifically, as tup upd begins enumerating directories searching for the .tup directory, it also searches for Tupfile.base (after looking for .tup). If a Tupfile.base is found, the requisite commands are run to initialize things and then tup continues on with upd.

I'm willing to tackle implementing this feature, but I'd like some input on the design first. Is this a good / desirable idea? Do people have comments / suggestions for improving it?

@bradjc
Copy link
Contributor

bradjc commented Jul 31, 2013

I try to encourage people to use tup and adding a new step other than just typing make always seems like going backwards to them. Therefore I support something that will just run tup init automatically the first time.

My suggestion is to not only add support for a new file (I like Tupfile.root) but also for a new command, say root that you could add to a Tupfile in the root directory (if it exists) that would tell tup to run tup init in that folder. Maybe the parsing required to do this would be unreasonable, but if not it would cut down on an extra file in probably a lot of cases. The rest of the line after root could specify options somehow, maybe:

root variants=debug,release logging=true <whatever other options>

Just a thought.

@gittup
Copy link
Owner

gittup commented Aug 1, 2013

On Wed, Jul 31, 2013 at 1:37 PM, Pat Pannuto notifications@github.comwrote:

I've found it a minor weakness / annoyance of tup that the first time I
check out a copy of a repo, I have to remember to run tup init before I
can tup upd. There's a few challenges with running init automatically,
the most obvious of which is knowing where the root of the project
directory is (assuming it's the current directory feels risky).

My proposal is the addition of a new file: Tupfile.base (name flexible,
maybe Tupefile.root, Tupfile.ini?)

This file would be placed in the root directory of the project to identify
it. In addition to identifying the root, I would propose a simple syntax
(currently thinking .ini-style syntax) for expressing things such as
default variants for the project.

In my current mental model, this file is parsed if and only if tup cannot
find a .tup directory. More specifically, as tup upd begins enumerating
directories searching for the .tup directory, it also searches for
Tupfile.base (after looking for .tup). If a Tupfile.base is found, the
requisite commands are run to initialize things and then tup continues on
with upd.

I'm willing to tackle implementing this feature, but I'd like some input
on the design first. Is this a good / desirable idea? Do people have
comments / suggestions for improving it?

This sounds good to me. I don't have any preference on the name - if it's
ini-style syntax, I guess .ini would be preferred for syntax-highlighting
reasons (tup already has the inih library builtin).

One question I have is what would the behavior be if multiple Tupfile.ini
files are found up the tree? I think in this case it might make sense to
pick the highest one (so, the last one found during the "up-the-tree"
traversal), so that if a sub-project has a Tupfile.ini, and a master
project also has a Tupfile.ini, the master project would be initialized.

ie:

super/Tupfile.ini
super/sub/Tupfile.ini

If there is no .tup directory, I think we want to make it at super/.tup

-Mike

@gittup
Copy link
Owner

gittup commented Aug 1, 2013

On Wed, Jul 31, 2013 at 7:22 PM, Brad Campbell notifications@github.comwrote:

I try to encourage people to use tup and adding a new step other than just
typing make always seems like going backwards to them. Therefore I
support something that will just run tup init automatically the first
time.

My suggestion is to not only add support for a new file (I like
Tupfile.root) but also for a new command, say root that you could add to
a Tupfile in the root directory (if it exists) that would tell tup to run
tup init in that folder. Maybe the parsing required to do this would be
unreasonable, but if not it would cut down on an extra file in probably a
lot of cases. The rest of the line after root could specify options
somehow, maybe:

root variants=debug,release logging=true

Just a thought.

I think this would be difficult to add to the current Tupfile parser,
unless it is parsed with a separate parser that ignores all of the rule &
variable definitions normally in a Tupfile (otherwise, tup will try to
process dependent Tupfiles & such). I'd favor a separate file for this
purpose.

-Mike

@kurige
Copy link

kurige commented Aug 1, 2013

I don't understand why this is so problematic that it requires adding a new file? You've already got '.tup', 'Tupfile', and 'Tuprules.tup' at the root directory. Adding a fourth 'tup' related item to a project's root is a little excessive, don't you think?

@ppannuto
Copy link
Contributor Author

ppannuto commented Aug 1, 2013

No, in this case you do not have .tup yet, that is the whole point of
this feature / file, to obviate tup init as a separate command. To do
that, you need some way of identifying the root directory of the project;
this is the purpose of the new file.
On Jul 31, 2013 6:38 PM, "Christopher Gateley" notifications@github.com
wrote:

I don't understand why this is so problematic that it requires adding a
new file? You've already got '.tup', 'Tupfile', and 'Tuprules.tup' at the
root directory. Adding a fourth 'tup' related file to a project's root is a
little excessive, don't you think?


Reply to this email directly or view it on GitHubhttps://github.com//issues/132#issuecomment-21908785
.

@ppannuto
Copy link
Contributor Author

ppannuto commented Aug 1, 2013

My current plan for implementation is to build on the existing option mechanism. Specifically, to create a dynamic array of locations adding a reference for each Tupfile.ini encountered from the directory where tup was invoked down to {the filesystem root, the .tup directory}, whichever is found first. Configuration lookup precedence would then first search this array and then fall back to the existing files.

As these are run-time configurations and orthogonal to the build, I believe they should not be tracked in tup's database (this also alleviates the chicken-egg problem for the first run). As these options potentially change the behavior of tup when executing, I believe the appropriate place to hook this scan is tup/main.c::127 (just before the comment /* Commands that don't use a normal tup_init() */.

As for nested .ini files, my solution would be to have a configuration "root" variable that defaults to false. When parsing the .ini's, if tup encounters root=true it will check the directory containing that .ini file for a .tup directory. If one is not found, it will execute tup init in that directory and stop traversing the directory tree.

This model alleviates some potential confusion for nested .ini's by making the marking of the root directory explicit and opens them to more general directory-sensitive configuration (which is really the only reason I could imagine nesting .ini files).

Thoughts?

@ppannuto
Copy link
Contributor Author

ppannuto commented Aug 1, 2013

It was brought to my attention that just the root=true idea would probably not be the best way of solving this. In particular, it would have weird / undesired effects on things like submodules in git, where you have nested projects, both of whom would define root=true.

The plans w.r.t. leveraging the options mechanism stand, but the new plan for directory search would be to go until a .tup directory is encountered or the root directory. If the search makes it all the way to the root, it will backtrack to the last Tupfile.ini it found. The expectation would be that this Tupfile.ini has root=true set and to use this as the project root. I think this is a good / flexible compromise.

As a final detail, if the shallowest .ini file didn't have root=true set, I think we'd have to emit a warning and ask the user what he'd like to do:

  1. Run tup init in that directory anyway
  2. Ignore that Tupfile.ini and go further back up the tree to the next one.

@gittup
Copy link
Owner

gittup commented Aug 4, 2013

On Thu, Aug 1, 2013 at 6:30 PM, Pat Pannuto notifications@github.comwrote:

It was brought to my attention that just the root=true idea would
probably not be the best way of solving this. In particular, it would have
weird / undesired effects on things like submodules in git, where you have
nested projects, both of whom would define root=true.

The plans w.r.t. leveraging the options mechanism stand, but the new plan
for directory search would be to go until a .tup directory is encountered
or the root directory. If the search makes it all the way to the root, it
will backtrack to the last Tupfile.ini it found. The expectation would be
that this Tupfile.ini has root=true set and to use this as the project
root. I think this is a good / flexible compromise.

I don't see exactly why you'd need to use the options mechanism here. I was
thinking it would just do a stat("Tupfile.ini") on the way up the tree in
find_tup_dir(), and keep the topmost one (see attached patch).

As a final detail, if the shallowest .ini file didn't have root=true set,
I think we'd have to emit a warning and ask the user what he'd like to do:

  1. Run tup init in that directory anyway
  2. Ignore that Tupfile.ini and go further back up the tree to the next
    one.

Can you explain what the root=true is supposed to do? Why would I want a
Tupfile.ini that doesn't have root=true? My understanding is that a
Tupfile.ini file means that "this directory can house a .tup directory", so
they should all be "roots". To work properly with submodules, you would
want to init at the top-most root, which should be solved by just grabbing
the top-most Tupfile.ini as mentioned above.

Thanks!
-Mike

@bradjc
Copy link
Contributor

bradjc commented Aug 4, 2013

I've talked to Pat about this and it seems that Tupfile.ini folds in nicely with the existing option infrastructure and option file hierarchy. Tupfile.ini would be added to the search order for tup options and would add repository specific options to the existing user specific options. This could potentially be useful for some projects to set, for instance, updater.full_deps to 1 if that particular project is expecting a lot of system changes.

Once there is a repo specific way to add options, setting the root location with root=true is a very explicit way to do so. Perhaps just using the Tupfile.ini closest to / makes more sense, however. It's possible that there might be folder specific Tupfile.ini files in subfolders in the future, but choosing the top Tupfile.ini would still work for finding the root. Maybe it's best to just not add support for root=true.

Also, making Tupfile.ini part of the options system allows for other repository specific options to be included with the repository. A very useful one would be default variants so that users don't have to run tup variant x.config and variant folders wouldn't have to be committed to the repository.

@ppannuto
Copy link
Contributor Author

ppannuto commented Aug 5, 2013

Brad covered the option point pretty well, as for the 'root' thing, you are
correct, it's not necessary if we choose the "lowest" initial file.

I have this mostly implemented, but will be traveling much of this week.
I'll push a branch out for review once I get a chance to finish things.
On Aug 4, 2013 3:58 PM, "Brad Campbell" notifications@github.com wrote:

I've talked to Pat about this and it seems that Tupfile.ini folds in
nicely with the existing option infrastructure and option file hierarchy.
Tupfile.ini would be added to the search order for tup options and would
add repository specific options to the existing user specific options. This
could potentially be useful for some projects to set, for instance,
updater.full_deps to 1 if that particular project is expecting a lot of
system changes.

Once there is a repo specific way to add options, setting the root
location with root=true is a very explicit way to do so. Perhaps just
using the Tupfile.ini closest to / makes more sense, however. It's
possible that there might be folder specific Tupfile.ini files in
subfolders in the future, but choosing the top Tupfile.ini would still work
for finding the root. Maybe it's best to just not add support for
root=true.

Also, making Tupfile.ini part of the options system allows for other
repository specific options to be included with the repository. A very
useful one would be default variants so that users don't have to run tup
variant x.config and variant folders wouldn't have to be committed to the
repository.


Reply to this email directly or view it on GitHubhttps://github.com//issues/132#issuecomment-22077746
.

@gittup
Copy link
Owner

gittup commented Dec 17, 2015

I think we can close this, given the #133 has been merged.

@gittup gittup closed this as completed Dec 17, 2015
@Qix-
Copy link

Qix- commented Dec 18, 2015

Why did we need an .ini file for this? I was always confused; whats wrong with just traversing upwards until we find a Tupfile, and if there isn't a .tup/ in the same directory tup init gets called? This seems like the correct behavior. Git does this, and nobody has ever had a problem with it.

@gittup
Copy link
Owner

gittup commented Dec 18, 2015

That's a good idea, though you may not have a Tupfile at the root of your project if you don't build anything there.

@ppannuto
Copy link
Contributor Author

@qix, Tupfile.ini was always part of a greater vision. Currently there is nowhere to store project-specific configuration for tup. Tupfile.ini was intended to be the home for that. See #144 for an old implementation of this as well as the beginning of this thread for some of the motivation.

As for recursing for Tupfiles, that wouldn't work in all scenarios. Easy example, we have some projects that use git submodules where recursing all the way to the "most root" Tupfile would not work correctly. At the same time, something like a tup_root directive in a Tupfile could solve this.

@Qix-
Copy link

Qix- commented Dec 18, 2015

@ppannuto recurse until a .tup/ or a Tupfile or a Tupfile.ini is found. Pretty easy solution there. Doing nothing is annoying.

Specifically saying where a parent Tupfile lives breaks the parent-child relationship, and makes no sense. Submodules that build with tup shouldn't have to know anything about their parent.

As well, never said Tupfile.ini didn't serve a purpose, but having to make another file just so Tup can discover the original file is a little silly when you don't need .ini directives.

If you don't have a Tupfile at the root of your project, that's when you should run tup init. There's no denying that, and that's fine. CMake works in a very similar fashion, so that is of course a required step.

In all honesty this is what I expected Tup to do in the first place.

@ppannuto
Copy link
Contributor Author

I think we have a miscommunication somewhere. Tup does recurse right now, until it finds a Tupfile.ini indicating the root of the project it should build. The point is that you cannot correctly stop at the first Tupfile (plenty of Tupfiles in subdirectories of projects) or the "last" Tupfile (submodule example) you find.

W.r.t. parent-child, I think we agree. The current .ini system stops a tup call from a child submodule from recursing into the parent.

As for Tupfile.ini being another file, think of it more as a partially implemented feature perhaps? There was an intention to use the .ini directives.

The explicit goal was to eliminate the need to ever run tup init. There's a simple tradeoff in cognitive load there. In one case, a project maintainer creates a Tupfile.ini once, in the other case everyone who gets a copy of the project must remember to run tup init before doing anything. Sure, there's some precedent for a "init" command of some form (./configure; cmake -> make), but I'm not convinced that makes them good ideas / the right design choice (I would certainly prefer to just type cmake every time and have it call make. Right now you have the weird situation where every time after the first time, make will pick up changes and re-run cmake for you, but the first time you have to remember to run cmake first. That feels broken IMHO. There's nothing fundamentally special about the first run.).

@Qix-
Copy link

Qix- commented Dec 18, 2015

First thing, pointing out that cmake doesn't just compile to make ;) Just being fair here. There's a reason why CMake exists aside from Make.

In cases that don't need a Tupfile.ini, though, why should I still have to type tup init when the Tupfile is literally right there? Extra output stating that Tup is initializing a new cache would be really helpful in that it would tell me where it found the Tup configuration. In the event it's incorrect, that should indicate I need to run tup init.

It doesn't take long at all to do repeated dirname() calls on the getcwd() to traverse upwards. Enumerate what's there, and favor tupfile.ini if you must.

@bradjc
Copy link
Contributor

bradjc commented Dec 18, 2015

If you have this file structure:

/project
    Tupfile
    /library
        Tupfile

Tup allows you to run tup in the /project/library folder and it will build the whole project. If you didn't have a Tupfile.ini, and you ran tup on a new checkout in /project/library, it would put .tup in the wrong spot.

Now, someone could realize this was an issue and fix it if tup prints what it's doing and the person catches the mistake. However, if one wants to ensure one's tool doesn't get adoption and use by the community one makes it hard to use from the first command. If one wants people to actually give it a chance, one makes it as foolproof as possible. I think the current solution is the best approach for making tup easy to use.

@Qix-
Copy link

Qix- commented Dec 18, 2015

Eh? Running tup in /project/library would just traverse upwards and see the only viable configuration file is in /project and init there. What's the problem with that? Just be verbose about it.

$ cd /project/library
$ tup
[ tup ] configuration found in /project/
[ tup ] .tup repository initialized
. . .

@ppannuto
Copy link
Contributor Author

What is "the only viable configuration file" if not Tupfile.ini?

On Thu, Dec 17, 2015 at 10:02 PM Josh Junon notifications@github.com
wrote:

Eh? Running tup in /project/library would just traverse upwards and see
the only viable configuration file is in /project and init there. What's
the problem with that?


Reply to this email directly or view it on GitHub
#132 (comment).

@ppannuto
Copy link
Contributor Author

Recall, this is dealing with the case where the .tup directory does not yet
exist, so you cannot rely on that.

On Thu, Dec 17, 2015 at 10:03 PM Pat Pannuto pat.pannuto@gmail.com wrote:

What is "the only viable configuration file" if not Tupfile.ini?

On Thu, Dec 17, 2015 at 10:02 PM Josh Junon notifications@github.com
wrote:

Eh? Running tup in /project/library would just traverse upwards and see
the only viable configuration file is in /project and init there. What's
the problem with that?


Reply to this email directly or view it on GitHub
#132 (comment).

@Qix-
Copy link

Qix- commented Dec 18, 2015

@ppannuto A Tupfile? I realize Tupfiles don't have to exist at the project root. I get that. Believe me, I do. If a Tupfile.ini is found, use it instead. Be smart about it; a Tupfile.ini is used to signify where the project root should be. Modules that don't need a Tupfile.ini and have a Tupfile in the root shouldn't have to have a Tupfile.ini in order for easy use of Tup.

I don't know how I can make that more clear.

@ppannuto
Copy link
Contributor Author

Expanding on @bradjc's case:

/project
    Tupfile
    Tupfile.ini
    /submodule
        Tupfile
        Tupfile.ini
        /subfolder
             Tupfile

You type tup in the subfolder directory, what is the expected behavior? How does tup figure this out correctly without Tupfile.ini?

@Qix-
Copy link

Qix- commented Dec 18, 2015

Find the root-most .tup/, Tupfile.ini or Tupfile - in that order.

@erikbrinkman
Copy link
Contributor

Tup currently / soon will support a significant number of automatically generated files, e.g. Tup*{,.lua,.py}. Why not just treat any valid tup-specific file as a root marker, and define a precedence order?

On somewhat of a side note, but still relevant to what's decided. If Tupfile.ini is also intended as a root marker, shouldn't tup init create an empty one if it doesn't exist?

@Qix-
Copy link

Qix- commented Dec 23, 2015

@erikbrinkman No, because that introduces more fluff into the repository. Again, having two build files (Tupfile and Tupfile.ini) with one being completely meaningless if not used (Tupfile.ini) when they're both at the project root in smaller projects creates extra baggage in a repository.

I'd go so far as to vote to have Tupfile.ini go away altogether and put those directives in the Tupfile or Tupfile.lua (Tupfile.py, whatever) files themselves.

@erikbrinkman
Copy link
Contributor

@Qix- I assume you're responding to having tup init create a blank Tupfile.ini. I agree that if tup by default searches for the root most tup-specific file, then that's a bad thing. I'm specifically referring to the current behavior. Since currently tup searches for a Tupfile.ini if no .tup directory can be found, then it seems like tup init should also create one.

The reason why I included the suggestion to create a Tupfile.ini is because the optimal way to resolve all of these files hasn't been agreed upon in this thread, and the idea of creating a blank root market seems genuinely useful given the current behavior.

As for for removing Tupfile.ini all together, it seems good to separate tup options that aren't critical to the result of a tup build from the rules that specifically define the results of the build. I think Pat's original idea was that if you're defining options that are useful for a build, you might want to check them in, and it could also serve as an indicator of the tup root directory. There may be better places to put the configuration options than in Tupfile.ini, and I would probably prefer having a blank root Tupfile to a blank root Tupfile.ini, but I definitely don't think configuration options should go within a Tupfile given that by definition they do not affect the result of the build.

I can only assume that you agree with generally searching for any related tup-specific file instead of just Tupfile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants