-
Notifications
You must be signed in to change notification settings - Fork 35
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
3.24.x deps 2024 09 06 13 39 17 290986 #1494
3.24.x deps 2024 09 06 13 39 17 290986 #1494
Conversation
@cf-bottom jenkins no test |
Sure, I triggered a build: [NO TESTS] Jenkins: https://ci.cfengine.com/job/pr-pipeline/11186/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-11186/ |
91c528d
to
231fd44
Compare
@cf-bottom jenkins no tests |
Alright, I triggered a build: [NO TESTS] Jenkins: https://ci.cfengine.com/job/pr-pipeline/11192/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-11192/ |
1bfbc78
to
5d1ad9a
Compare
@cf-bottom jenkins, think I'm ready for the tests |
Alright, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/11197/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-11197/ |
Oops. Forgot a fix somewhere from cf-bottom not quite working.
will fix. |
5d1ad9a
to
fb33e3b
Compare
@cf-bottom jenkins |
Alright, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/11200/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-11200/ |
fb33e3b
to
119ee9d
Compare
@cf-bottom jenkins |
Sure, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/11202/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-11202/ |
119ee9d
to
257d668
Compare
@vpodzime as you can see looking back in comments here we have a green jenkins build above with these changes. I just squashed the two fixup commits for openssl into one commit with the upgrade. Ready for review. |
Maybe you can update the comment above
I got a bit confused at first ;) |
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.
Looks good to me otherwise. Please check things work not just build on exotics (unless you already did).
Funny, that's the initial comment that github makes from the initial PR. That wrong version was generated by cf-bottom and then I fixed it. The commit comment is correct and now I have fixed the PR comment. 👍 Good catch. |
Right. I will build and test on exotics. |
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.
@craigcomstock please update the tables in the README (Version numbers in master column and add 3.24.x column).
@craigcomstock why does PR title say 3.24.x but target branch is master? |
Good catch. I used cf-bottom to start the PR but it did a bad job due to a few deps not working well for various reasons. I will add a todo to the ticket to check this in all three branches: 3.21.x, 3.24.x and master. |
No idea. I am not actively working on this PR right now, but rather the master one here: #1493 So I will fix things up in due course. |
257d668
to
d4a1cb1
Compare
reworked the commits from the master deps PR #1493 |
Added cherry pick of 3.21.x removal of dependency table to this PR since that will go inline with this change as well. @cf-bottom jenkins with exotics |
Sure, I triggered a build: (with exotics) Jenkins: https://ci.cfengine.com/job/pr-pipeline/11221/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-11221/ |
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 the last commit should be part of the OpenSSL version bump. Also please remove the cherry picked
references to private branches/commits.
@@ -1 +1 @@ | |||
https://www.openssl.org/source/ | |||
https://github.com/openssl/openssl/releases/download/openssl-3.3.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.
This change of the source is worth a comment in the commit message. Also, the commit message has a cherry-picked from
reference to your private branch.
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.
FWIW https://openssl-library.org/source/ links every release to github.com, there are no releases and openssl.org/source/ anymore.
build-scripts/install-dependencies
Outdated
$PATCH -p1 < $BASEDIR/buildscripts/build-scripts/perl-488307ffa6.patch | ||
gzip -dc perl-${PERL_VERSION}.tar.gz | tar xf - | ||
cd perl-${PERL_VERSION} | ||
# $PATCH -p1 < $BASEDIR/buildscripts/build-scripts/perl-488307ffa6.patch |
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.
Time to drop this patching and the patch itself?
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.
yeah, probably so. 👍
There are 4 commits related to OpenSSL version bump. Do you think all of them should be in one? fixup perl on centos-7, needs newer List::Utils module so build from source. fixup openssl windows patch, 0007 for openssl 3.3.2 Upgrade Perl version to latest for platforms like centos-7 that have insufficient version available |
Adjusted postgresql release-monitoring ID to reflect current supported version 16.x 301832 was 15.x and we already upgraded to 16.3 recently on master. https://release-monitoring.org/project/5601/ (PostgreSQL~current) was returning an RC version: 17 currently. We didn't want that. But now 17.0 is release so we will take it. 369409 pointed to 17 when it was an RC so also not helpful.
…insufficient version available
Ticket: ENT-12140 Changelog: none
Added note to refer to master README.md and individual files instead of README.md in LTS branches. Ticket: ENT-12140 Changelog: none
d4a1cb1
to
9fc47e6
Compare
This PR is proposed to merge to master. This is incorrect for this 3.24.x deps update. Will resubmit a new PR. |
No description provided.