fix(internal/command): look up executables in custom path environments#6273
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a custom lookPath function in internal/command/command.go to search for executable commands within a specified PATH environment variable, falling back to the system PATH if not provided, and adds corresponding unit tests. Feedback on these changes includes using the comma-ok idiom to correctly handle an explicitly empty PATH in the environment map, checking for any path separator to improve cross-platform compatibility (such as on Windows), and verifying executable permission bits on Unix-like systems to ensure the resolved file is actually executable.
|
I think relying on PATH env var resolves #6251. How does this (reliance on PATH env var) address the original problem of "librarian generate" not using the go tools installed by "librarian install"? |
The current |
suztomo
left a comment
There was a problem hiding this comment.
Note for myself: mergeEnv prioritizes getInstallDir.
🤖 I have created a release *beep* *boop* --- ## [0.17.0](v0.16.0...v0.17.0) (2026-06-09) ### Features * **internal/cache:** add `BinDirectory` and `LIBRARIAN_BIN` override ([#6315](#6315)) ([ac43e52](ac43e52)), closes [#5850](#5850) [#6199](#6199) * **librarian:** add `Discovery` field to Swift config ([#6320](#6320)) ([2ee0a36](2ee0a36)) * **nodejs:** update gapic generator to v4.12.0 ([#6341](#6341)) ([fae4158](fae4158)) * **sidekick/rust:** use consolidated `LroRecorder` in tracing decorator ([#6259](#6259)) ([0d318a9](0d318a9)) * **sidekick/swift:** generate `with` helper ([#6309](#6309)) ([36d2aa1](36d2aa1)) * **sidekick/swift:** map-based pagination ([#6268](#6268)) ([082e996](082e996)) ### Bug Fixes * **internal/command:** look up executables in custom path environments ([#6273](#6273)) ([7278ace](7278ace)), closes [#6271](#6271) * **internal/fetch:** add support for symlink extraction ([#6321](#6321)) ([7fa61e4](7fa61e4)), closes [#6313](#6313) * **internal/librarian/java:** allow omitting ReleasedVersion with fill and tidy ([#6274](#6274)) ([9552dcd](9552dcd)), closes [#6244](#6244) * **internal/librarian:** disable API path derive for Java ([#6287](#6287)) ([bb3119f](bb3119f)) * **librarian/internal/java:** explicitly list released_version as config ([5917f20](5917f20)) * **librarian/swift:** configuration fields ([#6316](#6316)) ([a1bd1c2](a1bd1c2)) * **nodejs:** manually create symlinks during librarian install ([#6314](#6314)) ([bbdc773](bbdc773)), closes [#6312](#6312) * **nodejs:** remove google/cloud/common_resources.proto after generation ([#6333](#6333)) ([6a9e325](6a9e325)), closes [#6024](#6024) * **python:** avoid adding to existing core lib ([#6324](#6324)) ([9ebe312](9ebe312)) * **sidekick/rust:** fix tracing template generation for discovery-based LROs ([#6258](#6258)) ([33ef923](33ef923)) * **sidekick/swift:** warnings in snippets ([#6284](#6284)) ([23bfa8d](23bfa8d)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
When building commands via
buildCmd, Go'sexec.CommandContextresolves non-absolute executable names using the parent process's systemPATHbefore custom environment maps (such as custom tool directories) are attached. This fails if the target executable is not globally installed on the parentPATH.To resolve this, a lookPath helper is added that manually resolves the executable's absolute path using the provided custom
PATHenvironment beforeexec.CommandContextis invoked. Since the binary is resolved to a fully qualified absolute path, Go successfully bypasses the parent systemPATHlookup.Tested in JoeWang1127/google-cloud-go#27
Fixes #6271