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
openssl: 1.1.0 -> 1.1.1 #46524
openssl: 1.1.0 -> 1.1.1 #46524
Conversation
If it is "API and ABI compliant with 1.1.0", why do we need the aliases? Are there actual cases where 1.1.1 causes a failure? |
Not that I know of, but didn't want to break a convention by removing the |
Oh I just noticed that you actually did remove the The commit message is a bit confusing though. You don't actually update openssl, you just update the dependency for a few versions right? Or am I misreading that? |
I updated OpenSSL from 1.1.0 to 1.1.1. However, |
LGTM assuming there are no build failures @GrahamcOfBorg build ldns nodejs-10_x tor |
Failure on x86_64-linux (full log) Attempted: ldns, nodejs-10_x, tor Partial log (click to expand)
|
Failure on aarch64-linux (full log) Attempted: ldns, nodejs-10_x, tor Partial log (click to expand)
|
Timed out, unknown build status on x86_64-darwin (full log) Attempted: nodejs-10_x The following builds were skipped because they don't evaluate on x86_64-darwin: ldns, tor Partial log (click to expand)
|
pkgs/top-level/all-packages.nix
Outdated
@@ -11454,7 +11456,7 @@ with pkgs; | |||
}; | |||
}) | |||
openssl_1_0_2 | |||
openssl_1_1_0; | |||
openssl_1_1_1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should not introduce openssl_1_1_1
attribute, we should rename openssl_1_1_0
to openssl_1_1
and use that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same with openssl_1_0
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it does not have to be changed in this PR, and it may be left as is: AFAIK openssl_1_0_1
and openssl_1_0_2
were incompatible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't introduce openssl_1_0
alias for openssl_1_0_2
yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, please use openssl_1_1
and openssl_1_0_2
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel strongly either way, but if future 1.1.x versions are expected to break backwards compatibility to 1.1.1, having that version explicitly makes sense to me.
@@ -1,4 +1,4 @@ | |||
{ stdenv, lib, rustPlatform, fetchFromGitHub, pkgconfig, file, perl, curl, cmake, openssl_1_1_0, libssh2, libgit2, libzip, Security }: | |||
{ stdenv, lib, rustPlatform, fetchFromGitHub, pkgconfig, file, perl, curl, cmake, openssl_1_1, libssh2, libgit2, libzip, Security }: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you use openssl
here, and call powerline-rs
with openssl = openssl_1_1
in all-packages.nix
?
Other than the change to |
The ofBorg failure is probably unrelated. It would be nice to still fix them in a separate commit though. They look like sandbox issues ( |
@@ -15,7 +15,7 @@ rustPlatform.buildRustPackage rec { | |||
cargoSha256 = "184s432a6damzvl0lv6jar1iml9dq60r190aqjy44lcg938981zc"; | |||
|
|||
nativeBuildInputs = [ pkgconfig file perl cmake curl ]; | |||
buildInputs = [ openssl_1_1_0 libssh2 libgit2 libzip ] ++ lib.optional stdenv.isDarwin Security; | |||
buildInputs = [ openssl_1_1 libssh2 libgit2 libzip ] ++ lib.optional stdenv.isDarwin Security; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please replace this with openssl
too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops. Fixed.
88c72be
to
9cef3ee
Compare
@GrahamcOfBorg build ldns nodejs-10_x tor powerline-rs |
Failure on aarch64-linux (full log) Attempted: ldns, nodejs-10_x, tor, powerline-rs Partial log (click to expand)
|
Failure on x86_64-linux (full log) Attempted: ldns, nodejs-10_x, tor, powerline-rs Partial log (click to expand)
|
Timed out, unknown build status on x86_64-darwin (full log) Attempted: nodejs-10_x, powerline-rs The following builds were skipped because they don't evaluate on x86_64-darwin: ldns, tor Partial log (click to expand)
|
nodejs-10_x builds locally for me. Seems to be a timeout issue but doesn't make much sense according to the log. @grahamc Is there a timeout per package? |
No, the timeout is per build including all dependencies. |
Then the question is how following error can be possible (from the x86_64-linux log):
|
Hm that seems odd for a timeout. |
No attempt on x86_64-darwin (full log) The following builds were skipped because they don't evaluate on x86_64-darwin: tor Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: tor Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: tor Partial log (click to expand)
|
@GrahamcOfBorg build openssl_1_1 |
Success on aarch64-linux (full log) Attempted: openssl_1_1 Partial log (click to expand)
|
Success on x86_64-darwin (full log) Attempted: openssl_1_1 Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: openssl_1_1 Partial log (click to expand)
|
Motivation for this change
From https://www.openssl.org/blog/blog/2018/09/11/release111/:
Added
openssl_1_0
andopenssl_1_1
aliases because that's the level of granularity packages almost certainly want, and will make future upgrades like this much easier.Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)