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

Build updates #148

Merged
merged 3 commits into from
Feb 12, 2024
Merged

Build updates #148

merged 3 commits into from
Feb 12, 2024

Conversation

hdholm
Copy link
Contributor

@hdholm hdholm commented Feb 6, 2024

This does two straightforward things. One, stop doing mass rebuilds when only the documentation has changed, and two update to actions/checkout@v4 in reaction to https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/ which will obsolete actions/checkout@v3. Unfortunately, the update to checkout (and thus node 20) seems to require libraries not available in CentOS 7. I therefore marked it as unstable in the config to keep its failure from stopping all other builds. But I'm really not certain what is the best way to proceed. It seems like checkout v3 will go end-of-life and not be usable "spring 2024" from the blog entry above. CentOS 7 is scheduled to go end-of-life at the end of June. So it seems like something will have to change before CentOS 7 is EOL to keep all the other builds working. The solution here, is probably not ideal, but the choices seem to be remove CentOS 7 altogether or duplicate a bunch of build script to use checkout v3 for CentOS 7. But that would only save it for a few months, still not quite to the end of June. So marking it unstable is a half-measure to raise the issue until there's a better decision.

Update checkout to avoid deprecation warnings and skip rebuilding just for documentation updates
Remove CentOS7 stability flag to check if it's just that OS failing actions/checkout@v4
@sarroutbi
Copy link
Collaborator

Hello @hdholm. Thanks for the PR. From my perspective, we could keep CentOS7 until it is EOL, and them remove it from the matrix. However, I don´t understand what you mean with this:
"So it seems like something will have to change before CentOS 7 is EOL to keep all the other builds working"
The rest of the builds (I understand non CentOS7) work with Node20/checkoutV4, right?

@hdholm
Copy link
Contributor Author

hdholm commented Feb 7, 2024

What I was trying to convey was that while CentOS 7 is still in the matrix in this patch, it doesn't actually build anymore. So it kind of seems pointless to leave it there. But the only way to make it build is to not do anything - and everything will fail when Node 16 goes away - or split the builds into CentOS 7 with v3 and everything else with v4. But that is only a stopgap measure because it seems like v3 won't be around through CentOS 7 EOL. i.e., the something that will have to change is v4 getting added to all the other builds either with this patch or another. It just wasn't clear to me that leaving CentOS 7 in had any real benefit to anyone given the choices. I hope that's more clear. It feels like I'm explaining my thinking badly here.

@sarroutbi
Copy link
Collaborator

Hello @hdholm. I understand your point now, thanks for clarifying. IMHO, I would directly remove the CentOS7 compile/test option in this PR. Once we merge it, if someone requires to include CentOS7 at CI level, we can discuss what to do.

Remove CentOS7 since it's nearing EOL and fails to build using actions/checkout@v4 and v3 will soon not be an option.
@hdholm
Copy link
Contributor Author

hdholm commented Feb 9, 2024

I removed CentOS 7 entirely given it's pending EOL and inability to support the Node 20 checkout changes.

Copy link
Collaborator

@sarroutbi sarroutbi left a comment

Choose a reason for hiding this comment

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

Change LGTM

@sarroutbi sarroutbi merged commit dae5654 into latchset:master Feb 12, 2024
22 checks passed
@hdholm hdholm deleted the BuildUpdates branch February 12, 2024 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants