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

What config scope should be the default? #2184

Open
tgamblin opened this issue Oct 31, 2016 · 7 comments
Open

What config scope should be the default? #2184

tgamblin opened this issue Oct 31, 2016 · 7 comments

Comments

@tgamblin
Copy link
Member

It came up while documenting #2152 that Spack is inconsistent about which config scope is edited from command to command. Currently, the sort-of-inconsistently-implemented default behavior is to edit the highest-precedence available scope, which used to be user, but as of #2030 it's the platform-specific scope for the current platform (e.g. ~/.spack/bgq/config.yaml).

I figure I should bounce this off of people before editing it. See if you think what I'm proposing below is reasonable.

Currently there are 11 commands that take a scope argument. I've labeled them as either "read" operations or "modify" operations. I think they should behave as follows:

Read operations:

  • spack config get
  • spack compilers / spack compiler list
  • spack mirror list
  • spack repo list

I think all of these should show merged results by default, and allow the user to select a specific scope with --scope=<scope> if they want to.

Modify operations:

  • spack config edit
  • spack compiler add / spack compiler find
  • spack compiler remove
  • spack mirror add
  • spack mirror remove
  • spack repo add
  • spack repo remove

I think instead of taking the highest precedence scope (which among other things might change from spack version to spack version if we implement additional scopes), we should implement this policy:

  1. Introduce a default_edit_scope option in config.yaml, and out of the box it would be set to edit the user scope by default.
  2. If the option is set to <scope>, commands should edit the generic <scope> config by default, but it should edit an architecture-specific <scope>/<platform> config if it exists. I think this is a) natural and b) prevents the user from getting confused if they, say, forgot they made a platform-specific config.
  3. Finally, compilers are a special case (in that they are actually platform-specific) and spack compiler add/find should modify the platform-specific scope by default.

Does this policy seem reasonable? the other option would be to require a scope parameter for edit commands, but I think it's nice for users to be able to say, e.g., spack config edit or spack repo add without thinking about scopes.

@alalazo @davydden @adamjstewart @citibeth

@citibeth
Copy link
Member

Read operations:

I had no idea any of these commands exist. Why should I learn them to look at a single scope if I have emacs/vim/etc?

Now that there are so many overriding scopes... I would be interested in a "merge" command that shows you the resulting configuration file (I believe it already exists?) Something like: spack cat config.yaml, spack cat packages.yaml, etc.

Something akin to git blame would be nice here too... so we can see where each line of the resulting effective config files came from.

Modify operations...
Does this policy seem reasonable?

To be honest... I don't use these commands much either. My modus operandus seems to be to edit config files directly.

I think the defaults seem reasonable.

But allowing the defaults to be changed in config might be overkill. I would lean toward hard-coding the way commands choose which scope to edit, and let the user override with a command line option. If they're not hard-coded, then it becomes harder to tell people which commands to issue. Because the right command for one installation will edit the wrong scope on another. Especially since there are now eight places where the default scope to edit can be changed.

  1. Finally, compilers are a special case (in that they are actually platform-specific) and spack compiler add/find should modify the platform-specific scope by default.

Does this take cross-compilers into account? (Sorry, cross-compilers make my head hurt).

@tgamblin
Copy link
Member Author

I had no idea any of these commands exist. Why should I learn them to look at a single scope if I have emacs/vim/etc?

You don't have to. But I'm not really asking you to -- our users like these, and it's easier for me to send commands in an email to copy/paste than to tell a user to edit a file in a precise way. cf. the command line instructions on every single PR on github.

I would be interested in a "merge" command that shows you the resulting configuration file (I believe it already exists?) Something like: spack cat config.yaml, spack cat packages.yaml, etc.

spack config get packages

I think the defaults seem reasonable.

yay!

But allowing the defaults to be changed in config might be overkill.

I'm trying to anticipate the project scope you want, but maybe you're right. Default could be user, or project if in a project directory. That is probably all that is needed.

Does this take cross-compilers into account? (Sorry, cross-compilers make my head hurt).

They're actually below this level. Each platform can have multiple OS/targets that live on it. e.g, BG/Q has a different OS/target combo on the login nodes vs. on the compute nodes. Cray has different OS's. That's all handled within compilers.yaml at the moment; compilers can have different OS's and they should also be able to have different targets (that they can't is currently a bug).

Platform-specific configs are there mainly so that we can ship good defaults for machines like BG/Q and Cray.

@citibeth
Copy link
Member

But allowing the defaults to be changed in config might be overkill.

I'm trying to anticipate the project scope you want, but maybe you're
right. Default could be user, or project if in a project directory. That
is probably all that is needed.

As the saying goes... let's burn that bridge when we get to it!

@tgamblin tgamblin mentioned this issue Oct 31, 2016
10 tasks
@adamjstewart
Copy link
Member

Read operations:

I think all of these should show merged results by default, and allow the user to select a specific scope with --scope=<scope> if they want to.

Agreed!

Something akin to git blame would be nice here too... so we can see where each line of the resulting effective config files came from.

Also agreed! This might be too much clutter to do by default, but we should at least have a flag to enable this.

As for Modify operations, I don't personally need platform-specific configurations, but I can see why others would. A setting in config.yaml could make it easy to compromise and let everyone get what they want.

@tgamblin I'm glad you're taking a look at this! This inconsistency caused #2042 and #2164 for me. As long as those are fixed, I don't care that much about the implementation. I'm hoping that I can run spack compiler find and spack config edit and spack compiler remove and have things just work without ever having to specify a scope flag.

@tgamblin
Copy link
Member Author

@scheibelp

@becker33
Copy link
Member

becker33 commented Jul 3, 2019

#11919 is superseded by the default config edit scope proposed here.

@scheibelp @tgamblin That would also alleviate concerns from Chris Nelson and Ross Bartlett about polluting user home directories, and others have brought up concerns. Should be easy to implement.

@citibeth
Copy link
Member

citibeth commented Jul 3, 2019

I don't think we need a special command to edit a file; that's what editors are for. Documentation on what the scopes are, and where they are found (in case you want to edit them) is useful.

The advent of Spack Environments removes need to pollute your home directory; and now the scopes are moved to within the environment.

For those two reasons, I wouldn't mind seeing this Issue closed.

@scheibelp scheibelp assigned scheibelp and unassigned alalazo Jul 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants