Replace symlinks in the output of cargo build scripts

[havasd]

#2948 breaks building of rdkafka with cmake because of dangling symlinks.

When building with latest version we get the following error:

ERROR: /home/wincent/.cache/bazel/_bazel_wincent/394c4c1d21c5490d4a70260a2cfccaf5/external/rules_rust~~crate~crates__rdkafka-sys-4.8.0-2.3.0/BUILD.bazel:68:19: error while validating output tree artifact external/rules_rust~~crate~crates__rdkafka-sys-4.8.0-2.3.0/_bs.out_dir: child lib/cmake/RdKafka/FindLZ4.cmake is a dangling symbolic link
ERROR: /home/wincent/.cache/bazel/_bazel_wincent/394c4c1d21c5490d4a70260a2cfccaf5/external/rules_rust~~crate~crates__rdkafka-sys-4.8.0-2.3.0/BUILD.bazel:68:19: Running Cargo build script rdkafka-sys failed: not all outputs were created or valid
Target @@rules_rust~~crate~crates__rdkafka-0.37.0//:rdkafka failed to build
Use --verbose_failures to see the command lines of failed build steps.
ERROR: /home/wincent/.cache/bazel/_bazel_wincent/394c4c1d21c5490d4a70260a2cfccaf5/external/rules_rust~~crate~crates__rdkafka-sys-4.8.0-2.3.0/BUILD.bazel:18:13 Compiling Rust rlib rdkafka_sys v4.8.0+2.3.0 (7 files) failed: not all outputs were created or valid

Fixes #3099

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[illicitonion]

Hmm... Do you know what the symlink is pointing at which is missing? In generally we want to preserve symlinks if possible - is it a symlink to an absolute path? Or a relative path inside in the outdir? Or a relative path inside the sandbox?

[havasd]

Hmm... Do you know what the symlink is pointing at which is missing? In generally we want to preserve symlinks if possible - is it a symlink to an absolute path? Or a relative path inside in the outdir? Or a relative path inside the sandbox?

It was an absolute path symlink. Essentially when cmake installs the header files as is. So if they were symlinks it will keep them symlinks. It means that due to the linked change now we symlink the runfiles instead of copying therefore nearly all of the native code dependencies will have absolute path symlinks to the sandbox.

I can try to make some change so that relative symlinks in the outdir is preserved.

[illicitonion]

Yeah, I think inlining absolute symlinks feels a lot more reasonable - it's still not obviously the correct semantics, but it's probably reasonable to do. Thanks!

[havasd]

I have changed it to skip resolving relative symlinks.

Could you please clarify what would be the correct semantics? It feels to me that when the entire source tree of a dependency which needs to be built is symlinked into the sandbox we must resolve them. Otherwise the symlinks will point to the sandbox

[illicitonion]

Looks good, thanks! One last piece - can you add a couple of tests to https://github.com/bazelbuild/rules_rust/tree/0.55.6/test/cargo_build_script? I'm imagining:

1. A cargo_build_script which writes a tempfile to /tmp/ with some contents, symlinks to it in $OUT_DIR, then a rust_test which include!s the $OUT_DIR path and sees the contents is correct, and which lstats with $OUT_DIR path and checks it's a file not a symlink
2. The same test, but which writes another file into $OUT_DIR with some contents, creates a relative symlink pointing at the other file in $OUT_DIR, and which does the same checks but checks it's a symlink not a file

[havasd]

I have added tests

EDIT: I tried to make testing the symlink existence work but it doesn't seem to me that we could properly validate from rust_test. We need to include it as a data dependency in order ot be able to make it accessible but depending on the setup there is a variety of ways how these files are made available. Some configurations include them as symlink some uses copy etc...

Therefore, I had to skip testing the symlink existence

[illicitonion]

@UebelAndre You have a lot more context than I do here - can you weigh in on this vs #3111 vs something else?

[havasd]

@UebelAndre if you have some time could you please review this PR?

[UebelAndre]

I can take a look this week!

[UebelAndre]

Just a few nits but this otherwise seems good to me! Thank you for your patience with me!

[UebelAndre]

CI seems to have flaked so this will need one more push. But if you also have time to look into https://github.com/bazelbuild/rules_rust/pull/3067/files#r1902089155 that'd be sweet!

[UebelAndre]

Awesome! Thank you so much!

[spags-lacework]

Thanks for fixing this. Do you know when you will do include this in a new version of rules_rust?