Skip to content

Commit

Permalink
gnutls: try to fix build on Darwin, after update
Browse files Browse the repository at this point in the history
  • Loading branch information
@vcunat
vcunat committed Apr 11, 2017
1 parent 6d13742 commit d6454e6
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion pkgs/development/libraries/gnutls/3.5.nix
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{ callPackage, fetchurl, libunistring, ... } @ args:
{ callPackage, stdenv, fetchurl, libunistring, darwin, ... } @ args:

callPackage ./generic.nix (args // rec {
version = "3.5.11";
Expand All @@ -15,4 +15,6 @@ callPackage ./generic.nix (args // rec {
sed '2iexit 77' -i tests/pkgconfig.sh
sed '/^void doit(void)/,$s/{/{ exit(77);/; t' -i tests/trust-store.c
'';

buildInputs = stdenv.lib.optional stdenv.isDarwin darwin.Security;
})

11 comments on commit d6454e6

@copumpkin
Copy link
Member

Choose a reason for hiding this comment

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

I'd be wary of adding frameworks to core packages until we implement #24693. I think this will probably make gnutls crash, but I haven't tested it yet.

@vcunat
Copy link
Member Author

@vcunat vcunat commented on d6454e6 Apr 11, 2017

Choose a reason for hiding this comment

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

Oh. It seemed required to fix the build. For now we could leave darwin on 3.5.10, but that version will probably become vulnerable at some point...

@copumpkin
Copy link
Member

Choose a reason for hiding this comment

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

No, I wouldn't want to separate the version, but there might be a flag to turn off the Security.framework requirement (for now at least). I'll try to take a look later. And yeah, this does add a segfault. I guess we could also fix the segfault 😄

@vcunat
Copy link
Member Author

@vcunat vcunat commented on d6454e6 Apr 11, 2017

Choose a reason for hiding this comment

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

I see no option for this and the CF includes are only guarded by #ifdef __APPLE__ (similarly LDFLAGS), so we would have to touch the code a bit. The motivation:

libgnutls: Added support for MacOSX key chain for obtaining trust store's root CA certificates.

@copumpkin
Copy link
Member

@copumpkin copumpkin commented on d6454e6 Apr 11, 2017 via email

Choose a reason for hiding this comment

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

@vcunat
Copy link
Member Author

@vcunat vcunat commented on d6454e6 Apr 12, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Well, curl/fetchurl can optionally depend on it, instead of our default openssl. Still, I think bootstrapping should always be able to avoid gnutls – even if it meant using lighter than default configurations for the bootstrap-intermediate packages.

@vcunat
Copy link
Member Author

@vcunat vcunat commented on d6454e6 Apr 12, 2017

Choose a reason for hiding this comment

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

@copumpkin: what (temporary) approach do you suggest now on staging so we don't block it?

@vcunat
Copy link
Member Author

@vcunat vcunat commented on d6454e6 Apr 13, 2017

Choose a reason for hiding this comment

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

I reverted Darwin to gnutls-3.5.10 so that Hydra can keep working 42fd720, but we can easily choose another solution anytime...

@copumpkin
Copy link
Member

@copumpkin copumpkin commented on d6454e6 Apr 13, 2017 via email

Choose a reason for hiding this comment

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

@vcunat
Copy link
Member Author

@vcunat vcunat commented on d6454e6 Jun 7, 2017

Choose a reason for hiding this comment

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

gnutls now has a security update, but it's only DOS in server code (null pointer dereference), so I guess Darwin can still go without updating it.

@vcunat
Copy link
Member Author

@vcunat vcunat commented on d6454e6 Mar 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Let me open a tagged issue, at least: #58481 (I'm not motivated to work on a solution.)

Please sign in to comment.