Skip to content

util/mkinstallvars.pl: replace List::Util::pairs with our own#25367

Closed
levitte wants to merge 2 commits intoopenssl:masterfrom
levitte:fix-25366
Closed

util/mkinstallvars.pl: replace List::Util::pairs with our own#25367
levitte wants to merge 2 commits intoopenssl:masterfrom
levitte:fix-25366

Conversation

@levitte
Copy link
Member

@levitte levitte commented Sep 3, 2024

Unfortunately, List::Util::pairs didn't appear in perl core modules
before 5.19.3, and our minimum requirement is 5.10.

Fortunately, we already have a replacement implementation, and can
re-apply it in this script.

Fixes #25366

@levitte levitte added branch: master Applies to master branch approval: review pending This pull request needs review by a committer approval: otc review pending triaged: bug The issue/pr is/fixes a bug tests: exempted The PR is exempt from requirements for testing branch: 3.3 Applies to openssl-3.3 labels Sep 3, 2024
Unfortunately, List::Util::pairs didn't appear in perl core modules
before 5.19.3, and our minimum requirement is 5.10.

Fortunately, we already have a replacement implementation, and can
re-apply it in this script.

Fixes openssl#25366
@levitte
Copy link
Member Author

levitte commented Sep 3, 2024

@friesoft, @alexdotgov2, please try out this patch

@friesoft
Copy link

friesoft commented Sep 3, 2024

Looks good to me on RHEL 7 + 8 + 9

@friesoft
Copy link

friesoft commented Sep 3, 2024

Works for me on RHEL 7 + 8 + 9 :) Thanks for the lightning fast fix :)
Will this be released as a 3.3.2.1?

@t8m
Copy link
Member

t8m commented Sep 3, 2024

Works for me on RHEL 7 + 8 + 9 :) Thanks for the lightning fast fix :) Will this be released as a 3.3.2.1?

As there is a clear workaround by installing the perl module, I do not think this warrants a 3.3.2.1 release.

@t8m t8m requested review from a team and nhorman September 3, 2024 19:20
@t8m t8m added the severity: regression The issue/pr is a regression from previous released version label Sep 3, 2024
Copy link
Member

@beldmit beldmit left a comment

Choose a reason for hiding this comment

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

LGTM

@beldmit beldmit added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Sep 3, 2024
@asedeno
Copy link

asedeno commented Sep 3, 2024

How does failing to build with your listed minimum dependencies not warrant a point release? This release does not build for a bunch of platforms, and telling those users to upgrade their version of perl or some perl module may be a non-starter for them.

@levitte
Copy link
Member Author

levitte commented Sep 3, 2024

Small detail: the next patch (point) release will be 3.3.3, regardless of when or how much goes into it.

@rainerjung
Copy link
Contributor

LGTM, the patch fixes 3.3.2 builds for SLES 11 + 12, RHEL 6 + 7 .
Thanks a bunch for still supporting very basic (outdated) perl installations!

Copy link
Contributor

@shahsb shahsb left a comment

Choose a reason for hiding this comment

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

LGTM.

@levitte
Copy link
Member Author

levitte commented Sep 4, 2024

@t8m, @beldmit, re-approve, please?

@levitte levitte added approval: review pending This pull request needs review by a committer approval: otc review pending and removed approval: done This pull request has the required number of approvals labels Sep 4, 2024
@ion-elgreco
Copy link

@t8m why isn't a patch release being made? This would make many folks their work easier to resolve this

@t8m
Copy link
Member

t8m commented Sep 6, 2024

A patch release means a forced update for many users of OpenSSL so it creates additional work for many people. There is of course also some work required from the OpenSSL release managers to create the release but that is not so important for the decision.

So doing a new patch release should not be taken lightly and only fairly serious regressions should trigger such release.

@slontis
Copy link
Member

slontis commented Sep 6, 2024

I will be raising this issue in the next OTC meeting (next Tuesday).

rtyler added a commit to delta-io/delta-rs that referenced this pull request Sep 6, 2024
…inux release

This change enables the use of Rustls for the manylinux Python wheel,
which was originally disabled in #1224. That issue was only affecting MacOS
and has likely since been resolved.

The switch from openssl motivated by the issue described in
openssl/openssl#25367

Closes #2848
@asedeno
Copy link

asedeno commented Sep 7, 2024

Not building on supported platforms is a serious regression, and honestly, this regression should have been caught by automation prior to release.

@ChristopherRabotin
Copy link

Merged to the master and 3.3 branches. Thank you.

Is this 3.3.2.1 release or other? thanks

No new release is going to be done for this. The patch needs to be applied on top of the 3.3.2 tarball. Please see #25394

It is very disappointing. This means that a large number of CI scripts need to be revised for this, and it is very difficult. Because many Rust components involve OpenSSL and are automatically compiled by default. To improve compatibility, the commonly used base environment is manylinux2014. In this case, the patch tarball does not work. However, revising the basic environment may conflict with compatibility specifications and cannot be executed. :(

Sorry to jump in here, but I've been facing the same issue with a manylinux: auto config and a Rust+Python project (https://github.com/nyx-space/anise/actions/runs/10748687666/job/29812702558?pr=311). Is there a workaround we can use, or are we condemned to not build our libs until the next release of openssl? I admit that the crux of the issue is that the manylinux build just uses an old version of rhel here, but I don't know of a workaround to change that.

Thanks

P.S.: Openssl dev team, you're a core tenet of all software out there, so I'll take this opportunity to thank you for all of the hard work you've put into this lib over the decades.

@levitte
Copy link
Member Author

levitte commented Sep 7, 2024

Is there a workaround we can use, or are we condemned to not build our libs until the next release of openssl?

Yes there is. You can take the patch that came out of this PR (commit 40c8ebe).

BTW, we created a discussion category Release addendums a couple of days ago, and you'll find Release 3.3.2 errata there.

glasser added a commit to apollographql/rover that referenced this pull request Sep 8, 2024
openssl 3.3.2 does not build on manylinux.

See openssl/openssl#25367
@glasser
Copy link

glasser commented Sep 8, 2024

If you found your way here because you are building on manylinux via Rust, I found that cargo update openssl-src --precise 300.3.1+3.3.1 was the command I needed to roll back.

my @subdirs = pairs (PREFIX => [ qw(BINDIR LIBDIR INCLUDEDIR APPLINKDIR) ],
LIBDIR => [ qw(ENGINESDIR MODULESDIR PKGCONFIGDIR
CMAKECONFIGDIR) ]);
my @subdirs = _pairs (PREFIX => [ qw(BINDIR LIBDIR INCLUDEDIR APPLINKDIR) ],
Copy link

Choose a reason for hiding this comment

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

Why do it with _pairs when you can just type 2 pairs of [ / ]?

array-refs have been core in perl for some time

Choose a reason for hiding this comment

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

Exactly, this is just a very round-about way of writing them as literal array refs, since you're only supplying 4 items to pairs or _pairs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I had done it like this initially 'cause I found it more elegant, simple as that.

But, now it's in with _pairs. If someone is really hell bent on changing this bit again, feel free to raise a PR.

@metercai
Copy link

metercai commented Sep 9, 2024

If you found your way here because you are building on manylinux via Rust, I found that cargo update openssl-src --precise 300.3.1+3.3.1 was the command I needed to roll back.

The only way is to force a rollback to 300.3.1+3.3.1 and wait for the new version to be released. That patch has no effect on this case.

@glasser
Copy link

glasser commented Sep 9, 2024

If you found your way here because you are building on manylinux via Rust, I found that cargo update openssl-src --precise 300.3.1+3.3.1 was the command I needed to roll back.

The only way is to force a rollback to 300.3.1+3.3.1 and wait for the new version to be released. That patch has no effect on this case.

Yes, that's what my command does — I'm just noting that this took me a while to figure out as a non-Cargo-expert (and it seems like I'm not the only person on this thread running into this via a manylinux build of Rust software) and that perhaps others might appreciate the explicit command.

@NehaShrivastava2022
Copy link

Works for me on amazon-linux2

@ChristopherRabotin
Copy link

If you found your way here because you are building on manylinux via Rust, I found that cargo update openssl-src --precise 300.3.1+3.3.1 was the command I needed to roll back.

Adding onto this, if you don't have a Cargo.lock, simply add glasser's command as a build step before the "build wheel" step: nyx-space/anise@02599b3 .

TheEnbyperor added a commit to AS207960/certbot-onion that referenced this pull request Oct 1, 2024
ChristopherRabotin added a commit to nyx-space/hifitime that referenced this pull request Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Applies to master branch branch: 3.3 Applies to openssl-3.3 severity: regression The issue/pr is a regression from previous released version tests: exempted The PR is exempt from requirements for testing triaged: bug The issue/pr is/fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OpenSSL 3.3.2 build fails on RHEL 7