.version: don't read from `.version` and deduplicate `.version-suffix` references #39416

Ma27 · 2018-04-24T18:37:02Z

Motivation for this change

See commit messages for further details.
The following changes were done:

Reverted newline fix I introduced this morning (using fileContents the problem doesn't exist anymore)
Deduplicated suffix logic (reading .version and .version-suffix is implemented in nixos/version and lib/trivial.nix).
Fixed .version reference in the pkgs.osquery derivation (using stdenv.lib.nixpkgsVersion now)

Running nix-build nixos/release.nix produces an output path with the proper version and suffix.

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- other Linux distributions
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

GrahamcOfBorg · 2018-04-24T18:46:53Z

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: osquery

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.

@infinisil

When reading the `nixpkgs` version from `.version` you always have a `\n` at the end because of the final newline. This issue exists since b7d15ed and had to be fixed several times according to the history of `.version`. Furthermore @infinisil recommended I explicitly configured `.editorconfig` to avoid newlines in `.version`.

GrahamcOfBorg · 2018-04-24T18:50:54Z

Success on x86_64-linux (full log)

Attempted: osquery

Partial log (click to expand)

post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/zih5v86ddlspnz6srbpmh5nhffcyvfb0-osquery-3.2.2
shrinking /nix/store/zih5v86ddlspnz6srbpmh5nhffcyvfb0-osquery-3.2.2/bin/osqueryd
shrinking /nix/store/zih5v86ddlspnz6srbpmh5nhffcyvfb0-osquery-3.2.2/bin/osqueryi
strip is /nix/store/j75dgadrff2d1fyc4fczmcgqkid2imdx-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/zih5v86ddlspnz6srbpmh5nhffcyvfb0-osquery-3.2.2/bin
patching script interpreter paths in /nix/store/zih5v86ddlspnz6srbpmh5nhffcyvfb0-osquery-3.2.2
/nix/store/zih5v86ddlspnz6srbpmh5nhffcyvfb0-osquery-3.2.2/etc/init.d/osqueryd: interpreter directive changed from "/bin/sh" to "/nix/store/xn5gv3lpfy91yvfy9b0i7klfcxh9xskz-bash-4.4-p19/bin/sh"
checking for references to /build in /nix/store/zih5v86ddlspnz6srbpmh5nhffcyvfb0-osquery-3.2.2...
/nix/store/zih5v86ddlspnz6srbpmh5nhffcyvfb0-osquery-3.2.2

fpletz · 2018-04-24T19:02:17Z

lib/trivial.nix

-    let suffixFile = ../.version-suffix; in
-    fileContents ../.version
-    + (if pathExists suffixFile then fileContents suffixFile else "pre-git");
+  nixpkgsVersion = version + suffix;


I think we should adopt a similar naming as the system.nixos NixOS module is using.

Suggestion:

version -> release

suffix -> versionSuffix

nixpkgsVersion -> version

Maybe prefix some or all with nixpkgs or nixos or split out an extra lib namespace for this kind of information?

Any more ideas?

A problem with the renaming is that e.g. the nix repo itself is referring to nixpkgsVersion, so it will become really painful.

I guess you could keep a compatibility alias for nixpkgsVersion at least.

Ma27 · 2018-04-26T08:35:20Z

@fpletz I just did the renames you suggested (and @globin agreed on). I thought that the ACK of two core maintainers should be sufficiently fair, it's currently in another commit to make reverts easier in case several folks are against that change %)

GrahamcOfBorg · 2018-04-26T08:48:59Z

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: osquery

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.

GrahamcOfBorg · 2018-04-26T19:19:50Z

Success on x86_64-linux (full log)

Attempted: osquery

Partial log (click to expand)

post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/c5lsaw4ixz0h315fys14bzrz4d6z53ya-osquery-3.2.2
shrinking /nix/store/c5lsaw4ixz0h315fys14bzrz4d6z53ya-osquery-3.2.2/bin/osqueryd
shrinking /nix/store/c5lsaw4ixz0h315fys14bzrz4d6z53ya-osquery-3.2.2/bin/osqueryi
strip is /nix/store/j75dgadrff2d1fyc4fczmcgqkid2imdx-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/c5lsaw4ixz0h315fys14bzrz4d6z53ya-osquery-3.2.2/bin
patching script interpreter paths in /nix/store/c5lsaw4ixz0h315fys14bzrz4d6z53ya-osquery-3.2.2
/nix/store/c5lsaw4ixz0h315fys14bzrz4d6z53ya-osquery-3.2.2/etc/init.d/osqueryd: interpreter directive changed from "/bin/sh" to "/nix/store/xn5gv3lpfy91yvfy9b0i7klfcxh9xskz-bash-4.4-p19/bin/sh"
checking for references to /build in /nix/store/c5lsaw4ixz0h315fys14bzrz4d6z53ya-osquery-3.2.2...
/nix/store/c5lsaw4ixz0h315fys14bzrz4d6z53ya-osquery-3.2.2

Ma27 · 2018-04-28T12:11:24Z

@dezgeg I guess you're right, we shouldn't break this espcially when it's in lib and nix references to this attribute.
I'll adjust my last commit to retain lib.nixpkgsVersion as alias with deprecation warning and write about it in the release notes :-)

This reverts commit e109784.

@dezgeg

This way easier to understand and the officially recommended approach. /cc @dezgeg @fpletz

The logic regarding the generated `.version-suffix` file is already defined in `lib/trivial.nix` and shouldn't be duplicated in `nixos/version`.

As suggested in NixOS#39416 (comment) the versioning attributes in `lib` should be consistent to `nixos/version` which implicates the following changes: * `lib.trivial.version` -> `lib.trivial.release` * `lib.trivial.suffix` -> `lib.trivial.versionSuffix` * `lib.nixpkgsVersion` -> `lib.version` As `lib.nixpkgsVersion` is referenced several times in `NixOS/nixpkgs`, `NixOS/nix` and probably several user's setups. As the rename will cause a notable impact it's better to keep `lib.nixpkgsVersion` as alias with a warning yielded by `builtins.trace`.

Ma27 · 2018-04-28T12:25:28Z

I added an alias as @dezgeg suggested, furthermore I rebased onto master as my changes in the release notes caused a conflict with master.

GrahamcOfBorg · 2018-04-28T12:38:05Z

Success on x86_64-linux (full log)

Attempted: osquery

Partial log (click to expand)

/nix/store/c5lsaw4ixz0h315fys14bzrz4d6z53ya-osquery-3.2.2

GrahamcOfBorg · 2018-04-28T12:38:44Z

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: osquery

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.

As suggested in NixOS#39416 (comment) the versioning attributes in `lib` should be consistent to `nixos/version` which implicates the following changes: * `lib.trivial.version` -> `lib.trivial.release` * `lib.trivial.suffix` -> `lib.trivial.versionSuffix` * `lib.nixpkgsVersion` -> `lib.version` As `lib.nixpkgsVersion` is referenced several times in `NixOS/nixpkgs`, `NixOS/nix` and probably several user's setups. As the rename will cause a notable impact it's better to keep `lib.nixpkgsVersion` as alias with a warning yielded by `builtins.trace`.

@edolstra

Originally introduced in NixOS#39416 (1), however it causes confusion and shouldn't be deprecated (2)(3). /cc @edolstra (1) NixOS#39416 (comment) (2) NixOS@9274ea3#commitcomment-29951594 (3) NixOS/nix#2291

FRidh · 2019-07-13T08:06:25Z

It happened now several times that lib.version ended up in a derivation because rec was not used. Yes, the use of with lib can be dangerous, but obtaining a version attribute from it is really annoying. We should not bring everything that is in lib in a certain namespace into top-level lib.

I very much want to get rid of lib.version in #64693.

Ma27 requested review from edolstra and nbp as code owners April 24, 2018 18:37

GrahamcOfBorg added 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Apr 24, 2018

fpletz reviewed Apr 24, 2018

View reviewed changes

GrahamcOfBorg added the 8.has: documentation label Apr 26, 2018

globin approved these changes Apr 26, 2018

View reviewed changes

Ma27 force-pushed the fix-.version-config branch from 7d8fe73 to 49f9fa4 Compare April 28, 2018 12:22

Ma27 added 4 commits April 28, 2018 14:23

Revert ".version: remove final newline"

9f2b67d

This reverts commit e109784.

osquery: use stdenv.lib.nixpkgsVersion

f66cdc7

This way easier to understand and the officially recommended approach. /cc @dezgeg @fpletz

lib: deduplicate version/suffix references

3990928

The logic regarding the generated `.version-suffix` file is already defined in `lib/trivial.nix` and shouldn't be duplicated in `nixos/version`.

Ma27 force-pushed the fix-.version-config branch from 49f9fa4 to 9274ea3 Compare April 28, 2018 12:24

GrahamcOfBorg added the 8.has: changelog label Apr 28, 2018

7c6f434c merged commit fd8dcdf into NixOS:master Apr 30, 2018

Ma27 deleted the fix-.version-config branch April 30, 2018 09:01

Ma27 mentioned this pull request Aug 5, 2018

lib/trivial: remove deprecation warning for lib.nixpkgsVersion. #44498

Closed

9 tasks

edolstra mentioned this pull request Jul 31, 2019

nix-channel man: lib.nixpkgsVersion -> lib.version NixOS/nix#3015

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

.version: don't read from `.version` and deduplicate `.version-suffix` references #39416

.version: don't read from `.version` and deduplicate `.version-suffix` references #39416

Ma27 commented Apr 24, 2018 •

edited

GrahamcOfBorg commented Apr 24, 2018

GrahamcOfBorg commented Apr 24, 2018

fpletz Apr 24, 2018

dezgeg Apr 28, 2018

dezgeg Apr 28, 2018

Ma27 commented Apr 26, 2018

GrahamcOfBorg commented Apr 26, 2018

GrahamcOfBorg commented Apr 26, 2018

Ma27 commented Apr 28, 2018

Ma27 commented Apr 28, 2018

GrahamcOfBorg commented Apr 28, 2018

GrahamcOfBorg commented Apr 28, 2018

FRidh commented Jul 13, 2019

.version: don't read from .version and deduplicate .version-suffix references #39416

.version: don't read from .version and deduplicate .version-suffix references #39416

Conversation

Ma27 commented Apr 24, 2018 • edited

Motivation for this change

Things done

GrahamcOfBorg commented Apr 24, 2018

GrahamcOfBorg commented Apr 24, 2018

fpletz Apr 24, 2018

Choose a reason for hiding this comment

dezgeg Apr 28, 2018

Choose a reason for hiding this comment

dezgeg Apr 28, 2018

Choose a reason for hiding this comment

Ma27 commented Apr 26, 2018

GrahamcOfBorg commented Apr 26, 2018

GrahamcOfBorg commented Apr 26, 2018

Ma27 commented Apr 28, 2018

Ma27 commented Apr 28, 2018

GrahamcOfBorg commented Apr 28, 2018

GrahamcOfBorg commented Apr 28, 2018

FRidh commented Jul 13, 2019

.version: don't read from `.version` and deduplicate `.version-suffix` references #39416

.version: don't read from `.version` and deduplicate `.version-suffix` references #39416

Ma27 commented Apr 24, 2018 •

edited