-
-
Notifications
You must be signed in to change notification settings - Fork 14k
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
lib/systems: rename examples.nix to all.nix, include alias #175341
Conversation
When I had to figure out how `lib/systems` worked in order to enable bootstrapping on mips64el and powerpc64el, by far the most confusing thing was that nixpkgs' official list of platforms (i.e. those identifiers which can be used in `pkgsCross.<platform>`) is kept in a file called `examples.nix`. When most people, including myself, see a codebase with files or directories named `examples/` they assume it contains *examples* which are for tutorial purposes only, and are not used by the deployed codebase. This commit attempts to save future nixpkgs contributors the confusion I encountered. It renames `lib/systems/examples.nix` to `lib/systems/all.nix`, since what it contains is *a list of all systems (or platforms) that nixpkgs currently knows about*. This commit also includes an alias in `lib/systems/examples.nix`, so it is not a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes please! This was pretty annoying the first time I saw it
Future Changes
If this commit is merged I will submit separate PRs to update all the references to
lib.systems.examples
in nixpkgs so they point tolib.systems.all
instead.
Why plural "PRs"? Better to do it in one swoop to avoid more new references being added.
At some point in the future I would like to gradually migrate towards removing the alias, in steps: merge this commit, then replace the
examples.nix
alias with athrow
, then dropexamples.nix
entirely -- with no more than one step taken in each twice-per-year release.
Did you mean trace
instead of throw
? That'd seem more reasonable and would also let us know if there's anything blocking removal. (think Nix uses this somewhere)
@@ -0,0 +1,336 @@ | |||
# These can be passed to nixpkgs as either the `localSystem` or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to add this to lib/systems/default.nix
(here). It doesn't seem to do any magic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why plural "PRs"? Better to do it in one swoop to avoid more new references being added.
Okay!
Did you mean
trace
instead ofthrow
?
Ah, I was wondering how to have nixpkgs print a "warning". I assumed trace
was supposed to be for debugging only. If it is okay to commit expressions that use it to print warnings, then I would like to add that as an additional step:
At some point in the future I would like to gradually migrate towards removing the alias, in steps:
- merge this commit
- replace the examples.nix alias with a trace
- replace the trace with a throw
- drop examples.nix entirely
...with no more than one step taken in each twice-per-year release.
I have edited the topmost comment to reflect this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant:
all = import ./all.nix { inherit lib; };
examples = import ./examples.nix { inherit lib; };
We can't change the file structure yet since people might be importing lib/systems/examples.nix
directly for whatever reason, so this file should just have a 1-1 mapping of the directory structure.
I don't like renaming it, because they really are just examples. Anything that |
To clarify, for those following along:
I don't like the fact that Instead of this PR, I would be happy to see us migrate towards forcing
@Ericson2314, would you support such an effort, including acting as shepherd for an RFC if one is deemed necessary? I would prefer that instead of this PR. But the current ambiguity is much worse than either of those solutions. I've marked this as draft so nobody merges it before we hear back from you. |
@a-m-joseph I would support that. In fact, I would support just getting rid of |
So how will you do this? |
@@ -6,7 +6,7 @@ rec { | |||
parse = import ./parse.nix { inherit lib; }; | |||
inspect = import ./inspect.nix { inherit lib; }; | |||
platforms = import ./platforms.nix { inherit lib; }; | |||
examples = import ./examples.nix { inherit lib; }; | |||
examples = import ./all.nix { inherit lib; }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a big no, as explained by @Ericson2314.
We can tolerate a vague name sometimes, but we can not have factually incorrect ones.
This seems like a valid requirement. Maybe talk to Eelco about merging |
If I have understood correctly, the idea is Edit, in case this resolves confusion: the valid arguments to
It will still be |
Closing in favor of this approach, which I much prefer. It will be about two weeks before I can start drafting an early-revision proposal, though -- have a few other things ahead of it in the to-do queue. |
Background
When I had to figure out how
lib/systems
worked in order to enable bootstrapping on mips64el and powerpc64le, by far the most confusing thing was that nixpkgs' official list of platforms (i.e. those identifiers which can be used inpkgsCross.<platform>
) is kept in a file calledexamples.nix
. When most people, including myself, see a codebase with files or directories namedexamples/
they assume it contains examples which are for tutorial purposes only, and are not used by the deployed codebase.This commit attempts to save future nixpkgs contributors the confusion I encountered.
Description of changes
This commit renames
lib/systems/examples.nix
tolib/systems/all.nix
, since what it contains is a list of all systems (or platforms) that nixpkgs currently knows about. This commit also includes an alias inlib/systems/examples.nix
, so it is not a breaking change.Future Changes
If this commit is merged I will submit separate PRs to update all the references to
lib.systems.examples
in nixpkgs so they point tolib.systems.all
instead.At some point in the future I would like to gradually migrate towards removing the alias, in steps:
...with no more than one step taken in each twice-per-year release.
Things done
./result/bin/
)