Skip to content

Cd/create debian package on release #5464

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

Conversation

hannes-steffenhagen-diffblue
Copy link
Contributor

@hannes-steffenhagen-diffblue hannes-steffenhagen-diffblue commented Aug 17, 2020

  • 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.

@hannes-steffenhagen-diffblue hannes-steffenhagen-diffblue force-pushed the CD/create-debian-package-on-release branch from afa4801 to e9ae5a3 Compare August 17, 2020 14:10
@hannes-steffenhagen-diffblue
Copy link
Contributor Author

The windows build here is broken not because of any of these changes, but because sourceforge links we were downloading packages from went stale.

@NlightNFotis
Copy link
Contributor

NlightNFotis commented Aug 17, 2020

This is a sick PR by the way - this has the potential to make all our packaging super easy if I understood correctly?

So it's possible now to build a package in all platforms just by doing a build package (or ninja package)?

Nicely done in any case 💯

Copy link
Collaborator

@martin-cs martin-cs left a comment

Choose a reason for hiding this comment

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

This would be a good piece of functionality. I assume you have spoken to @tautschnig about this as he is the current maintainer for the Debian package : https://tracker.debian.org/pkg/cbmc

@hannes-steffenhagen-diffblue
Copy link
Contributor Author

@NlightNFotis Not all our packaging, unfortunately. But we can produce debian binary packages, WiX installers, simple binary zips etc from this.

For an Ubuntu PPA we would need to produce debian source packages apparently and I am not completely sure what's involved with that yet.

@martin-cs I wasn't actually aware of that.

@martin-cs
Copy link
Collaborator

@hannes-steffenhagen-diffblue he has the power to get a package into Debian (and thus, I think, downstream, to Ubuntu) plus has all of the control.tar.gz meta-information you need to get through the Debian linter.

@hannes-steffenhagen-diffblue
Copy link
Contributor Author

hannes-steffenhagen-diffblue commented Aug 17, 2020

@martin-cs Our short term goal right now is actually to have an ‘unstable’ Ubuntu PPA so that people can keep their cbmc up-to-date through that. If I understand the process correctly – and I am not completely sure I do – this wouldn’t really work with the “normal” mainstream Debian or Ubuntu release process. Checking out your link it seems that @tautschnig has some scripts to keep debian-unstable up to date, but if I understood correctly using that would mean also pulling in a lot of other debian-unstable dependencies. We’re trying to get it working with stable Ubuntu dependencies, but rolling releases for cbmc (if that makes sense).

I think we should talk to Michael regardless because of course long term making sure cbmc stays up to date in Ubuntu/Debian mainline is also important.

@martin-cs
Copy link
Collaborator

@hannes-steffenhagen-diffblue having your own package repo and getting people to pull from there seems like a positive step and a good first target. Longer-term I believe it might be possible (with the assistance of @tautschnig or at least @tautschnig 's scripts) to have the unstable package up-to-date and (as long as it doesn't have any dependencies beyond the current stable) set the repository priority so that you get just that one and not accidentally upgrade everything to unstable.

@codecov
Copy link

codecov bot commented Aug 17, 2020

Codecov Report

Merging #5464 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #5464   +/-   ##
========================================
  Coverage    68.25%   68.25%           
========================================
  Files         1180     1180           
  Lines        97722    97722           
========================================
  Hits         66698    66698           
  Misses       31024    31024           
Flag Coverage Δ
#cproversmt2 42.81% <ø> (ø)
#regression 65.42% <ø> (ø)
#unit 32.23% <ø> (ø)

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 e58dad8...4e5ba60. Read the comment docs.

@@ -0,0 +1,34 @@
set(CPACK_PACKAGE_NAME "cbmc") # XXX should this be cprover instead?
set(CPACK_PACKAGE_VENDOR "Diffblue Ltd.")
set(CPACK_PACKAGE_CONTACT "[email protected]") # XXX should this be a Diffblue email address instead?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Clarification on this point would be great, this is the address used in in src/cbmc/dist-linux, the current Ubuntu package lists “Ubuntu Developers” as the maintainer and @tautschnig as the ‘Original Maintainer’, so I’m genuinely unsure what to put here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you put multiple contacts in? Might also be worth asking @peterschrammel for an opinion too.

Copy link
Member

Choose a reason for hiding this comment

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

If possible I'd put [email protected] to have a stable address.

Copy link
Contributor

@chrisr-diffblue chrisr-diffblue left a comment

Choose a reason for hiding this comment

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

Generally looks good - as you say, you need to clarify some details but I'm not sure I have answers for you there.

run: |
cd build
ninja package
ls *.deb
Copy link
Contributor

Choose a reason for hiding this comment

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

What does the ls *.deb achieve, other than a nice output? Will it fail the build in someway? Or will it mask any failures from the ninja package because the exit code of the ls will be success?

Copy link
Contributor

@thomasspriggs thomasspriggs Aug 20, 2020

Choose a reason for hiding this comment

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

My experience of debugging the run sections of github actions suggests that it will stop on the first line where an error code is returned. Similar to writing a bash script where set -e has been specified. Therefore this extra line should not mask any other failures.

Copy link
Contributor Author

@hannes-steffenhagen-diffblue hannes-steffenhagen-diffblue Aug 21, 2020

Choose a reason for hiding this comment

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

@chrisr-diffblue ls returns a non-zero exit code if called on non-existent files... so this is just checking whether or not there is a file ending with .deb in the directory after ninja package. See here for a demonstration of how this works.

Failures propagate in github action scripts because I believe set -e is set by default.

@@ -0,0 +1,34 @@
set(CPACK_PACKAGE_NAME "cbmc") # XXX should this be cprover instead?
set(CPACK_PACKAGE_VENDOR "Diffblue Ltd.")
set(CPACK_PACKAGE_CONTACT "[email protected]") # XXX should this be a Diffblue email address instead?
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you put multiple contacts in? Might also be worth asking @peterschrammel for an opinion too.

Copy link
Contributor

@thomasspriggs thomasspriggs left a comment

Choose a reason for hiding this comment

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

LGTM.

It may be worth us investigating whether we can use cpack to make the .msi for the windows packaging as well. @markrtuttle added a handwritten xml file with the configuration to create it with WiX. But there is a WiX plugin for cpack which could potentially generate it for us and reduce the amount of packaging setup which would otherwise be duplicated.

- name: Build using Ninja
run: |
cd build
ninja
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 This could be done as a single line -
run: ninja -C build

@@ -0,0 +1,34 @@
set(CPACK_PACKAGE_NAME "cbmc") # XXX should this be cprover instead?
Copy link
Member

Choose a reason for hiding this comment

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

I think cbmc is more well-known.

@@ -0,0 +1,34 @@
set(CPACK_PACKAGE_NAME "cbmc") # XXX should this be cprover instead?
set(CPACK_PACKAGE_VENDOR "Diffblue Ltd.")
set(CPACK_PACKAGE_CONTACT "[email protected]") # XXX should this be a Diffblue email address instead?
Copy link
Member

Choose a reason for hiding this comment

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

If possible I'd put [email protected] to have a stable address.

@hannes-steffenhagen-diffblue
Copy link
Contributor Author

@thomasspriggs Yes, my plan was to try the WiX generation of CPack to reduce our maintenance burden on these things.

@hannes-steffenhagen-diffblue hannes-steffenhagen-diffblue force-pushed the CD/create-debian-package-on-release branch from 3469760 to 4e5ba60 Compare August 21, 2020 12:49
@hannes-steffenhagen-diffblue hannes-steffenhagen-diffblue merged commit b76314b into diffblue:develop Aug 21, 2020
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.

6 participants