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

unset XDG_HOME_CONFIG as gitea manages configuration locations #33067

Merged
merged 6 commits into from
Jan 1, 2025

Conversation

eeyrjmr
Copy link
Contributor

@eeyrjmr eeyrjmr commented Jan 1, 2025

unset XDG_CONFIG_HOME early to enable gitea to manage git configuration. simple error checking to satisfy the linting. Closes #33039

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 1, 2025
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jan 1, 2025
@github-actions github-actions bot added modifies/go Pull requests that update Go code modifies/cli PR changes something on the CLI, i.e. gitea doctor or gitea admin labels Jan 1, 2025
@eeyrjmr eeyrjmr force-pushed the drop_XDG_CONFIG_HOME branch from c9bb2c4 to c7eb088 Compare January 1, 2025 02:52
cmd/main.go Outdated Show resolved Hide resolved
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 1, 2025
@lunny
Copy link
Member

lunny commented Jan 1, 2025

Is it better to move it closer to commonBaseEnvs?

@wxiaoguang
Copy link
Contributor

commonBaseEnvs

commonBaseEnvs doesn't "unset"

@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 Jan 1, 2025
@wxiaoguang
Copy link
Contributor

Hmm, find another case: NewMainApp is the entry for the program, while the unset should also apply to the "testing" code. So maybe there could be a common function like setting.InitGiteaEnvVars() then call it in NewMainApp and unittest.InitSettings.

@eeyrjmr
Copy link
Contributor Author

eeyrjmr commented Jan 1, 2025

Hmm, find another case: NewMainApp is the entry for the program, while the unset should also apply to the "testing" code. So maybe there could be a common function like setting.InitGiteaEnvVars() then call it in NewMainApp and unittest.InitSettings.

good point,
so if I understand this correctly.

  1. add a function to ... modules/settings/config_env.go ( seems the most logical)
  2. call said function in NewMainApp and unittest.InitSettings

@wxiaoguang wxiaoguang marked this pull request as draft January 1, 2025 10:18
@eeyrjmr eeyrjmr marked this pull request as ready for review January 1, 2025 14:53
@techknowlogick
Copy link
Member

This may impact let's encrypt as I believe certmagic uses xdg to store the certs. I think this is overridden, but I can't fully remember. Just putting this note here so I remember to check once I'm at a computer

@wxiaoguang
Copy link
Contributor

This may impact let's encrypt as I believe certmagic uses xdg to store the certs. I think this is overridden, but I can't fully remember. Just putting this note here so I remember to check once I'm at a computer

I do not think any user could start Gitea with XDG_HOME_CONFIG to provide services to other users. Have they manually written XDG_HOME_CONFIG in their systemd gitea.service or docker-compose?

@lunny
Copy link
Member

lunny commented Jan 1, 2025

This may impact let's encrypt as I believe certmagic uses xdg to store the certs. I think this is overridden, but I can't fully remember. Just putting this note here so I remember to check once I'm at a computer

It seems these paths are provided by Gitea.
图片

@techknowlogick
Copy link
Member

@lunny thanks for confirming:)

@wxiaoguang wxiaoguang merged commit 233b795 into go-gitea:main Jan 1, 2025
26 checks passed
@GiteaBot GiteaBot added this to the 1.24.0 milestone Jan 1, 2025
@wxiaoguang
Copy link
Contributor

@lunny thanks for confirming:)

But the ACME path doesn't seem right. -> Try to fix ACME directory problem #33072

@eeyrjmr eeyrjmr deleted the drop_XDG_CONFIG_HOME branch January 1, 2025 23:55
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jan 2, 2025
* giteaofficial/main:
  [skip ci] Updated translations via Crowdin
  unset XDG_HOME_CONFIG as gitea manages configuration locations (go-gitea#33067)
  Refactor repo-new.ts (go-gitea#33070)
  Refactor pull-request compare&create page (go-gitea#33071)
  feat: link to nuget dependencies (go-gitea#26554)
  Remove some unnecessary template helpers (go-gitea#33069)
  Inherit submodules from template repository content (go-gitea#16237)
  [skip ci] Updated translations via Crowdin
  feat(action): issue change title notifications (go-gitea#33050)
  Use project's redirect url instead of composing url (go-gitea#33058)
  Fix unittest and repo create bug (go-gitea#33061)
  Fix locale type (go-gitea#33059)
  Refactor maven package registry (go-gitea#33049)
  Optimize the installation page (go-gitea#32994)
  [Feature] Private README.md for organization (go-gitea#32872)
  Make issue suggestion work for new PR page (go-gitea#33035)
  Add IntelliJ Gateway's .uuid to gitignore (go-gitea#33052)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/cli PR changes something on the CLI, i.e. gitea doctor or gitea admin modifies/go Pull requests that update Go code size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gitea can overwrite users gitconfig when using XDG spec
6 participants