Repository navigation
buck2 calling/nesting cleanups - #290
Merged
Merged
Conversation
martinpitt
commented
Oct 10, 2026
martinpitt
commented
Oct 10, 2026
Buck takes the isolation dir from `--isolation-dir` or `$BUCK_ISOLATION_DIR`, but the wrapper's parser did not check the latter. Signed-off-by: Martin Pitt <martin@amutable.com>
Nothing uses them, and by now our buck2 is modified so heavily that using a different/non-tested buck2 is "here be dragons" territorry. Signed-off-by: Martin Pitt <martin@amutable.com>
Every driver built its nested Buck command by hand from `nested_buck()` and passed the binary path through a lot of helper functions. Centralize building a command argv, so that in the future, passing global options like `--no-buckd` happen in this single place. Also stop falling back to a plain `buck` binary. By now we hard-depend on our fork. Let `tine` control the buck2 binary that runs. Signed-off-by: Martin Pitt <martin@amutable.com>
The `BUCK2_BINARY` variable previously had two different jobs: Tell tools running under a `build` where the binary lives, but also as a flag whether it was already running under a parent `buck2`. In that case, `tine` skipped re-reading mount declarations and refreshing shared configuration. But nothing actually uses this: tools under a build call `buck2` directly. The only special case is the `cache_roundtrip` test, but it scrubs the environment to act as a developer's top-level call. Treat every wrapper invocation as top-level. This clarifies what `BUCK2_BINARY` is about, and opens the door for further simplifications. Signed-off-by: Martin Pitt <martin@amutable.com>
Use `util.buck2_argv()` for all of tine.py's top-level buck commands as well. Rename `buck2_binary()` to `ensure_buck2_binary()` and let that export the environment. It only reports success, so that nothing can break the "go via `buck2_argv()`" contract any more. Signed-off-by: Martin Pitt <martin@amutable.com>
This constant has confused me too many times. Signed-off-by: Martin Pitt <martin@amutable.com>
1 task done
daandemeyer
approved these changes
Oct 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Some things which I found to be unnecessarily complicated/confusing while developing #289. See individual commits for details. This already kills 51 LoC.
Plus the
$BUCK_ISOLATION_DIRfix, which is unrelated, but easy enough to ride along.This goes before #289 which then can become simpler.