bazelbuild / rules_pkg

Bazel rules for creating packages of many types (zip, tar, deb, rpm, ...)
Apache License 2.0
214 stars 167 forks source link

`remap_paths` from `pkg_tar` should apply to runfiles #790

Closed dieortin closed 5 months ago

dieortin commented 7 months ago

The new implementation of include_runfiles for pkg_tar performs path remapping only on the binaries, and then roots the runfiles in <remapped_binary_path>.runfiles. The runfiles themselves are not remapped, so placing them in a different location is not possible.

My use case for this is placing C++ .so dependencies in a different path (/lib) This would be easy to achieve if remapping applied to runfiles.

My proposed change would be applying remapping to individual runfiles, instead of to the root folder only. I have it working in https://github.com/bazelbuild/rules_pkg/compare/main...dieortin:rules_pkg:tar-runfile-mapping and would be happy to work on a PR if you think this makes sense.

aiuto commented 7 months ago

I see what you are doing there. That might be subtly different behavior than other people are expecting. I think what I did mimics the old behavior.

Sure, send a PR, but include a bunch of tests that show the various ways this could be used.

But... in the long run, remap_paths (as it exists today) is dumb. You want an intelligent regex based remapper that can say things like "move *.so to /usr/lib/myapp/lib, and move everything else to /usr/lib/myapp/share/... I want that kind of thing in pkg_files, so it is not specific to tar.

dieortin commented 7 months ago

Okay, I will be away for some days but I’ll get back to you with a PR when I return.

I agree, a regex based remapped would be great. I’m not familiar with the pkg_files implementation, would that be very challenging? I could have a go at it.

Meanwhile, as dumb as remap_paths is, it is the smartest or more flexible option that currently exists (much more so than renames from pkg_files)