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

Reduce CI go module downloads, add make targets #18708

Merged
merged 13 commits into from
Feb 12, 2022
Merged

Reduce CI go module downloads, add make targets #18708

merged 13 commits into from
Feb 12, 2022

Conversation

silverwind
Copy link
Member

The CI currently downloads all go modules in each pipeline step because go modules reside outside the project directory. Fix this by introducing a volume for the /go directory [1] so modules are only downloaded once per pipeline using a new deps-backend make target.

For completeness, I also included new deps and deps-frontend targets and the frontend one is also triggered explicitly on CI where needed.

[1] https://docs.drone.io/pipeline/kubernetes/examples/language/golang/#dependencies

@silverwind
Reduce CI go module downloads, add make targets
d40c814 
The CI currently downloads all go modules in each pipeline step because
go modules reside outside the project directory. Fix this by introducing
a volume for the `/go` directory [1] so modules are only downloaded once per
pipeline using a new `deps-backend` make target.

For completeness, I also included new `deps` and `deps-frontend` targets
and the frontend one is also triggered explicitly on CI where needed.

[1] https://docs.drone.io/pipeline/kubernetes/examples/language/golang/#dependencies
@silverwind silverwind added the topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile label Feb 10, 2022
@silverwind
Copy link
Member Author

silverwind commented Feb 10, 2022 •
edited
Loading

Seems there is a concurrency issue with the lint steps which all try to install golangci-lint in parallel into the shared volume, resulting in golangci-lint: Text file busy. Probably need to extract tool installation to a separate step or include it in deps-backend so it only runs once.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 10, 2022
@silverwind silverwind changed the title reduce CI go module downloads, add make targets Reduce CI go module downloads, add make targets Feb 10, 2022
@techknowlogick
Copy link
Member

@silverwind would "privileged" mode need to be enabled for this? If so, I can sign this yaml

@silverwind
Copy link
Member Author

Don't think it need privileges.

@silverwind
Copy link
Member Author

silverwind commented Feb 10, 2022 •
edited
Loading

Few more downloads to eliminate, on average, this saves about 15 seconds per step that previously downloaded modules. Failures on the database services are unrelated to the PR.

@techknowlogick
Copy link
Member

Awesome, thanks :)

I've restarted the build

@silverwind silverwind added the pr/wip This PR is not ready for review label Feb 10, 2022
@silverwind silverwind removed the pr/wip This PR is not ready for review label Feb 11, 2022
@silverwind
Copy link
Member Author

Should be ready for review.

There is one unrelated change included here which may speed up golangci-lint a bit by skipping over certain directories (thought it will not work for all linters, see golangci/golangci-lint#913.

@silverwind silverwind added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Feb 11, 2022
lunny
lunny reviewed Feb 11, 2022
View reviewed changes
.drone.yml Show resolved Hide resolved
@silverwind
Copy link
Member Author

@lunny I've gone through all pull statements and fixed them so they always are used on the first occurence of an image in a pipeline and the order in yaml is now image followed by pull which makes more sense to me.

@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 Feb 11, 2022
@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 Feb 11, 2022
@zeripath
Copy link
Contributor

make lgtm work

@zeripath zeripath merged commit 2ddff74 into go-gitea:main Feb 12, 2022
@zeripath
Copy link
Contributor

I think we might want to consider backporting this to 1.16.

@silverwind silverwind deleted the modules-cache-ci branch February 12, 2022 09:08
@silverwind
Copy link
Member Author

Right, there's no reason not to.

silverwind added a commit to silverwind/gitea that referenced this pull request Feb 12, 2022
@silverwind
Reduce CI go module downloads, add make targets (go-gitea#18708)
0e66d1d 
The CI currently downloads all go modules in each pipeline step because
go modules reside outside the project directory. Fix this by introducing
a volume for the `/go` directory [1] so modules are only downloaded once per
pipeline using a new `deps-backend` make target.

For completeness, I also included new `deps` and `deps-frontend` targets
and the frontend one is also triggered explicitly on CI where needed.

[1] https://docs.drone.io/pipeline/kubernetes/examples/language/golang/#dependencies
@silverwind silverwind mentioned this pull request Feb 12, 2022
Merged
zjjhot added a commit to zjjhot/gitea that referenced this pull request Feb 12, 2022
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
be1b906 
* giteaofficial/main:
  Send mail to issue/pr assignee/reviewer also when OnMention is set (go-gitea#18707)
  Reduce CI go module downloads, add make targets (go-gitea#18708)
  Add number in queue status to monitor page (go-gitea#18712)
  Fix source code line highlighting (go-gitea#18729)
  Fix forked repositories missed tags (go-gitea#18719)
  [skip ci] Updated translations via Crowdin
  Fix release typo (go-gitea#18728)
  Display template path of current page in dev mode (go-gitea#18717)
  Separate the details links of commit-statuses in headers (go-gitea#18661)
  Add LDAP group sync to Teams, fixes go-gitea#1395 (go-gitea#16299)
  Change git.cmd to RunWithContext (go-gitea#18693)
zeripath pushed a commit that referenced this pull request Feb 12, 2022
@silverwind
Reduce CI go module downloads, add make targets (#18708, #18475, #18443…
c04a4af 
…) (#18741)

Backport #18708 
Backport #18475 
Backport #18443 

The CI currently downloads all go modules in each pipeline step because go modules reside outside the project directory. Fix this by introducing a volume for the `/go` directory [1] so modules are only downloaded once per pipeline using a new `deps-backend` make target.

For completeness, I also included new `deps` and `deps-frontend` targets and the frontend one is also triggered explicitly on CI where needed.

[1] https://docs.drone.io/pipeline/kubernetes/examples/language/golang/#dependencies

* Also backports #18475 and #18443 so that is was able to merge cleanly.
Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: techknowlogick <techknowlogick@gitea.io>
@zeripath zeripath added the backport/done All backports for this PR have been created label Feb 12, 2022
@zeripath zeripath added this to the 1.17.0 milestone Feb 12, 2022
Caellion added a commit to Caellion/gitea that referenced this pull request Feb 16, 2022
@Caellion
Merge branch 'main' of https://github.com/go-gitea/gitea
8adf230 
* 'main' of https://github.com/go-gitea/gitea: (87 commits)
  Fix template bug of LFS lock (go-gitea#18784)
  Various Mermaid improvements (go-gitea#18776)
  [skip ci] Updated translations via Crowdin
  Fix display time of milestones (go-gitea#18753)
  [skip ci] Updated translations via Crowdin
  Prevent dangling GetAttribute calls (go-gitea#18754)
  Add example to render html files (go-gitea#18736)
  Fix a broken link in `commits_list_small.tmpl` (go-gitea#18763)
  Fix broken cancel button link on patch page (go-gitea#18718)
  Ignore the migrate if u2f_registration is not exist (go-gitea#18760)
  [skip ci] Updated translations via Crowdin
  Increase the size of the webauthn_credential credential_id field 
(go-gitea#18739)
  Fix isempty detection of git repository (go-gitea#18746)
  [skip ci] Updated translations via Crowdin
  Send mail to issue/pr assignee/reviewer also when OnMention is set 
(go-gitea#18707)
  Reduce CI go module downloads, add make targets (go-gitea#18708)
  Add number in queue status to monitor page (go-gitea#18712)
  Fix source code line highlighting (go-gitea#18729)
  Fix forked repositories missed tags (go-gitea#18719)
  [skip ci] Updated translations via Crowdin
  ...
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
@silverwind
Reduce CI go module downloads, add make targets (go-gitea#18708)
bfb81f7 
The CI currently downloads all go modules in each pipeline step because
go modules reside outside the project directory. Fix this by introducing
a volume for the `/go` directory [1] so modules are only downloaded once per
pipeline using a new `deps-backend` make target.

For completeness, I also included new `deps` and `deps-frontend` targets
and the frontend one is also triggered explicitly on CI where needed.

[1] https://docs.drone.io/pipeline/kubernetes/examples/language/golang/#dependencies
@zeripath zeripath mentioned this pull request Apr 19, 2022
Closed
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lunny lunny lunny left review comments

@lafriks lafriks lafriks approved these changes

@techknowlogick techknowlogick techknowlogick approved these changes

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. 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.17.0
Development

Successfully merging this pull request may close these issues.
6 participants
@silverwind @techknowlogick @zeripath @lunny @lafriks @GiteaBot