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

cmd/dist: internal import detection is generating false positives #68570

Open
4a6f656c opened this issue Jul 24, 2024 · 4 comments
Open

cmd/dist: internal import detection is generating false positives #68570

4a6f656c opened this issue Jul 24, 2024 · 4 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-OpenBSD
Milestone

Comments

@4a6f656c
Copy link
Contributor

After updating the Go port for OpenBSD to start testing 1.23rc2, the build fails with:

===>  Building for go-1.23rc2
Building Go cmd/dist using /usr/ports/pobj/go-1.23rc2/go-openbsd-amd64-bootstrap. (go1.22.5 openbsd/amd64)
Building Go toolchain1 using /usr/ports/pobj/go-1.23rc2/go-openbsd-amd64-bootstrap.
go tool dist: /usr/ports/pobj/go-1.23rc2/go/src/cmd/internal/obj/x86/asm_test.go.orig.port:10: bootstrap-copied source file cannot import internal/testenv

This appears to be a result of code added in 805f6b3, combined with the fact that the OpenBSD ports build applies patches (and hence patch produces a backup .orig.port file). This approach has worked without issue for Go 1.22 (and every earlier version) - the new code seems to be unnecessarily excessive since this is not a .go file, does not have any build tags and is additionally test code.

In order to avoid packaging complications, can we restrict this check to files that Go will actually compile?

@ianlancetaylor
Copy link
Member

Rather than try to have cmd/dist predict how the tools might change in the future, we have a list of ignored file prefixes and suffixes (ignorePrefixes and ignoreSuffixes in cmd/dist/buildtool.go. I suppose we could extend one of those lists.

@ianlancetaylor ianlancetaylor added this to the Go1.23 milestone Jul 24, 2024
@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 24, 2024
@ianlancetaylor
Copy link
Member

As far as I know .orig.port is not a normal output of the patch program. How hard would it be for the OpenBSD process to use a different file name? It's not clear to me that the Go build process needs to work with unexpected files sprinkled into the tree.

@dmitshur dmitshur moved this to In Progress in Release Blockers Jul 24, 2024
@mknyszek mknyszek added OS-OpenBSD WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. and removed release-blocker labels Jul 31, 2024
@4a6f656c
Copy link
Contributor Author

The .orig.port extension is used by OpenBSD's ports in order to avoid conflicts with .orig files that are bundled:

https://man.openbsd.org/bsd.port.mk.5#PATCHORIG

This could be changed back to the patch(1) default of .orig for the lang/go port if need be.

It seems a bit strange that the general Go source file rules are not applied here, but I can provide a diff that adds .orig to the ignoreSuffixes list if that is the preferred option.

@gopherbot gopherbot modified the milestones: Go1.23, Go1.24 Aug 13, 2024
@ianlancetaylor
Copy link
Member

I would be OK with changing bootstrapFixImports to only run on files whose names end with .go.

@seankhliao seankhliao removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Dec 14, 2024
@gopherbot gopherbot modified the milestones: Go1.24, Go1.25 Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-OpenBSD
Projects
Status: In Progress
Development

No branches or pull requests

6 participants