bash: Add a toolchain for local Bash. #4980
Closed
Conversation
This file contains 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
katre
reviewed
Apr 9, 2018
tools/bash/BUILD.tools
Outdated
| exports_files(["bash_def.bzl"]) | ||
|
|
||
| toolchain_type( | ||
| name = "bash_toolchain_type", |
There was a problem hiding this comment.
For consistency with other toolchains (and to save typing) I recommend naming this simply "toolchain_type".
laszlocsomor
force-pushed
the
bash-removal-1-toolchain-rule
branch
4 times, most recently
from
f078872
to
a61ccfc
Compare
katre
approved these changes
Apr 12, 2018
tools/shell/shell_toolchain.bzl
Outdated
| """shell_toolchain rule implementation.""" | ||
| return [platform_common.ToolchainInfo(path = ctx.attr.path)] | ||
|
|
||
| def _is_windows(repository_ctx): |
There was a problem hiding this comment.
Most of this file is the repository rule that autodetects the shell toolchain. Is it worth splitting the definition of the actual shell_toolchain rule to a separate .bzl file, so that people defining standalone shell toolchains aren't loading all of this?
There was a problem hiding this comment.
Good point, done.
tools/shell/shell_toolchain.bzl
Outdated
| """shell_toolchain rule implementation.""" | ||
| return [platform_common.ToolchainInfo(path = ctx.attr.path)] | ||
|
|
||
| def _is_windows(repository_ctx): |
There was a problem hiding this comment.
Also, and this isn't part of this change but a general comment, we need a general purpose library of repository functions for common tasks like os detection.
|
I'll fix up #5001 and submit it today to unblock you. |
laszlocsomor
force-pushed
the
bash-removal-1-toolchain-rule
branch
from
0155984
to
60ed7af
Compare
katre
approved these changes
Apr 12, 2018
laszlocsomor
force-pushed
the
bash-removal-1-toolchain-rule
branch
2 times, most recently
from
7e65c29
to
1a544c3
Compare
Bazel automatically detects the local Bash and creates a custom toolchain rule for it. Later, rules that use Bash will require this toolchain and retrieve Bash's path from it instead of relying on hardcoded paths or the --shell_executable flag. See bazelbuild#4319 Change-Id: Idd8242a20d202b1f5a56cddac95b625c6c08ede9
Change-Id: If36e21ae29fb56f8f7e02e9d97205b5a6cb0117e
Change-Id: Ib2c75a834ac850e70ac4408842ce3ce28d9a9e08
Change-Id: I6e6ed809c8efb8ad2dd73bfa1335a17ec0790070
Change-Id: I62598bb5e0b6c2da1eddb9d8958347e96583a3d3
Change-Id: I88462c3b24b278ac0b60bdbf00d48bd88b36096b
Change-Id: Ic8798b198e49d8759b9c52d04ff1bcbfa5fcf854
Change-Id: Ie3a2169fe8217660399217650b1e8b0207c912b9
Change-Id: I9a40499f1a980187487541b21042c80dd35459d8
Change-Id: Iabe85677694216068a8011b8f8bbc18048e45e06
Change-Id: Ide80a8813809cb932a089916cedf42abf84d307c
The assertion in line bazelbuild#235 passes on Linux if the limit is 9, on Darwin if it is 10. Commenting out the code in BazelRulesModule.java that automatically loads the sh_configure() rule which instantiates @local_config_sh makes the test pass with the old limit (8). Change-Id: Ic2406a4da260b284e15852070d58472ca18340af
laszlocsomor
force-pushed
the
bash-removal-1-toolchain-rule
branch
from
1a544c3
to
8da6df1
Compare
Change-Id: I30658deb98e7964989e761f63e63ab595c47f578
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.
Bazel automatically detects the local Bash and
creates a custom toolchain rule for it.
Later, rules that use Bash will require this
toolchain and retrieve Bash's path from it instead
of relying on hardcoded paths or the
--shell_executableflag.See #4319
Change-Id: Idd8242a20d202b1f5a56cddac95b625c6c08ede9