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

Tupfile.ini support #133

Closed
wants to merge 3 commits into from
Closed

Tupfile.ini support #133

wants to merge 3 commits into from

Conversation

ppannuto
Copy link
Contributor

These commits cover the features requested in #132.

The first commit handles the discovery and parsing of Tupfile.ini files as well as the automatic running of tup init as necessary.

The second commit obsoletes the command tup upd in favor of simply tup.

The individual commit messages go into greater detail on each point.

@bradjc
Copy link
Contributor

bradjc commented Aug 19, 2013

I just gave this a whirl and it works pretty well. Not having to type four extra characters each time is actually pretty useful.

@ppannuto
Copy link
Contributor Author

This currently doesn't work if tup has the setuid bit set. It will create the .tup directory with root as the owner.

I know how to fix this, but hold of on pulling / examining until I do.

@ppannuto
Copy link
Contributor Author

The Tupfile.ini commit is fixed to temporarily drop privileges while running tup init now. I've tested it on OS X, don't have linux handy. @bradjc can you verify it works correctly? (Not that I imagine it wouldn't..)

@bradjc
Copy link
Contributor

bradjc commented Aug 21, 2013

I ran all the tests on this branch. Two things:

  1. It seems that tup_option_exit() gets called multiple times when the monitor is used (test t7005). This causes a segfault as the same pointer is freed multiple times. This diff works around the issue (or maybe fixes it, I'm not sure if tup_option_exit() is supposed to be called multiple times):
diff --git a/src/tup/option.c b/src/tup/option.c
index b1fd945..86d15b6 100644
--- a/src/tup/option.c
+++ b/src/tup/option.c
@@ -279,6 +279,8 @@ void tup_option_exit(void)
                c = c->next;
                free(temp);
        }
+
+       dynamic_configs = NULL;
 }

 static int tup_boolify_flag(const char *value) {
  1. Test t7052 fails with this pull request. The issue is actually the touch Tupfile.ini in tup.sh. It causes the monitor to run immediately (before the Tupfile and source files are created) and the test detects this as the monitor compiling it twice. I believe this is an issue with the test, but I'm not sure.

@ppannuto
Copy link
Contributor Author

I've updated the commit to reset dynamic_configs = NULL in tup_option_exit to address point 1. Whether or not its necessary to be able to call tup_option_exit multiple times, it's certainly not incorrect to reset that pointer to NULL.

As for point 2, I think that's best addressed by you Mike.

@gittup
Copy link
Owner

gittup commented Aug 22, 2013

Hi Pat,

Thanks for taking the time to work on this! See comments below...

On Wed, Aug 21, 2013 at 2:47 AM, Pat Pannuto notifications@github.comwrote:

I've updated the commit to reset dynamic_configs = NULL in
tup_option_exit to address point 1. Whether or not its necessary to be able
to call tup_option_exit multiple times, it's certainly not incorrect to
reset that pointer to NULL.

As for point 2, I think that's best addressed by you Mike.

Yeah, this test is just expecting the autoupdater to run once, when the
Tupfile.ini change is causing it to run twice. Not a big deal - we would
just change the expected value.

Some questions/feedback on Tupfile.ini:

  1. I don't like that Tupfile.ini is now "mandatory" in that if you don't
    have one, you get the warning messages. I think if there is no Tupfile.ini
    and no .tup directory, you should just get the tup usage information. Users
    should still be able to 'tup init' manually as before, without getting the
    warnings.

  2. I still think hooking it in to the options mechanism is not the right
    approach. The options are supposed to be user or machine preferences, not
    project preferences. Ie: it doesn't make sense for me to check-in a
    Tupfile.ini that specifies updater.num_jobs, or display.color, etc. Those
    are up to the user. The only mention I've seen for why this is necessary is
    for updater.full_deps, which is an option I'd prefer to actually get rid of
    (by making it always do full deps). Before that happens, I think we would
    make Tupfile.ini support only that option by creating the .tup/options file
    after calling init(). There's no need to read Tupfile.ini after .tup is
    already created.

  3. In line with part 2, I don't see a reason to parse multiple
    Tupfile.ini's if there are several up the tree (for sub-projects, and the
    like). The top-most one should override the lower level one as the place
    for where init() is called.

  4. Part of the plan for Tupfile.ini was to create default variants - that
    isn't here yet, correct? I don't see it, but just wanted to make sure I'm
    not missing something.

As for making 'tup' do a 'tup upd' - the rationale behind having to
explicitly do 'tup upd' in the first place is so that just a 'tup' doesn't
change the filesystem, like most other tools (except 'make' and its
descendants, of course). This can always be changed with a local alias, but
I'm happy to add an option or something to support that in tup directly. I
don't think it should be the default, however. This thread also has some
info:

https://groups.google.com/d/msg/tup-users/s5rR10Nb7X0/WB66mI8R7tUJ

The bug mentioned in my last post was fixed in t7045.

-Mike

@ppannuto
Copy link
Contributor Author

On Thu, Aug 22, 2013 at 8:13 AM, gittup notifications@github.com wrote:

Hi Pat,

Thanks for taking the time to work on this! See comments below...

On Wed, Aug 21, 2013 at 2:47 AM, Pat Pannuto <notifications@github.com

wrote:

I've updated the commit to reset dynamic_configs = NULL in
tup_option_exit to address point 1. Whether or not its necessary to be
able
to call tup_option_exit multiple times, it's certainly not incorrect to
reset that pointer to NULL.

As for point 2, I think that's best addressed by you Mike.

Yeah, this test is just expecting the autoupdater to run once, when the
Tupfile.ini change is causing it to run twice. Not a big deal - we would
just change the expected value.

Some questions/feedback on Tupfile.ini:

  1. I don't like that Tupfile.ini is now "mandatory" in that if you don't
    have one, you get the warning messages. I think if there is no Tupfile.ini
    and no .tup directory, you should just get the tup usage information. Users
    should still be able to 'tup init' manually as before, without getting the
    warnings.

The rationale behind this warning was to incentivize current tup users to
add a .ini file to their existing projects. The overarching goal with this
series of commits is to make tup much more user-friendly for people who have never
heard of tup (nor, at least when they're encountering it for the first
time, do they probably want to learn).

Users of current build systems don't have to know anything about automake
to know that the pattern ./configure && make will get them from tarball to
software. In the current system, tup init && tup upd won't even work as an
idiom, since repeated calls to tup init will fail. The point of the
warning then is to i.) define, and ii.) help enforce a tup "best practice".

Under the current system, an explicit tup init call does still work, it
just prints the warning as well.

I'm willing to remove it, but I'd like to push back at least once before I do.

  1. I still think hooking it in to the options mechanism is not the right
    approach. The options are supposed to be user or machine preferences, not
    project preferences. Ie: it doesn't make sense for me to check-in a
    Tupfile.ini that specifies updater.num_jobs, or display.color, etc. Those
    are up to the user. The only mention I've seen for why this is necessary is
    for updater.full_deps, which is an option I'd prefer to actually get rid of
    (by making it always do full deps). Before that happens, I think we would
    make Tupfile.ini support only that option by creating the .tup/options file
    after calling init(). There's no need to read Tupfile.ini after .tup is
    already created.

Then where do project preferences go? ./tup/options is not the right place
for project preferences, as that file is machine-specific, it's not checked into
the repository like Tupfile.ini would be and changes would therefore not be synced.

My thought was that Tupfile.ini is the place to set project defaults, while
a user can use ./tup/options to override those settings locally if necessary.
Going further up the chain, ~/.tupoptions are then user-specific,
machine-specific options, and /etc/tup/options are global machine-specific
options. Each level solves a different need.

As a strawman for other project-specific options that make sense, some of
the more ancient build tools don't uniquely name temp files, and therefore
cannot have multiple instances running at once. In that case, being able to
set updater.num_jobs as a project-specific option is logical.

  1. In line with part 2, I don't see a reason to parse multiple
    Tupfile.ini's if there are several up the tree (for sub-projects, and the
    like). The top-most one should override the lower level one as the place
    for where init() is called.

As a motivating example for this, consider the init.default_variants that
will
be added shortly. In a project, I have a tree:

|- generic/
|--- coreA/
|--- coreB/

The root Tupfile.ini, would specify building both the coreA and coreB variants
by default. Inside of the coreA directory, however, the default variants would
be overridden to only build coreA.

I'm not certain it's the most necessary / useful feature, but it's there and
doesn't really seem to add much complexity to the code, so why not give
users the option?

  1. Part of the plan for Tupfile.ini was to create default variants - that
    isn't here yet, correct? I don't see it, but just wanted to make sure I'm
    not missing something.

Yes, I thought of that a logically different feature, but I can add it to
this pull request shortly (read: as soon as I write it :).

As for making 'tup' do a 'tup upd' - the rationale behind having to
explicitly do 'tup upd' in the first place is so that just a 'tup' doesn't
change the filesystem, like most other tools (except 'make' and its
descendants, of course). This can always be changed with a local alias, but
I'm happy to add an option or something to support that in tup directly. I
don't think it should be the default, however. This thread also has some
info:

https://groups.google.com/d/msg/tup-users/s5rR10Nb7X0/WB66mI8R7tUJ

Ugh.. another example of a discussion lost to the mailing list...

I would push back against this as well. Commands like gcc don't print out
a usage block, they try to run, they simply don't have enough information
to run successfully:

$ gcc
i686-apple-darwin11-llvm-gcc-4.2: no input files

If you want the usage info from a program, you run program -?, that's the
established paradigm. Tup has a massive amount of metadata available to
it, both in the database and the various tupfiles strewn about. The point of
this metadata is to supply the needed information so tup can run.

Tup is fundamentally a build system tool, when I run tup, odds are I want
to build my project. If I want tup to do something else, I need to specify it,
but by default tup should do what it was built to do: build my project.

I see this as one more point of friction in bringing over make / other build
system converts, and with the demotion of init to a secondary command,
this leaves upd as the only remaining "primary command".

In discussions between @bradjc and I, getting rid of upd was actually the
original motivation for this whole series of improvements, the starting
questions was, "what would need to change so that we can get rid of upd"?

The bug mentioned in my last post was fixed in t7045.

-Mike


Reply to this email directly or view it on GitHubhttps://github.com//pull/133#issuecomment-23097751
.

Pat Pannuto
NSF/NDSEG/Qualcomm Fellow
Computer Engineering
University of Michigan
248.990.4548

@ppannuto
Copy link
Contributor Author

Pull request updated to include default variants support. Also fixes a small bug in the original ini commit.

@gittup
Copy link
Owner

gittup commented Aug 28, 2013

Hi Pat,

On Thu, Aug 22, 2013 at 4:05 PM, Pat Pannuto notifications@github.comwrote:

On Thu, Aug 22, 2013 at 8:13 AM, gittup notifications@github.com wrote:

Hi Pat,

Thanks for taking the time to work on this! See comments below...

On Wed, Aug 21, 2013 at 2:47 AM, Pat Pannuto <notifications@github.com

wrote:

I've updated the commit to reset dynamic_configs = NULL in
tup_option_exit to address point 1. Whether or not its necessary to be
able
to call tup_option_exit multiple times, it's certainly not incorrect
to
reset that pointer to NULL.

As for point 2, I think that's best addressed by you Mike.

Yeah, this test is just expecting the autoupdater to run once, when the
Tupfile.ini change is causing it to run twice. Not a big deal - we would
just change the expected value.

Some questions/feedback on Tupfile.ini:

  1. I don't like that Tupfile.ini is now "mandatory" in that if you don't
    have one, you get the warning messages. I think if there is no
    Tupfile.ini
    and no .tup directory, you should just get the tup usage information.
    Users
    should still be able to 'tup init' manually as before, without getting
    the
    warnings.

The rationale behind this warning was to incentivize current tup users to
add
a .ini file to their existing projects. The overarching goal with this
series of
commits is to make tup much more user-friendly for people who have never
heard of tup (nor, at least when they're encountering it for the first
time, do
they probably want to learn).

I like trying to make tup more user-friendly, but I don't think forcing
people to change their existing habits & scripts is the way to go.

Users of current build systems don't have to know anything about automake
to know that the pattern ./configure && make will get them from tarball
to
software. In the current system, tup init && tup upd won't even work as
an
idiom, since repeated calls to tup init will fail. The point of the
warning then
is to i.) define, and ii.) help enforce a tup "best practice".

I think we can fix that by making a 'tup init' inside an existing hierarchy
just print a warning and exit successfully, right?

  1. I still think hooking it in to the options mechanism is not the right
    approach. The options are supposed to be user or machine preferences,
    not
    project preferences. Ie: it doesn't make sense for me to check-in a
    Tupfile.ini that specifies updater.num_jobs, or display.color, etc.
    Those
    are up to the user. The only mention I've seen for why this is necessary
    is
    for updater.full_deps, which is an option I'd prefer to actually get rid
    of
    (by making it always do full deps). Before that happens, I think we
    would
    make Tupfile.ini support only that option by creating the .tup/options
    file
    after calling init(). There's no need to read Tupfile.ini after .tup is
    already created.

