-
Notifications
You must be signed in to change notification settings - Fork 269
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
CBMC version 6 release process changes #7987
CBMC version 6 release process changes #7987
Conversation
V6 is currently in flux, with a not-very well established release process and potentially breaking changes to the binaries. Avoid pushing experimental releases to third party services while in preview.
Previously, it would be passed on to CMake which cannot handle suffixes following a `-`.
3a3ec3d
to
821d58c
Compare
REGEX "CBMC_VERSION = ([0-9.]+).*") | ||
string(REGEX REPLACE "CBMC_VERSION = ([0-9.]+).*" "\\1" CBMC_VERSION ${CBMC_VERSION}) |
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 section is to avoid a problem with CMake.
Essentially CMake accepts version of type major[.minor[.patch[.tweak]]]
and fails if anything else is fed.
Before we were passing anything specified in config.inc
as version and leaving CMake to validate it.
In this way we are just passing the part of the version that is composed of digits and .
s, so 6.0.0-preview
is considered a valid version and for CMake it is interpreted as 6.0.0
allowing CBMC itself to use the full version specified in config.inc
.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #7987 +/- ##
===========================================
+ Coverage 77.72% 78.37% +0.64%
===========================================
Files 1701 1701
Lines 196386 196386
===========================================
+ Hits 152647 153923 +1276
+ Misses 43739 42463 -1276 ☔ View full report in Codecov by Sentry. |
SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK }} | ||
SLACK_MESSAGE: "${{ job.status == 'success' && 'Homebrew PR submitted successfully' || 'Homebrew PR failed' }}" | ||
run: go run scripts/slack_notification_action.go | ||
# homebrew-pr: |
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.
⛏️ It might be good to put a short comment on each of these disabled sections to explain why it is disabled. It is possible that someone might see that they are not receiving new versions from Docker / homebrew and go looking here. So some comments could save some confusion.
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 will consider this for a next PR, but I want to move this one forward to unblock us.
In the meantime, I'm hoping that someone coming across this change would see the blame
long which would lead him to the commit 2d14e99 and its message that explains the situation.
|
||
## Stack | ||
|
||
* Revert changes to homebrew PR push (`.github/workflows/release-packages.yaml`) |
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.
Commit hashes could be really handy here, assuming that we can avoid invalidating them in a re-base before the PR is merged.
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'm not sure if we need commit hashes here - this was supposed to be a casual thing just to verify our reverted changes against the log here, sort of a double-entry bookkeeping practice but for the purpose of managing the process.
But if keen, I can also add this in a new PR.
Please wait with merging until we've got an agreed plan for version 6. |
@kroening I have now outlined a plan for the process to use for the major release on the Version 6 RFC here - #7743 (comment) I have outlined it on the RFC rather than here, so that the plan along with the discussion around it is straightforward to reference even once PR(s) are merged / closed. |
Hello @kroening, may I request another look at this one please? We're keen to move with the implementation, as we're in a relatively tight timeline with version 6, so if there are no blockers, we want to move with this one and sort out any minor disagreements in follow-up PRs. |
Is the time-line and plan for version 6 available anywhere? There are a bunch of interface-breaking changes it would be good to make to |
As raised (a long time ago) in #7743 we'd hoped to be we've been planning this for some time. The exact details of which Version 6 labelled PRs are merged will depend on if they're ready. The actual timeline is ASAP, I'd suggest raising the PR soon if you can. We'd love to start merging PRs and have unstable/pre-releases from... now. (In practice we don't want to cause too much collateral and so have not merged anything breaking yet.) |
Hello, I've merged this one now in the interest of moving the stack of dependent PRs forward (along with the rest of the process). Feel free to leave any more comments here, or in #7743 This is just the foundation of the rest of the work, so if you find anything objectionable here after the merge, feel free to ask for it and we can raise a new PR to cover that. |
This PR contains (temporary) changes while we're orchestrating the new (v6) release of CBMC going forward.
For now, it deactivates pushing build artefacts to third party services, and increments the version number.