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

x/pkgsite/cmd/pkgsite: remove most external dependencies #61399

Open
matloob opened this issue Jul 17, 2023 · 41 comments
Open

x/pkgsite/cmd/pkgsite: remove most external dependencies #61399

matloob opened this issue Jul 17, 2023 · 41 comments
Assignees
@matloob
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
pkgsite/backlog

Comments

@matloob
Copy link
Contributor

matloob commented Jul 17, 2023

We'd like to get cmd/pkgsite to a point where it can be used to provide documentation from the go command. To do that we'd have to remove most of its external dependencies to a point where we can integrate it into part of the go command. Currently a lot of the dependencies are not necessary for the functionality of cmd/pkgsite but are needed for the frontend. We should be able to inject a lot of the dependencies and remove others so that the build of cmd/pkgsite has very few dependencies so that it could be vendored into the go repo.
@gopherbot gopherbot added this to the Unreleased milestone Jul 17, 2023
@hyangah hyangah modified the milestones: Unreleased, pkgsite/later Jul 17, 2023
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 18, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/514521 mentions this issue: internal/middleware: move stats to its own package

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/514519 mentions this issue: internal/vuln: remove the dependency on golang.org/x/exp

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/514518 mentions this issue: internal/frontend: add an interface for creating request caches

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/514675 mentions this issue: internal/middleware: delete Language and languageTag

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/514520 mentions this issue: internal/frontend: remove experiment middleware from newTestServer

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/514522 mentions this issue: all: switch from github.com/ghodss/yaml to gopkg.in/yaml.v3

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/514676 mentions this issue: internal/frontend: remove dependency on cloud error reporting client

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/515376 mentions this issue: internal/config: remove dependency on MonitoredResource proto

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/515375 mentions this issue: cmd/log: move stackdriver logging to its own package

gopherbot pushed a commit to golang/pkgsite that referenced this issue Aug 4, 2023
@matloob
internal/frontend: add an interface for creating request caches
4434dd5 
This change adds a new Cacher interface that is used to create
middlewares for caching requests. This abstracts away the use of redis
so that the frontend doesn't depend on redis. The tests still depend
on redis for the 404 page testing logic, but the 404 page logic will
be moved out into a different package so those tests will go too.

The Expirer and Middleware interfaces are not present on the Cache
function so that the interface can be defined in package
internal/frontend without needing the dependency on the Middleware
package.

For golang/go#61399

Change-Id: I6518b2ed1d772cb4deda3308c4190f0f1b8a35a0
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/514518
kokoro-CI: kokoro <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Jamal Carvalho <[email protected]>
Run-TryBot: Michael Matloob <[email protected]>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Aug 4, 2023
@matloob
internal/vuln: remove the dependency on golang.org/x/exp
30cc364 
It's used for slices.Compact. This change makes a copy of that
function, which should be removed once we can depend on it being
present in the standard library.

For golang/go#61399

Change-Id: I3fcd49a3d4cf1a40c402ca137efa9a16929c2cc1
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/514519
kokoro-CI: kokoro <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Michael Matloob <[email protected]>
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Aug 4, 2023
@matloob
internal/frontend: remove experiment middleware from newTestServer
051a825 
No experiments are actually set in the callers to newTestServer or
testServer.

For golang/go#61399

Change-Id: Iae58a670cc17356bd24634e6ff17ee9f70ca5f69
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/514520
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
Run-TryBot: Michael Matloob <[email protected]>
kokoro-CI: kokoro <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Aug 4, 2023
@matloob
internal/middleware: move stats to its own package
9e6bdc9 
Make a package internal/middleware/stats for middleware.Stats and
middleware.ElapsedStat. This is part of removing the dependency from
internal/frontend on internal/middleware.

For golang/go#61399

Change-Id: I44afbfc9b9e28e1caabab8fe700376ec026c863d
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/514521
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Michael Matloob <[email protected]>
Reviewed-by: Jamal Carvalho <[email protected]>
kokoro-CI: kokoro <[email protected]>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Aug 4, 2023
@matloob
internal/middleware: delete Language and languageTag
b486dfa 
The Language middleware isn't referenced anywhere else in the code, so
the context tag can't be set anywhere and languageTag must always be
returning English. Just hardcode English everywhere instead

For golang/go#61399

Change-Id: Id8cee6b9fb87a7f8f3460fb6eaf2f14ea0af7c8d
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/514675
Run-TryBot: Michael Matloob <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
kokoro-CI: kokoro <[email protected]>
Reviewed-by: Jamal Carvalho <[email protected]>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Aug 4, 2023
@matloob
all: switch from github.com/ghodss/yaml to gopkg.in/yaml.v3
1e93998 
yaml.v3 is the more widely used yaml. The main difference between the
two is that ghodss converts yaml to JSON as an intermediate to
marshalling and unmarshalling. That means we need to add yaml struct
tags to use yaml.v3.

Add yaml tags to ConfigOverride and QuotaSettings because yaml.v3
lowercases the names of fields by default and the configs have used
uppercase names. Change a test experiment config to use lowercase
names since that's what's used in most other configs.

For golang/go#61399

Change-Id: Id7f09f2635ee013506b1573bfe555ec0348e60e4
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/514522
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Michael Matloob <[email protected]>
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
Reviewed-by: Jamal Carvalho <[email protected]>
kokoro-CI: kokoro <[email protected]>
kijimaD pushed a commit to kd-collective/pkgsite that referenced this issue Aug 6, 2023
@matloob
internal/frontend: remove dependency on cloud error reporting client
52eb228 
This removes the dependency of internal/frontend on the cloud error
reporting client, both directly, and through the derrors package by
introducing a new interface Reporter that is used both to set the
reporting client for internal/derrors, and on the Server.

For golang/go#61399

Change-Id: Id4d4def522cda9b4e49f53cff6708019dec2693c
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/514676
Reviewed-by: Jamal Carvalho <[email protected]>
kokoro-CI: kokoro <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Michael Matloob <[email protected]>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Aug 7, 2023
@matloob
cmd/log: move stackdriver logging to its own package
5237f18 
This change breaks out the stackdriver logging to its own package. It
also only does logging of trace ids and labels to the stackdriver
logger, to keep the logging interface simple.

For golang/go#61399

Change-Id: I9a25d0c6391d1667fe476e5fdc30fc057f07c40f
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/515375
TryBot-Result: Gopher Robot <[email protected]>
kokoro-CI: kokoro <[email protected]>
Run-TryBot: Michael Matloob <[email protected]>
Reviewed-by: Jamal Carvalho <[email protected]>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Aug 7, 2023
@matloob
internal/config: remove dependency on MonitoredResource proto
3918c80 
For golang/go#61399

Change-Id: I90070de1c2fcde2bb4056c30637d4ff4fdf100f5
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/515376
kokoro-CI: kokoro <[email protected]>
Run-TryBot: Michael Matloob <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Jamal Carvalho <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/517976 mentions this issue: internal/frontend: move base types to their own packages

gopherbot pushed a commit to golang/pkgsite that referenced this issue Aug 15, 2023
@matloob
internal/frontend: move base types to their own packages
712da68 
This change moves serverError, basePage and errorPage out to different
packages so that the fetch page logic can use them but be moved out of
internal/frontend.

For golang/go#61399

Change-Id: I72ccee40d1847d3211ca851a320530c4c1dcf2e2
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/517976
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Michael Matloob <[email protected]>
kokoro-CI: kokoro <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/519335 mentions this issue: internal/frontend: move fetchserver and versions to their own packages

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/518817 mentions this issue: internal/frontend: separate fetch and 404 logic into fetchserver

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/520615 mentions this issue: internal/testing/fakedatasource: add a fake for internal.DataSource

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/520878 mentions this issue: internal/testing/fakedatasource: implement PostgresDB interface

gopherbot pushed a commit to golang/pkgsite that referenced this issue Aug 21, 2023
@matloob
internal/frontend: separate fetch and 404 logic into fetchserver
d493045 
ServeFetch and ServePathNotFoundPage only work when a postgres
database is present. Separate them out into a different fetchserver
type which can be moved into a different package (in a followup cl).
This will allow their behavior, which is not used by cmd/pkgsite, to
be removed from that server. More importantly, their tests, which
depend on a real postgres database can be separated from the tests of
package internal/frontend.

For golang/go#61399

Change-Id: I73677fd06750fd48580071b8a895b322d9e3ac5d
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/518817
kokoro-CI: kokoro <[email protected]>
Run-TryBot: Michael Matloob <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/538767 mentions this issue: internal/source: move some tests into another package

gopherbot pushed a commit to golang/pkgsite that referenced this issue Nov 7, 2023
@matloob
internal/source: move some tests into another package
8d5d35d 
The goal of this CL is to remove the transitive test of cmd/pkgsite on
the github.com/google/go-replayers/httpreplay package. The dependency
happens through the tests of the source package, so move the test out
of the source package into its own package.

For golang/go#61399

Change-Id: Ia0d5e4310e25ec0a034cd84f0d5cbcb06b5a75d8
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/538767
kokoro-CI: kokoro <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Run-TryBot: Michael Matloob <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/542058 mentions this issue: internal/testing/htmlcheck: replace use of cascadia in In

gopherbot pushed a commit to golang/pkgsite that referenced this issue Nov 15, 2023
@matloob
internal/testing/htmlcheck: replace use of cascadia in In
21ef660 
This cl implements the :nth-child() and :nth-of-type() pseudoclasses
for the css selector query function, allowing it to be used for
htmlcheck.In.

It also replaces the single use of a a child combinator '>' in a test
with a descendant combinator ' ' so that we don't need to implement
the child combinator. The test should have the same behavior.

For golang/go#61399

Change-Id: I09d9b8fbcd0eafd37aceb5994515687c85244ef8
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/542058
kokoro-CI: kokoro <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/550037 mentions this issue: internal/postgres: delete dead code depending on goldmark

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/550035 mentions this issue: internal/frontend: use GFM features in markdown parser

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/548255 mentions this issue: internal/frontend: change id generation to use parsed markdown text

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/550039 mentions this issue: internal/frontend: remove use of goldmark for readme rendering

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/550038 mentions this issue: internal/frontend: replace styleguide rendering with markdown package

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/548216 mentions this issue: internal/frontend: handle emph and strong in rewriteLinks

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/550036 mentions this issue: switch from goldmark to markdown for markdown processing

gopherbot pushed a commit to golang/pkgsite that referenced this issue Dec 18, 2023
@matloob
internal/frontend: change id generation to use parsed markdown text
56a70e8 
To produce heading ids that match between the goldmark version of the
code and the rsc.io/markdown version of the code, use the markdown
parser to parse the markdown and then extract the text from it. We do
this because rsc.io/markdown doesn't provide the raw markdown for us
to generate the ids with. This will change the ids that are generated
for some headings.

For golang/go#61399

Change-Id: Id0f26b311b59e848ff1753e058d413ed3168926d
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/548255
LUCI-TryBot-Result: Go LUCI <[email protected]>
kokoro-CI: kokoro <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Dec 18, 2023
@matloob
internal/frontend: handle emph and strong in rewriteLinks
035a41c 
For golang/go#61399

Change-Id: Iaf9be9ccd4d9deef61750b50ce3ea7699cf3c16b
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/548216
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>
kokoro-CI: kokoro <[email protected]>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Dec 18, 2023
@matloob
internal/frontend: use GFM features in markdown parser
7246abc 
This enables all the GFM features in the markdown parser, and adds
table support to walkBlocks and cases in rewriteHeadingsBlocks.

For golang/go#61399

Change-Id: I6563a5c8b0b9a838e8875e3a2ef5afe3cc155281
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/550035
LUCI-TryBot-Result: Go LUCI <[email protected]>
kokoro-CI: kokoro <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Dec 18, 2023
@matloob
internal/frontend: switch from goldmark to markdown
1c81b52 
For golang/go#61399

Change-Id: Iabf61eb3ffc432f91a5b83e68a39596af9567048
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/550036
kokoro-CI: kokoro <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Dec 18, 2023
@matloob
internal/postgres: delete dead code depending on goldmark
a1db965 
For golang/go#61399

Change-Id: Iac6e41368d2bfbc0ff1c4c02aa51671ff5a24b82
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/550037
kokoro-CI: kokoro <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Dec 18, 2023
@matloob
internal/frontend: replace styleguide rendering with markdown package
e7538de 
Use the markdown package instead of goldmark to render the styleguide.
I diffed the html files produced by each running cmd/pgksite and there
were zero diffs.

For golang/go#61399

Change-Id: Ife4d3ac22174f1ee73450df9d8d92736f6e61ac1
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/550038
LUCI-TryBot-Result: Go LUCI <[email protected]>
kokoro-CI: kokoro <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/550875 mentions this issue: internal/proxy/proxytest: use standard transport

gopherbot pushed a commit to golang/pkgsite that referenced this issue Dec 18, 2023
@matloob
internal/frontend: remove use of goldmark for readme rendering
8d0563a 
delete the code that uses goldmark and clean up the variants of the
code that use the markdown parser

For golang/go#61399

Change-Id: I03e8c303086110278dd0f3994ba97729e0cbf7c1
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/550039
Reviewed-by: Jonathan Amsterdam <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
TryBot-Bypass: Michael Matloob <[email protected]>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Dec 18, 2023
@matloob
internal/proxy/proxytest: use standard transport
b2a01f8 
proxytest is just used for tests so we don't need opencensus. Remove
the transport and the dependency.

For golang/go#61399

Change-Id: If2ef831deb098c393e625fdc288a8f6015540b48
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/550875
Reviewed-by: Jonathan Amsterdam <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
TryBot-Bypass: Michael Matloob <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/550936 mentions this issue: internal: move templatecheck tests to their own package

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/551515 mentions this issue: internal/tests: add a dependency test for cmd/pkgsite

gopherbot pushed a commit to golang/pkgsite that referenced this issue Dec 19, 2023
@matloob
internal: move templatecheck tests to their own package
c63424d 
The new package the templatecheck tests are in is not a transitive
dependency of cmd/pkgsite, so that cmd/pkgsite no longer has a
transitive test dependency on github.com/jba/templatecheck.

To make this work we had to expose the templates from the
internal/frontend and internal/godoc packages for the tests to use.

For golang/go#61399

Change-Id: I1290ec24b53af77a82671c8fc068867e12857ce3
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/550936
LUCI-TryBot-Result: Go LUCI <[email protected]>
Run-TryBot: Michael Matloob <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
kokoro-CI: kokoro <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Dec 19, 2023
@matloob
internal/tests: add a dependency test for cmd/pkgsite
4356cea 
Ensuring that it keeps its limited set of dependencies.
The test is rather slow: it takes about 5 seconds for me. I tried
parallelizing it and only got it down to about 3 seconds so I figured
it wasn't worth it.

For golang/go#61399

Change-Id: I34b75a1f77480e02368ddb7d552e89ab106bf28e
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/551515
LUCI-TryBot-Result: Go LUCI <[email protected]>
kokoro-CI: kokoro <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/552115 mentions this issue: third_party: delete autoComplete.js

gopherbot pushed a commit to golang/pkgsite that referenced this issue Dec 21, 2023
@matloob
third_party: delete autoComplete.js
70c492b 
The code using autoComplete.js was deleted in golang.org/cl/279133, so
delete the code. It was originally copied from
https://github.com/TarekRaafat/autoComplete.js. The non-minimized file
was deleted in golang.org/cl/340389 but the min files didn't seem to
be deleted around that time.

For golang/go#61399

Change-Id: I04f6437f8ba0c0a6a3972c1e52933d2478ae03ff
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/552115
kokoro-CI: kokoro <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/552295 mentions this issue: third_party/wait-for-it: replace with a simple go program

gopherbot pushed a commit to golang/pkgsite that referenced this issue Dec 24, 2023
@matloob
third_party/wait-for-it: replace with a simple go program
475d4c5 
The program doesn't have all the features of wait-for-it, but it
implements the core functionality we need.

For golang/go#61399

Change-Id: Ia5498523e44b74dcd5af1c984521f1a46208d2c5
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/552295
kokoro-CI: kokoro <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>
This was referenced Jun 21, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@matloob matloob

Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
pkgsite/backlog
Development

No branches or pull requests
4 participants
@hyangah @gopherbot @seankhliao @matloob