Conversation
|
I like If I understand correctly, with this PR I'll be able to tell |
Exactly. Or for machines that require |
|
All the tests pass, except the doc test. I'm getting the following error from Travis: When I try it myself, I get this behavior too: Looks like this PR still needs a little work... |
|
Maybe you can't parse args multiple times? It seemed a little hacky anyway. |
|
Also, I can't believe the documentation tests are the only thing that catches such a grave bug. |
Good guess, but actually you can.
“Life is pain, highness. Anyone who says differently is selling something.” This PR is too good to throw away (IMHO), so I just re-did my I find the position of the subcommand by first parsing the command line with a parser that has just the args for main Spack, leaving everything from the Spack command on as unparsed. Then I use list lengths to split up the original args.
I'm hoping I don't have to find a solution to that problem in this PR. |
|
I like the concept of simplifying the spack command line, but I have reservations about this for the same reason the git maintainers do. Specifically:
I tend to agree with the issues raised above that if you allow everything in this PR, then it's going to be very hard to write a script for Spack that works reproducibly for different users. I would like to keep Spack scriptable and to ensure a little bit that when we talk to a user on GitHub, that we're talking about the same semantics for a given command. Also, the settings here overlap with some of those in
Why not just set That said, I would welcome a PR that allows real aliases, similar to git's. e.g.: aliases:
dinstall: install --dirty
in: install
rm: uninstallThe one caveat would be the same as the one The other cool thing about So basically I am saying I like this, but I think we should take a page from Thoughts? |
Let's look at the situation more holistically. Spack scripts are already non-reproducible between users, due to the large number of things that can ALREADY be in Luckily, it is possible to use Spack in reproducible way. With #2686 in place, I have completely emptied my By moving configuration from the user-level scope to command line scopes, I have made things reproducible where they were not before. I could do the same thing with my Finally, let's look at what I'm actually using my As you can see, none of the alias commands I've selected have any effect on reproducibility. Conclusion: Reproducibility in Spack is a significant issue, and user can shoot themselves in the foot. This PR does not particularly increase or decrease that possibility. However, #2686 DOES provide a tool that allows users to work in a reproducible way, if they so choose.
I suppose that's another way to go about it, and would be appropriate for the aliases I wanted this PR for. However... this PR was quick, and in one fell swoop adds all possibility functionality --- without changing the code that uses those configuration options. Adding the parameters one by one to
That doesn't cover the use case I want this PR for. Now users have to type Nor is your example convincing. Because typically
It provides none of the power I want from this PR. If I'm willing to type non-standard Spack commands (eg This would be minorly inconvenient for me, but I might get used to it. However, it could be embarassing in other cases. Suppose I'm on a system using some crazy compiler that requires Now I sit my users down with a Spack tutorial and tell them to type
As long as you only put
Again... not the functionality I'm looking for in this PR.
The proposal to add appropriate options to |
|
I would definitely like the ability to set default flags for Spack subcommands. I almost always want: I don't particularly care whether this is done by adding an |
|
Do others have an opinion on this? @alalazo? @davydden? @mamelara @mplegendre? @becker33 ? @lee218llnl ? sounds like I might be in the minority. |
|
Personal minor concern, mostly OT: I would not grow the array of configuration files. If there is already |
|
I'm wondering why we have separate YAML files to begin with. They could
all be concatenated into one.
…On Tue, Jan 10, 2017 at 10:55 AM, Denis Davydov ***@***.***> wrote:
Personal minor concern, mostly OT: I would not grow the array of
configuration files. If there is already config.yaml, maybe the
functionality should be added through it. Otherwise in a year one may end
up having tens of files and it would be a pain to remember which one
actually influence what.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2705 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AB1cd92qwXAdRAzSZoLozdD4ZKo45AAWks5rQ6oPgaJpZM4LYn1r>
.
|
|
@tgamblin I tend to agree with your opinion above, in particular:
and with @davydden on the number of configuration files. |
|
and I don't mind adding more settings in config.yaml.
Looking at the specific command-line options that @adamjstewart and I have
said we want to use this for... do you mind adding *those* to *config.yaml*?
…On Tue, Jan 10, 2017 at 11:14 AM, Massimiliano Culpo < ***@***.***> wrote:
@tgamblin <https://github.com/tgamblin> I tend to agree with your opinion
above, in particular:
Also, the settings here overlap with some of those in config.yaml.
Why not just set dirty: true in config.yaml? That exists. color: true may
soon exist (see #2441 <#2441>). We
have control over what defaults we allow to be set here (i.e., we can keep
them sane), and I don't mind adding more settings in config.yaml.
and with @davydden <https://github.com/davydden> on the number of
configuration files.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2705 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AB1cdxBGlrNxkYF8YzUE_MHmHwiiJNpDks5rQ659gaJpZM4LYn1r>
.
|
Just because they could doesn't mean they should. They provide logical separation, so if I ask you for your compiler settings, you can give me your I don't mind a new |
Nope. I like those actually. I also think making |
|
RE: configuration proliferation: I also like fewer config files, though I am not sure I would like a single one. So far:
One near-term concern I have about concatenating is that commands edit the YAML files, and while I've hacked the YAML parser a bit, we haven't hacked it to round-trip comments. So you'll lose comments for things like That said, consolidating is easy. The files all start with unique section keys (this was intentional) so concatenating them doesn't actually make them harder to parse or differentiate. @mplegendre and i have talked about allowing the top-level sections to be either separate files or concatenated into one. I think that would be easy to pull off. |
|
Could an argument |
|
@mplegendre To be more specific... I would like the option of providing configuration scopes as a consolidated single file, rather than a directory of YAML files. I don't necessarily want to do this to my As we formalize ideas around Spack environments, I think it's worthwhile separating options that change how concretization works into a separate file. A quick check through docs on |
I said more than that. Actually, I said that Spack is already not scriptable because of The article on |
|
Superseded by #17229 |
This PR adds command line defaults to Spack. Default command line arguments can now be specified in a configuration file called
aliases.yaml. For example:Example: When the user types
spack spec xz, that will be converted tospack --debug spec --install-status --long xz.Question: What should the YAML file be called?
aliases.yaml(because it's kind of like Bash aliases)?defaults.yaml(because it sets defaults for command lines)?default-args.yaml(because that's more specific thandefaults.yaml)?