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

sunxi-tools: 20171130 -> 20181113 #51007

Merged
merged 1 commit into from Dec 2, 2018

Conversation

samueldr
Copy link
Member

Motivation for this change

The update is necessary to flash the SPI NOR flash available on some AllWinner devices.

The revision chosen is the current tip of master.

Things done
  • ✔️ Tested using sandboxing (nix.useSandbox on NixOS, or option 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/)
  • ⬜ Determined the impact on package closure size (by running nix path-info -S before and after)
  • ✔️ Assured whether relevant documentation is up to date
  • ✔️ Fits CONTRIBUTING.md.

This was verified as working, as the instructions on the NixOS wiki were followed using that build.

@GrahamcOfBorg
Copy link

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: sunxi-tools

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
Copy link

Success on aarch64-linux (full log)

Attempted: sunxi-tools

Partial log (click to expand)

shrinking /nix/store/r0fgszb6nvmikvrbqjggzb0vf69d8rhr-sunxi-tools-20181113/bin/sunxi-pio
shrinking /nix/store/r0fgszb6nvmikvrbqjggzb0vf69d8rhr-sunxi-tools-20181113/bin/sunxi-nand-part
shrinking /nix/store/r0fgszb6nvmikvrbqjggzb0vf69d8rhr-sunxi-tools-20181113/bin/sunxi-fel
shrinking /nix/store/r0fgszb6nvmikvrbqjggzb0vf69d8rhr-sunxi-tools-20181113/bin/sunxi-bootinfo
shrinking /nix/store/r0fgszb6nvmikvrbqjggzb0vf69d8rhr-sunxi-tools-20181113/bin/sunxi-fexc
strip is /nix/store/6dpnd5aniypn8124mmy8f88s4mq2zl07-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/r0fgszb6nvmikvrbqjggzb0vf69d8rhr-sunxi-tools-20181113/bin
patching script interpreter paths in /nix/store/r0fgszb6nvmikvrbqjggzb0vf69d8rhr-sunxi-tools-20181113
checking for references to /build in /nix/store/r0fgszb6nvmikvrbqjggzb0vf69d8rhr-sunxi-tools-20181113...
/nix/store/r0fgszb6nvmikvrbqjggzb0vf69d8rhr-sunxi-tools-20181113

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: sunxi-tools

Partial log (click to expand)

shrinking /nix/store/wqr5lf47xbxg13j2q07kfrhv6h77hlfj-sunxi-tools-20181113/bin/sunxi-fel
shrinking /nix/store/wqr5lf47xbxg13j2q07kfrhv6h77hlfj-sunxi-tools-20181113/bin/phoenix_info
shrinking /nix/store/wqr5lf47xbxg13j2q07kfrhv6h77hlfj-sunxi-tools-20181113/bin/sunxi-nand-part
shrinking /nix/store/wqr5lf47xbxg13j2q07kfrhv6h77hlfj-sunxi-tools-20181113/bin/sunxi-bootinfo
shrinking /nix/store/wqr5lf47xbxg13j2q07kfrhv6h77hlfj-sunxi-tools-20181113/bin/sunxi-pio
strip is /nix/store/rpbg8gmqxhz8g61p1plz5d2srs84pvmv-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/wqr5lf47xbxg13j2q07kfrhv6h77hlfj-sunxi-tools-20181113/bin
patching script interpreter paths in /nix/store/wqr5lf47xbxg13j2q07kfrhv6h77hlfj-sunxi-tools-20181113
checking for references to /build in /nix/store/wqr5lf47xbxg13j2q07kfrhv6h77hlfj-sunxi-tools-20181113...
/nix/store/wqr5lf47xbxg13j2q07kfrhv6h77hlfj-sunxi-tools-20181113

@@ -1,13 +1,13 @@
{ stdenv, fetchFromGitHub, pkgconfig, libusb, zlib }:

stdenv.mkDerivation {
name = "sunxi-tools-20171130";
name = "sunxi-tools-20181113";
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be sunxi-tools-unstable-2018-11-13.

Copy link
Member Author

Choose a reason for hiding this comment

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

I know that fact, but because of how nix-env handles version comparisons, I believe unstable-yyyy-mm-dd is always seen as older than yyyymmdd.

nix-repl> builtins.compareVersions "20181113" "20171234"
1

nix-repl> builtins.compareVersions "unstable-2018-11-13" "20171234"
-1

builtins.compareVersions s1 s2
Compare two strings representing versions and return -1 if version s1 is older than version s2, 0 if they are the same, and 1 if s1 is newer than s2. The version comparison algorithm is the same as the one used by nix-env -u.

The idea here is to ensure end-users' software is upgraded. Not sure if there's a bigger issue to be tackled with regards to that existing discrepancy inside Nixpkgs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, and in fact this sounds very problematic. More so, I don't think a lot of nix devs are interested in ensuring nix-env works properly, so 👍 for bringing this to my attention.

Copy link
Contributor

@c0bw3b c0bw3b Nov 26, 2018

Choose a reason for hiding this comment

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

But unstable is going to be part of the pkg name, not the version part:

nix-repl> name = "pkg-unstable-20180101"
nix-repl> builtins.parseDrvName name
{ name = "pkg-unstable"; version = "20180101"; }

It will "break" pkg upgrade not because of versions comparison but because of the name change.
I agree this "rule" of the manual comes from a good idea but raises some practical questions.

I later read @fpletz mentioning the idea of this rule was to allow maintaining two versions of a package (unstable and normal/stable). Which is sound too. But unclear in the manual.

For case like this when we fetch a non-release commit, maybe the unstable tag should come at the end of the version part as to not break version comparison logic?

nix-repl> name = "pkg-20180101-unstable"
nix-repl> builtins.parseDrvName name
{ name = "pkg"; version = "20180101-unstable"; }

nix-repl> builtins.compareVersions "2018-01-01-unstable" "2017-10-01"
1

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh! misremembered that detail about why it would fail. I was sure the version attribute would have been in use there. Though I knew for sure it would fail. Thanks for the clarification and confirmation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Adding -unstable at the end of the version number is not good either because we still have a version comparison issue when a package comes back to a stable release number. We had the case recently in #51087.

Anyway, that's a broader issue. Let's not stall this update for this.

@c0bw3b c0bw3b merged commit 97ac980 into NixOS:master Dec 2, 2018
@samueldr samueldr deleted the sunxi-tools/update-2018-11-13 branch February 12, 2019 02:31
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

4 participants