Skip to content

Comments

nim: fix dynamic library loading, wrap for cross-compilation#95692

Merged
2 commits merged intomasterfrom
unknown repository
Sep 7, 2020
Merged

nim: fix dynamic library loading, wrap for cross-compilation#95692
2 commits merged intomasterfrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented Aug 17, 2020

Motivation for this change

See nim-lang/Nim#15194.

Wait for nim-lang/Nim#15196 to close.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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.

@ofborg ofborg bot requested a review from jbedo August 17, 2020 15:19
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Aug 17, 2020
Copy link
Contributor

@jbedo jbedo left a comment

Choose a reason for hiding this comment

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

This is great, tested mosdepth successfully. I hope the patch gets merged upstream.

@ghost
Copy link
Author

ghost commented Aug 24, 2020

I've pushed a second commit that wraps the compiler for cross-compilation, but at the moment I don't have access to the right hardware to test cross-compilation.

This adds nim-unwrapped and nim-stdlib to the top-level of nixpkgs, which can be overridden in overlays to affect the final nim wrapper.

@FRidh
Copy link
Member

FRidh commented Aug 24, 2020

Copy link
Member

Choose a reason for hiding this comment

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

Do these need to be listed at top-level?

Copy link
Author

Choose a reason for hiding this comment

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

I listed those at the top-level so that callPackage feeds them back to the nim-wrapper. I did this because I want to be able to patch the stdlib from an overaly without rebuilding the compiler, just the wrapper. Not sure this is the best way to do this.

@ghost
Copy link
Author

ghost commented Aug 24, 2020

For cross a test could be added that uses a qemu emulator.

Unfortunately I only found two packages in the tree to cross-compile, one has a dependency that is broken for cross-compiles and the other is quite expensive to build. I've used this method to cross-compile Nim from Linux to Genode so I think it should work, and if it doesn't I'll fix it eventually.

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Aug 24, 2020
@ofborg ofborg bot requested a review from jbedo August 24, 2020 11:12
@ghost
Copy link
Author

ghost commented Aug 24, 2020

Now Nimble gives me the dreaded Invalid section: . error:

     Error: Could not validate package:
        ... Could not read package info file in /home/repo/nimpkgs/geminim/geminim.nimble;
        ...   Reading as ini file failed with:
        ...     Invalid section: .
        ...   Evaluating as NimScript file failed with:
        ...     /home/repo/nimpkgs/geminim/geminim_20571.nims(5, 22) Warning: imported and not used: 'strutils' [UnusedImport]
        ... printPkgInfo() failed.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants