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

modularity: Document the ability to use non-files in imports #50503

Merged
merged 2 commits into from Nov 28, 2018

Conversation

Baughn
Copy link
Contributor

@Baughn Baughn commented Nov 17, 2018

Motivation for this change

There are no examples of using anything but files in the imports list. Users might be confused into thinking that's all that's possible.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@Baughn Baughn requested a review from nbp as a code owner November 17, 2018 15:01
Copy link
Member

@markuskowa markuskowa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is indeed a useful hint (I did not realize that this is an option).

nixos/doc/manual/configuration/modularity.xml Outdated Show resolved Hide resolved
@markuskowa markuskowa self-assigned this Nov 24, 2018
Co-Authored-By: Baughn <svein@google.com>
@markuskowa
Copy link
Member

Can you please squash the commits.

@Baughn
Copy link
Contributor Author

Baughn commented Nov 28, 2018

In a few hours. ...can't you use the "squash and merge" option?

@markuskowa markuskowa merged commit 2486596 into NixOS:master Nov 28, 2018
@domenkozar
Copy link
Member

This is a bad idea to document, because you lose source tracking at errors. Your errors with not include in what line the error happened and it's going to be very painful to debug.

@markuskowa
Copy link
Member

It seems like it was designed to also accept configuration sets and not just paths. That should be documented and not just be a hidden feature? I can see your point that it makes things harder to debug but should that not be the users choice in the end?
I personally find that feature quite useful for defining nixops deployments, where machines share some configurations but not so much that I would want to put in a separate file.

@domenkozar
Copy link
Member

It's a slippery slope, you'll soon regret saving those 5s once you're deep debugging some coercion gone wrong, without having the line number where it happened.

I think we should at very least make this clear (in red) in the documentation.

@markuskowa
Copy link
Member

I agree it should be mentioned in the documentation.
However, from a technical point it is not clear to me why or when nix loses track of the line numbers. I ran into this type of problem many times without using the above mentioned construct. Is that mentioned somewhere in the documentation?

@domenkozar
Copy link
Member

You only run into this if you use nixos modules imports without filenames.

@markuskowa
Copy link
Member

markuskowa commented Nov 28, 2018

When you misspell an option in a nixos configuration file the error message does not contain the line number (it does tell the filename though). So, this statement would then only apply to syntactical errors (where nix usually shows the line numbers)?
Btw.: I did just try importing a module by function and it actually does tell me the line number of a syntax error. I could not see any difference in behavior.
Under what circumstance would that debugging problem, you refer to, occur?

@domenkozar
Copy link
Member

Maybe @edolstra can explain precisely, I don't have time to really research this, I'm just warning this is going to create more pain than gain as it currently stands if documented.

@edolstra
Copy link
Member

Well, this feature is used in lots of places, so documenting it makes sense, even if it's not really a good idea to use.

@Baughn
Copy link
Contributor Author

Baughn commented Nov 29, 2018

For instance, I'm using it to generate NixOS systems for a number of machines, most of which differ only in their hostname.

Is this a bad idea? Should I do it some other way? I don't want to generate configuration.nix files for each of them; I'd rather have the machines all defined in a single file, one line per machine.

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

Successfully merging this pull request may close these issues.

None yet

5 participants