Then where do project preferences go? ./tup/options is not the right
place
for project preferences, as that file is machine-specific, it's not
checked
into
the repository like Tupfile.ini would be and changes would therefore not
be
synced.

Can you clarify which options you think are project preferences? For these
options I believe it is wrong to make a default for the project, since they
are specific to a user's preferences (eg: I want to override
monitor.foreground) and/or a machine (eg: this machine details horribly
with colors, so disable display.color):

updater.num_jobs
updater.keep_going
display.color
display.width
display.progress
display.job_numbers
display.job_time
monitor.autoupdate
monitor.autoparse
monitor.foreground
graph.dirs
graph.ghosts
graph.environment
graph.combine
db.sync

That leaves only:

updater.warnings
updater.full_deps

And arguably both of those could go away. I'm trying to move toward making
full_deps the default and just tossing the option entirely, because tup
isn't always accurate if you ignore dependencies outside the hierarchy. And
the warnings thing probably isn't all that useful - we might be better off
committing to a better strategy for what to do with .hidden files.

So exactly what options are you hoping to commit to the project as project
preferences?

As a strawman for other project-specific options that make sense, some of
the more ancient build tools don't uniquely name temp files, and therefore
cannot have multiple instances running at once. In that case, being able
to
set updater.num_jobs as a project-specific option is logical.

Tup already supports this in Linux and OSX since the temporary files are
uniquely stored in .tup/tmp before being renamed to the output file. See
t6048 for the test. I'd love to get that working correctly in Windows too.

  1. In line with part 2, I don't see a reason to parse multiple
    Tupfile.ini's if there are several up the tree (for sub-projects, and
    the
    like). The top-most one should override the lower level one as the place
    for where init() is called.

As a motivating example for this, consider the init.default_variants that
will
be added shortly. In a project, I have a tree:

|- generic/
|--- coreA/
|--- coreB/

The root Tupfile.ini, would specify building both the coreA and coreB
variants
by default. Inside of the coreA directory, however, the default variants
would
be overridden to only build coreA.

I'm not certain it's the most necessary / useful feature, but it's there
and
doesn't really seem to add much complexity to the code, so why not give
users the option?

I don't think I understand your terminology here. So coreA and coreB are
both independent projects (I can check them out of git or whatever), and
are also built as sub-projects under generic? Why are you referring to them
as variants as well?

  1. Part of the plan for Tupfile.ini was to create default variants -
    that
    isn't here yet, correct? I don't see it, but just wanted to make sure
    I'm
    not missing something.

Yes, I thought of that a logically different feature, but I can add it to
this
pull request shortly (read: as soon as I write it :).

Ok :).

As for making 'tup' do a 'tup upd' - the rationale behind having to
explicitly do 'tup upd' in the first place is so that just a 'tup'
doesn't
change the filesystem, like most other tools (except 'make' and its
descendants, of course). This can always be changed with a local alias,
but
I'm happy to add an option or something to support that in tup directly.
I
don't think it should be the default, however. This thread also has some
info:

https://groups.google.com/d/msg/tup-users/s5rR10Nb7X0/WB66mI8R7tUJ

Ugh.. another example of a discussion lost to the mailing list...

I would push back against this as well. Commands like gcc don't print out
a usage block, they try to run, they simply don't have enough information
to run successfully:

$ gcc
i686-apple-darwin11-llvm-gcc-4.2: no input files

If you want the usage info from a program, you run program -?, that's
the
established paradigm. Tup has a massive amount of metadata available to
it, both in the database and the various tupfiles strewn about. The point
of
this metadata is to supply the needed information so tup can run.

Tup is fundamentally a build system tool, when I run tup, odds are I want
to build my project. If I want tup to do something else, I need to specify
it,
but by default tup should do what it was built to do: build my project.

I see this as one more point of friction in bringing over make / other
build
system converts, and with the demotion of init to a secondary command,
this leaves upd as the only remaining "primary command".

In discussions between @bradjc and I, getting rid of upd was actually the
original motivation for this whole series of improvements, the starting
questions was, "what would need to change so that we can get rid of upd"?

