Support path mapping in Rust Starlark actions

[dabanki]

Transition from passing raw string paths (.path) to passing File objects directly to action arguments (Args). This allows Bazel to mutate paths during execution, which is required for path mapping compatibility.

For more details on path mapping best practices, see: bazelbuild/bazel#22658

[UebelAndre]

Wasn't this done in #4011 ? Is there a gap in the regression testing?

[dabanki]

#4011 seems have covered most things. We just noticed that target_flag_value was not updated to pass in File object instead of path (as string). When doing e2e testing for some of our targets this was causing failures for us

[UebelAndre]

#4011 seems have covered most things. We just noticed that target_flag_value was not updated to pass in File object instead of path (as string). When doing e2e testing for some of our targets this was causing failures for us

Fixes are welcome but can you also add regression testing for the missed case?

[dabanki]

I'm not familiar with this codebase, so the best way I could find to do this was to add an assertion on the flag type which implies that path mapping is supported. Let me know if that works.

[krasimirgg]

Could we model such elements of emit as (string, Path) tuples?
We've got some in-progress extensions internally that we'd like to upstream as soon as we're confident they work properly that use a similar pattern, such as --emit=thin-link-bitcode=<path>

[dabanki]

Newest pushed commit should address this if I understand correctly - let me know if it looks like what you had in mind.

[UebelAndre]

I'm not familiar with this codebase, so the best way I could find to do this was to add an assertion on the flag type which implies that path mapping is supported. Let me know if that works.

I think an integration test would be best here. Can you define some target that naturally triggers this code path?

[krasimirgg]

I'm not familiar with this codebase, so the best way I could find to do this was to add an assertion on the flag type which implies that path mapping is supported. Let me know if that works.

I think an integration test would be best here. Can you define some target that naturally triggers this code path?

To provide more context as someone on both sides --
Happy to help out with setting up an integration test, (but we may need to add some additional features to rules_rust / toolchain-repository-sets to be able to set this up):

Internally we define rust_toolchains manually; we use the target_json functionality.

To get to a behavior that this changes, you need something like that: a rust library that's set up to use a rust toolchain that uses a target json:

# MODULE.bazel

bazel_dep(name = "rules_rust", version = "0.70.0")

rust = use_extension("@rules_rust//rust:extensions.bzl", "rust")

# Configure a toolchain with a custom target specification file
rust.toolchain(
    name = "custom_toolchain",
    edition = "2021",
    versions = ["1.85.0"],
    # Path to your custom target specification JSON file
    # doesn't work today: 
    # Error: in 'toolchain' tag, unknown attribute 'target_json' provided
    target_json = "//tools/platforms:my_custom_target.json", 
)

use_repo(rust, "rust_toolchains")
register_toolchains("@rust_toolchains//:all")

I don't know of a good way to spawn such a custom toolchain. @UebelAndre -- do you have suggestions?

[krasimirgg]

Chatted about the (lack of) regression test coverage for this yesterday with @UebelAndre. I've taken up #4078 to set this up. Let's focus on the technicals in this PR, and I'll make sure to add some test coverage later as part of that bug.

[krasimirgg]

Thank you!