Skip to content

Self-execute the path from os.Executable in more places #3338

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

Merged
merged 5 commits into from
Jun 30, 2025

Conversation

DrJosh9000
Copy link
Contributor

@DrJosh9000 DrJosh9000 commented Jun 12, 2025

Description

Remove checkBinaryPaths, and self-execute using the result from os.Executable in more places.

Unfortunately we can't yet totally solve the sorry-Mario-your-buildkite-agent-is-in-another-castle problem (buildkite-agent within a command or script run as part of a step can still be a separate binary).

Context

checkBinaryPaths creates the need for some ugly workarounds elsewhere for those who build the agent themselves.

https://linear.app/buildkite/issue/PS-732

Changes

  • Revert Notify when host and bootstrap agent paths mismatch #3123
  • Add a self internal package for managing path to self, and provide a way to override during tests
  • Use self.Path for artifact phase, setting metadata, and the credential helper.
  • Use self.OverridePath within the bootstrap subcommand, if a magic combination of undocumented env vars is present

Testing

  • Tests have run locally (with go test ./...). Buildkite employees may check this if the pipeline has run automatically.
  • Code is formatted (with go fmt ./...)

@DrJosh9000 DrJosh9000 force-pushed the ps-732-remove-path-check branch 2 times, most recently from 4b5584c to 9a60f52 Compare June 19, 2025 00:25
@DrJosh9000 DrJosh9000 marked this pull request as ready for review June 23, 2025 01:59
@DrJosh9000 DrJosh9000 requested a review from a team June 26, 2025 06:45
@DrJosh9000 DrJosh9000 force-pushed the ps-732-remove-path-check branch from 9a60f52 to 2a313c5 Compare June 26, 2025 06:49
Copy link
Contributor

@zhming0 zhming0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks good to me!

I do not 100% follow what happened in nixpkg though, do they attempt to build our development build?

@DrJosh9000
Copy link
Contributor Author

Yes, as I understand it, Nix packages tend to build everything from source. They also inject a wrapper to help control dependencies. I think the Nix approach is very sensible. But the wrapper causes the two paths to be different, and build from source lacked the buildVersion override, so our logic in #3123 considered the output to be a "development build".

@DrJosh9000 DrJosh9000 merged commit 077c31c into main Jun 30, 2025
1 check passed
@DrJosh9000 DrJosh9000 deleted the ps-732-remove-path-check branch June 30, 2025 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants