-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
Treewide: change all occurrences of "name" to "pname-version" #104045
Conversation
Related issue: #103997 |
Is it expected that none of these derivations will build differently because the interpolated strings are the same as they were before? |
Yes. It is just a rewriting of the expressions. |
@@ -14,7 +14,7 @@ stdenv.mkDerivation rec { | |||
# which will contain the links to all available *.debs for the arch. | |||
|
|||
src = fetchurl { | |||
url = "http://dl.google.com/linux/musicmanager/deb/pool/main/g/google-musicmanager-beta/${name}_amd64.deb"; | |||
url = "http://dl.google.com/linux/musicmanager/deb/pool/main/g/google-musicmanager-beta/${pname}-${version}_amd64.deb"; |
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.
Unrelated to this PR but the platform should probably be set to x84_64 only.
name = "jackmix-0.5.2"; | ||
pname = "jackmix"; | ||
version = "0.5.2"; | ||
|
||
src = fetchurl { |
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 should probably use fetchFromGitHub.
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.
It would change the final output. I am not trying to do this here and now.
src = fetchurl { | ||
url = "mirror://sourceforge/mp3gain/mp3gain-1_6_2-src.zip"; | ||
url = "mirror://sourceforge/mp3gain/${pname}-1_6_2-src.zip"; |
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.
url = "mirror://sourceforge/mp3gain/${pname}-1_6_2-src.zip"; | |
url = "mirror://sourceforge/mp3gain/${pname}-${builtins.replaceStrings ["."] ["_"] version}-src.zip"; |
I did not test 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.
This is also out of scope for this PR.
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.
Doesn't eval for the most part.
Fix: https://github.com/AndersonTorres/nixpkgs/pull/1
With my changes, Nixpkgs can be eval'd again and the sources that aren't already broken still produce the same output. Also, nit: Treewide -> treewide |
Oh man, this will not be easy. There are many "legitimate" uses of |
I think it's impossible for this to not trigger rebuilds since even in the simplest case, the builder gets after this change the new environment variable |
@doronbehar : then, in a certain sense, the expression itself becomes a "build input" of itself? And what is structuredAttrs? Well, in this sense I think we can sprint it to the next viable update. |
#72074 && https://nixos.mayflower.consulting/blog/2020/01/20/structured-attrs/
I'm not sure I understand your explanation, but to demonstrate how easily rebuilds can be triggered, consider a change such as: diff --git i/pkgs/development/compilers/gcc/9/default.nix w/pkgs/development/compilers/gcc/9/default.nix
index 305ed56df78..e39c787284b 100644
--- i/pkgs/development/compilers/gcc/9/default.nix
+++ w/pkgs/development/compilers/gcc/9/default.nix
@@ -83,6 +83,7 @@ in
stdenv.mkDerivation ({
pname = "${crossNameAddon}${name}${if stripped then "" else "-debug"}";
inherit version;
+ dummyVar = "bla";
builder = ../builder.sh;
That would rebuild the world, and you can see that via running The difference between the derivations can be found in the In general What's so frustrating about it is that our Even changes in the order of inputs in a Nix arrays causes rebuilds, consider this change: diff --git i/pkgs/development/compilers/gcc/9/default.nix w/pkgs/development/compilers/gcc/9/default.nix
index 305ed56df78..d73089e9c0c 100644
--- i/pkgs/development/compilers/gcc/9/default.nix
+++ w/pkgs/development/compilers/gcc/9/default.nix
@@ -168,7 +168,7 @@ stdenv.mkDerivation ({
++ optional targetPlatform.isLinux patchelf;
buildInputs = [
- gmp mpfr libmpc libelf
+ mpfr gmp libmpc libelf
targetPackages.stdenv.cc.bintools # For linking code at run-time
] ++ (optional (isl != null) isl)
++ (optional (zlib != null) zlib) It will also trigger a rebuild of the world. The problem is that structuredAttrs could perhaps allow us to write a |
@doronbehar order of build inputs matters since it can change the order setup hooks are run and we cannot guarantee arbitrary bash code commutes. Another example where it is a problem are environment variables like |
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 should not be merged until consensus is found in #103997.
This is just a parallel work for now, and way far from a clear finish. Anyway I will treat this as Draft for now. |
Another day, another huma nerror. Now I will try to recover. EDIT: is the interface broken? I can't reopen this! |
GH thinks you force-pushed to a commit already in master and, since that means there are no changes, won't let you open the PR. Should work again if you push another commit. Would probably also not be a bad idea to simply push commits and then squash them up once they're ready. Following changes between force-pushes isn't really something you can do very well in git. |
Not true as long as you don't do a rebase in the same force push. https://github.com/NixOS/nixpkgs/compare/32986af10085058e08125720467d84f5e1afe4b3..712c2405b1c4ddf461a1eafd640165583d1c0cca |
Reopening |
Motivation for this change
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)