[WIP] build: preserve explicit output names - #2362
Conversation
There was a problem hiding this comment.
Review: preserve explicit output name
The change correctly makes an explicit -o name authoritative for single-package ModeBuild (and file-path ModeTest): buildOutputPath now short-circuits to conf.OutFile verbatim, skipping the lib prefix / extension normalization in applyPrefix. This matches go build -o semantics and the added tests (exe_preserve_outfile_lib_prefix, c_shared_build_with_outfile, c_archive_build_with_outfile_without_extension) lock the intended behavior in well.
The build.go change from conf.AppExt to filepath.Ext(outFmts.Out) for libname is a necessary follow-on and a genuine fix: with an explicit -o custom.so on darwin (where AppExt=".dylib"), the old code would not have trimmed .so and would produce custom.so.h; the new code correctly yields custom.h.
A few non-blocking cleanups worth considering (inline). Nothing blocking.
Additional notes (no reliable inline location):
- Behavior is user-visible: with an explicit
-o, nolibprefix is added and no extension is appended (user owns the exact name).doc/Embedded_Cmd.md:5documents-o <file>only as "Specify output file name" — consider a one-line note that the explicit name is now used as-is, to matchgo buildsemantics. - Test coverage gaps: no end-to-end
buildOutFmtstest exercises the new early-return forModeTestwith an explicit file path, nor themultiPkg && OutFile != ""build case (which intentionally falls through to temp-file generation rather than honoring the early return). Pinning those boundaries would guard against regressions.
Additional findings
internal/build/outputs.go:71: [P2] determineBaseNameAndDir ModeBuild+OutFile branch is now dead work: For single-packageModeBuildwithconf.OutFile != "",buildOutputPathnow early-returnsconf.OutFileverbatim (line 145) beforebuildOutFmtsever uses thebaseName/dirthis branch produces. So theAppExttrimming,filepath.Dirsplit, and"."->""normalization here are computed but discarded. Consider simplifying this branch toreturn pkgName, ""(or removing it) and lettingbuildOutputPathown the explicit--ocase, so a future reader isn't misled into thinking this still drives the build-opath.internal/build/outputs.go:86: [P2] Stale comment: ModeTest baseName/dir now discarded by early return: For the explicit-fileModeTestcase (not a dir, not ending in/),buildOutputPathalso early-returnsconf.OutFileverbatim (lines 143-144), so thebaseName/dircomputed here are discarded. The comment on lines 84-85 ("Don't convert '.' to '' ... This preserves the information that user specified an output file") describes a mechanism that no longer has any effect. Update the comment or remove this now-moot computation.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
a2a5b17 to
5b7b525
Compare
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Compared with |
0a16b3e to
de7560d
Compare
de7560d to
3e8bf4e
Compare
Summary
-o-ois not specifiedTesting
go test ./internal/build -run 'TestApplyBuildModeNaming|TestBuildOutFmtsBuildModes|TestBuildOutFmtsPCLN| TestTestOutputFileLogic' -count=1