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

Add go licenses to licenses.txt #21034

Merged
merged 15 commits into from Sep 3, 2022
Merged

Add go licenses to licenses.txt #21034

merged 15 commits into from Sep 3, 2022

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Sep 2, 2022 •
edited

make go-licenses will generate assets/go-licenses.json which is then included in the webpack build.

This step depends on both go and node being present, so unfortunately, I could not automate the generation by hooking it up to tidy as that target is triggered on CI where we do not have a docker image with both go an node.

It should be ran from time to time, ideally after each go mod update.

@6543
Copy link
Member

6543 commented Sep 2, 2022

coverage-bodged.out should not be within this pull

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 2, 2022
@silverwind
Copy link
Member Author

Should be fixed, branch got messed up through rebases.

@silverwind
Add go licenses to licenses.txt
0caad2f 
`make go-licenses` will generate `assets/go-licenses.txt` which is then
included in the webpack build. This step depends on both go and node
being present, so unfortunately, I could not automate the generation by
hooking it up to `tidy` as that target is triggered on CI.

It should be ran from time to time, ideally after each go mod update.
@silverwind
remove coverage-bodged.out
4ea9fc5
@silverwind
use same regex as google/go-licenses
98930e0
@silverwind
fix sorts
6cb3a72
@silverwind
misc tweaks
e86480e
@silverwind
Update build/generate-go-licenses.js
dd6c035
@silverwind silverwind added this to the 1.18.0 milestone Sep 3, 2022
@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 Sep 3, 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 Sep 3, 2022
@silverwind
Copy link
Member Author

Small rewrite incoming, which will sort all modules at once.

@silverwind
Copy link
Member Author

Data is now passed through a JSON file to webpack, which enabled me to sort all modules at once, e.g. showing now preference for js vs. go modules like earlier.

@silverwind
Copy link
Member Author

One future addition might be to parse the go module version from go.sum, but I'm not feeling like that right now 😉.

@6543
Copy link
Member

6543 commented Sep 3, 2022 •
edited

@silverwind add go-licenses target to make tidy and make vendor ?

not as precond. but as command afterwards

@silverwind
Copy link
Member Author

silverwind commented Sep 3, 2022 •
edited

Possible, but it would require node to be installed for checks-backend ci target because of the dependency chain:

checks-backend > tidy-check > tidy > go-licenses

@silverwind
Copy link
Member Author

silverwind commented Sep 3, 2022 •
edited

Did that now, so make tidy now triggers the target if necessary. I didn't feel like adding it to make vendor because that is invoked in release pipelines and it seems just unnecessary there.

@6543 6543 merged commit 49efd1f into go-gitea:main Sep 3, 2022
@silverwind silverwind deleted the go-licenses branch September 4, 2022 10:47
@zeripath
Copy link
Contributor

zeripath commented Sep 4, 2022

This is still running on make vendor and it's running somewhat intermittently occasionally resulting in an empty file.

I would say that this should really be in make generate as it is a generation step not tidy.

zeripath added a commit to zeripath/gitea that referenced this pull request Sep 4, 2022
@zeripath
Move go-licenses to generate
2c8a1a2 
The go-licenses check introduced in go-gitea#21034 is being run on make vendor
and occassionally causes an empty go-licenses file if the vendors need to
change. This should be moved to the generate task as it is a generated file.

Ref go-gitea#21034

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this pull request Sep 4, 2022
Merged
zjjhot added a commit to zjjhot/gitea that referenced this pull request Sep 5, 2022
@zjjhot
Merge remote-tracking branch 'upstream/main'
ccd705d 
* upstream/main: (22 commits)
  [skip ci] Updated translations via Crowdin
  Webhook for Wiki changes (go-gitea#20219)
  test: use `T.TempDir` to create temporary test directory (go-gitea#21043)
  Set uploadpack.allowFilter etc on gitea serv to enable partial clones with ssh (go-gitea#20902)
  Fix 500 on time tracking in timeline API (go-gitea#21052)
  Add more checks in migration code (go-gitea#21011)
  Fill the specified ref in webhook test payload (go-gitea#20961)
  [skip ci] Updated licenses and gitignores
  Add go licenses to licenses.txt (go-gitea#21034)
  Added docs for agit-setup (go-gitea#21027)
  Add another index for Action table on postgres (go-gitea#21033)
  Delete unreferenced packages when deleting a package version (go-gitea#20977)
  Improve arc-green code theme (go-gitea#21039)
  Add down key check has tribute container (go-gitea#21016)
  Do not add links to Posters or Assignees with ID < 0 (go-gitea#20577)
  [skip ci] Updated translations via Crowdin
  Show language name on hover (go-gitea#20923)
  fix: PackageMetadataVersion deps (go-gitea#21017)
  Fix the quick-submit for pending review comment (go-gitea#20992)
  Kd/ci playwright go test (go-gitea#20123)
  ...
lunny pushed a commit that referenced this pull request Sep 5, 2022
@zeripath @delvh
Move go-licenses to generate and separate generate into a frontend an…
8080e23 
…d backend component (#21061)

The `go-licenses` make task introduced in #21034 is being run on make vendor
and occasionally causes an empty go-licenses file if the vendors need to
change. This should be moved to the generate task as it is a generated file.

Now because of this change we also need to split generation into two separate 
steps:

1. `generate-backend`
2. `generate-frontend`

In the future it would probably be useful to make `generate-swagger` part of `generate-frontend` but it's not tolerated with our .drone.yml

Ref #21034

Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: delvh <dev.lh@web.de>
@silverwind
Copy link
Member Author

silverwind commented Sep 5, 2022 •
edited

This is still running on make vendor and it's running somewhat intermittently occasionally resulting in an empty file.

I would say that this should really be in make generate as it is a generation step not tidy.

It has to run after tidy thought, otherwise it may produce outdated results. Thought if it runs after tidy-check, that will also work because that target ensure tidy is consistent.

@zeripath
Copy link
Contributor

zeripath commented Sep 5, 2022

It has to run after tidy thought, otherwise it may produce outdated results. Thought if it runs after tidy-check, that will also work because that target ensure tidy is consistent.

You can't really build without make vendor being up-to-date so that's probably not the issue.

In fact we should be able to make make make vendor when make backend if go.mod is newer than go.sum, and then if we make make go-licenses dependent on vendor and go.mod we should be able to make make make go-licenses when we make frontend

The only issue is how drone will behave.

@silverwind
Copy link
Member Author

silverwind commented Sep 5, 2022 •
edited

As I see it make vendor should be a complete standalone task. It just downloads what is in go.sum to the vendordirectory, used for release source tarballs. Build will work with or without this directory as it sources dependencies from GOPATH.

In any case, I think I will at least rewrite generate-go-licenses.js to go, to eliminate the node dependency of the step.

@silverwind
Copy link
Member Author

Go rewrite of script in #21078.

@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

@6543 6543 6543 approved these changes

@delvh delvh delvh 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.
Projects
None yet
Milestone
1.18.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@silverwind @6543 @zeripath @lunny @delvh @GiteaBot