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

Pad every frontend snap with workaround part (infra) #934

Merged
merged 2 commits into from
Jan 12, 2024

Conversation

Hook25
Copy link
Collaborator

@Hook25 Hook25 commented Jan 12, 2024

Description

snapd has added a minimal 16kb size of snap files in the following commit (canonical/snapd@5bae3c1)
but the automated review tool unaware that. This causes very small snaps to be automatically rejected by the review tool because they are automatically padded by snapcraft but not by the cross-verification process that the review tool does.

To work around this I propose introducing a new part that links back to the upstream bug, making the build pass for now.

Resolved issues

Failing daily builds

Documentation

This patch comes with a comment explaining why the part is there and when it should be removed.

Tests

I have built the snap offline and it is passing the validator (also, in the last few days due to another bug, the only snaps that were passing the validator were the snaps who included some other build log, making them slightly bigger!)

kissiel
kissiel previously approved these changes Jan 12, 2024
Copy link
Contributor

@kissiel kissiel left a comment

Choose a reason for hiding this comment

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

I'm suggesting having a predictable (and informative) content as "padding".
See below.

checkbox-snap/series_classic16/snap/snapcraft.yaml Outdated Show resolved Hide resolved
checkbox-snap/series_classic18/snap/snapcraft.yaml Outdated Show resolved Hide resolved
checkbox-snap/series_classic20/snap/snapcraft.yaml Outdated Show resolved Hide resolved
checkbox-snap/series_classic22/snap/snapcraft.yaml Outdated Show resolved Hide resolved
checkbox-snap/series_uc16/snap/snapcraft.yaml Outdated Show resolved Hide resolved
checkbox-snap/series_uc18/snap/snapcraft.yaml Outdated Show resolved Hide resolved
checkbox-snap/series_uc20/snap/snapcraft.yaml Outdated Show resolved Hide resolved
checkbox-snap/series_uc22/snap/snapcraft.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@kissiel kissiel left a comment

Choose a reason for hiding this comment

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

Thanks for the header!

@kissiel kissiel merged commit 58c6d38 into main Jan 12, 2024
4 checks passed
@kissiel kissiel deleted the workaroud_review_tool_bug branch January 12, 2024 14:09
LiaoU3 pushed a commit to LiaoU3/checkbox that referenced this pull request Mar 20, 2024
* Add new workaround part to snapcraft.yaml

* Added header to file and updated comment
binli pushed a commit to binli/checkbox that referenced this pull request Mar 22, 2024
* Add new workaround part to snapcraft.yaml

* Added header to file and updated comment
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