Skip to content

Comments

git: Depend on curl.dev to provide curl-config.#61527

Closed
nh2 wants to merge 1 commit intoNixOS:masterfrom
nh2:git-curl-config
Closed

git: Depend on curl.dev to provide curl-config.#61527
nh2 wants to merge 1 commit intoNixOS:masterfrom
nh2:git-curl-config

Conversation

@nh2
Copy link
Contributor

@nh2 nh2 commented May 14, 2019

Motivation for this change

Fixes building with pkgsStatic, where otherwise curl linker flags
would not be correctly determined.

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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@nh2
git: Depend on curl.dev to provide curl-config.
4d12eae 
Fixes building with `pkgsStatic`, where otherwise curl linker flags
would not be correctly determined.
@nh2 nh2 requested review from matthewbauer and peti May 14, 2019 23:58
matthewbauer
matthewbauer reviewed May 15, 2019
View reviewed changes
Copy link
Member

@matthewbauer matthewbauer left a comment

Choose a reason for hiding this comment

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

Is there a —with-curl option for git? I think that would be better. Otherwise you wind up linking to the nonstatic curl I think

@ofborg ofborg bot requested review from the-kenny and wmertens May 15, 2019 00:57
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 501-1000 This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 1001-2500 This PR causes many rebuilds on Linux and should target the staging branches. labels May 15, 2019
@nh2
Copy link
Contributor Author

nh2 commented May 15, 2019 •
edited
Loading

@matthewbauer Hmm, you're right, something isn't working yet.

Without this PR I get the failure:

/nix/store/5p8qcz6i13ymgf80kimhx2g9pb5r5nia-bash-4.4-p23/bin/bash: curl-config: command not found
    LINK git-remote-http
/nix/store/qpcc9hlyq5gnpaf2x4b6xfdkzas9dxcy-x86_64-unknown-linux-musl-binutils-2.31.1/bin/x86_64-unknown-linux-musl-ld: http.o: in function `process_curl_messages':
/build/git-2.21.0/http.c:261: undefined reference to `curl_multi_info_read'
/nix/store/qpcc9hlyq5gnpaf2x4b6xfdkzas9dxcy-x86_64-unknown-linux-musl-binutils-2.31.1/bin/x86_64-unknown-linux-musl-ld: /build/git-2.21.0/http.c:281: undefined reference to `curl_multi_info_read'
...

With this PR I apparently get further (though "further" might also just be parallel-compilation-interleaving), but do get a glibc error:

    LINK git-remote-http
    LINK git
/nix/store/qpcc9hlyq5gnpaf2x4b6xfdkzas9dxcy-x86_64-unknown-linux-musl-binutils-2.31.1/bin/x86_64-unknown-linux-musl-ld: warning: ld-linux-x86-64.so.2, needed by /nix/store/xqs95fqkjb1kd102yjv5h5q57gcsafb3-glibc-2.27/lib/libpthread.so.0, not found (try using -rpath or -rpath-link)
/nix/store/qpcc9hlyq5gnpaf2x4b6xfdkzas9dxcy-x86_64-unknown-linux-musl-binutils-2.31.1/bin/x86_64-unknown-linux-musl-ld: /nix/store/xqs95fqkjb1kd102yjv5h5q57gcsafb3-glibc-2.27/lib/libpthread.so.0: undefined reference to `_dl_allocate_tls@GLIBC_PRIVATE'

@nh2 nh2 mentioned this pull request May 15, 2019
Closed
10 tasks
@nh2
Copy link
Contributor Author

nh2 commented May 15, 2019

Is there a —with-curl option for git? I think that would be better. Otherwise you wind up linking to the nonstatic curl I think

OK, abandoning this PR in favour of PR #61552.

@nh2 nh2 closed this May 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@peti peti Awaiting requested review from peti

@wmertens wmertens Awaiting requested review from wmertens

@the-kenny the-kenny Awaiting requested review from the-kenny

1 more reviewer

@matthewbauer matthewbauer matthewbauer left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

10.rebuild-darwin: 501-1000 This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 1001-2500 This PR causes many rebuilds on Linux and should target the staging branches.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nh2 @matthewbauer