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.

Sign up for GitHub

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

Jump to bottom

idsk : init at unstable-2018-02-11 #35045

Merged
merged 4 commits into from Feb 20, 2018
Merged

idsk : init at unstable-2018-02-11 #35045

merged 4 commits into from Feb 20, 2018

Conversation

bignaux
Copy link
Contributor

@bignaux bignaux commented Feb 16, 2018

Motivation for this change
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.

lheckemann
lheckemann reviewed Feb 16, 2018
View reviewed changes
pkgs/tools/filesystems/idsk/default.nix Outdated
version = "0.16";
rev = "1846729ac3432aa8c2c0525be45cfff8a513e007";
short_rev = "${builtins.substring 0 7 rev}";
name = "${pname}-${version}-${short_rev}";
Copy link
Member

Choose a reason for hiding this comment

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

indentation is off here

@lheckemann
Copy link
Member

@GrahamcOfBorg build idsk

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Partial log (click to expand)

[100%] Built target iDSK
installing
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/m2kxyc5jbd0vxffy2gmbxnhxnfg02007-idsk-0.16-1846729
shrinking /nix/store/m2kxyc5jbd0vxffy2gmbxnhxnfg02007-idsk-0.16-1846729/bin/iDSK
strip is /nix/store/adidfx4pa7vmvby0gjqqmiwg2x49yr27-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/m2kxyc5jbd0vxffy2gmbxnhxnfg02007-idsk-0.16-1846729/bin
patching script interpreter paths in /nix/store/m2kxyc5jbd0vxffy2gmbxnhxnfg02007-idsk-0.16-1846729
checking for references to /build in /nix/store/m2kxyc5jbd0vxffy2gmbxnhxnfg02007-idsk-0.16-1846729...
/nix/store/m2kxyc5jbd0vxffy2gmbxnhxnfg02007-idsk-0.16-1846729

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Partial log (click to expand)

[100%] Built target iDSK
installing
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/8mlj7c8kad04f55d9xb39yswyzlgr6wr-idsk-0.16-1846729
shrinking /nix/store/8mlj7c8kad04f55d9xb39yswyzlgr6wr-idsk-0.16-1846729/bin/iDSK
strip is /nix/store/skd6ix5ipkyhxzq7naylj4digawakl4j-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/8mlj7c8kad04f55d9xb39yswyzlgr6wr-idsk-0.16-1846729/bin
patching script interpreter paths in /nix/store/8mlj7c8kad04f55d9xb39yswyzlgr6wr-idsk-0.16-1846729
checking for references to /build in /nix/store/8mlj7c8kad04f55d9xb39yswyzlgr6wr-idsk-0.16-1846729...
/nix/store/8mlj7c8kad04f55d9xb39yswyzlgr6wr-idsk-0.16-1846729

@GrahamcOfBorg
Copy link

Failure on x86_64-darwin (full log)

Partial log (click to expand)

Package ‘idsk-0.16-1846729’ in /private/var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/lnl7-mac/pkgs/tools/filesystems/idsk/default.nix:13 is not supported on ‘x86_64-darwin’, refusing to evaluate.

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

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

nlewo
nlewo reviewed Feb 16, 2018
View reviewed changes
pkgs/tools/filesystems/idsk/default.nix Outdated
stdenv.mkDerivation rec {

pname = "idsk";
version = "0.16";
Copy link
Member

@nlewo nlewo Feb 16, 2018 •
edited

Choose a reason for hiding this comment

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

Where did you find the version number? I'm able to get it from there github.
When then revision is not a release, we have to use version = "unstable-YYYY-MM-DD" as mentioned in https://nixos.org/nixpkgs/manual/#sec-package-naming

Copy link
Contributor Author

@bignaux bignaux Feb 16, 2018 •
edited

Choose a reason for hiding this comment

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

It refers to the version display :
"iDSK version 0.16 (by Demoniak, Sid, PulkoMandy), http://github.com/cpcsdk"
But i will change to unstable-YYYY-MM-DD if it more compliant.
The problem i see with such name, they don't really express the version + git for a user (that use 0.14 for example, and will get a unstable-2018-xx-xx thing).

@bignaux
Copy link
Contributor Author

bignaux commented Feb 16, 2018

I've fixed version, but i'm unhappy with the idsk-unstable-2018-02-11 name. giving date show it's not a release, unstable is redundant and give no information about what it is.

@lheckemann
Copy link
Member

https://nixos.org/nixpkgs/manual/#sec-syntax

  • Use 2 spaces of indentation per indentation level in Nix expressions, 4 spaces in shell scripts.
  • Do not use tab characters, i.e. configure your editor to use soft tabs. For instance, use (setq-default indent-tabs-mode nil) in Emacs. Everybody has different tab settings so it’s asking for trouble.

I believe the -unstable naming convention is to make sure that in the presence of a stable and an unstable version of a package, nix-env will not automatically select the unstable one because it has a higher version number. See the description; this isn't a great behaviour IMO and nobody who knows the difference will prefer nix-env -i over nix-env -iA but hey, such is life. I also agree that if upstream has some sort of version number it should be included. I don't know the exact policies around this though.

@bignaux bignaux changed the title idsk : init at 0.16-1846729 idsk : init at unstable-2018-02-11 Feb 16, 2018
@nlewo
Copy link
Member

nlewo commented Feb 20, 2018

I also don't know if there is a policy regarding this case. But I think we can consider this is a release. When we use a tag to fetch a release, this tag is mutable and set by application developers. This is really close from this displayed version number.
@bignaux could you switch back to the version number (instead of unstable-date)?

@bignaux
Copy link
Contributor Author

bignaux commented Feb 20, 2018

I'd prefer keep the name as it, and merge, and later i found time to fix bug in idsk and ask for a bug release.

@nlewo nlewo merged commit a1526e5 into NixOS:master Feb 20, 2018
@nlewo
Copy link
Member

nlewo commented Feb 20, 2018

Thanks

@bignaux bignaux deleted the idsk branch February 20, 2018 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lheckemann lheckemann lheckemann left review comments

@nlewo nlewo nlewo left review comments

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@bignaux @lheckemann @GrahamcOfBorg @nlewo