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

Fix snapcraft.yaml to properly use stage-packages #137

Merged
merged 3 commits into from
Sep 6, 2023

Conversation

dilyn-corner
Copy link
Contributor

@dilyn-corner dilyn-corner commented Aug 28, 2023

Expanding the PYTHONPATH is required to pickup the germinate stage-package

Without specifying PTYHONPATH you may hit such an error:

[8] germinate
Traceback (most recent call last):
  File "/snap/ubuntu-image/512/usr/bin/germinate", line 25, in <module>
    from germinate.scripts.germinate_main import main
ModuleNotFoundError: No module named 'germinate'

Additionally, debootstrap is a stage-package but will not be used -- if debootstrap is not installed on the host, you'll run into both a "missing from $PATH" error and, after you correct the $PATH variable used in the snapcraft.yaml, you end up with debootstrap being unable to find some relevant files in /usr/share -- this can be resolved by setting an environment variable DEBOOTSTRAP_DIR.

The last commit in this PR adds two new stage-packages: devscripts and grub-common. It's possible that a user will not have these packages installed, and they are requirements for building some variations of our pc-gadget snap (https://github.com/snapcore/pc-gadget/tree/classic). I leave it up to whomever reviews this to determine if these are valid inclusions for ubuntu-image.

@codecov
Copy link

codecov bot commented Aug 28, 2023

Codecov Report

Merging #137 (2b663a1) into main (28b7b85) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #137   +/-   ##
=======================================
  Coverage   88.71%   88.71%           
=======================================
  Files          11       11           
  Lines        2383     2383           
=======================================
  Hits         2114     2114           
  Misses        265      265           
  Partials        4        4           

Required to pickup the germinate stage-package

Signed-off-by: Dilyn Corner <[email protected]>
debootstrap is a stage-package but is not in the $PATH. Additionally,
DEBOOTSTRAP_DIR must be set because the canonical path of
/usr/share/debootstrap is prefixed with $SNAP as a snap.

Signed-off-by: Dilyn Corner <[email protected]>
When building classic images, ubuntu-image will optionally run make to
build the gadget tree. Canonical's gadget snaps use a Makefile to create
certain artifacts, and some important commands used in this Makefile
(chdist, grub-mkimage) are provided by devscripts and grub-common. Add
these dependencies as stage-packages so that ubuntu-image can readily
build some gadget snaps, especially on non-Ubuntu systems.

Signed-off-by: Dilyn Corner <[email protected]>
@dilyn-corner dilyn-corner changed the title snapcraft.yaml: add PYTHONPATH to environment Fix snapcraft.yaml to properly use stage-packages Sep 4, 2023
@sil2100
Copy link
Collaborator

sil2100 commented Sep 5, 2023

Thank you for this PR @dilyn-corner ! The changes themselves look good, I just need to run some tests to see if everything is still working as expected. The additional stage-packages feel okay as well - we wanted to leave those out and leave this to gadget owners to recommend packages to install, but I think they're common enough in the gadget-build world that they make sense.
Would be nice if we had an interface for the gadget tree to inform ubuntu-image and the user of what build dependencies it requires. But that'll be made obsolete with imagecraft.

@dilyn-corner
Copy link
Contributor Author

Yes I was very hesitant on adding the requirements for building some gadgets -- this would be less of an issue if ubuntu-image would use snapcraft to build the gadget instead of make, but this is a whole other debacle :)

I'm certain my testing isn't as comprehensive as yours, but I've been building a few dozen different (Classic) images over the last few days with these changes and haven't noticed any breaks FWIW.

Copy link
Collaborator

@sil2100 sil2100 left a comment

Choose a reason for hiding this comment

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

Looks good indeed!

As for using snapcraft for building the gadgets - we actually long time ago initially used it indeed! Then had to change to Makefiles due to snapcraft being in Ubuntu universe. Now we could revisit this eventually, but I think all of these problems will go away when we switch to imagecraft.

@sil2100 sil2100 merged commit c1abc94 into canonical:main Sep 6, 2023
9 of 10 checks passed
@dilyn-corner dilyn-corner deleted the fix-pythonpath branch October 27, 2023 21:52
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