Skip to content

fix: snap build (main branch)#37645

Closed
wxiaoguang wants to merge 2 commits into
go-gitea:mainfrom
wxiaoguang:fix-snap
Closed

fix: snap build (main branch)#37645
wxiaoguang wants to merge 2 commits into
go-gitea:mainfrom
wxiaoguang:fix-snap

Conversation

@wxiaoguang
Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang commented May 11, 2026

  1. make "pull" and "build" testable and debuggable
  2. add more comments for how the build works
  3. separate 1.26 and main build tags
  4. fix incorrect tag describe (the current snap info gitea outputs version 1.22)

Legacy logic is kept as is although some of them don't seem good (e.g.: snap version grep, tag finding, etc)

ATTENTION:

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 11, 2026
@wxiaoguang wxiaoguang changed the title fix snap build (main branch) fixL snap build (main branch) May 11, 2026
@wxiaoguang wxiaoguang changed the title fixL snap build (main branch) fix: snap build (main branch) May 11, 2026
@wxiaoguang wxiaoguang requested a review from Copilot May 11, 2026 07:21
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the Snap build configuration on main to make the snapcraft pull/build steps easier to test/debug locally, improves inline documentation of the build flow, and adjusts version/tag handling so the produced snap version is derived more reliably from Git tags.

Changes:

  • Replace inline override-pull/override-build shell blocks in snapcraft.yaml with dedicated scripts (part-gitea-pull.sh, part-gitea-build.sh).
  • Implement tag/version/grade computation in part-gitea-pull.sh using git describe --tags and a normalized snap version string.
  • Adjust build tags for the main branch snap build in part-gitea-build.sh and document the intended differences vs the 1.26 snap build.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
snap/snapcraft.yaml Switches snapcraft part overrides to standalone scripts and updates the snap description text.
snap/part-gitea-pull.sh New pull-step script to decide whether to build a release tag vs current branch and to set snap version/grade.
snap/part-gitea-build.sh New build-step script to run the Gitea build with main-branch tags and stage artifacts into the snap part install dir.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread snap/part-gitea-pull.sh
Comment thread snap/part-gitea-pull.sh Outdated
Comment thread snap/part-gitea-pull.sh Outdated
Comment thread snap/part-gitea-build.sh
Comment thread snap/part-gitea-build.sh Outdated
@@ -0,0 +1,21 @@
#!/bin/sh
set -e
Copy link
Copy Markdown
Member

@silverwind silverwind May 11, 2026

Choose a reason for hiding this comment

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

Use set -euo pipefail (bash strict mode), like we already have in a number of other scripts.

Also recommend to run shellcheck on that script, not yet implemented in this repo, but I plan to add it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's just your guess.

If you think you are right, edit the code directly.

Don't bother me.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There's no guess in my assertions, sloppy scripts are just that, sloppy. I will push a fix to make it strict and shellcheck-clean.

Copy link
Copy Markdown
Member

@silverwind silverwind May 11, 2026

Choose a reason for hiding this comment

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

Don't bother me.

Wrong mentality, reviews should not bother. Unless you prefer to be sloppy/lazy which you always remind me not to be. Why is it ok if you do it?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fixed in 8e89cc0.

Switch shebang to bash so `set -o pipefail` is available, then enable
`set -euo pipefail` so the scripts fail fast on any unset variable,
failed command, or broken pipe (e.g. `snap info gitea` failing while
piped into `awk`).

Replace `[ p -o q ]` with `[ p ] || [ q ]` (SC2166), guard env var reads
with `${VAR:-}` so they're safe under `set -u`, fold the `[ -z $x ] &&
x=$(...)` idioms into `${x:-$(...)}` parameter expansion, and rewrite
the `A && B || C` ternary as an explicit `if/else` (SC2015).

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@wxiaoguang
Copy link
Copy Markdown
Contributor Author

I intentionally didn't use "bash" because sometimes Ubuntu only has "dash".

So if you'd like to maintain it, you can open a PR.

@wxiaoguang wxiaoguang closed this May 11, 2026
@wxiaoguang wxiaoguang deleted the fix-snap branch May 11, 2026 09:13
@silverwind
Copy link
Copy Markdown
Member

Ubuntu only has "dash"

These scripts run on ubuntu 24.04 or higher which has bash pre-installed.

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

Ubuntu only has "dash"

These scripts run on ubuntu 24.04 or higher which has bash pre-installed.

Yes, it is, but what's wrong with old code's "sh"? It is 1:1 translated from the legacy "snapcraft.yml"

I have no motivation or interest to spend any time on such bikeshedding.

You can open your PR and ask the awesome owners & TOC to approve.

@silverwind
Copy link
Copy Markdown
Member

You could just revert my commit, it is at most a suggestion, not binding.

@lunny
Copy link
Copy Markdown
Member

lunny commented May 13, 2026

Or maybe we can revert my solution and add a comment the tag could be removed when 1.27 is released because it will only affect stable versions.

@silverwind
Copy link
Copy Markdown
Member

silverwind commented May 13, 2026

Or do it on snapcraft.io like described in #37602 (comment) as a short-term fix, but will need adjustments next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants