Try pkg.meta.position before unsafeGetAttrPos #247
Conversation
pbsds
force-pushed
the
use-meta-position
branch
2 times, most recently
from
e2d9ad0 to
1aede84
Compare
nix_update/eval.py
Outdated
| else if (builtins.unsafeGetAttrPos "src" pkg) != null then | ||
| sanitizePosition (builtins.unsafeGetAttrPos "src" pkg) | ||
| else | ||
| sanitizePosition (positionFromMeta pkg); |
There was a problem hiding this comment.
If we have meta.position, should it not be preferred over src?
There was a problem hiding this comment.
i was trying to change as little of existing behaviour as possible, by if you're on board with this change then why not.
pbsds
changed the title
nix_update/eval.py
Outdated
| @@ -142,13 +142,22 @@ def eval_expression( | |||
| return f""" | |||
| let | |||
| {indent(dedent(let_bindings), " ")} | |||
| positionFromMeta = pkg: let | |||
| parts = builtins.match "(.*):([0-9]+)" pkg.meta.position; | |||
| in if pkg ? meta.position then {{ | |||
There was a problem hiding this comment.
Do we need this if here? It's already checked on line 158. Also would make the code slightly simpler.
nix_update/eval.py
Outdated
| raw_version_position = sanitizePosition (builtins.unsafeGetAttrPos "version" pkg); | ||
|
|
||
| position = if pkg ? isRubyGem then | ||
| raw_version_position | ||
| else if pkg ? isPhpExtension then | ||
| raw_version_position | ||
| else | ||
| else if pkg ? meta.position then | ||
| sanitizePosition (positionFromMeta pkg) |
There was a problem hiding this comment.
I think we can even overrule the quirks for ruby gems and php here, I would say.
pbsds
changed the title
Fixes `nixpkgs.mpvScripts.*`, discovered while reviewing NixOS/nixpkgs#308062
|
@mergify queue |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at 63b5a4e |
|
what is meta.position set to in this case? |
|
To be more specific, here's my (Nixpkgs) patching attempt of trying to set the diff --git i/pkgs/applications/radio/gnuradio/wrapper.nix w/pkgs/applications/radio/gnuradio/wrapper.nix
index f468aadb27b5..107efe90bc80 100644
--- i/pkgs/applications/radio/gnuradio/wrapper.nix
+++ w/pkgs/applications/radio/gnuradio/wrapper.nix
@@ -163,6 +163,14 @@ let
;
pkgs = packages;
};
+ meta = unwrapped.meta // {
+ position = ./. + (
+ if unwrapped.versionAttr.major == "3.10" then
+ "default"
+ else
+ unwrapped.versionAttr.major
+ ) + ".nix:0";
+ };
self = if doWrap then
stdenv.mkDerivation {
inherit pname version passthru;
@@ -197,11 +205,12 @@ let
wrapProgram "$i" ${makeWrapperArgs}
done
'';
- inherit (unwrapped) meta;
+ inherit meta;
}
else
unwrapped.overrideAttrs(_: {
inherit passthru;
+ inherit meta;
})
;
in selfWith and without this patch, this command prints the same output: nix eval --json -L --impure --expr '
let
pkgs = (builtins.getFlake "'$PWD'").legacyPackages.${builtins.currentSystem};
in
pkgs.gnuradio.meta.position
' | jq --raw-output '.'Output: Instead of the expected: |
|
Does lib.traceVal print your |
|
If you meant this: nix eval --json -L --impure --expr '
let
pkgs = (builtins.getFlake "'$PWD'").legacyPackages.${builtins.currentSystem};
in
pkgs.lib.traceVal pkgs.gnuradio.meta.position
' | jq --raw-output '.'Then the output is: |
|
Even for the simplest package: diff --git i/pkgs/by-name/he/hello/package.nix w/pkgs/by-name/he/hello/package.nix
index 7b8db4c7c3c2..c96f372399d8 100644
--- i/pkgs/by-name/he/hello/package.nix
+++ w/pkgs/by-name/he/hello/package.nix
@@ -52,6 +52,7 @@ stdenv.mkDerivation (finalAttrs: {
license = licenses.gpl3Plus;
maintainers = [ maintainers.eelco ];
mainProgram = "hello";
+ position = "bla.nix";
platforms = platforms.all;
};
})This command: nix eval --json -L --impure --expr '
let
pkgs = (builtins.getFlake "'$PWD'").legacyPackages.${builtins.currentSystem};
in
pkgs.lib.traceVal pkgs.hello.meta.position
' | jq --raw-output '.'Prints: |
|
we could move the |
That would work for me probably. I hope though that it won't break your setup with the |
|
It seems as of recent that mkDerivation overrides meta.position: Proposed fix: NixOS/nixpkgs#329900 |
| raw_version_position | ||
| else if pkg ? isPhpExtension then | ||
| raw_version_position | ||
| else | ||
| else | ||
| sanitizePosition (builtins.unsafeGetAttrPos "src" pkg); |
There was a problem hiding this comment.
Indeed if it is possible to set meta.position, this nix-update PR here makes a bit more sense. However, since mkDerivation always sets meta.position, the pkg.src attribute will never be retrieved here. Or perhaps it will be retrieved for other derivation builders? I doubt if there are builders that are not based on mkDerivation for which it will happen.
|
This also break packages defined by nix-update 0.14.0: nix-update 0.13.1: Notice nix-update 0.14.0 try to update file While nix-update 0.13.0 updates file |
|
I revert back to my initial proposal in #267, please test it out and give us feedback :) |
Some package builders set
meta.positionmanually due to clobbering the origin ofsrc.Fixes
nix-update -u mpvScripts.*, discovered while reviewing NixOS/nixpkgs#308062.