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.

Sign up for GitHub

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

Jump to bottom

Default variants #144

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Default variants #144

wants to merge 2 commits into from

Conversation

ppannuto
Copy link
Contributor

This pull request adds support for default variants. It depends on PR #133.

Currently it the same as before, it extends the option mechanism to add support for default variants. I'm open to suggestions for how you'd prefer to handle this, but I wanted to have this out here as (1) it's a place to have a discussion and (2) this is used extensively by our lab already, so I didn't want to drop support for it.

One suggestion was to simply have two separate sets of options. The current tup-specific options (e.g. updater.num_jobs) and then project-specific options (e.g. default variants and the aliases previously discussed), where the latter are set in the .ini files but .ini files cannot set the former.

@ppannuto
Add option parsing to Tupfile.ini
aefc524 
Any `Tupfile.ini` encountered along the way to the root is parsed as a
configuration file (such as the `.tup/options` or `/etc/tup/options`
files). The ini files are added to the configuration search in the order
they are found, that is a `Tupfile.ini` in the directory `tup` is run
from has the highest precedence. Note: This has the interesting property
that things in `Tupfile.ini` files are explicitly not project-global
unless they are in the root directory of the project.

The contents of `Tupfile.ini` files are not tracked in tup's database,
rather they are re-parsed on every invocation of tup. This is a design
choice, as the configuration elements in the ini files are designed to
affect the behavoir of tup as a program, not tup as a build system. In a
correct system, the options exposed by the ini files should not be able
to affect net result of a build.
@ppannuto
Add post_init.varaints hook for default variants
8d09099 
This commit adds support for default variants. It adds an option entry
called post_init.variants that takes a list of variants to be run. The
hook is called any time that `init_command()` is run, whether implicitly
or by an explicit tup init.

The init() process is re-arranged slightly so that tup is fully init'd
when the post_init hooks run. For similar reasons, find_tup_dir is
converted to a singleton.
@ppannuto
Copy link
Contributor Author

These commits have been updated since #133 was merged. It should apply cleanly to latest master (caf954c).

@gittup Let me know what you're thinking and how you'd like to move this feature forward?

@gittup
Copy link
Owner

gittup commented Oct 28, 2013

On Wed, Oct 23, 2013 at 7:37 PM, Pat Pannuto notifications@github.comwrote:

These commits have been updated since #133https://github.com/gittup/tup/pull/133was merged. It should apply cleanly to latest master (
caf954c caf954c8).

@gittup https://github.com/gittup Let me know what you're thinking and
how you'd like to move this feature forward?

I support using Tupfile.ini to supply default variants, though I still
think using the options mechanism is the wrong place for it. We can still
use inih to process it without using options, in case that was the primary
reason for putting it there. My thinking here is as follows:

  1. As described in 133, the majority of the existing options are strictly
    user preferences, and have no business going into a Tupfile.ini file that
    gets checked into the project.

  2. Similarly, I'd argue that the new post_init.variants option only makes
    sense in a Tupfile.ini and would be incorrect to set this option in any of
    the existing options files (ie: why would ~/.tupoptions want to define
    default variants for all projects? Not all projects may support those
    particular variants).

So I'd like to propose that post_init.variants not be an in the options
table. Instead, I'd suggest having an explicit format for Tupfile.ini that
can only support variants. This could still use the inih parser:

[variants]
some text here

But it wouldn't affect any of the existing options, and no current options
file could set the default variants.

I'd also like to suggest that we create a new option that determines
whether or not Tupfile.ini is enabled. Something like 'ini.add_variants' or
whatever. It sounds like you wanted to have Tupfile.ini always parsed so
that as new variants are added to the project, all developers get those
variants built when they pull the latest code. However, I believe we should
still support other developers who may not want to use variants at all.
With this option, if 'ini.add_variants' is true, Tupfile.ini gets parsed on
every 'tup' invocation, and any new variants are created/built. If it's
false, Tupfile.ini variants are unused and can still be created manually by
the developer as desired.

How does that sound? I think this would support your use-cases while still
addressing my concerns.

Thanks again for splitting these out,
-Mike

@ppannuto ppannuto mentioned this pull request Dec 18, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ppannuto @gittup