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

snap: refresh snap for base: core22 #1679

Merged
merged 7 commits into from Apr 17, 2023
Merged

snap: refresh snap for base: core22 #1679

merged 7 commits into from Apr 17, 2023

Conversation

Saviq
Copy link
Contributor

@Saviq Saviq commented Mar 22, 2023

This refreshes the snap packaging so that https://snapcraft.io/sccache can be served better.

@Saviq
Copy link
Contributor Author

Saviq commented Mar 22, 2023

This is now building here:

https://launchpad.net/~saviq/+snap/edge - it failed on s390x and ppc64el due to dependency issues.

I will follow up with a rework of the configuration hook to work with a configuration file rather than the environment.

@sylvestre
Copy link
Collaborator

oh, thanks for doing this. We noticed that it didn't get update for a while.

Could you please make sure it doesn't run as root in snap? thanks

Could you please add me as maintainer too ? (sylvestre@debian.org is my login - I am doing that as Mozilla upstream)

snap/snapcraft.yaml Outdated Show resolved Hide resolved
snap/snapcraft.yaml Outdated Show resolved Hide resolved
@sylvestre
Copy link
Collaborator

About the s390x port, see #1442

snap/snapcraft.yaml Outdated Show resolved Hide resolved
@Saviq
Copy link
Contributor Author

Saviq commented Mar 22, 2023

oh, thanks for doing this. We noticed that it didn't get update for a while.

Yeah, I never dotted all the "i"'s on this…

Could you please make sure it doesn't run as root in snap? thanks

Could you please add me as maintainer too ? (sylvestre@debian.org is my login - I am doing that as Mozilla upstream)

You've got mail, and we've requested it to get moved to Mozilla:

https://forum.snapcraft.io/t/publisher-change-request-for-sccache/34440

About the s390x port, see #1442

ACK.

I've added workflows to build the snap, but you'd have to enable actions from canonical/actions/* ideally. Or we can drop them, or avoid using actions that aren't allowed with some copy-paste:

https://github.com/mozilla/sccache/actions/runs/4489852240

@sylvestre
Copy link
Collaborator

yeah, I will ask the team to enable it, thanks :)

@Saviq
Copy link
Contributor Author

Saviq commented Mar 22, 2023

yeah, I will ask the team to enable it, thanks :)

Cool, for those to upload to the snap store, you'll need to set the SNAPCRAFT_TOKEN secret to the output of:

snapcraft export-login --snaps sccache --channels="*edge*" --acls package_access,package_push,package_release -

Or drop that part and store as artifacts instead, your call :)

Personally I use a bot account for those uploads, so if you have one like that - and want CI to upload to the store - let's add it as a collaborator as well?

@sylvestre
Copy link
Collaborator

Uploading them on tagged releases works for me

@Saviq
Copy link
Contributor Author

Saviq commented Mar 22, 2023

Could you please make sure it doesn't run as root in snap? thanks

I've just dropped the daemon altogether. From what I understand about how sccache operates, it doesn't make sense to have a central server.

@Saviq
Copy link
Contributor Author

Saviq commented Mar 22, 2023

Uploading them on tagged releases works for me

I'd say it only makes sense to upload from PRs (that's how it's currently set up), to edge/pr<number> channels, for easy testing of the changes.

Builds into other channels should be set up at https://snapcraft.io/sccache/builds, or through Launchpad snap recipes, once we merge this PR (no point before we do) - this will get you more architectures and a more stable build environment than the Actions one.

If you have other means of testing, we may well skip publishing the snap from PRs. I'd still build from PRs in case something breaks in there.

@codecov-commenter
Copy link

codecov-commenter commented Mar 22, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.07 🎉

Comparison is base (0af46b1) 29.33% compared to head (ac1d5ac) 29.40%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1679      +/-   ##
==========================================
+ Coverage   29.33%   29.40%   +0.07%     
==========================================
  Files          49       49              
  Lines       17056    17056              
  Branches     8272     8271       -1     
==========================================
+ Hits         5004     5016      +12     
- Misses       7000     7002       +2     
+ Partials     5052     5038      -14     

see 12 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

snap/snapcraft.yaml Outdated Show resolved Hide resolved
.github/workflows/snap.yml Outdated Show resolved Hide resolved
@sylvestre
Copy link
Collaborator

@Saviq Seems that it needs also another action:

Bad request - diddlesnaps/snapcraft-multiarch-action@v1 is not allowed to be used in mozilla/sccache.

Can we ignore it?

@Saviq
Copy link
Contributor Author

Saviq commented Apr 5, 2023

@sylvestre I could move the build-snap to @release to avoid that, but then it could only build for amd64.

Your call!

@Xuanwo Xuanwo changed the title snap: refresh snap for base: core22` snap: refresh snap for base: core22 Apr 5, 2023
@Saviq
Copy link
Contributor Author

Saviq commented Apr 5, 2023

@sylvestre feel free to push this:

diff --git a/.github/workflows/snap.yml b/.github/workflows/snap.yml
index f002d37..84c532c 100644
--- a/.github/workflows/snap.yml
+++ b/.github/workflows/snap.yml
@@ -10,16 +10,6 @@ jobs:
 
     timeout-minutes: 45
 
-    strategy:
-      matrix:
-        platform:
-        - amd64
-        - armhf
-        - arm64
-        # Broken dependencies: - ppc64el
-        # https://github.com/mozilla/sccache/issues/1442
-        # Broken environment: - s390x
-
     steps:
     - name: Check out code
       uses: actions/checkout@v3
@@ -27,9 +17,8 @@ jobs:
         fetch-depth: 0  # needed for version determination
 
     - name: Build and publish the snap
-      uses: canonical/actions/build-snap@multiarch
+      uses: canonical/actions/build-snap@release
       with:
-        architecture: ${{ matrix.platform }}
         snapcraft-token: ${{ secrets.SNAPCRAFT_TOKEN }}
         publish: ${{ github.event_name == 'pull_request' && github.repository == github.event.pull_request.head.repo.full_name }}
         publish-channel: edge/pr${{ github.event.number }}

@sylvestre
Copy link
Collaborator

@Saviq "I could move the build-snap to @Release to avoid that, but then it could only build for amd64."
=> yeah, this is fine, thanks

@Saviq
Copy link
Contributor Author

Saviq commented Apr 11, 2023

yeah, this is fine, thanks

Done.

Saviq and others added 6 commits April 12, 2023 18:09
There's not really a point running a system-wide daemon of sccache. At
least not by default. `sccache` forks a local server if one isn't
running already, and does it as the current user. The server needs write
access to the compilation path, so a central server… doesn't work very
well.
Co-authored-by: Sylvestre Ledru <sledru@mozilla.com>
Other arches require untrusted actions.
@sylvestre
Copy link
Collaborator

I think we fixed the server side configuration but we have an error:


Creating new base instance 'base-instance-snapcraft-buildd-base-v00--a12c0d0071c13c372948' from instance.
readlink: missing operand
Try 'readlink --help' for more information.
Run REVIEW_OPTS=()
Errors
------
 - lint-snap-v2:confinement_classic
	(NEEDS REVIEW) confinement 'classic' not allowed. If your snap needs classic confinement to function, please make a request for this snap to use classic by creating a new topic in the forum using the 'store-requests' category and detail the technical reasons why classic is required.
	https://forum.snapcraft.io/t/process-for-reviewing-classic-confinement-snaps/1460
sccache_v0.4.1-25-g7cba138_amd64.snap: FAIL

@Saviq
Copy link
Contributor Author

Saviq commented Apr 13, 2023

I think we fixed the server side configuration but we have an error:

Fixed, sorry!

@sylvestre sylvestre merged commit 2a7db60 into mozilla:main Apr 17, 2023
36 of 37 checks passed
@sylvestre
Copy link
Collaborator

no worries :)

@sylvestre
Copy link
Collaborator

@Saviq sorry for the dumb question but how do we publish it on snap ?
I see see the old release
https://snapcraft.io/sccache

@Saviq
Copy link
Contributor Author

Saviq commented Apr 17, 2023

@Saviq sorry for the dumb question but how do we publish it on snap ? I see see the old release https://snapcraft.io/sccache

Hey, so https://snapcraft.io/docs/releasing-your-app manually, but if you set the SNAPCRAFT_TOKEN secret, we can upload from CI.

@Saviq
Copy link
Contributor Author

Saviq commented Apr 17, 2023

@sylvestre to get builds from main, best probably hook it up under https://snapcraft.io/sccache/builds - you'll then just need to move the edge revisions to beta/candidate/stable as appropriate, here: https://snapcraft.io/sccache/releases

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.

None yet

3 participants