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 gitea/test_env image instead of golang #23455

Merged
merged 2 commits into from
Mar 15, 2023
Merged

Use gitea/test_env image instead of golang #23455

merged 2 commits into from
Mar 15, 2023

Conversation

silverwind
Copy link
Member

The safe.directory setting was not executed for pull requests, which made subsequent deps-backend target fail at go mod download. To fix it, split thep and perform the git config unconditionally.

Example: https://drone.gitea.io/go-gitea/gitea/69477/4/3

@silverwind silverwind added topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Mar 13, 2023
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 14, 2023
@silverwind
Copy link
Member Author

silverwind commented Mar 14, 2023 •
edited

The step order is still messed up and it sometimes does not execute early enough it seems.

https://drone.gitea.io/go-gitea/gitea/69480/3/12

image

@silverwind silverwind marked this pull request as draft March 14, 2023 00:11
@jolheiser
Copy link
Member

May need to set depends_on for the step order.

@silverwind
Copy link
Member Author

silverwind commented Mar 14, 2023 •
edited

This should be better, but drone does not show order graph yet, I guess we need to wait until execution begins or ends.

@silverwind
Copy link
Member Author

Looks better now

image

@silverwind
Copy link
Member Author

silverwind commented Mar 14, 2023 •
edited

Nope, it does not fix it: https://drone.gitea.io/go-gitea/gitea/69485/2/6

The command writes into ~/.gitconfig which drone throws away after every step because it's outside the working directory. The joys of drone CI...

I think we need a make target or something that does it before each test run, or find some other way to convince git that the directory is "safe" (I still despise git authors for adding this silly check).

@silverwind
Copy link
Member Author

silverwind commented Mar 14, 2023 •
edited

Local config does not work as git will not even read it.

I'm out of ideas besides sudo chown everything, but I wonder where the issue arises intitially. Shouldn't all files in a drone checkout already be owned by the user running it? Do we alter this somewhere?

@techknowlogick
Copy link
Member

Do we alter this somewhere?

Yes, the test_env container runs as UID 1000, and this was changed when we disabled the binary being able to run as root.

@techknowlogick
Copy link
Member

#23464 may solve some issues, as in the container I set the safe directory of git to * so it'll be that for each time the container is used.

@silverwind
Copy link
Member Author

silverwind commented Mar 14, 2023 •
edited

deps-backend currently executes in golang:1.20, so it won't help. But I guess we could just move deps-backend and possibly more steps to use the test_env container.

@techknowlogick
Copy link
Member

deps-backend currently executes in golang:1.20, so it won't help. But I guess we could just move deps-backend and possibly more steps to use the test_env container.

I wouldn't mind that, as with the recent improvements to test_env builds, it's much easier to update them as needed.

@silverwind
Copy link
Member Author

I removed the configure-git step again and switched all images:

gitea/test_env:linux-1.20-amd64 instead of golang:1.20
gitea/test_env:linux-1.20-arm64 instead of golang:1.20
gitea/test_env:linux-1.19-arm64 instead of golang:1.19

The release pipelines also had this git config command, but I think it never had any effect there because it modifies files outside the working directory, which are discarded when drone enters the next step in a pipeline.

@silverwind
Use gitea/test_env image instead of golang
d83fb70 
This avoids errors related to git's safe.directory in the golang image.
@silverwind silverwind changed the title Split up fetch-tags ci step Use gitea/test_env image instead of golang Mar 14, 2023
@silverwind silverwind marked this pull request as ready for review March 14, 2023 21:46
@silverwind
Copy link
Member Author

silverwind commented Mar 14, 2023 •
edited

Squashed and renamed PR.

@techknowlogick One more question regarding test_env: Is updating to new patch-level releases of golang automated? If not, we might start to fall behind the official golang periodically, which is a concern for security-related golang patches.

@techknowlogick
Copy link
Member

techknowlogick commented Mar 15, 2023 •
edited

@silverwind thank you so
much! Xgo is mostly automated, and once cron is a part of actions we can finalize the automation for test_env. Until then there are alerts for the security team that a rebuild is needed.

@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 Mar 15, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 15, 2023
@lunny lunny added this to the 1.20.0 milestone Mar 15, 2023
@techknowlogick techknowlogick merged commit ea1b926 into go-gitea:main Mar 15, 2023
@lunny lunny removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 15, 2023
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 15, 2023
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
a4a7dce 
* giteaofficial/main: (33 commits)
  Bump webpack from 5.75.0 to 5.76.0 (go-gitea#23484)
  Replace Less with CSS (go-gitea#23481)
  Fix 'View File' button in code search (go-gitea#23478)
  Use `gitea/test_env` image instead of `golang` (go-gitea#23455)
  Skip DB tests duplicate runs on push to branches (go-gitea#23476)
  Update app.example.ini (go-gitea#23480)
  [skip ci] Updated translations via Crowdin
  Fix due date being wrong on issue list (go-gitea#23475)
  test_env: hardcode major go version in use (go-gitea#23464)
  Push option bonus for PTC docs (go-gitea#23473)
  Lint Markdown pass
  Push to create docs (go-gitea#23458)
  Convert GitHub event on actions and fix some pull_request events. (go-gitea#23037)
  Remove wrongly added column on migration test fixtures (go-gitea#23456)
  Refactor branch/tag selector to Vue SFC (go-gitea#23421)
  add admin API email endpoints (go-gitea#22792)
  add user rename endpoint to admin api (go-gitea#22789)
  Add workflow error notification in ui (go-gitea#23404)
  Make branches list page operations remember current page (go-gitea#23420)
  fix markdown lint issue (go-gitea#23457)
  ...
@silverwind silverwind deleted the configure-git branch March 15, 2023 07:50
@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

@lunny lunny lunny approved these changes

@techknowlogick techknowlogick techknowlogick approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile
Projects
None yet
Milestone
1.20.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@silverwind @jolheiser @techknowlogick @lunny @GiteaBot