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

nim: 0.20.2 -> 1.0.0 #69362

Merged
merged 1 commit into from Sep 29, 2019
Merged

nim: 0.20.2 -> 1.0.0 #69362

merged 1 commit into from Sep 29, 2019

Conversation

xzfc
Copy link
Contributor

@xzfc xzfc commented Sep 24, 2019

Motivation for this change

https://nim-lang.org/blog/2019/09/23/version-100-released.html

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 nix-review --run "nix-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @ehmry @peterhoeg

xzfc
xzfc commented Sep 24, 2019
View reviewed changes
pkgs/development/compilers/nim/default.nix
@@ -30,7 +30,7 @@ stdenv.mkDerivation rec {
# as part of building it, so it cannot be read-only

checkInputs = [
nodejs-slim-11_x tzdata coreutils
nodejs-slim tzdata coreutils
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Node 11.x is EOL (#69008), so I switched to current LTS.

@peterhoeg
Copy link
Member

Looks good - thanks. Can I trouble you to remove me as a maintainer as part of this please?

@peterhoeg
Copy link
Member

@GrahamcOfBorg build nim

ehmry
ehmry approved these changes Sep 25, 2019
View reviewed changes
Copy link
Contributor

@ehmry ehmry left a comment

Choose a reason for hiding this comment

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

Looks fine, my only comment is that I think Nimble should be build seperately (99c24c5), but thats another issue ( #67878).

@Mic92
Copy link
Member

Mic92 commented Sep 25, 2019

It looks like two tests are failing on aarch64. Did this happen in the old version?

@xzfc
Copy link
Contributor Author

xzfc commented Sep 25, 2019

It looks like two tests are failing on aarch64. Did this happen in the old version?

No, these are new/updated tests.

PR update:

  • Disabled broken tests on aarch64. I haven't tested though.
  • Fortify hardening is disabled for tests and binaries created by nim compiler. It appends -O2 to gcc flags which is unwanted for unoptimized nim builds. Nim compiler itself is still built with fortify hardening enabled.
    • It works but I'm not sure that wrapProgram --run it a proper way of disabling it.
    • Re-enabled tests/misc/tstrace.nim that was broken by fortify hardening.

@xzfc
Copy link
Contributor Author

xzfc commented Sep 25, 2019

@GrahamcOfBorg build nim

@xzfc
Copy link
Contributor Author

xzfc commented Sep 25, 2019

Could anyone trigger @GrahamcOfBorg build nim please?

@Mic92
Copy link
Member

Mic92 commented Sep 26, 2019

@GrahamcOfBorg build nim

@marsam marsam merged commit 9b42f37 into NixOS:master Sep 29, 2019
@marsam
Copy link
Contributor

marsam commented Sep 29, 2019

Thank you @xzfc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ehmry ehmry ehmry approved these changes

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@xzfc @peterhoeg @Mic92 @marsam @ehmry