rules_docker icon indicating copy to clipboard operation
rules_docker copied to clipboard

Support runfiles root_symlinks in images

Open wolfd opened this issue 1 year ago • 1 comments

Fixes symlink support and adds root_symlink support.

PR Checklist

Please check if your PR fulfills the following requirements:

  • [x] Tests for the changes have been added (for bug fixes / features)
  • [ ] Docs have been added / updated (for bug fixes / features)

^ I'm not aware of docs that mention what specifically happens to runfiles.

PR Type

What kind of change does this PR introduce?

  • [x] Bugfix
  • [ ] Feature
  • [ ] Code style update (formatting, local variables)
  • [ ] Refactoring (no functional changes, no api changes)
  • [ ] Build related changes
  • [ ] CI related changes
  • [ ] Documentation content changes
  • [ ] Other... Please describe:

What is the current behavior?

Previously I was seeing that runfiles.symlinks (regardless of whether the target file existed in runfiles.files), did not point to the correct location in the image (they were broken).

Additionally, root_symlinks were entirely ignored.

Issue Number: N/A

What is the new behavior?

The overall method we take here is to create real files where the [root_]symlink is declared if the target File is not seen in file_map.

We now use _final_file_path instead of layer_file_path. This seems to work, but I am not sure if I'm missing a use-case here.

This PR also adds some extra test functionality to check that symlinks point at the expected location.

Does this PR introduce a breaking change?

  • [ ] Yes
  • [x] No

I don't think that the previous behavior is considered working, and I'm having trouble imagining a case where that broken behavior would be desired.

Other information

wolfd avatar Apr 21 '23 23:04 wolfd

For what it's worth, using root_symlinks or symlinks without also including the real file in runfiles seems to be buggy when using --remote_download_toplevel, as mentioned here: https://github.com/bazelbuild/bazel/issues/18249

Regardless, this PR can be used to make layers with or without the target file existing in the same runfiles object. Just thought I'd mention it.

wolfd avatar May 12 '23 17:05 wolfd