Replace usages of ctx.action with ctx.actions.{run, run_shell}
#151
Conversation
- Required making all imports absolute to work around bazelbuild/bazel#3493
- Uses zipper instead of /usr/bin/zip - Does not use symlink'd dependencies dir because it's not necessary.
rust/private/rustc.bzl
Outdated
| dylibs = [l for l in dep.cc.libs if l.basename.endswith(toolchain.dylib_ext)] | ||
| staticlibs = [l for l in dep.cc.libs if l.basename.endswith(toolchain.staticlib_ext)] | ||
| if not dylibs and not staticlibs: | ||
| fail("Did not find ususable library outputs in {}".format(dep)) |
There was a problem hiding this comment.
bug 2: osx fails this check ... not sure why
rust/private/rustc.bzl
Outdated
| out_dir = ctx.actions.declare_directory(ctx.label.name + ".out_dir") | ||
| ctx.actions.run_shell( | ||
| # TODO: Remove /bin/tar usage | ||
| command = "mkdir {dir} && /bin/tar -xzf {tar} -C {dir}".format(tar = tar_file.path, dir = out_dir.path), |
There was a problem hiding this comment.
bug 1: change back to tar, it's in a different place on osx
rust/private/rustc.bzl
Outdated
|
|
||
| return toolchain.compilation_mode_opts[comp_mode] | ||
|
|
||
| def _get_lib_name(lib): | ||
| # def get_lib_name(lib: File) -> str: |
There was a problem hiding this comment.
Yeah. I was trying to think what the best way to include type annotations was, doesn't seem like the docstring convention is quite figured out.
There was a problem hiding this comment.
I'm having a pretty hard time efficiently separating out the minor refactoring changes from additions. I'd really like to scrutinize any behavior changes more closely, but it's hard for me to pick those out.
Anyway, I trust your judgment generally. If theres anything that merits a closer look, point it out and I'll take a look at it. Else, LGTM.
| setup_cmd += [_symlink_dep_cmd(lib, deps_dir, in_runfiles)] | ||
| crate_list = transitive_crates.to_list() | ||
| transitive_libs = depset([c.output for c in crate_list]) + transitive_staticlibs + transitive_dylibs | ||
| indirect_crates = depset([crate for crate in crate_list if crate not in direct_crates]) |
There was a problem hiding this comment.
What's the distinction being made here beween "indirect crates" and "transitive libs'? Is indirect crates supposed to be just the rust-lang transitive dependencies?
I'm having a hard time efficiently working through the diff, so I'll ask to be spoonfed: What motivated this particular addition?
There was a problem hiding this comment.
transitive_libs is everything, as opposed to just crates.
I added to compensate for removing the setup_deps symlink hermiticity, by not passing unnecessary link directories to rustc. (This could be entirely pointless, I'm not attached to it)
|
@acmcarther most of the individual commits are relatively digestible, but I think overall the change is uncontentious. In hindsight I shouldn't have split up files at the same time as making other changes. |
In order to test those flags, PR bazelbuild#151 broke some usage of those flags that make `cargo raze` build file not work any more.
* Add flags and features to the default library In order to test those flags, PR #151 broke some usage of those flags that make `cargo raze` build file not work any more. * Having 2 flags in the same string in rustc is no longer allowed and that's fine I'll send a PR to cargo-raze to fix it on cargo raze side rather. * Fix the feature passing Features needs to be quoted on the command line apparently. * readability: use single quote to avoid escaping double quotes
This got fairly yak-shavy, but addresses most of the remainder of #117
Relevant commit messages:
DepInfoprovider and update accessor syntax.