Use correct path to snap binary #17274
Conversation
techknowlogick
added
type/bug
skip-changelog
This comment has been minimized.
This comment has been minimized.
GiteaBot
added
the
lgtm/need 2
| snapPath := os.Getenv("SNAP") | ||
| if snapPath != "" && strings.HasPrefix(appPath, snapPath) { | ||
| revision := os.Getenv("SNAP_REVISION") | ||
| appPath = strings.Replace(appPath, revision, "current", 1) |
There was a problem hiding this comment.
I think we should be more careful about the string replacement. Snap document says that the SNAP_REVISION can be something like "x1" (https://snapcraft.io/docs/environment-variables) , a very short string. Maybe there will be other revision strings in future.
So maybe we should just split the appPath and check if the second last field is SNAP_REVISION and replace it, and check the gitea binary existence again. And print a log here to tell users we changed the app path.
There was a problem hiding this comment.
I agree. Then the code would be like what I proposed in PR 17157, but with an added check that the second last field is SNAP_REVISION. That would also include a check with exec.LookPath() that the modified path will actually work.
| @@ -51,6 +52,8 @@ For documentation about each of the variables available, refer to the | |||
| - `USER`: System user that Gitea will run as. Used for some repository access strings. | |||
| - `USERNAME`: if no `USER` found, Gitea will use `USERNAME` | |||
| - `HOME`: User home directory path. The `USERPROFILE` environment variable is used in Windows. | |||
| - `SNAP`: Path to directory that snap uses to store Gitea's configuration. Snap defaults to using `$GITEA_APP_PATH`. | |||
| - `SNAP_REVISION`: If Gitea is installed using snap, revision number of snap in use. | |||
There was a problem hiding this comment.
SNAP and SNAP_REVISION are not intended to be used by real users. So Maybe they can be written in other sections and warn users do not set these variables manually.
Co-authored-by: zeripath <[email protected]>
|
in terms of hacktoberfest i think it would not count for OleHusgaard ... but you could push final version of this pull HEAD back to his HEAD ... or I could do so ;) etc... |
|
@6543 it wouldn't count because the other PR was made before oct1 |
|
oh ... ok then it doesnt matter :D |
|
Hey, I just have a new idea about this problem. Why should we spend so much time on guessing the gitea binary path inside snap .... Can't we just check and update the repository's hooks on every push? As long as every git push changes many files, we check two or three hook files it doesn't cost much. Once we can make sure the content of the hooks are correct, we do not need to guess the snap path. |
|
The new idea is implemented in #17335 , I think the new PR can fully resolve the problem |
|
@techknowlogick Could you confirm that #17335 replaced this PR. |
replace #17157
Fixes #16209
co-authored-by: @OleHusgaard