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

Deb generation and upload #1676

Merged
merged 13 commits into from Mar 11, 2024
Merged

Deb generation and upload #1676

merged 13 commits into from Mar 11, 2024

Conversation

bennibbelink
Copy link
Contributor

This PR fixes the CMake build to once again provide Debian package generation functionality. It also adds stages to the Dockerfile and publish_release.yml workflow to build the Debian package and upload it automatically as a release asset (see my release). Feel free to download that .deb and install it on a fresh ubuntu image. Also - confirmation that the images pushed to GHCR are tagged correctly by the publish_release.yml workflow.

Another thing that is potentially relevant to this PR but I'm not sure about.... @gonuke we've spoken briefly before about the versioning scheme and how it is related to the datatypes used by cyclus. Does the versioning system need to be reworked at all? and/or do we just need to be careful about manually updating version numbers on each release?

@bennibbelink bennibbelink linked an issue Feb 28, 2024 that may be closed by this pull request
@coveralls
Copy link
Collaborator

coveralls commented Feb 28, 2024

Pull Request Test Coverage Report for Build 8226613785

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.001%) to 32.589%

Totals Coverage Status
Change from base Build 8226579368: 0.001%
Covered Lines: 52974
Relevant Lines: 128976

💛 - Coveralls

Copy link

Downstream Build Status Report

Build FROM cyclus_20.04_apt/cyclus
  • Cycamore: Success
  • Cymetric: Success
Build FROM cyclus_20.04_conda/cyclus
  • Cycamore: Success
  • Cymetric: Success
Build FROM cyclus_22.04_apt/cyclus
  • Cycamore: Success
  • Cymetric: Success
Build FROM cyclus_22.04_conda/cyclus
  • Cycamore: Success
  • Cymetric: Success

Copy link
Member

@gonuke gonuke left a comment

Choose a reason for hiding this comment

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

A few little things

Comment on lines +161 to +162
FROM scratch as deb-package
COPY --from=deb-generation /cyclus/build/cyclus*.deb /
Copy link
Member

Choose a reason for hiding this comment

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

What does this target accomplish?

Copy link
Member

Choose a reason for hiding this comment

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

Don't we want to save this as an artifact somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is how we can pass the .deb file back out to the GitHub runner. Otherwise it only exists within the docker image and we cannot access it on the runner. The code coverage workflow follows a similar pattern to pass coverage reports back out to the client. The files contained in this scratch layer are outputted to a destination that we specify when we build the target - see line 76 of publish_release.yml

src/env.cc.in Outdated
Comment on lines 43 to 46
if (instdir.length() > 0)
if (instdir.length() <= 0)
Copy link
Member

Choose a reason for hiding this comment

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

This seems like a big deal?!?!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right? The _ env var (and thus instdir) should be the path to the executable. If that env var is nonexistent then we use the path passed in by CMake, otherwise we use the location of the running executable. This was causing cyclus to always look in /root/.local for things even if it is installed elsewhere. Pretty sure the issue manifested itself when I installed the deb on a fresh machine it couldn't find the rng schema located in the share directory

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I don't think this fix is robust enough. I've opened an issue #1688

However, #1688 should not block this PR in my opinion

@gonuke
Copy link
Member

gonuke commented Mar 8, 2024

This looks like it needs a rebase for CHANGELOG

@gonuke
Copy link
Member

gonuke commented Mar 11, 2024

This may need a rebase

@gonuke gonuke merged commit 551535a into main Mar 11, 2024
17 checks passed
@bennibbelink bennibbelink deleted the deb-generation branch March 12, 2024 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

cyclus python package broke deb generation
3 participants