Skip to content

Add --work-path and --custom-path to git hooks #25121

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

Closed
wants to merge 3 commits into from
Closed

Add --work-path and --custom-path to git hooks #25121

wants to merge 3 commits into from

Conversation

DavidGregory084
Copy link

Fixes #25107.

This adds explicit --work-path and --custom-path flags to the generated git hooks.

This should prevent hooks from inheriting different values for AppWorkPath and CustomPath than those used in the main gitea process.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 7, 2023
@delvh
Copy link
Member

delvh commented Jun 7, 2023

Hmm… I'm not quite sure how similar this is to #22754, especially if the same discussion still applies.

@DavidGregory084
Copy link
Author

DavidGregory084 commented Jun 7, 2023

Hmm that is an interesting discussion.

The root of this entire problem seems to be that the app.ini is not authoritative.

There are too many Gitea entry points in a single deployment to allow command-line and environment variable overrides that affect something fundamental like the working directories.

In the case of the snap package, this manifests itself in the form of hardcoded environment variables in snapcraft.yml that make it difficult for users to override the default paths consistently throughout the application.

I guess that runs counter to the 12-Factor-App philosophy, but the alternative is that the environment and command line flags are propagated consistently to any entry points.

@lunny
Copy link
Member

lunny commented Jun 7, 2023

Maybe we should store them into some runtime files and share for different processes. Otherwise everytime, the paths changed, we need to update all the repositories which will take too much time for a big instance. Or store them into database or memory for internal route endpoint.

@lunny lunny requested a review from wxiaoguang June 7, 2023 15:44
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jun 7, 2023

We shouldn't do this. There are more problems for these "config options".

I will fix the path problem in 1.21 (I do not want to introduce too many breaking changes in 1.20 since it is releasing soon)

@DavidGregory084
Copy link
Author

Yeah I just read #24818 and I agree that it's a much better solution 👍

@DavidGregory084 DavidGregory084 deleted the explicit-hook-path-config branch June 7, 2023 15:54
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Sep 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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.

Inconsistent value for [repository].ROOT causes errors on git push
5 participants