[WIP/RFC] make-derivation: default name if pname and version are both set #41627

Synthetica9 · 2018-06-07T11:24:08Z

Motivation for this change

This is done in, for example, buildPythonPackage

TODO

…ersion

…omeSource

dtzWill · 2018-06-07T12:45:52Z

I'm not sure how important it is but I think it's common for these to be rec only for this reason, making them non-recursive would be nice: simpler for human understanding, friendlier to eval at least in theory.

matthewbauer

Definitely looks like something very useful! pname/version is what lots of packages are already using so hopefully everyone is okay with this.

matthewbauer · 2018-06-07T13:56:06Z

pkgs/stdenv/generic/make-derivation.nix

@@ -12,7 +12,9 @@ rec {
  # * https://nixos.org/nix/manual/#ssec-derivation
  #   Explanation about derivations in general
  mkDerivation =
-    { name ? ""
+    { name ? if builtins.hasAttr "pname" attrs && builtins.hasAttr "version" attrs


Can you update "pos" above as well? It is used to find line numbers in traces. I think we need to also look for "version" before name for this to work correctly.

Would that be something like builtins.unsafeGetAttrPos "pname" attrs?

Yeah but I actually think version would be better because it's more frequently used than pname.

Okay, I'll add it.

Does 0c1d5d1 look good to you?

I'd like to have an assertion for name == "${pname}-${version}" if all of the three are set

Oh, yeah, that would make sense. Adding that.

@globin e0d2348

This should be attrs ? pname && attrs ? version.

… version

Synthetica9 · 2018-06-07T15:00:45Z

e0d2348 seems to break Python packages, since they define the following:

name: python2.7-Twisted-17.9.0
pname: Twisted
version: 17.9.0

What should we do with that?

@globin
Edit: adding an explicit exception (hasPrefix "python" attrs.name) seems to work, is that the way to go?

matthewbauer · 2018-06-07T15:17:32Z

@globin I suspect we should document this as well?

matthewbauer · 2018-06-07T15:41:00Z

Try removing pname & version from attrs in python here:

https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/interpreters/python/mk-python-derivation.nix#L68

Synthetica9 · 2018-06-07T15:46:46Z

@matthewbauer Looks like a good first step, but crashes here (and someone foresaw it :P):

https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/python-modules/setuptools/default.nix#L8

matthewbauer · 2018-06-07T15:49:52Z

Yeah we might be able to just remove the prefix there as well.

/cc @FRidh

FRidh · 2018-06-07T16:00:47Z

I'm of the opinion version should not actually be an attribute of the derivation but instead of the source.

matthewbauer · 2018-06-07T16:03:12Z

I'm of the opinion version should not actually be an attribute of the derivation but instead of the source.

That's a good point - but the way Nixpkgs works now is that version is an attribute of the derivation through name - this just lets you separate those two out.

FRidh · 2018-06-07T16:04:56Z

That's a good point - but the way Nixpkgs works now is that version is an attribute of the derivation through name - this just lets you separate those two out.

We include it in the name, but it is not an attribute. Therefore, it is possible to have name = "${pname}-${src.version}.

FRidh · 2018-06-07T16:07:33Z

I've pushed b5d7bb4 (removing name from Python packages) to staging.

FRidh · 2018-06-07T16:11:16Z

Try removing pname & version from attrs in python here:

Looks like a good first step, but crashes here (and someone foresaw it :P):

In the case of Python, you could set

pname = "${python.libPrefix}-${pname}"

and remove the name and version parameters.

Synthetica9 · 2018-06-07T16:54:28Z

@FRidh

The following should also work I think:

let
  pname = "setuptools";
  version = "39.0.1";
in stdenv.mkDerivation rec {
  name = "${python.libPrefix}-${pname}-${version}";

…tch source

Synthetica9 · 2018-06-07T17:24:16Z

Try removing pname & version from attrs in python here:

@matthewbauer Actually, that also seems to break things:

https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/python-modules/cryptography/default.nix#L25

too greedy find-replace

Synthetica9 · 2018-06-07T19:27:34Z

I think I'm starting to come to the conclusion that dropping pname and/or version from pythonpackages is just a bad idea (people find creative ways to use both), and we should just change the assert to hasSuffix name "${pname}-${version}"

@matthewbauer @FRidh

FRidh · 2018-06-08T11:53:40Z

I think I'm starting to come to the conclusion that dropping pname and/or version from pythonpackages is just a bad idea (people find creative ways to use both), and we should just change the assert to hasSuffix name "${pname}-${version}"

You are referring to the following code:

  psutil_1 = self.psutil.overrideAttrs (oldAttrs: rec {
    name = "${oldAttrs.pname}-${version}";
    version = "1.2.1";
    src = oldAttrs.src.override {
      inherit version;
      sha256 = "0ibclqy6a4qmkjhlk3g8jhpvnk0v9aywknc61xm3hfi5r124m3jh";
    };
});

Here they need to set name because they use overrideAttrs which overrides the call to stdenv.mkDerivation, and creation of name is done in buildPythonPackage. Instead, if overridePythonAttrs is used, then there would not have been any need for setting name.

With your proposed change, stdenv.mkDerivation will fix name, so then they technically could use overrideAttrs and without passing in a custom name. So, it's not that dramatic at all.

If you like to I can have a look at how buildPython* needs to be changed. Tomorrow I should have time for it.

edolstra · 2018-06-08T12:49:29Z

A fundamental change like this is something that should be done via the RFC process, IMHO.

GrahamcOfBorg · 2018-06-08T12:57:43Z

Failure on x86_64-linux (full log)

Attempted: ibus

Partial log (click to expand)

cannot build derivation '/nix/store/fyhirxcdx0qr8fp23r2hg6wgjd6k556x-gtk+-2.24.32.drv': 5 dependencies couldn't be built
cannot build derivation '/nix/store/jkm8kzr36nnn7lsbsvliny9nnzc5fxja-gtk+3-3.22.30.drv': 10 dependencies couldn't be built
cannot build derivation '/nix/store/6lh2nrixpfdzpsriid7fi8f2klp2i1rj-libgsf-1.14.42.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/yjz4ai6nk0a02amqji2z696cnmk10add-libnotify-0.7.7.drv': 2 dependencies couldn't be built
building of '/nix/store/wzdajvsb9ha87a5y3vfr8ipyfip88jpc-rustc-1.25.0.drv' timed out after 4000 seconds
cannot build derivation '/nix/store/6qxx6yqysikx04yrb2183k2z466pxqdb-cargo-0.26.0.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/gq9pfw9gllzd8xvwi5kvm89gjz16a4xz-librsvg-2.42.4.drv': 8 dependencies couldn't be built
cannot build derivation '/nix/store/k127j3g713zjn8wacwiqnax3dnanm20h-hook.drv': 3 dependencies couldn't be built
cannot build derivation '/nix/store/7lai30gy6z5sn3wsi4snplq7c5g68i7x-ibus-1.5.17.drv': 12 dependencies couldn't be built
error: build of '/nix/store/7lai30gy6z5sn3wsi4snplq7c5g68i7x-ibus-1.5.17.drv' failed

GrahamcOfBorg · 2018-06-08T13:49:25Z

Failure on aarch64-linux (full log)

Attempted: ibus

Partial log (click to expand)

head: write error: Broken pipe
checking for references to /build in /nix/store/mc25k6xxy1wxz0p630fwglrmi4ri788r-vala-0.38.9...
shrinking RPATHs of ELF executables and libraries in /nix/store/l75gy3lykywjjzs76gzmasa65cr29f3l-vala-0.38.9-devdoc
strip is /nix/store/3ki967c3ij4zi1q9ycc3g1vfwr608nrb-binutils-2.30/bin/strip
patching script interpreter paths in /nix/store/l75gy3lykywjjzs76gzmasa65cr29f3l-vala-0.38.9-devdoc
checking for references to /build in /nix/store/l75gy3lykywjjzs76gzmasa65cr29f3l-vala-0.38.9-devdoc...
cannot build derivation '/nix/store/mnk1w6513fp3l0pxkcp1fci7nck96kd9-librsvg-2.42.4.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/xkxygk167rq52xdgxqlrligsjpw0rrfv-hook.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/8cb2gdbazbwa6a0spqc0i7n6p9d69wnp-ibus-1.5.17.drv': 1 dependencies couldn't be built
error: build of '/nix/store/8cb2gdbazbwa6a0spqc0i7n6p9d69wnp-ibus-1.5.17.drv' failed

GrahamcOfBorg · 2018-06-08T14:16:05Z

Failure on aarch64-linux (full log)

Attempted: ibus

Partial log (click to expand)

   Compiling rls-span v0.4.0
   Compiling rls-data v0.15.0
   Compiling rustc_errors v0.0.0 (file:///build/rustc-1.25.0-src/src/librustc_errors)
   Compiling backtrace v0.3.5
building of '/nix/store/kp3saf9dwqpf3pna959fvacv3f428lax-rustc-1.25.0.drv' timed out after 3600 seconds
cannot build derivation '/nix/store/gma90vw0ym5d85k8my6r7k5k1g9600x9-cargo-0.26.0.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/mnk1w6513fp3l0pxkcp1fci7nck96kd9-librsvg-2.42.4.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/xkxygk167rq52xdgxqlrligsjpw0rrfv-hook.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/8cb2gdbazbwa6a0spqc0i7n6p9d69wnp-ibus-1.5.17.drv': 1 dependencies couldn't be built
error: build of '/nix/store/8cb2gdbazbwa6a0spqc0i7n6p9d69wnp-ibus-1.5.17.drv' failed

GrahamcOfBorg · 2018-06-08T14:48:31Z

Failure on x86_64-linux (full log)

Attempted: ibus

Partial log (click to expand)

shrinking /nix/store/zbg4wh6fw3h48my03s1vsv5fpfz2dj1p-libgit2-0.26.0/lib/libgit2.so.0.26.0
strip is /nix/store/4mf2xm9p32lzrim927yk92xhx35yaz62-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/zbg4wh6fw3h48my03s1vsv5fpfz2dj1p-libgit2-0.26.0/lib
patching script interpreter paths in /nix/store/zbg4wh6fw3h48my03s1vsv5fpfz2dj1p-libgit2-0.26.0
checking for references to /build in /nix/store/zbg4wh6fw3h48my03s1vsv5fpfz2dj1p-libgit2-0.26.0...
cannot build derivation '/nix/store/6qxx6yqysikx04yrb2183k2z466pxqdb-cargo-0.26.0.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/gq9pfw9gllzd8xvwi5kvm89gjz16a4xz-librsvg-2.42.4.drv': 8 dependencies couldn't be built
cannot build derivation '/nix/store/k127j3g713zjn8wacwiqnax3dnanm20h-hook.drv': 3 dependencies couldn't be built
cannot build derivation '/nix/store/7lai30gy6z5sn3wsi4snplq7c5g68i7x-ibus-1.5.17.drv': 12 dependencies couldn't be built
error: build of '/nix/store/7lai30gy6z5sn3wsi4snplq7c5g68i7x-ibus-1.5.17.drv' failed

Synthetica9 · 2018-10-16T11:47:27Z

Superseded by NixOS/rfcs#35

Synthetica9 added 6 commits June 6, 2018 22:04

make-derivation: use pname-version as default name if both are present

c313e07

treewide: removed name from a few pythonpackages defining pname and v…

b5d7bb4

…ersion

treewide: removed name from packages that already defined pname and v…

ef6ece0

…ersion

gnome3: don't re-define lib.versions.majorMinor and implement fetchGn…

af1b4f4

…omeSource

gnome3: make some refinements to fetchGnomeSource

e3d9978

Make fetchGnomeSource availabable as top-level pkg infra

02f45b6

Synthetica9 requested a review from FRidh as a code owner June 7, 2018 11:24

GrahamcOfBorg added 6.topic: emacs 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: python 6.topic: stdenv Standard environment labels Jun 7, 2018

Synthetica9 changed the title ~~[WIP] make-derivation: default name if pname and version are both set~~ [WIP/RFC] make-derivation: default name if pname and version are both set Jun 7, 2018

GrahamcOfBorg added 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100 labels Jun 7, 2018

matthewbauer requested changes Jun 7, 2018

View reviewed changes

make-derivation: get position info from version

0c1d5d1

matthewbauer approved these changes Jun 7, 2018

View reviewed changes

make-derivation: add check that the name is consistent with pname and…

e0d2348

… version

Synthetica9 added 2 commits June 7, 2018 18:55

make-derivation: fix for python packages

6bc5d26

fictx: remove name from all derivations, add a consistent way to fe…

0f5c2ae

…tch source

mk-python-derivation: re-add version as argument to mkDerivation

33932e3

Synthetica9 force-pushed the pname-version-default branch 2 times, most recently from 952049c to 2b1c0fc Compare June 7, 2018 17:58

fcitx-unikey: split name into pname and version

b03d631

Synthetica9 force-pushed the pname-version-default branch from 2b1c0fc to b03d631 Compare June 7, 2018 18:14

Synthetica9 added 4 commits June 7, 2018 20:17

pip: fix bootstrapped-pip

7cd8034

fcitx-qt5: fix

a2d4ae2

fcitx-libpinyin: fix

b6d8df3

too greedy find-replace

fcitx-mozc: fix

44be91f

mk-python-derivation: try a different fix

ee16a1d

GrahamcOfBorg added 8.has: clean-up 10.rebuild-darwin: 501+ 10.rebuild-linux: 501+ and removed 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100 labels Jun 7, 2018

ibus: fix build

26adf6f

Synthetica9 added 2 commits June 8, 2018 13:54

pkgs/applications/altcoins: fixup names

f8e3964

pkgs/applications/audio: fixup names

0786975

Synthetica9 closed this Oct 16, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP/RFC] make-derivation: default name if pname and version are both set #41627

[WIP/RFC] make-derivation: default name if pname and version are both set #41627

Synthetica9 commented Jun 7, 2018 •

edited

dtzWill commented Jun 7, 2018

matthewbauer left a comment

matthewbauer Jun 7, 2018

Synthetica9 Jun 7, 2018

matthewbauer Jun 7, 2018

Synthetica9 Jun 7, 2018

Synthetica9 Jun 7, 2018

globin Jun 7, 2018

Synthetica9 Jun 7, 2018

Synthetica9 Jun 7, 2018

edolstra Oct 29, 2018

Synthetica9 commented Jun 7, 2018 •

edited

matthewbauer commented Jun 7, 2018

matthewbauer commented Jun 7, 2018

Synthetica9 commented Jun 7, 2018

matthewbauer commented Jun 7, 2018

FRidh commented Jun 7, 2018

matthewbauer commented Jun 7, 2018

FRidh commented Jun 7, 2018

FRidh commented Jun 7, 2018

FRidh commented Jun 7, 2018

Synthetica9 commented Jun 7, 2018

Synthetica9 commented Jun 7, 2018 •

edited

Synthetica9 commented Jun 7, 2018 •

edited

FRidh commented Jun 8, 2018 •

edited

edolstra commented Jun 8, 2018

GrahamcOfBorg commented Jun 8, 2018

GrahamcOfBorg commented Jun 8, 2018

GrahamcOfBorg commented Jun 8, 2018

GrahamcOfBorg commented Jun 8, 2018

Synthetica9 commented Oct 16, 2018

[WIP/RFC] make-derivation: default name if pname and version are both set #41627

[WIP/RFC] make-derivation: default name if pname and version are both set #41627

Conversation

Synthetica9 commented Jun 7, 2018 • edited

Motivation for this change

TODO

dtzWill commented Jun 7, 2018

matthewbauer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Synthetica9 commented Jun 7, 2018 • edited

matthewbauer commented Jun 7, 2018

matthewbauer commented Jun 7, 2018

Synthetica9 commented Jun 7, 2018

matthewbauer commented Jun 7, 2018

FRidh commented Jun 7, 2018

matthewbauer commented Jun 7, 2018

FRidh commented Jun 7, 2018

FRidh commented Jun 7, 2018

FRidh commented Jun 7, 2018

Synthetica9 commented Jun 7, 2018

Synthetica9 commented Jun 7, 2018 • edited

Synthetica9 commented Jun 7, 2018 • edited

FRidh commented Jun 8, 2018 • edited

edolstra commented Jun 8, 2018

GrahamcOfBorg commented Jun 8, 2018

GrahamcOfBorg commented Jun 8, 2018

GrahamcOfBorg commented Jun 8, 2018

GrahamcOfBorg commented Jun 8, 2018

Synthetica9 commented Oct 16, 2018

Synthetica9 commented Jun 7, 2018 •

edited

Synthetica9 commented Jun 7, 2018 •

edited

Synthetica9 commented Jun 7, 2018 •

edited

Synthetica9 commented Jun 7, 2018 •

edited

FRidh commented Jun 8, 2018 •

edited