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

par: fix static build #107961

Merged
merged 2 commits into from Jan 13, 2021
Merged

par: fix static build #107961

merged 2 commits into from Jan 13, 2021

Conversation

KAction
Copy link
Contributor

@KAction KAction commented Dec 30, 2020

  • 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.

@KAction KAction force-pushed the par-static branch 2 times, most recently from 0d585e2 to 5d7c54d Compare December 31, 2020 13:03
Comment on lines 21 to 26
buildPhase = ''
make -f protoMakefile \
CC="${stdenv.cc.targetPrefix}cc -c" \
LINK1=${stdenv.cc.targetPrefix}cc
'';
Copy link
Member

@Mic92 Mic92 Jan 1, 2021

Choose a reason for hiding this comment

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

I did not saw this earlier, but should it not be feasible to just use makeFlags here?

makeFlags = [ 
  "-f" "protoMakefile"
  "CC=${stdenv.cc.targetPrefix}cc -c"
  "LINK1=${stdenv.cc.targetPrefix}cc"
];

This would be a bit nicer since it is available in nix-shell i.e. eval make $makeFlags
and it make things like enableParallelBuilding work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does not work for me due space inside of CC. -c gets interpreted as make(1) flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But can work with makeFlagsArray. Do you like this approach (just pushed) better?

This change causes rebuild of both glibc and musl dynamic variants, but
no cascade rebuilds.
buildPhase = ''make -f protoMakefile'';
makefile = "protoMakefile";
preBuild = ''
makeFlagsArray=( CC="${stdenv.cc.targetPrefix}cc -c" \
Copy link
Member

Choose a reason for hiding this comment

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

This allows to still add stuff to makeFlagsArray in an override:

Suggested change
makeFlagsArray=( CC="${stdenv.cc.targetPrefix}cc -c" \
makeFlagsArray+=( CC="${stdenv.cc.targetPrefix}cc -c" \

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, force-pushed.

@Mic92
Copy link
Member

Mic92 commented Jan 13, 2021

@GrahamcOfBorg build par

@Mic92 Mic92 merged commit dbb48b4 into NixOS:master Jan 13, 2021
@KAction KAction deleted the par-static branch October 29, 2021 13:55
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

2 participants