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

wolfssl: 3.9.0 -> 3.9.6, split package #16389

Merged
merged 1 commit into from
Jun 21, 2016

Conversation

mcmtroffaes
Copy link
Contributor

Motivation for this change
  1. Update wolfssl.
  2. Split package similar to openssl so it can be used as a drop-in replacement for openssl.
Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • OS X
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Note that no package immediately depends on wolfssl, so for testing I've built curl with wolfssl as a replacement for openssl using the following nix shell script:

with import <nixpkgs> {};
let
  curlwolfssl = (curl.override {
    openssl = wolfssl;
  }).overrideDerivation (oldAttrs: {
    configureFlags = oldAttrs.configureFlags ++ [
      "--with-cyassl=${wolfssl}"
    ];
  });
in
{
  my-env = stdenv.mkDerivation {
    name = "my-env";
    buildInputs = [
      curlwolfssl
    ];
  };
}

The resulting curl binary was tested with

curl https://nixos.org/
curl --sslv2 https://nixos.org/

which work as expected (the latter command gives curl: (35) CyaSSL does not support SSLv2 confirming that it indeed uses wolfssl).

Note that I had to break a dependency cycle. I did this simply by removing the "wolfssl-config" binary. The dev package comes with a pkg-config .pc file anyway, so I hope this is an acceptable solution. If there is a more standard solution for this problem, I'd be happy to be pointed to it and to fix it accordingly.

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @zimbatm to be a potential reviewer

@fpletz fpletz added 6.topic: closure size The final size of a derivation, including its dependencies 8.has: package (update) This PR updates a package to a newer version labels Jun 21, 2016
postInstall = ''
# fix recursive cycle:
# wolfssl-config points to dev, dev propagates bin
rm $bin/bin/wolfssl-config
Copy link
Contributor

@dezgeg dezgeg Jun 21, 2016

Choose a reason for hiding this comment

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

Does moveToOutput bin/wolfssl-config "$dev" work? (similar is used by e.g. curl)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion - I'll check this out...

@mcmtroffaes
Copy link
Contributor Author

Updated patch now using moveToOuptut on bin/wolfssl-config rather than deleting it. Because there is nothing else in $out besides this file, moveToOutput ends up removing $out itself causing the build to fail. Possibly moveToOutput could be fixed not to do so, but there wasn't an obvious way to do this, and since this situation is pretty rare anyway, I've simply decided to do a quick mkdir -p $out as well in the post-install phase.

Is this an acceptable approach?

@zimbatm zimbatm merged commit 6596d1e into NixOS:master Jun 21, 2016
@mcmtroffaes
Copy link
Contributor Author

Thanks so much for the quick merge, @zimbatm!

@mcmtroffaes mcmtroffaes deleted the feature/wolfssl-3.9.6 branch June 21, 2016 13:24
@zimbatm
Copy link
Member

zimbatm commented Jun 21, 2016

👍 nice work too!

@marsam marsam mentioned this pull request Aug 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: closure size The final size of a derivation, including its dependencies 8.has: package (update) This PR updates a package to a newer version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants