Now build core24 by valentindavid · Pull Request #90 · canonical/core-base

valentindavid · 2022-11-23T09:11:16Z

No description provided.

valentindavid · 2022-12-02T15:57:01Z

valentindavid · 2023-02-10T14:15:21Z

Waiting on snapcore/snapd#12345

Was merged.

alfonsosanchezbeato

Looks nice! I have some comments, in general I think we should try to minimize the references to coreXX to just core/base whenever possible so this is less annoying in the future.

alfonsosanchezbeato · 2023-02-13T12:10:35Z

Makefile

-	if [ -e /build/core22 ]; then \
-		/bin/cp $(DESTDIR)/usr/share/snappy/dpkg.list /build/core22/core22-$$(date +%Y%m%d%H%M)_$(DPKG_ARCH).manifest; \
-		/bin/cp $(DESTDIR)/usr/share/snappy/dpkg.yaml /build/core22/core22-$$(date +%Y%m%d%H%M)_$(DPKG_ARCH).dpkg.yaml; \
+	if [ -e /build/core24 ]; then \


Could we have a makefile variable to avoid us repeting the current base too much?

alfonsosanchezbeato · 2023-02-13T12:13:40Z

hooks/001-extra-packages.chroot

-     > /etc/apt/sources.list.d/ubuntu-image.list
-
-cat >/etc/apt/trusted.gpg.d/ucdev-base-ppa.asc <<'EOF'
+if false; then


Why are not we doing this anymore?

The PPA does not exist yet. It would fail.

Well, it exists, but maybe is it failing because there is no kinetic package in the PPA? Please add some TODO it that is the case.

Would be nice to open a PR readding this so it's visible

Ah, re-adding. Well it will be needed once we need to backport some packages. Which is probably not going to happen until we get close to or passed 24.04. It does not need to be re-added if we do not have any package.

Ah yes, excuse my lack of hyphen, that definitely reads wrong. I just wanted to make sure that it's added again if neccessary, but then a TODO is probably good enough,

alfonsosanchezbeato · 2023-02-13T12:16:43Z

snapcraft.yaml

-      - LTS: jammy
-      - BASE: ${LTS}-base-${CRAFT_TARGET_ARCH}.tar.gz
-      - DIR_URL: https://cdimage.ubuntu.com/ubuntu-base/${LTS}/daily/current
+      - LTS: "22.10"


Nitpick: not really an LTS now, maybe this should be named differently

alfonsosanchezbeato · 2023-02-13T12:18:21Z

tests/lib/prepare-uc.sh


 # install dependencies
-install_core22_deps
+install_core24_deps


Renaming to install_base_deps would be nice

alfonsosanchezbeato · 2023-02-13T12:18:37Z

tests/lib/prepare-uc.sh


 # download snaps required for us to build the image
-download_core22_snaps "$SNAP_BRANCH"
+download_core24_snaps "$SNAP_BRANCH"


download_base_snaps or similar

Well, given the implementation is only for core24, I am not sure I want to change that.

Ok, that's fine

alfonsosanchezbeato · 2023-02-13T12:18:58Z

tests/lib/prepare-uc.sh


 # finally build the uc image
-build_core22_image
+build_core24_image


build_base_image or similar

alfonsosanchezbeato · 2023-02-13T12:21:10Z

.github/workflows/tests.yaml

-          repository: 'snapcore/snapd'
+          repository: 'valentindavid/snapd'
          path: snapd
+          ref: 'de7785a57c3517d8d08700ed638709509b1148a6'


This should not be needed now, right?

alfonsosanchezbeato · 2023-02-13T12:23:01Z

tests/lib/prepare-utils.sh


    # get the model
-    curl -o ubuntu-core-amd64-dangerous.model https://raw.githubusercontent.com/snapcore/models/master/ubuntu-core-22-amd64-dangerous.model
+    #curl -o ubuntu-core-amd64-dangerous.model https://raw.githubusercontent.com/snapcore/models/master/ubuntu-core-24-amd64-dangerous.model


Maybe add a TODO here to get it from the models repo when we have the modle there?

alfonsosanchezbeato · 2023-02-13T12:24:03Z

tests/lib/prepare-utils.sh

    # download neccessary images
    snap download pc-kernel --channel=22/${snap_branch} --basename=upstream-pc-kernel
    snap download pc --channel=22/${snap_branch} --basename=upstream-pc-gadget
+    unsquashfs -d upstream-pc-gadget upstream-pc-gadget.snap


Please add TODO for when the 24 channel is available

pc gadget is not yet available for core24 branch

alfonsosanchezbeato

LGTM

valentindavid force-pushed the valentindavid/core24 branch 8 times, most recently from a23b390 to ec0d2b4 Compare November 23, 2022 12:46

valentindavid force-pushed the valentindavid/core24 branch from f62c07f to 72be7d9 Compare December 2, 2022 15:35

valentindavid marked this pull request as ready for review February 8, 2023 11:29

alfonsosanchezbeato reviewed Feb 13, 2023

View reviewed changes

valentindavid force-pushed the valentindavid/core24 branch 2 times, most recently from 92a571b to c4b53b1 Compare February 13, 2023 13:42

valentindavid added 4 commits February 14, 2023 14:52

Now build core24

ccd1d7b

Add model for core24

aacfd10

Patch pc gadget

fb6ec40

pc gadget is not yet available for core24 branch

Add missing systemd-resolved

4ecf8e3

valentindavid force-pushed the valentindavid/core24 branch from c4b53b1 to 4ecf8e3 Compare February 14, 2023 13:52

alfonsosanchezbeato approved these changes Feb 14, 2023

View reviewed changes

valentindavid merged commit 6260c70 into canonical:main Feb 17, 2023

Conversation

valentindavid commented Nov 23, 2022

Uh oh!

valentindavid commented Dec 2, 2022

Uh oh!

valentindavid commented Feb 10, 2023

Uh oh!

alfonsosanchezbeato left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alfonsosanchezbeato left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants