Mic92 / nixpkgs-review

Review pull-requests on https://github.com/NixOS/nixpkgs
MIT License
356 stars 59 forks source link

build and symlink each output of the packages #307

Closed figsoda closed 1 year ago

figsoda commented 1 year ago

fixes #306

the output paths will be similar to the regular nix results, the following results are tested with https://github.com/NixOS/nixpkgs/pull/215728

4 packages built:
geogram geogram.bin geogram.dev geogram.lib

$ tree results
results
├── geogram -> /nix/store/jd1ys0jn7sp9bw647ch8ydp5ic3l09g6-geogram-1.8.2
├── geogram.bin -> /nix/store/j64i56kvgg5wp5m1snhxwmm9wcbg2zm1-geogram-1.8.2-bin
├── geogram.dev -> /nix/store/cr6wlvc22gj072r0fw0j52kbmqmpr6my-geogram-1.8.2-dev
└── geogram.lib -> /nix/store/299za097p633g4w1ig63g8k5zcf86nk6-geogram-1.8.2-lib
davidak commented 1 year ago

The result is different.

[davidak@gaming:~/code/nixpkgs]$ nix-build . -A openssl.all
these 3 paths will be fetched (10.30 MiB download, 17.28 MiB unpacked):
  /nix/store/mqrkghijrs8cl35z6pqgm79qljwj24pd-openssl-3.0.8-doc
  /nix/store/qpkc8189fwa37faanivhfz5sdjnsfi33-openssl-3.0.8-man
  /nix/store/v84frgj57lkz1svhlsnbdrd20lgak4ms-openssl-3.0.8-debug
copying path '/nix/store/qpkc8189fwa37faanivhfz5sdjnsfi33-openssl-3.0.8-man' from 'https://cache.nixos.org'...
copying path '/nix/store/v84frgj57lkz1svhlsnbdrd20lgak4ms-openssl-3.0.8-debug' from 'https://cache.nixos.org'...
copying path '/nix/store/mqrkghijrs8cl35z6pqgm79qljwj24pd-openssl-3.0.8-doc' from 'https://cache.nixos.org'...
/nix/store/69cw78hmw7lsgjicxyp7dcky372fb4dy-openssl-3.0.8-bin
/nix/store/81h1lh105i20xyks9qw1c58bim79jmvx-openssl-3.0.8-dev
/nix/store/dsf1m9azqqz6c3nqj9yk0nnardqmaia0-openssl-3.0.8
/nix/store/qpkc8189fwa37faanivhfz5sdjnsfi33-openssl-3.0.8-man
/nix/store/mqrkghijrs8cl35z6pqgm79qljwj24pd-openssl-3.0.8-doc
/nix/store/v84frgj57lkz1svhlsnbdrd20lgak4ms-openssl-3.0.8-debug

[davidak@gaming:~/code/nixpkgs]$ ll
total 84
-rw-r--r--  1 davidak users 8391 Feb 15 18:21 CONTRIBUTING.md
-rw-r--r--  1 davidak users 1097 Feb 15 18:21 COPYING
-rw-r--r--  1 davidak users  971 May 28  2022 default.nix
drwxr-xr-x 12 davidak users 4096 Feb 15 18:21 doc
-rw-r--r--  1 davidak users 2266 Feb 15 18:21 flake.nix
drwxr-xr-x  5 davidak users 4096 Feb 15 18:21 lib
drwxr-xr-x  3 davidak users 4096 Feb 15 18:21 maintainers
drwxr-xr-x  7 davidak users 4096 Feb 15 18:21 nixos
drwxr-xr-x 18 davidak users 4096 May 28  2022 pkgs
-rw-r--r--  1 davidak users 6189 Feb 15 18:21 README.md
lrwxrwxrwx  1 davidak users   57 Feb 15 23:19 result -> /nix/store/dsf1m9azqqz6c3nqj9yk0nnardqmaia0-openssl-3.0.8
lrwxrwxrwx  1 davidak users   61 Feb 15 23:19 result-bin -> /nix/store/69cw78hmw7lsgjicxyp7dcky372fb4dy-openssl-3.0.8-bin
lrwxrwxrwx  1 davidak users   63 Feb 15 23:19 result-debug -> /nix/store/v84frgj57lkz1svhlsnbdrd20lgak4ms-openssl-3.0.8-debug
lrwxrwxrwx  1 davidak users   61 Feb 15 23:19 result-dev -> /nix/store/81h1lh105i20xyks9qw1c58bim79jmvx-openssl-3.0.8-dev
lrwxrwxrwx  1 davidak users   61 Feb 15 23:19 result-doc -> /nix/store/mqrkghijrs8cl35z6pqgm79qljwj24pd-openssl-3.0.8-doc
lrwxrwxrwx  1 davidak users   61 Feb 15 23:19 result-man -> /nix/store/qpkc8189fwa37faanivhfz5sdjnsfi33-openssl-3.0.8-man

Can you make it consistent?

How can i test this easily?

figsoda commented 1 year ago

The result is different.

If you are referring to the change from - to ., this is intentional to make sure the attributes don't conflict with each other, since even though unlikely, it is still technically possible for e.g. openssl-lib to be a valid attribute that evaluates something other than openssl.lib

How can i test this easily?

You can test it on a PR with something like this

nix run github:figsoda/nixpkgs-review/outputs pr 215728

If you want to test it locally, just add a space in one of the phases and run it with wip or rev HEAD

davidak commented 1 year ago

I like having one results directory instead of many cluttering the current one. Maybe Nix should do it this way as well? Do you want to bring your argument also there?

figsoda commented 1 year ago

Requesting another review just to triple check that I don't break anything

Mic92 commented 1 year ago

bors merge

bors[bot] commented 1 year ago

Build succeeded: