Skip to content
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

a single version attribute for expressions previously using "majorVersion" #36168

Merged
merged 2 commits into from Mar 4, 2018

Conversation

ryantm
Copy link
Member

@ryantm ryantm commented Mar 1, 2018

Many packages in nixpkgs split the version string into a "major" and "minor" part because the upstream source distribution URL is something like "/project/2.3/projectname-2.3.4.tar.gz". This PR introduces a new function majorMinorVersion that lets us declare the version with a single attribute, allowing for easier expression updating via tooling.

Say the package is at version "1.2.3", the expressions currently use the terminology majorVersion to mean "1.2", and minorVersion to mean "3". I chose to use the Semantic Versioning terminology in this function, which means majorMinorVersion refers to "1.2" ("3" would be the "patch" part of the version, but isn't relevant to this). I don't care too much about the name, though I did think about it carefully, so if someone some other preference that's fine.

This PR includes an example usage of majorMinorVersion in the haproxy package.

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Partial log (click to expand)

for x in intro SPOE close-options netscaler-client-ip-insertion-protocol 51Degrees-device-detection DeviceAtlas-device-detection network-namespaces linux-syn-cookies proxy-protocol WURFL-device-detection lua cookie-options architecture management configuration; do \
	install -m 644 doc/$x.txt "/nix/store/ly8avzlpfmhid434cpks1nqi65lhr05v-haproxy-1.7.9/doc/haproxy" ; \
done
post-installation fixup
moving /nix/store/ly8avzlpfmhid434cpks1nqi65lhr05v-haproxy-1.7.9/doc to /nix/store/ly8avzlpfmhid434cpks1nqi65lhr05v-haproxy-1.7.9/share/doc
gzipping man pages under /nix/store/ly8avzlpfmhid434cpks1nqi65lhr05v-haproxy-1.7.9/share/man/
strip is /nix/store/4sdh09gmvl15cy0zb6i7mbvxh5syz206-cctools-binutils-darwin/bin/strip
stripping (with command strip and flags -S) in /nix/store/ly8avzlpfmhid434cpks1nqi65lhr05v-haproxy-1.7.9/sbin
patching script interpreter paths in /nix/store/ly8avzlpfmhid434cpks1nqi65lhr05v-haproxy-1.7.9
moving /nix/store/ly8avzlpfmhid434cpks1nqi65lhr05v-haproxy-1.7.9/sbin/* to /nix/store/ly8avzlpfmhid434cpks1nqi65lhr05v-haproxy-1.7.9/bin

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Partial log (click to expand)

shrinking RPATHs of ELF executables and libraries in /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9
shrinking /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/sbin/haproxy
shrinking /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/sbin/haproxy-systemd-wrapper
gzipping man pages under /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/share/man/
strip is /nix/store/b0zlxla7dmy1iwc3g459rjznx59797xy-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/sbin 
patching script interpreter paths in /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9
checking for references to /tmp/nix-build-haproxy-1.7.9.drv-0 in /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9...
moving /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/sbin/* to /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/bin
/nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Partial log (click to expand)

shrinking RPATHs of ELF executables and libraries in /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9
shrinking /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9/sbin/haproxy-systemd-wrapper
shrinking /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9/sbin/haproxy
gzipping man pages under /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9/share/man/
strip is /nix/store/lvx1acn1ig1j2km8jds5x3ggh3f2wa8v-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9/sbin
patching script interpreter paths in /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9
checking for references to /build in /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9...
moving /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9/sbin/* to /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9/bin
/nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Partial log (click to expand)

/nix/store/ly8avzlpfmhid434cpks1nqi65lhr05v-haproxy-1.7.9

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Partial log (click to expand)

/nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Partial log (click to expand)

shrinking RPATHs of ELF executables and libraries in /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9
shrinking /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/sbin/haproxy
shrinking /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/sbin/haproxy-systemd-wrapper
gzipping man pages under /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/share/man/
strip is /nix/store/b0zlxla7dmy1iwc3g459rjznx59797xy-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/sbin 
patching script interpreter paths in /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9
checking for references to /tmp/nix-build-haproxy-1.7.9.drv-0 in /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9...
moving /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/sbin/* to /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/bin
/nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9

@jtojnar
Copy link
Contributor

jtojnar commented Mar 1, 2018

See also gnome3.versionBranch from recently merged #33086

@ryantm
Copy link
Member Author

ryantm commented Mar 1, 2018

@jtojnar Cool, that's the same function. What do you think of this change?

lib/strings.nix Outdated
majorMinorVersion "1.2.3"
=> "1.2"
*/
majorMinorVersion = v: builtins.concatStringsSep "." (lib.lists.take 2 (splitString "." v));
Copy link
Member

@Mic92 Mic92 Mar 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about lib.semver.majorVersion and lib.semver.minorVersion or is this too verbose?
but maybe this also causes ambiguity.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about lib.semver.major, lib.semver.minor, lib.semver.majorMinor?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the name semver common enough, that people will figure out the meaning on there own?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But maybe this becomes clear when it is used in the code and the passed variable is called version.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not limited to semver – see the GNOME example.

Copy link
Member Author

@ryantm ryantm Mar 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like "major" and "minor" are pretty universally the first and second parts of a version, so how about we make lib.version.major, lib.version.minor, and lib.version.majorMinor? We can also move lib.strings.versionOlder, and lib.strings.versionAtLeast into there as lib.version.older and lib.version.atLeast later.

@ryantm
Copy link
Member Author

ryantm commented Mar 3, 2018

I updated the code to match my latest proposal.

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Partial log (click to expand)

for x in intro SPOE close-options netscaler-client-ip-insertion-protocol 51Degrees-device-detection DeviceAtlas-device-detection network-namespaces linux-syn-cookies WURFL-device-detection lua cookie-options architecture proxy-protocol management configuration; do \
	install -m 644 doc/$x.txt "/nix/store/ly8avzlpfmhid434cpks1nqi65lhr05v-haproxy-1.7.9/doc/haproxy" ; \
done
post-installation fixup
moving /nix/store/ly8avzlpfmhid434cpks1nqi65lhr05v-haproxy-1.7.9/doc to /nix/store/ly8avzlpfmhid434cpks1nqi65lhr05v-haproxy-1.7.9/share/doc
gzipping man pages under /nix/store/ly8avzlpfmhid434cpks1nqi65lhr05v-haproxy-1.7.9/share/man/
strip is /nix/store/4sdh09gmvl15cy0zb6i7mbvxh5syz206-cctools-binutils-darwin/bin/strip
stripping (with command strip and flags -S) in /nix/store/ly8avzlpfmhid434cpks1nqi65lhr05v-haproxy-1.7.9/sbin
patching script interpreter paths in /nix/store/ly8avzlpfmhid434cpks1nqi65lhr05v-haproxy-1.7.9
moving /nix/store/ly8avzlpfmhid434cpks1nqi65lhr05v-haproxy-1.7.9/sbin/* to /nix/store/ly8avzlpfmhid434cpks1nqi65lhr05v-haproxy-1.7.9/bin

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Partial log (click to expand)

shrinking RPATHs of ELF executables and libraries in /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9
shrinking /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9/sbin/haproxy-systemd-wrapper
shrinking /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9/sbin/haproxy
gzipping man pages under /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9/share/man/
strip is /nix/store/lvx1acn1ig1j2km8jds5x3ggh3f2wa8v-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9/sbin
patching script interpreter paths in /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9
checking for references to /build in /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9...
moving /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9/sbin/* to /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9/bin
/nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9

@jtojnar
Copy link
Contributor

jtojnar commented Mar 3, 2018

The problem with nested attributes is that it is not very idiomatic. stdenv.lib is otherwise flat.

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Partial log (click to expand)

shrinking RPATHs of ELF executables and libraries in /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9
shrinking /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/sbin/haproxy-systemd-wrapper
shrinking /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/sbin/haproxy
gzipping man pages under /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/share/man/
strip is /nix/store/b0zlxla7dmy1iwc3g459rjznx59797xy-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/sbin 
patching script interpreter paths in /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9
checking for references to /tmp/nix-build-haproxy-1.7.9.drv-0 in /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9...
moving /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/sbin/* to /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/bin
/nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9

@ryantm
Copy link
Member Author

ryantm commented Mar 3, 2018

Thanks for pointing that out. I'll work on fixing it.

@Mic92
Copy link
Member

Mic92 commented Mar 3, 2018

We also have lib.systems and lib.platforms, which are not flat, but I find them clean.
So what is the reason for making everything flat except that we always did it that way?

@romildo
Copy link
Contributor

romildo commented Mar 3, 2018

@ryantm Notice that now nix has a splitVersion primop: NixOS/nix#1870. Consider using it.

@ryantm
Copy link
Member Author

ryantm commented Mar 3, 2018

@Mic92 I don't really have a preference between those two. If I made it flat, I'd be doing something like:

versionParts
majorVersion
minorVersion
patchVersion
majorMinorVersion

The non-flat version is in the code.

@romildo Is it too early to use that feature in nixpkgs?

@jtojnar
Copy link
Contributor

jtojnar commented Mar 3, 2018

I vote versionBranch as defined in gnome3.

@Mic92
Copy link
Member

Mic92 commented Mar 3, 2018

Actually I also don't have strong preference, but without looking at the source code I would not figure out what versionBranch does. @romildo has a point versionParts should be probably named splitVersion.

@ryantm
Copy link
Member Author

ryantm commented Mar 3, 2018

versionBranch isn't really immediately obvious to me either. If I try to think about it, it seems like it refers to when a project makes a stable branching that they maintain in parallel with their unstable development branch. I prefer majorMinor to that because it feels more general and directly related to manipulating the version string.

@romildo
Copy link
Contributor

romildo commented Mar 3, 2018

Is it too early to use that feature in nixpkgs?

Probably splitVersion should be used instead of lib.splitString "." as soon as nix-2.0 (which has already been released and has the builtin splitVersion) is the default in nixpkgs.

Edited: #34636 has just been merged.

@Mic92
Copy link
Member

Mic92 commented Mar 4, 2018

@GrahamcOfBorg eval

@ryantm
Copy link
Member Author

ryantm commented Mar 4, 2018

Updated to use builtins.splitVersion but still be backwards compatible.

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Partial log (click to expand)

/nix/store/ly8avzlpfmhid434cpks1nqi65lhr05v-haproxy-1.7.9

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Partial log (click to expand)

shrinking RPATHs of ELF executables and libraries in /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9
shrinking /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/sbin/haproxy-systemd-wrapper
shrinking /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/sbin/haproxy
gzipping man pages under /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/share/man/
strip is /nix/store/b0zlxla7dmy1iwc3g459rjznx59797xy-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/sbin 
patching script interpreter paths in /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9
checking for references to /tmp/nix-build-haproxy-1.7.9.drv-0 in /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9...
moving /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/sbin/* to /nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9/bin
/nix/store/l4n8lj83gx0krhlrzxrh94m1f3nxvsic-haproxy-1.7.9

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Partial log (click to expand)

shrinking RPATHs of ELF executables and libraries in /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9
shrinking /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9/sbin/haproxy-systemd-wrapper
shrinking /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9/sbin/haproxy
gzipping man pages under /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9/share/man/
strip is /nix/store/lvx1acn1ig1j2km8jds5x3ggh3f2wa8v-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9/sbin
patching script interpreter paths in /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9
checking for references to /build in /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9...
moving /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9/sbin/* to /nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9/bin
/nix/store/5hkdbhg0kqixvfrl4zv7qflqxs7dg99r-haproxy-1.7.9

@Mic92 Mic92 merged commit 73774ef into NixOS:master Mar 4, 2018
@Mic92
Copy link
Member

Mic92 commented Mar 4, 2018

Thanks!

@ryantm ryantm deleted the majorminor branch March 4, 2018 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants