Skip to content

cmd/go: provide package path for main packages to cmd/compile #34480

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
Closed
mdempsky opened this issue Sep 23, 2019 · 7 comments
Closed

cmd/go: provide package path for main packages to cmd/compile #34480

mdempsky opened this issue Sep 23, 2019 · 7 comments
Labels
FeatureRequest Issues asking for a new feature that does not need a proposal. GoCommand cmd/go NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone
Unplanned

Comments

@mdempsky
Copy link
Contributor

Can cmd/go provide cmd/compile with the full package path to the source package, even when compiling main packages?

When benchmarking cmd/compile changes, it's useful to key stuff by myimportpath (i.e., the -p command-line flag) and just spit everything across an entire "go build -a std cmd" build into a single file, and then let benchcmp or benchstat handle it.

But this currently doesn't work for main packages, because cmd/go sets -p main for these:

go/src/cmd/go/internal/work/gc.go

Lines 50 to 56 in 7eef0ca

pkgpath := p.ImportPath
if cfg.BuildBuildmode == "plugin" {
pkgpath = pluginPath(a)
} else if p.Name == "main" && !p.Internal.ForceLibrary {
pkgpath = "main"
}
gcargs := []string{"-p", pkgpath}

So you end up with a bunch of "BenchmarkFoo:main" lines, which muddle the benchcmp/benchstat output.

I figure two main options:

  1. Change cmd/go to just set -p to the package path regardless, and cmd/compile can rewrite it to "main" where/if necessary. (Looking briefly at myimportpath, some uses would be unaffected; but DWARF and the new ABI stuff might be impacted.)

  2. Add another command-line flag for cmd/go to provide the package path, which cmd/compile can use for tagging benchmarking data with instead.

/cc @rsc @aclements
@mdempsky mdempsky added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. FeatureRequest Issues asking for a new feature that does not need a proposal. labels Sep 23, 2019
@jayconrod
Copy link
Contributor

cc @cherrymui

@cherrymui
Copy link
Member

The compiler uses myimportpath for symbol names and such. But as you said we can rewrite it to "main" as necessary. So either option will work for this use case.

I'm slightly inclined to option 2, for an option specific for benchmarking. User could also overwrite it if necessary.

@jayconrod
Copy link
Contributor

For option 1, a binary may have multiple packages named main. That comes up a lot when testing a main package and when using -coverpkg with a pattern that covers multiple main packages. So don't rewrite in any context where this needs to be unique.

For option 2, would you want to use this for import paths (strings that appear in import declarations)? Import paths are user-friendly but may not be unique. Package paths are unique but less friendly (resolved vendor directories and such). Currently, when a package is compiled, the compiler does not know how it will be imported by other packages. A package may be imported with multiple distinct import paths because of minimal module compatibility, too, so we have to be careful here.

@mdempsky
Copy link
Contributor Author

I'm slightly inclined to option 2, for an option specific for benchmarking. User could also overwrite it if necessary.

That makes sense.

So don't rewrite in any context where this needs to be unique.

Perhaps cmd/go could provide -p "foo/bar" -main instead of rewriting -p "foo/bar" into -p "main"? That would let cmd/compile know when it should use "main" instead.

For option 2, would you want to use this for import paths (strings that appear in import declarations)? Import paths are user-friendly but may not be unique. Package paths are unique but less friendly (resolved vendor directories and such).

Hm, so I want that when I run "go build [pkgs...]", that each individual compile operation has some unique stable identifier so I can correlate benchmarks across builds.

That makes me think we actually want the resolved package path, which then also means we can't always rely on -p even for non-main packages.

@seankhliao seankhliao added this to the Unplanned milestone Aug 27, 2022
@seankhliao seankhliao added the GoCommand cmd/go label Feb 22, 2025
@matloob
Copy link
Contributor

matloob commented Mar 27, 2025 •
edited
Loading

Would this still be helpful?

@matloob matloob added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Mar 27, 2025
@cherrymui
Copy link
Member

It may still be helpful, but not urgent. There workarounds, e.g. using a script to build one main package a time.

@gopherbot
Copy link
Contributor

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@gopherbot gopherbot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
FeatureRequest Issues asking for a new feature that does not need a proposal. GoCommand cmd/go NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
6 participants
@mdempsky @jayconrod @gopherbot @seankhliao @cherrymui @matloob