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.

Sign up for GitHub

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

Jump to bottom

git: Don't depend curl-config when cross-compiling. #61552

Closed
wants to merge 1 commit into from
Closed

git: Don't depend curl-config when cross-compiling. #61552

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions pkgs/applications/version-management/git-and-tools/git/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{ fetchurl, stdenv, buildPackages
, symlinkJoin
, curl, openssl, zlib, expat, perlPackages, python, gettext, cpio
, gnugrep, gnused, gawk, coreutils # needed at runtime by git-filter-branch etc
, openssh, pcre2
Expand Down Expand Up @@ -76,6 +77,9 @@ stdenv.mkDerivation {
configureFlags = stdenv.lib.optionals (stdenv.buildPlatform != stdenv.hostPlatform) [
"ac_cv_fread_reads_directories=yes"
"ac_cv_snprintf_returns_bogus=no"
# Provide curl manually, do not rely on `curl-config`
# See also `CURL_LDFLAGS` below.
"--with-curl=${symlinkJoin { name = "curl-libs-and-headers"; paths = [ curl.out curl.dev ]; }}"
Copy link
Member

Choose a reason for hiding this comment

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

Actually maybe try CURL_CONFIG=${curl.dev}/bin/curl-config? from here: https://github.com/git/git/blob/master/configure.ac#L597

Copy link
Member

Choose a reason for hiding this comment

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

Using symlinkJoin is usually a bad idea

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using symlinkJoin is usually a bad idea

@matthewbauer Why?

Copy link
Member

Choose a reason for hiding this comment

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

It breaks the closure size reductions from multiple outputs. When you link to that lib directory, you will also get the headers in closures.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I see. That makes sense.

What should I do though, given that git has only a single flag as opposed to one for headers and libs each?

I can try the curl-config approach, but using explicit flags sems to have less surprises so that might be preferable.

Do you no longer have the concerns about curl-config from #61527, or is the difference of your proposal above that we'd pass the CURL_CONFIG manually and that would somehow be better / address your previous concern of

Otherwise you wind up linking to the nonstatic curl I think

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@matthewbauer A short ping for my questions above :)

Copy link
Member

Choose a reason for hiding this comment

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

Sorry. Yeah you can use curl-config, you just can't put it in nativeBuildInputs, otherwise you get the build system's curl-config binary, not the targeted system. I believe putting "CURL_CONFIG=${curl.dev}/bin/curl-config" in configureFlags should be okay (if a little awkward).

Copy link
Member

Choose a reason for hiding this comment

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

@matthewbauer Except that curl-config won't work if you're cross-compiling to a different architecture, e.g. armv7l.

Copy link
Member

Choose a reason for hiding this comment

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

@kisik21 Some packages natively compile the config tools that go in the dev output precisely for this reason. It's ugly either way, but doing that is practical.

];

preBuild = ''
Expand All @@ -86,6 +90,9 @@ stdenv.mkDerivation {
"prefix=\${out}"
"SHELL_PATH=${stdenv.shell}"
]
# Provide curl manually, do not rely on `curl-config` for cross compilation.
# See also `--with-curl` above.
++ stdenv.lib.optionals (stdenv.buildPlatform != stdenv.hostPlatform) ["CURL_LDFLAGS=-lcurl"]
++ (if perlSupport then ["PERL_PATH=${perlPackages.perl}/bin/perl"] else ["NO_PERL=1"])
++ (if pythonSupport then ["PYTHON_PATH=${python}/bin/python"] else ["NO_PYTHON=1"])
++ stdenv.lib.optionals stdenv.isSunOS ["INSTALL=install" "NO_INET_NTOP=" "NO_INET_PTON="]
Expand Down