rules_rust icon indicating copy to clipboard operation
rules_rust copied to clipboard

Remove the CARGO_BAZEL_ISOLATED env var.

Open wt opened this issue 3 years ago • 3 comments

Manipulating the build environment with this environment variable doesn't seem super useful to me. Also, I am working on a patch to allow isolation of the building of the cargo_bazel binary when using a label as a generator instead of a generator_url. This variable interferes with that patch since the CARGO_BAZEL_ISOLATED env var doesn't really mean a lot for the bootstrap case, so we would have to duplicate the cargo isolation logic with slight differences if this variable is used in this context. Duplicating the logic seems less useful to me than just removing this var.

The other PR is #1230

wt avatar Apr 07 '22 23:04 wt

The environment variable is an optional override for the isolated attribute. It's there so users can have it one way or another but CI do things differently to enable faster iterations locally but more hermetic testing in CI. Is that not valuable functionality? Is there a better approach to changing this behavior per environment?

UebelAndre avatar Apr 08 '22 14:04 UebelAndre

I think it would be more in-brand to allow selecting a toolchain that is not isolated from the host. The CARGO_BAZEL_ISOLATED doesn't really seem to help since the REPIN will need to be used with it if you ever use the non-default setting. Perhaps lock files should also be linked to the toolchain so that you could generate a lock file for all the toolchains.

wt avatar Apr 08 '22 17:04 wt

I think it would be more in-brand to allow selecting a toolchain that is not isolated from the host. The CARGO_BAZEL_ISOLATED doesn't really seem to help since the REPIN will need to be used with it if you ever use the non-default setting. Perhaps lock files should also be linked to the toolchain so that you could generate a lock file for all the toolchains.

I don't think this is something that makes sense to associate with a toolchain. I'd liken it to something like --spawn_strategy=local where users opt out of a safer yet potentially slower means of building. I personally find the ability to toggle isolation valuable since I regularly update pins in my projects where other folks on my team don't and wouldn't be impacted by isolation.

Could you maybe restate the goal here? Is this change largely just cleanup? I wanna make sure I'm clear on the goal.

UebelAndre avatar Apr 12 '22 12:04 UebelAndre