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

boost: Clean, reducing duplication #33188

Merged
merged 1 commit into from Dec 31, 2017

Conversation

Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented Dec 29, 2017

Motivation for this change

This was motivated originally by my cross work, but that goal requires a few more commits to other things. Still, it's good to start the cleanup now / get things out of the way.

Things done

Native is almost exactly tested as part of #26805. However, In that version I had this setup hook replicating some logic with findInputs downstream that's since been removed. I removed the setup hook for this PR so now this is technically untested, but I doubt anything changed.

Actually its even better, the setup hook traverses propagated dependencies, but no version of boost has any, so its demonstrably a no-op!

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

@Ericson2314 Ericson2314 added the 6.topic: cross-compilation Building packages on a different sort platform than than they will be run on label Dec 29, 2017
@Ericson2314 Ericson2314 changed the title boost: Clean , reducing duplication boost: Clean, reducing duplication Dec 29, 2017
@Ericson2314
Copy link
Member Author

Ericson2314 commented Dec 29, 2017

I differed the native derivations of the default version, and nothing interesting is different: just argument order and extra nativeBuildInputs for cross.

@Ericson2314 Ericson2314 added this to Needed by the big PR---nice to move pick off pieces of it and move here, rebasing the big PR on top in Cross compilation Dec 30, 2017
@Ericson2314 Ericson2314 mentioned this pull request Dec 30, 2017
13 tasks
@Ericson2314
Copy link
Member Author

Looking to finally merge #26805, so Boost maintainer, whoever you are, I hope you aren't on an internet-free vacation! (This blocks that.)

Copy link
Member

@globin globin left a comment

Choose a reason for hiding this comment

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

I've not reviewed this too thoroughly but looks fine generally, you can even probably drop the PIC stuff, see inline comments.


withToolset = stdenv.lib.optionalString (toolset != null) "--with-toolset=${toolset}";
cxxflags = optionalString (enablePIC) "-fPIC";
Copy link
Member

Choose a reason for hiding this comment

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

You should be able to drop all PIC handling, "fixed" by hardening ages ago


genericB2Flags = [
linkflags = optionalString (enablePIC) "-fPIC";
Copy link
Member

Choose a reason for hiding this comment

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

dito

else
"";
cflags = concatStringsSep " "
(optional (enablePIC) "-fPIC" ++
Copy link
Member

Choose a reason for hiding this comment

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

dito

@Ericson2314
Copy link
Member Author

@globin Ah, you mentioned that before, and I guess I forgot! turns out the option was unused so it was dead code anyways.

This was motivated originally by my cross work, but that goal requires a
few more commits to other things. Still, it's good to start the cleanup
now / get things out of the way.
, enableRelease ? true
, enableDebug ? false
, enableSingleThreaded ? false
, enableMultiThreaded ? true
, enableShared ? !(hostPlatform.libc == "msvcrt") # problems for now
, enableStatic ? !enableShared
, enablePIC ? false
Copy link
Member Author

Choose a reason for hiding this comment

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

This is unused, and hardening provides it anyways

, enableRelease ? true
, enableDebug ? false
, enableSingleThreaded ? false
, enableMultiThreaded ? true
, enableShared ? !(hostPlatform.libc == "msvcrt") # problems for now
, enableStatic ? !enableShared
, enablePIC ? false
, enableExceptions ? false
Copy link
Member Author

Choose a reason for hiding this comment

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

this is unused

@Ericson2314 Ericson2314 merged commit da48603 into NixOS:staging Dec 31, 2017
@Ericson2314 Ericson2314 deleted the clean-boost branch December 31, 2017 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: cross-compilation Building packages on a different sort platform than than they will be run on 10.rebuild-darwin: 101-500 10.rebuild-linux: 501+
Projects
No open projects
Cross compilation
Needed by the big PR---nice to move p...
Development

Successfully merging this pull request may close these issues.

None yet

5 participants