Fair enough. We can probably make it so 'tup' does something like:

if inside tup hierarchy:
'tup upd'
else we find a Tupfile.ini up the tree:
run 'tup init' at the top-most Tupfile.ini and create its variants
run 'tup upd'
else:
print usage information (suggestiong to create a Tupfile.ini or run 'tup
init' at the top)

How does that sound?

-Mike

@ppannuto
Copy link
Contributor Author

On Tue, Aug 27, 2013 at 6:01 PM, gittup notifications@github.com wrote:

Hi Pat,

On Thu, Aug 22, 2013 at 4:05 PM, Pat Pannuto <notifications@github.com

wrote:

On Thu, Aug 22, 2013 at 8:13 AM, gittup notifications@github.com
wrote:

Hi Pat,

Thanks for taking the time to work on this! See comments below...

On Wed, Aug 21, 2013 at 2:47 AM, Pat Pannuto <notifications@github.com

wrote:

I've updated the commit to reset dynamic_configs = NULL in
tup_option_exit to address point 1. Whether or not its necessary to
be
able
to call tup_option_exit multiple times, it's certainly not incorrect
to
reset that pointer to NULL.

As for point 2, I think that's best addressed by you Mike.

Yeah, this test is just expecting the autoupdater to run once, when the
Tupfile.ini change is causing it to run twice. Not a big deal - we
would
just change the expected value.

Some questions/feedback on Tupfile.ini:

  1. I don't like that Tupfile.ini is now "mandatory" in that if you
    don't
    have one, you get the warning messages. I think if there is no
    Tupfile.ini
    and no .tup directory, you should just get the tup usage information.
    Users
    should still be able to 'tup init' manually as before, without getting
    the
    warnings.

The rationale behind this warning was to incentivize current tup users to
add
a .ini file to their existing projects. The overarching goal with this
series of
commits is to make tup much more user-friendly for people who have never
heard of tup (nor, at least when they're encountering it for the first
time, do
they probably want to learn).

I like trying to make tup more user-friendly, but I don't think forcing
people to change their existing habits & scripts is the way to go.

Agreed.

Users of current build systems don't have to know anything about automake
to know that the pattern ./configure && make will get them from tarball
to
software. In the current system, tup init && tup upd won't even work as
an
idiom, since repeated calls to tup init will fail. The point of the
warning then
is to i.) define, and ii.) help enforce a tup "best practice".

I think we can fix that by making a 'tup init' inside an existing hierarchy
just print a warning and exit successfully, right?

Yes, this is a good compromise. I'll change to this.

  1. I still think hooking it in to the options mechanism is not the
    right
    approach. The options are supposed to be user or machine preferences,
    not
    project preferences. Ie: it doesn't make sense for me to check-in a
    Tupfile.ini that specifies updater.num_jobs, or display.color, etc.
    Those
    are up to the user. The only mention I've seen for why this is
    necessary
    is
    for updater.full_deps, which is an option I'd prefer to actually get
    rid
    of
    (by making it always do full deps). Before that happens, I think we
    would
    make Tupfile.ini support only that option by creating the .tup/options
    file
    after calling init(). There's no need to read Tupfile.ini after .tup is
    already created.

Then where do project preferences go? ./tup/options is not the right
place
for project preferences, as that file is machine-specific, it's not
checked
into
the repository like Tupfile.ini would be and changes would therefore not
be
synced.

Can you clarify which options you think are project preferences? For these
options I believe it is wrong to make a default for the project, since they
are specific to a user's preferences (eg: I want to override
monitor.foreground) and/or a machine (eg: this machine details horribly
with colors, so disable display.color):

updater.num_jobs
updater.keep_going
display.color
display.width
display.progress
display.job_numbers
display.job_time
monitor.autoupdate
monitor.autoparse
monitor.foreground
graph.dirs
graph.ghosts
graph.environment
graph.combine
db.sync

That leaves only:

updater.warnings
updater.full_deps

And arguably both of those could go away. I'm trying to move toward making
full_deps the default and just tossing the option entirely, because tup
isn't always accurate if you ignore dependencies outside the hierarchy. And
the warnings thing probably isn't all that useful - we might be better off
committing to a better strategy for what to do with .hidden files.

So exactly what options are you hoping to commit to the project as project
preferences?

Sure, so some of the motivation here comes from some future ideas that
@bradjc and I have cooked up. In particular, we have been tossing about the
idea of "aliases", where aliases are additional user-defined subcommands.
An alias could run a script, a series of other tup commands, etc. This
would solve absence of a "make install" equivalent but allowing a project
to define an "install" alias. The idea is nowhere near fully-fledged, but
it's an example of a project-specific configuration.

As a strawman for other project-specific options that make sense, some of
the more ancient build tools don't uniquely name temp files, and
therefore
cannot have multiple instances running at once. In that case, being able
to
set updater.num_jobs as a project-specific option is logical.

Tup already supports this in Linux and OSX since the temporary files are
uniquely stored in .tup/tmp before being renamed to the output file. See
t6048 for the test. I'd love to get that working correctly in Windows too.

Each subprocess gets its own unique tmp directory? That's cool, didn't
realize that.

  1. In line with part 2, I don't see a reason to parse multiple
    Tupfile.ini's if there are several up the tree (for sub-projects, and
    the
    like). The top-most one should override the lower level one as the
    place
    for where init() is called.

As a motivating example for this, consider the init.default_variants that
will
be added shortly. In a project, I have a tree:

|- generic/
|--- coreA/
|--- coreB/

The root Tupfile.ini, would specify building both the coreA and coreB
variants
by default. Inside of the coreA directory, however, the default variants
would
be overridden to only build coreA.

I'm not certain it's the most necessary / useful feature, but it's there
and
doesn't really seem to add much complexity to the code, so why not give
users the option?

I don't think I understand your terminology here. So coreA and coreB are
both independent projects (I can check them out of git or whatever), and
are also built as sub-projects under generic? Why are you referring to them
as variants as well?

No, the project in question is an architectural simulator. So there's a
simulator framework that is common and you select which core (e.g. msp430,
cc2538) you would like to build a simulator for. This is done at compile
time as the optimizer can do some powerful things once the core is selected
(if it's easier: github.com/ppannuto/M-ulator).

  1. Part of the plan for Tupfile.ini was to create default variants -
    that
    isn't here yet, correct? I don't see it, but just wanted to make sure
    I'm
    not missing something.

Yes, I thought of that a logically different feature, but I can add it to
this
pull request shortly (read: as soon as I write it :).

Ok :).

As for making 'tup' do a 'tup upd' - the rationale behind having to
explicitly do 'tup upd' in the first place is so that just a 'tup'
doesn't
change the filesystem, like most other tools (except 'make' and its
descendants, of course). This can always be changed with a local alias,
but
I'm happy to add an option or something to support that in tup
directly.
I
don't think it should be the default, however. This thread also has
some
info:

https://groups.google.com/d/msg/tup-users/s5rR10Nb7X0/WB66mI8R7tUJ

Ugh.. another example of a discussion lost to the mailing list...

I would push back against this as well. Commands like gcc don't print out
a usage block, they try to run, they simply don't have enough information
to run successfully:

$ gcc
i686-apple-darwin11-llvm-gcc-4.2: no input files

If you want the usage info from a program, you run program -?, that's
the
established paradigm. Tup has a massive amount of metadata available to
it, both in the database and the various tupfiles strewn about. The point
of
this metadata is to supply the needed information so tup can run.

Tup is fundamentally a build system tool, when I run tup, odds are I want
to build my project. If I want tup to do something else, I need to
specify
it,
but by default tup should do what it was built to do: build my project.

I see this as one more point of friction in bringing over make / other
build
system converts, and with the demotion of init to a secondary command,
this leaves upd as the only remaining "primary command".

In discussions between @bradjc and I, getting rid of upd was actually the
original motivation for this whole series of improvements, the starting
questions was, "what would need to change so that we can get rid of upd"?

Fair enough. We can probably make it so 'tup' does something like:

if inside tup hierarchy:
'tup upd'
else we find a Tupfile.ini up the tree:
run 'tup init' at the top-most Tupfile.ini and create its variants
run 'tup upd'
else:
print usage information (suggestiong to create a Tupfile.ini or run 'tup
init' at the top)

How does that sound?

That should be what it does now :)

-Mike


Reply to this email directly or view it on GitHubhttps://github.com//pull/133#issuecomment-23383066
.

Pat Pannuto
NSF/NDSEG/Qualcomm Fellow
Computer Engineering
University of Michigan
248.990.4548

@ppannuto
Copy link
Contributor Author

This latest series of commits should cover everything we've talked about.

It's also rebased on top of the latest tup-master.

@ppannuto
Copy link
Contributor Author

Hey Mike,

I know you were on vacation for a little while, are you back now / have you had a chance to look at this?

I just rebased onto tup-master again to make sure everything will work / merge cleanly, looks good for me.

-Pat

@bradjc
Copy link
Contributor

bradjc commented Oct 7, 2013

Would it maybe be useful to split the tupfile.ini processing and the move to just a tup command into separate pull requests? I have many tup projects on various machines and I've been seeing the no .tup folder found error quite a bit because I can't remember where I've run tup init. It seems like incorporating an improvement (the tupfile.ini support) that intelligently handles running tup init for the user is a no-brainer.

@gittup
Copy link
Owner

gittup commented Oct 13, 2013

Sorry for the delay - after vacation I've also had a busy work trip, which
I'm still trying to get caught up from.

On Sun, Oct 6, 2013 at 11:16 PM, Brad Campbell notifications@github.comwrote:

Would it maybe be useful to split the tupfile.ini processing and the move
to just a tup command into separate pull requests? I have many tup
projects on various machines and I've been seeing the no .tup folder founderror quite a bit because I can't remember where I've run tup
init. It seems like incorporating an improvement (the tupfile.ini
support) that intelligently handles running tup init for the user is a
no-brainer.

I'd be happy to accept a pull request that just allows 'tup' to do an init
where the top-most Tupfile.ini file exists, and run 'tup upd' otherwise. I
think the only thing I still have an issue with in the tup-ini branch is
the use of Tupfile.ini as an in-tree project options file. I still don't
think it's correct for someone to be able to commit a file to the project
that allows setting a default updater.num_jobs, for example.

As I understand it, the options parsing of Tupfile.ini is just used for the
default variants - if we can skip that part for now and get just the single
'tup' command, that would work for me.

Thanks,
-Mike

This commit seeks to obviate the explicit calling of `tup init`. When
tup is run, it will now search for both a `.tup` directory or
`Tupfile.ini` files on the path down to the root directory. If no `.tup`
directory is found, tup will consider the directory closest to the root
that had a `Tupfile.ini` file as the root of the project and automatically
call `tup init` in that directory.

To appease the idiom of `tup init && tup upd`, repeated calls to init
will print a warning indicating that tup has already been init'd, but
will still return success.
This commit deprecates the command `tup upd` in favor of the simpler,
shorter command `tup`. Now that `tup init` is not required, there was only
one "basic" tup command remaining: upd. This allows for much easier
migration to tup for refugees from other "build systems", using tup is now
as simple as just typing 'tup'.

The command `tup upd` is preserved for legacy / backwards-compatability
reasons. It is silently accepted.
None of the tests failed (on OS X at least), but these changes eliminate
the large numbers of warnings that printed.

Test t0001 is updated to create a Tupfile.ini in the custom init
directory that it creates.

Test t7052 is updated to expect the monitor to run twice, as it detects
the call to `touch Tupfile.ini` from the test framework.
@ppannuto
Copy link
Contributor Author

This pull request has been split. These commits cover:

1.) Adding Tupfile.ini
2.) Deprecating 'upd'
3.) Fixing up test cases

@ppannuto ppannuto mentioned this pull request Oct 21, 2013
@gittup
Copy link
Owner

gittup commented Oct 23, 2013

On Sun, Oct 20, 2013 at 8:46 PM, Pat Pannuto notifications@github.comwrote:

This pull request has been split. These commits cover:

1.) Adding Tupfile.ini
2.) Deprecating 'upd'
3.) Fixing up test cases


Reply to this email directly or view it on GitHubhttps://github.com//pull/133#issuecomment-26687392
.

Thanks for splitting this out! This is now in master. I've slightly
modified 2 of the commits, and had 2 follow-up commits. The main changes I
made are to not show the warnings if there is no Tupfile.ini, and to not
show usage information during a partial update (like 'tup foo.o' - if foo.o
failed to build, the usage information would be displayed).

-Mike

@ppannuto ppannuto closed this Oct 23, 2013
@ppannuto ppannuto deleted the tup-ini branch October 23, 2013 23:24
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

Successfully merging this pull request may close these issues.

3 participants