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

darwin.cctools: 895 -> 927.0.2 #73721

Merged

Conversation

virusdave
Copy link
Contributor

@virusdave virusdave commented Nov 18, 2019

Update the version of cctools on darwin. The older version fails
to work on some modern packages, such as bazel 1.1.

Motivation for this change

Latest bazel package builds, but doesn't work at runtime on darwin, due to this dependency. When i attempted rerunning the runtime failures using the (newer) system cctools binaries from 927.0.2, they work fine, hence this update PR.

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

(*) This command succeeded after the rebase onto staging, but i don't believe it did anything interesting. It certainly didn't build any derivations.

Notify maintainers

cc @matthewbauer

@FRidh
Copy link
Member

FRidh commented Nov 19, 2019

please rebase onto staging

Update the version of `cctools` on darwin.  The older version fails
to work on some modern packages, such as `bazel 1.1`.
@virusdave virusdave force-pushed the dnicponski/scratch/update_darwin_cctools branch from d2c7d60 to 7b77c09 Compare November 19, 2019 14:20
@virusdave
Copy link
Contributor Author

@FRidh done.

@ofborg ofborg bot removed 6.topic: TeX Issues regarding texlive and TeX in general 6.topic: emacs 6.topic: erlang labels Nov 19, 2019
@virusdave
Copy link
Contributor Author

Any thoughts on this?

@FRidh
Copy link
Member

FRidh commented Nov 20, 2019

Thanks. I don't know anything about Darwin packages so probably best is to wait for or ping Darwin maintainers that are interested in this.

@virusdave
Copy link
Contributor Author

Friendly ping for @matthewbauer also @alyssais and @volth , "recent" committers. Would love to get this landed soon if possible.

@virusdave
Copy link
Contributor Author

@FRidh I've pinged all the maintainers and folks who have "recently" changed this derivation, but with no response. Suggestions on who to ping now / how to get this merged?

@veprbl
Copy link
Member

veprbl commented Nov 23, 2019

@virusdave This PR appears to be a desirable update, however, please keep in mind that nixpkgs is a community-driven project and maintainers are volunteers who work on their schedule. I believe, @matthewbauer is the most knowledgeable for reviewing this PR, so let's wait for what he has to say.

If you would like to help us with reviewing the PR, please document carefully what steps did you take to validate your change (we are looking to ensure that nothing is broken by this). From my personal experience, seeing that you did your "homework" helps maintainers to prioritize reviewing your PR over other work.

@veprbl veprbl requested a review from LnL7 November 23, 2019 22:50
Copy link
Member

@LnL7 LnL7 left a comment

Choose a reason for hiding this comment

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

I don't have experience with bumping cctools so I'm not sure if the version should match the rest of the stdenv and apple SDKs. In which case it might require more testing, other then that it looks good to me.

libtool is not really needed and it interferes with
updateAutotoolsGnuConfigScriptsHook. So remove it when
cross-compiling, but leave it in native to preserve hashes.
@matthewbauer
Copy link
Member

Looks good, have included a few misc iOS fixes on top of it.

@matthewbauer matthewbauer merged commit 9d56a06 into NixOS:staging Nov 26, 2019
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

5 participants