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.

Sign up for GitHub

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

Jump to bottom

Use git.HOME_PATH for Git HOME directory #20114

Merged
merged 18 commits into from
Jul 8, 2022
Merged

Use git.HOME_PATH for Git HOME directory #20114

merged 18 commits into from
Jul 8, 2022

Conversation

wxiaoguang
Copy link
Contributor

@wxiaoguang wxiaoguang commented Jun 24, 2022 •
edited
Loading

This PR follows:

As discussed in discord, it seems that it is preferred to should use a separate dir for git home.

This PR introduces a new config option: git.HOME_PATH, which is default to %(APP_DATA_PATH)/home

@wxiaoguang wxiaoguang added the pr/wip This PR is not ready for review label Jun 24, 2022
@wxiaoguang wxiaoguang marked this pull request as draft June 24, 2022 06:04
@wxiaoguang wxiaoguang added the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label Jun 24, 2022
@wxiaoguang wxiaoguang changed the title [WIP] Add GIT_HOME_PATH instead of always using repository.ROOT Add git.HOME_PATH instead of always using repository.ROOT Jun 26, 2022
@wxiaoguang wxiaoguang removed status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR pr/wip This PR is not ready for review labels Jun 26, 2022
@wxiaoguang wxiaoguang marked this pull request as ready for review June 26, 2022 09:47
@wxiaoguang wxiaoguang requested a review from zeripath June 26, 2022 09:47
@codecov-commenter

This comment was marked as off-topic.

Sign in to view
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 26, 2022
@wxiaoguang wxiaoguang mentioned this pull request Jun 26, 2022
Merged
@justusbunsi

This comment was marked as off-topic.

Sign in to view
@wxiaoguang

This comment was marked as off-topic.

Sign in to view
@justusbunsi

This comment was marked as off-topic.

Sign in to view
@justusbunsi
Copy link
Member

justusbunsi commented Jun 26, 2022 •
edited
Loading

Code L-G-T-M. Just wondering if it would be a problem that the directory already exists but permissions cannot be changed? Scenario in mind: It's a mount point from a Kubernetes pod already having correct permissions but changing them is not possible unless running as root.

Is there a compiled executable I can use? I have issues with dependency installation and cannot compile/test right now.

Edit: add dashes to magic abbreviation.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jun 26, 2022
@luwol03

This comment was marked as off-topic.

Sign in to view
@wxiaoguang wxiaoguang removed the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Jun 26, 2022
@justusbunsi

This comment was marked as off-topic.

Sign in to view
@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Jun 27, 2022 •
edited
Loading

I will hide some unrelated comments (GitHub "bug") above 😊

Try to answer: "Just wondering if it would be a problem that the directory already exists but permissions cannot be changed?"

I think there will be no problem, it works like other directories like "avatar" / "repo-archive", etc, created and used. The permissions are likely not necessary to be changed at the moment. But it's still good to do confirmation for different environments. If the gnupg requires some special permissions, then it could be redirected to another directory, like the GNUPGHOME discussed before.

This PR is almost ready for review. There is one thing that I am uncertain, sec.Key("HOME_PATH").MustString("home"), what name should be used for the directory.

@wxiaoguang wxiaoguang added this to the 1.17.0 milestone Jun 27, 2022
@wxiaoguang wxiaoguang changed the title Add git.HOME_PATH instead of always using repository.ROOT Use git.HOME_PATH for Git HOME directory Jun 27, 2022
@lunny
Copy link
Member

lunny commented Jul 6, 2022

Some documents need to be updated, otherwise LGTM

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 6, 2022
lunny
lunny requested changes Jul 8, 2022
View reviewed changes
Copy link
Member

@lunny lunny left a comment

Choose a reason for hiding this comment

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

Wait for our discuss about documents

@lunny lunny merged commit 496b8e3 into go-gitea:main Jul 8, 2022
@wxiaoguang wxiaoguang deleted the add-git-home-path branch July 8, 2022 08:40
This was referenced Jul 8, 2022
Merged
Closed
wxiaoguang added a commit to wxiaoguang/gitea that referenced this pull request Jul 8, 2022
@wxiaoguang
Use git.HOME_PATH for Git HOME directory (go-gitea#20114)
f5f58ff 
Before, in go-gitea#19732, the old home directory is not correct.
This PR introduces a new config option for git home: git.HOME_PATH,
which is default to %(APP_DATA_PATH)/home

And pass env GNUPGHOME to git command, force Gitea to use a stable GNUPGHOME directory
@wxiaoguang wxiaoguang added the backport/done All backports for this PR have been created label Jul 8, 2022
@wxiaoguang wxiaoguang mentioned this pull request Jul 8, 2022
Merged
lunny pushed a commit that referenced this pull request Jul 8, 2022
@wxiaoguang
Use git.HOME_PATH for Git HOME directory (#20114) (#20293)
5e5ff77 
Before, in #19732, the old home directory is not correct.
This PR introduces a new config option for git home: git.HOME_PATH,
which is default to %(APP_DATA_PATH)/home

And pass env GNUPGHOME to git command, force Gitea to use a stable GNUPGHOME directory
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jul 9, 2022
@zjjhot
Merge remote-tracking branch 'giteaoffical/main'
62c4c96 
* giteaoffical/main:
  Implement sync push mirror on commit (go-gitea#19411)
  Use git.HOME_PATH for Git HOME directory (go-gitea#20114)
dineshsalunke pushed a commit to dineshsalunke/gitea that referenced this pull request Jul 9, 2022
@wxiaoguang @zeripath
@6543 @justusbunsi @lunny @dineshsalunke
Use git.HOME_PATH for Git HOME directory (go-gitea#20114)
c59d9c4 
* Add git.HOME_PATH

* add legacy file check

* Apply suggestions from code review

Co-authored-by: zeripath <[email protected]>

* pass env GNUPGHOME to git command, move the existing .gitconfig to new home, make the fix for 1.17rc more clear.

* set git.HOME_PATH for docker images to default HOME

* Revert "set git.HOME_PATH for docker images to default HOME"

This reverts commit f120101.

* force Gitea to use a stable GNUPGHOME directory

* extra check to ensure only process dir or symlink for legacy files

* refactor variable name

* The legacy dir check (for 1.17-rc1) could be removed with 1.18 release, since users should have upgraded from 1.17-rc to 1.17-stable

* Update modules/git/git.go

Co-authored-by: Steven Kriegler <[email protected]>

* remove initFixGitHome117rc

* Update git.go

* Update docs/content/doc/advanced/config-cheat-sheet.en-us.md

Co-authored-by: zeripath <[email protected]>
Co-authored-by: 6543 <[email protected]>
Co-authored-by: Steven Kriegler <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
dineshsalunke pushed a commit to dineshsalunke/gitea that referenced this pull request Jul 15, 2022
@wxiaoguang @zeripath
@6543 @justusbunsi @lunny @dineshsalunke
Use git.HOME_PATH for Git HOME directory (go-gitea#20114)
3b81e40 
* Add git.HOME_PATH

* add legacy file check

* Apply suggestions from code review

Co-authored-by: zeripath <[email protected]>

* pass env GNUPGHOME to git command, move the existing .gitconfig to new home, make the fix for 1.17rc more clear.

* set git.HOME_PATH for docker images to default HOME

* Revert "set git.HOME_PATH for docker images to default HOME"

This reverts commit f120101.

* force Gitea to use a stable GNUPGHOME directory

* extra check to ensure only process dir or symlink for legacy files

* refactor variable name

* The legacy dir check (for 1.17-rc1) could be removed with 1.18 release, since users should have upgraded from 1.17-rc to 1.17-stable

* Update modules/git/git.go

Co-authored-by: Steven Kriegler <[email protected]>

* remove initFixGitHome117rc

* Update git.go

* Update docs/content/doc/advanced/config-cheat-sheet.en-us.md

Co-authored-by: zeripath <[email protected]>
Co-authored-by: 6543 <[email protected]>
Co-authored-by: Steven Kriegler <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
@parnic parnic mentioned this pull request Aug 1, 2022
Open
vsysoev pushed a commit to IntegraSDL/gitea that referenced this pull request Aug 10, 2022
@wxiaoguang @zeripath
@6543 @justusbunsi @lunny
Use git.HOME_PATH for Git HOME directory (go-gitea#20114)
99a4cff 
* Add git.HOME_PATH

* add legacy file check

* Apply suggestions from code review

Co-authored-by: zeripath <[email protected]>

* pass env GNUPGHOME to git command, move the existing .gitconfig to new home, make the fix for 1.17rc more clear.

* set git.HOME_PATH for docker images to default HOME

* Revert "set git.HOME_PATH for docker images to default HOME"

This reverts commit f120101.

* force Gitea to use a stable GNUPGHOME directory

* extra check to ensure only process dir or symlink for legacy files

* refactor variable name

* The legacy dir check (for 1.17-rc1) could be removed with 1.18 release, since users should have upgraded from 1.17-rc to 1.17-stable

* Update modules/git/git.go

Co-authored-by: Steven Kriegler <[email protected]>

* remove initFixGitHome117rc

* Update git.go

* Update docs/content/doc/advanced/config-cheat-sheet.en-us.md

Co-authored-by: zeripath <[email protected]>
Co-authored-by: 6543 <[email protected]>
Co-authored-by: Steven Kriegler <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
@ghost ghost mentioned this pull request Aug 13, 2022
Closed
s-hamann added a commit to s-hamann/ansible-gitea that referenced this pull request Nov 8, 2022
@s-hamann
Support for Gitea 1.17.0, drop support for older versions
f493175 
Gitea 1.17.0 changed the Gitea user's home directory. See:
go-gitea/gitea#20114
go-gitea/gitea#19732
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@justusbunsi justusbunsi justusbunsi left review comments

@Gusted Gusted Gusted left review comments

@zeripath zeripath zeripath left review comments

@6543 6543 6543 approved these changes

@lunny lunny lunny approved these changes

@techknowlogick techknowlogick Awaiting requested review from techknowlogick

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! type/bug
Projects
None yet
Milestone
1.18.0
Development

Successfully merging this pull request may close these issues.
9 participants
@wxiaoguang @codecov-commenter @justusbunsi @luwol03 @lunny @zeripath @6543 @Gusted @GiteaBot