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/blog: builds failing on release-branch.go1.12 due to missing dependencies #37337

Closed
bcmills opened this issue Feb 20, 2020 · 6 comments
Closed

x/blog: builds failing on release-branch.go1.12 due to missing dependencies #37337

bcmills opened this issue Feb 20, 2020 · 6 comments
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Unreleased

Comments

@bcmills
Copy link
Contributor

bcmills commented Feb 20, 2020

In CL 218517, we added an external dependency to the x/website repo.

Somehow, that resulted in test failures for the x/blog repo (https://build.golang.org/log/c8db93d879bffec55c15c341dc5082bdde79a770):

../website/markdown/markdown.go:16:2: cannot find package "github.com/yuin/goldmark" in any of:
	/workdir/go/src/github.com/yuin/goldmark (from $GOROOT)
	/workdir/gopath/src/github.com/yuin/goldmark (from $GOPATH)
../website/markdown/markdown.go:17:2: cannot find package "github.com/yuin/goldmark/renderer/html" in any of:
	/workdir/go/src/github.com/yuin/goldmark/renderer/html (from $GOROOT)
	/workdir/gopath/src/github.com/yuin/goldmark/renderer/html (from $GOPATH)

It appears that the x/blog tests on release-branch.go1.12 are not running in module mode. They probably should be.

CC @jayconrod @dmitshur @cagedmantis @toothrot
@bcmills bcmills added Testing An issue that has been verified to require only test changes, not just a test failure. Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done. modules labels Feb 20, 2020
@gopherbot gopherbot added this to the Unreleased milestone Feb 20, 2020
@dmitshur
Copy link
Contributor

dmitshur commented Feb 20, 2020 •
edited

golang.org/x repos are intentionally tested in GOPATH mode on release-branch.go1.12 branches, and in module mode (as long as a go.mod file is present) on release-branch.go1.13 branches and newer.

Builders have support for testing with third-party packages in golang.org/x repos only in module mode.

No packages in x/blog import x/website/markdown package yet, so it's strange that it is attempted to be built in GOPATH mode. Need to understand why that is.

@dmitshur
Copy link
Contributor

Okay, the golang.org/x/website/content/static package imports x/website/markdown, and x/blog imports golang.org/x/website/content/static.

@jayconrod
Copy link
Contributor

Builders have support for testing with third-party packages in golang.org/x repos only in module mode.

How does the builder set up the workspace? I assumed the builder would go get the packages to be tested, but when I tried that locally with Go 1.12.17, it checked out github.com/yuin/goldmark, and I didn't run into any error with the go test command.

No packages in x/blog import x/website/markdown package yet, so it's strange that it is attempted to be built in GOPATH mode. Need to understand why that is.

I didn't think about this when I added the Markdown rendering, but content/static/gen.go is not excluded by build constraints, so golang.org/x/website/markdown is imported by the regular version of golang.org/x/website/content/static.

Perhaps instead, all the generator code should be moved from gen.go to makestatic.go. gen.go should just have a //go:generate pragma.

@gopherbot
Copy link

Change https://golang.org/cl/220358 mentions this issue: content/static: move code generation to new package

@dmitshur
Copy link
Contributor

How does the builder set up the workspace?

Answered in person, but for posterity, the code is in buildStatus.runSubrepoTests of cmd/coordinator.

@gopherbot
Copy link

Change https://golang.org/cl/251120 mentions this issue: content/static: remove internal/gen

@golang golang locked and limited conversation to collaborators Aug 28, 2021
passionSeven added a commit to passionSeven/website that referenced this issue Oct 18, 2022
@passionSeven
content/static: move code generation to new package
bc433d1 
This change prevents ./cmd/golangorg and other binaries that depend on
./content/static from transitively importing ./markdown. At this time,
./markdown is only needed for code generation (via go generate) and
for testing. It does not need to be linked into ./cmd/golangorg.

x/blog imports ./content/static. The transitive import of
github.com/yuin/goldmark causes Go 1.12 builders to fail because they
run in GOPATH mode, and they forbid dependencies outside the
golang.org/x repos.

Fixes golang/go#37337

Change-Id: I938c06cdea66d6e08ac27d28f089026b42db9062
Reviewed-on: https://go-review.googlesource.com/c/website/+/220358
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@jayconrod @dmitshur @bcmills @gopherbot