Skip to content

Update CBMC version to 5.20.1 #5648

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

Merged
merged 1 commit into from
Dec 10, 2020

Conversation

piotr-grabalski
Copy link
Contributor

Version update to 5.20.1 for release.

  • Each commit message has a non-empty body, explaining why the change was made.
  • Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • White-space or formatting changes outside the feature-related changed lines are in commits of their own.

Version update to 5.20.1 for release.
@codecov
Copy link

codecov bot commented Dec 10, 2020

Codecov Report

Merging #5648 (63df2ea) into develop (f520c41) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #5648   +/-   ##
========================================
  Coverage    69.41%   69.41%           
========================================
  Files         1243     1243           
  Lines       100609   100609           
========================================
  Hits         69839    69839           
  Misses       30770    30770           
Flag Coverage Δ
cproversmt2 43.14% <ø> (ø)
regression 66.31% <ø> (ø)
unit 32.27% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f520c41...63df2ea. Read the comment docs.

@tautschnig
Copy link
Collaborator

@piotr-grabalski Could you please explain what the point of this change is? In light of #5647 just merged, the commit message quite frankly makes no sense to me.

@piotr-grabalski
Copy link
Contributor Author

Hi @tautschnig,

The release 5.20.0 got a wrong version number in the end due to a human (mine) error. We are running release one more time with version 5.20.1 to get the version number right this time.

Thanks,
Piotr

@tautschnig
Copy link
Collaborator

The release 5.20.0 got a wrong version number in the end due to a human (mine) error. We are running release one more time with version 5.20.1 to get the version number right this time.

Could you please be more specific? We all make mistakes, we should just avoid repeating the same mistakes. Understanding what the mistake was would help to figure out how we could put in place better mechanisms to avoid them. FWIW, #5647 looked sane to me, so it must have happened somewhere else?

@piotr-grabalski
Copy link
Contributor Author

Sorry @tautschnig, let me explain in more details:

In order to perform a release the steps from: https://github.com/diffblue/cbmc/blob/develop/doc/ADR/release_process.md should be followed. The ideal flow is:

  • Modify src/config.inc file to reflect the correct version,
  • Commit the change and issue a PR
  • Once PR is approved and merged:
    • Update local CBMC git repository to pull merged PR changes
    • Tag the repository with annotated tag containing the new version number
    • Push tag to the git repository
      At this point Github actions are building the packages

What went wrong here is in bold font - I have forgotten to pull the merged PR (with config.inc modified) and tagged the previous commit which contained the old version number in config.inc file (5.19.0) therefore all of the packages got generated with wrong (but correct for the tagged commit) version number. I hope to get this right this time and that I pull from git repository before tagging 5.20.1.

Could you please merge this pull request if my explanation makes sense?

Thanks,
Piotr

@piotr-grabalski
Copy link
Contributor Author

Could you please merge this pull request if my explanation makes sense?

@tautschnig I will merge this pull request in order to proceed with the release - I hope my explanation makes sens but please contact me if you need more information.

Thanks,
Piotr

@piotr-grabalski piotr-grabalski merged commit a9a3644 into diffblue:develop Dec 10, 2020
@tautschnig
Copy link
Collaborator

Piotr, thank you for elaborating! So it seems we have a manual step and that can of course go wrong. How about https://github.com/marketplace/actions/github-tag-bump to avoid this manual step? I'll try to create a PR to this effect.

@tautschnig
Copy link
Collaborator

Piotr, thank you for elaborating! So it seems we have a manual step and that can of course go wrong. How about https://github.com/marketplace/actions/github-tag-bump to avoid this manual step? I'll try to create a PR to this effect.

I've created #5649 to try this out.

@piotr-grabalski
Copy link
Contributor Author

Michael, sorry for my late reply, the action you recommended looks promising - I think we have to switch off automatic bumping though, the version from config.inc file should be in sync with the tag. It will also require changing the instruction to include #major, #minor and/or #patch in order to automatically bump the version to get it in sync when config.inc file is checked in.

It for sure would limit errors due to a human factor but I think it will not eliminate them - it still will be possible to check-in wrong version in config.inc or accidentally bump the version tag. I think the key here will be to have a clear instructions.

@piotr-grabalski
Copy link
Contributor Author

Michael, sorry for my late reply, the action you recommended looks promising - I think we have to switch off automatic bumping though, the version from config.inc file should be in sync with the tag. It will also require changing the instruction to include #major, #minor and/or #patch in order to automatically bump the version to get it in sync when config.inc file is checked in.

It for sure would limit errors due to a human factor but I think it will not eliminate them - it still will be possible to check-in wrong version in config.inc or accidentally bump the version tag. I think the key here will be to have a clear instructions.

And it looks like you have thought about all of this already! Thanks!

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.

4 participants