Add BAZEL_SKIP_XCODE_FETCH to reduce toolchain configuration time #191
Conversation
keith
requested review from
BalestraPatrick,
brentleyjones,
segiddins and
thii
as code owners
Since this toolchain requires Xcode, running xcode_locator during toolchain setup is potentially duplicative. The only things it's used for is to check that they exist, and to add to the allowed include directories. The include directories already includes `/Applications` which is where _most_ people likely have Xcode installed, so as long as that can be relied on, fetching these values isn't necessary. This saves a few seconds when the toolchain is reconfigured. https://bazelbuild.slack.com/archives/CD3QY5C2X/p1652819625121889
keith
force-pushed
the
ks/add-bazel_skip_xcode_fetch-to-reduce-toolchain-configuration-time
branch
from
5a46fc6
to
c5dc29e
Compare
|
maybe instead of this we should flip this around and do an env var if you want to allow Xcode versions outside of |
|
here's what that would look like #197 |
| (xcode_toolchains, xcodeloc_err) = run_xcode_locator(repository_ctx, xcode_locator) | ||
| if not xcode_toolchains: | ||
| return False | ||
| skip_xcode_fetch = "BAZEL_SKIP_XCODE_FETCH" in repository_ctx.os.environ and repository_ctx.os.environ["BAZEL_SKIP_XCODE_FETCH"] == "1" |
There was a problem hiding this comment.
What do you think about using bool to evaluate the environment variable value? It allows folks to pick their favorite representation.
There was a problem hiding this comment.
this is what the upstream CC toolchain vars enforce so I figure we might as well keep the same behavior
|
The one upside to this implementation vs #197. It may seem like a more understandable change in behavior for those who run into it. Of course, folks have to opt into the benefit. Perhaps, we can recommend that it be enabled in the useful flags doc. |
|
going with #197 in the spirit of trying to have the right defaults |
keith
deleted the
ks/add-bazel_skip_xcode_fetch-to-reduce-toolchain-configuration-time
branch
Since this toolchain requires Xcode, running xcode_locator during
toolchain setup is potentially duplicative. The only things it's used
for is to check that they exist, and to add to the allowed include
directories. The include directories already includes
/Applicationswhich is where most people likely have Xcode installed, so as long
as that can be relied on, fetching these values isn't necessary. This
saves a few seconds when the toolchain is reconfigured.
https://bazelbuild.slack.com/archives/CD3QY5C2X/p1652819625121889