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

x/pkgsite: make LICENSE detection optional in pkgsite codebase (not in pkg.go.dev) #39602

Closed
marwan-at-work opened this issue Jun 16, 2020 · 8 comments
Assignees
Labels
Milestone

Comments

@marwan-at-work
Copy link
Contributor

@marwan-at-work marwan-at-work commented Jun 16, 2020

Overview

Now that pkgsite is open sourced (congrats btw!), developers can run the server for sharing documentation of their private code amongst their internal teams.

However, private code almost always does not have a LICENSE file in its root repository because employees typically sign a separate agreement stating that the internal code is not to be copied or shared.

This means that pkgsite won't be able to show any documentation for most of the internal code out there (or at least the ones I've worked with as it is hard to quantify what you cannot access).

One option is to go around and add an appropriate LICENSE for all of the private code that both pkgsite can detect: https://pkg.go.dev/license-policy and is an appropriate LICENSE for private code. However, most of the licenses that pkgsite detects seem to be open-source oriented which is a non-starter for private modules. Even if there was a LICENSE that was meant for private-code, it doesn't guarantee that all teams can use it within their private repositories as it requires approvals from their company's legal entities.

Proposal

Therefore, it seems like the best option is to add a new configuration to pkgsite that bypasses the LICENSE check all together so that developers running the server for their internal teams don't need to go add a LICENSE to all of their private modules while still benefiting from using pkgsite and be able to give feedback!

Thanks!

@gopherbot gopherbot added this to the Unreleased milestone Jun 16, 2020
@julieqiu
Copy link
Contributor

@julieqiu julieqiu commented Jun 16, 2020

/cc @jba

@julieqiu julieqiu changed the title go.dev: make LICENSE detection optional in pkgsite codebase (not in pkg.go.dev) x/pkgsite: make LICENSE detection optional in pkgsite codebase (not in pkg.go.dev) Jun 16, 2020
@MicahParks
Copy link

@MicahParks MicahParks commented Aug 1, 2020

This does not solve this issue, as it is not a pull request, but I have a project that will hard code license respecting off and run pkgsite as a docker container. It has a few other use cases too.

I could figure out how to solve this with a flag, then create & submit pull request, unless the Go team is already on the issue.

I was able to use sed commands in the Dockerfile I made to manipulate some booleans that allowed the docs to build for projects without permissive licenses. You can see the affected files/lines in any of the athens.Dockerfile files in the below projects.

GitLab project
GitHub mirror

@jba jba self-assigned this Aug 4, 2020
@jba jba removed the NeedsInvestigation label Aug 4, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Aug 5, 2020

Change https://golang.org/cl/246957 mentions this issue: internal/postgres,internal/worker: support bypassing license restrictions

@gopherbot
Copy link

@gopherbot gopherbot commented Aug 6, 2020

Change https://golang.org/cl/247138 mentions this issue: internal/frontend: support bypassing license check

gopherbot pushed a commit to golang/pkgsite that referenced this issue Aug 6, 2020
Add a mode to postgres.DB where data is saved even if the module or
package is not redistributable.  This "bypassing license restrictions"
mode is off by default.

Add a flag to the worker binary to turn it on. Used a flag rather than
an environment variable so there is no way for ambient state to affect
the binary for this important decision.

Also:

- Add a test to verify that license data is removed/not removed
  depending on the bypass setting.

- Fix a bug where the new model insertion wasn't omitting
  documentation for non-redistributable modules.

- Fix a bug in internal/sample where the top-level readme wasn't
  getting populated.

A later CL will change the frontend to bypass the check.

For golang/go#39602
For golang/go#39629

Change-Id: I67a6d24c18f3b93cfbfc9ec2a20159c07a84e077
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/246957
Reviewed-by: Julie Qiu <julie@golang.org>
Run-TryBot: Julie Qiu <julie@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
@gopherbot
Copy link

@gopherbot gopherbot commented Aug 22, 2020

Change https://golang.org/cl/250017 mentions this issue: internal,internal/postgres: remove non-redist data on read

@jba jba added the NeedsFix label Aug 24, 2020
gopherbot pushed a commit to golang/pkgsite that referenced this issue Aug 25, 2020
The postgres package removes non-redistributable data during
insertion. Also remove it when reading, unless bypassLicenseCheck is
true.

Handling this in the DataSource layer simplifies frontend code, which
can avoid redistributability checks and knowing about the bypass flag
except when it constructs its DataSource.

As part of this work, move the removal of non-redistributable data to
the internal package, so it can eventually be used by the proxy data
source.

For golang/go#39602

Change-Id: Ia1362ead917b42f844b4c4d25ade74cdcb03d4dc
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/250017
Reviewed-by: Julie Qiu <julie@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Aug 25, 2020

Change https://golang.org/cl/250637 mentions this issue: internal/postgres: enforce redistributability and allow bypass for search

gopherbot pushed a commit to golang/pkgsite that referenced this issue Aug 26, 2020
…arch

- Unexport upsertSearchDocuments and verify that at its sole call
  site, non-redistributable data is removed unless the bypass flag is
  true. This "fixes" golang/go#40971, which was never actually broken.

- Remove non-redistributable data in
  GetPackagesForSearchDocumentUpsert. (This was a legitimate bug,
  although one that never had any effect.)

Fixes golang/go#40971
For golang/go#39602

Change-Id: I35493739de3a4ac0694ad3f07a371d9873f31b52
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/250637
Reviewed-by: Julie Qiu <julie@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Aug 26, 2020

Change https://golang.org/cl/250817 mentions this issue: cmd/frontend: add flag for bypassing license check

@gopherbot
Copy link

@gopherbot gopherbot commented Aug 26, 2020

Change https://golang.org/cl/250818 mentions this issue: internal/proxydatasource: remove non-redistributable data

gopherbot pushed a commit to golang/pkgsite that referenced this issue Aug 26, 2020
Remove non-redistributable data from modules that aren't
redistributable, unless a bypass option is set.

For golang/go#39602

Change-Id: I7832c60b2ac855b257ccfb776c87d05313e43cd2
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/250818
Reviewed-by: Julie Qiu <julie@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants