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

curl: enable Kerberos in non-fetchurl builds #29785

Merged
merged 2 commits into from
Dec 19, 2017
Merged

curl: enable Kerberos in non-fetchurl builds #29785

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7535385
curl: use the "kerberos" package rather than specifically GNU gss
catern Sep 25, 2017
68432fd
curl: enable gssSupport in non-fetchurl builds
catern Sep 25, 2017
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: 4 additions & 3 deletions pkgs/tools/networking/curl/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
, sslSupport ? false, openssl ? null
, gnutlsSupport ? false, gnutls ? null
, scpSupport ? false, libssh2 ? null
, gssSupport ? false, gss ? null
, gssSupport ? false, kerberos ? null
, c-aresSupport ? false, c-ares ? null
}:

Expand All @@ -19,6 +19,7 @@ assert !(gnutlsSupport && sslSupport);
assert gnutlsSupport -> gnutls != null;
assert scpSupport -> libssh2 != null;
assert c-aresSupport -> c-ares != null;
assert gssSupport -> kerberos != null;

stdenv.mkDerivation rec {
name = "curl-7.56.0";
Expand All @@ -43,7 +44,7 @@ stdenv.mkDerivation rec {
optional idnSupport libidn ++
optional ldapSupport openldap ++
optional zlibSupport zlib ++
optional gssSupport gss ++
optional gssSupport kerberos ++
optional c-aresSupport c-ares ++
optional sslSupport openssl ++
optional gnutlsSupport gnutls ++
Expand All @@ -66,7 +67,7 @@ stdenv.mkDerivation rec {
( if idnSupport then "--with-libidn=${libidn.dev}" else "--without-libidn" )
]
++ stdenv.lib.optional c-aresSupport "--enable-ares=${c-ares}"
++ stdenv.lib.optional gssSupport "--with-gssapi=${gss}";
++ stdenv.lib.optional gssSupport "--with-gssapi=${kerberos}";

CXX = "c++";
CXXCPP = "c++ -E";
Expand Down
4 changes: 3 additions & 1 deletion pkgs/top-level/all-packages.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ with pkgs;

# `fetchurl' downloads a file from the network.
fetchurl = import ../build-support/fetchurl {
inherit curl stdenv;
inherit stdenv;
curl = curl.override { gssSupport = false; };
Copy link
Member

@Mic92 Mic92 Oct 15, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I would not make a different version for just fetchurl. It is pretty likely that we end up with two different libcurl versions anyway since curl is one of the most used library. So either we enable it for curl by default or have curlFull as alternative.

Copy link
Contributor Author

@catern catern Oct 16, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I'm a little worried about putting it in "curl" because it grows the Nix closure. If I make a different version for just "fetchurl", I guess I don't have that problem? I don't want to put it in "curlFull", though, because curlFull isn't used when when derivations want to compile against libcurl.

What I think we should probably be doing is have a "curlBasic" version which is used by fetchurl and other things in the closure of Nix. And then make normal "curl" be "curlFull". Either we should do that, or do this.

Copy link
Member

@Mic92 Mic92 Oct 16, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

It will blow the closure in general. Curl is used anywhere. By having two version of libcurl it will only become worse.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The other issue (kind of the same issue) is that if we add Kerberos support to the curl used in fetchurl, those Kerberos dependencies need to use fetchurlboot when building or there will be a circular dependency. Right?

I can do that (change them to use fetchurlboot) if that is desired.

Copy link
Member

Choose a reason for hiding this comment

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

I would like to know the opinion of the other maintainers here.

};

fetchRepoProject = callPackage ../build-support/fetchrepoproject { };
Expand Down Expand Up @@ -1613,6 +1614,7 @@ with pkgs;
zlibSupport = true;
sslSupport = zlibSupport;
scpSupport = zlibSupport && !stdenv.isSunOS && !stdenv.isCygwin;
gssSupport = true;
};

curl_unix_socket = callPackage ../tools/networking/curl-unix-socket rec { };
Expand Down