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/website/cmd/golangorg: various open simplification opportunities after move from x/tools/cmd/godoc #41102

Closed
dmitshur opened this issue Aug 28, 2020 · 3 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@dmitshur
Copy link
Contributor

(This issue is factored out from #29206 (comment).)

The website has been factored out into the cmd/golangorg command in the x/website repository (see #29206). At this time, there remain viable simplifications that were not possible back when it was a shared godoc binary in x/tools. Primarily, this is about reducing the number of unused configurations and code paths.

Resolving this will make development easier and reduce maintenance costs (less code paths to maintain).

Simplification Opportunities

Currently, there are these independent configuration axes:

  1. cmd/golangorg can be built with golangorg build constraint or without
  2. the content/static directory can be read from disk or via the static package that needs to be go generate
    • new files need to be manually added to gen.go
      • it is possible to make a mistake of forgetting to do this step (e.g., see CL 213937)
  3. the GOROOT directory can be provided via a .zip file (-zip flag) or directly on disk (-goroot flag).

For 1, the build constraint can be removed. It was needed back when the website was the same binary as godoc, but that is no longer the case. However, need to consider how memcache and datastore clients will be configured for local testing vs prod (currently, they're always turned on when golangorg build constraint is satisfied, always off otherwise).

For 2, the static package was needed so users would be able to run godoc binary without its x/tools source code on disk (especially back when godoc was bundled into main distribution). When the golangorg binary is deployed, the repository filesystem is already included, and it should be always available during development. The static package needs to be constantly re-generated during development.

For 3, the ability to provide GOROOT contents via a zip instead of a directory on disk is also not being used at all right now, and I don't foresee a need for this. I think it was needed back when the website was deployed to an App Engine with some limits on filesystem access that are no longer applicable. If there is a need in the future (unlikely), we can re-introduce this feature.

Proposed State

I suggest we remove all 3 aforementioned options, the end result will be a single golangorg command without build constraints. It will read GOROOT directory from disk, and it will be provided the x/website/content directory on disk.

If we find that evaluating templates or rendering Markdown on each HTTP request is found to be slow or expensive, I think we can address that by adding a caching layer on top. We already do this on some pages, e.g., the https://golang.org/dl/ page uses a cache.

Deployment to production (both to golang.org, and to tip.golang.org) will continue to work. It'll be reading content from filesystem instead of the generated static package. It's already reading the favicon.ico and robots.txt files from the filesystem, as well as the entire GOROOT directory.

/cc @rsc @andybons @toothrot @cagedmantis

@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 28, 2020
@dmitshur dmitshur added this to the Unreleased milestone Aug 28, 2020
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/291690 mentions this issue: content/static: use embed and io/fs

gopherbot pushed a commit to golang/website that referenced this issue Feb 16, 2021
This lets us delete the generated static.go.

For golang/go#41102

Change-Id: Ie09f34a83f114592eec4ba2dd9263285169374ae
Reviewed-on: https://go-review.googlesource.com/c/website/+/291690
Trust: Russ Cox <[email protected]>
Run-TryBot: Russ Cox <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/293413 mentions this issue: cmd/golangorg: simplify local vs prod programs

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/293412 mentions this issue: cmd/golangorg: delete index support code

gopherbot pushed a commit to golang/website that referenced this issue Feb 24, 2021
Now that search is turned off, all the index code can be deleted.

For golang/go#41102.

Change-Id: Idb82f2b80cadb5a2d209b5e5995d38c35ffaf8db
Reviewed-on: https://go-review.googlesource.com/c/website/+/293412
Trust: Russ Cox <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
@golang golang locked and limited conversation to collaborators Feb 24, 2022
passionSeven added a commit to passionSeven/website that referenced this issue Oct 18, 2022
There was too much duplicated code between main.go
and appinit.go and too many build-tagged-out files.

Make main.go the func main for both prod and local.
Introduce local.go, merging dl.go and play.go.
Introduce prod.go, holding the prod-specific bits of appinit.go
(the rest are in main.go).

Rename the build tag to prod instead of golangorg
(the whole program is golangorg; it's very confusing).

Fixes golang/go#41102.

Change-Id: I261ce8e9171110f01798025f8218ce9f8253af81
Reviewed-on: https://go-review.googlesource.com/c/website/+/293413
Trust: Russ Cox <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

2 participants