bazel-skylib icon indicating copy to clipboard operation
bazel-skylib copied to clipboard

feature request: Create common rule for symlinking

Open Yannic opened this issue 5 years ago • 6 comments

It would be great if skylib had a rule similar to copy_file that, instead of manually copying the file, uses ctx.actions.symlink (since Bazel 3.2).

This could be either implemented as a new rule (symlink_file?), or as attribute of copy_file (which could potentially become the default behavior in the future).

Yannic avatar May 18 '20 13:05 Yannic

I have some reservations about this. I don't have time to do a deep explanation this week, so I'm just asking questions about the need and the PR you sent. Because knowing the needs is important to evaluate the PR.

why does someone need a symlink?

  • Do we just want the file under a new name and this is speed optimization over copy?
  • Why do we want a new name in the first place?
  • Is this for packaging later in tar or zip? (e.g. BUILD_dist in source, but need BUILD in the tgz distribution).
  • Do we want to support naked symlinks, which are a different real need. Again, the motivations are the same

If we only are doing this to rename in a performant way, why can't we make this a fast_copy rule which promises to do a symlink if it can or copy if it can't, but it leaves a real artifiact either way.

If we are doing this to define packaging structures, then I don't think we want to build the symlinks. We want package rules that describe the need for the symlink, and the packager gets to pick what it builds. For example, it is reasonable for a developer with remote build to have a windows host, but build a tarball that contains a symlink.

aiuto avatar May 29 '20 17:05 aiuto

The purpose of this new rule is to have a faster way to give a file a new name. The reason why we need to make files available under different names is that some tools unfortunately behave differently depending on their name (e.g. sha256(clang) == sha256(clang++), but they do different things when called as clang vs clang++ 😡). Obviously, symlinking a large binary is faster than copying it.

I'm not sure what you mean with naked symlinks. Are you talking about the experimental unresolved symlinks in Bazel which allow creating symlinks which point to arbitrary strings (https://github.com/bazelbuild/bazel/issues/10298)? If so, then, at this moment, I don't have any intention in supporting them in the symlink_file rule. IMO, unresolved symlinks are tricky (caching, remote execution, ...) and should only be used to reach absolute paths outside of a WORKSPACE (e.g. /usr/bin/gcc). Making them easier to use will most-likely cause more harm than good.

For the actual implementation, I think you may not yet be aware of the brand-new ctx.actions.symlink that landed in Bazel 3.2 which also allows Artifact -> Artifact (https://github.com/bazelbuild/bazel/pull/10695) symlinks. It behaves exactly like the fast_copy rule you're describing.

Yannic avatar May 29 '20 18:05 Yannic

Thanks. The use case makes things much clearer.

If you want it for speed, let's make a rule name that declares that intent, something like 'lightweight_copy. Of course, that name is ugly, but the point is that we hint that it is less resource consumptive and that does not mean "always a symlnk".

Or maybe this could just be a parameter to copy_file, like "allow_fast_copy".

By naked, I did mean dangling. I'm aware of the symlink action.

My intent is that the rule name here does not make people believe they are getting deep properties of a symlink. I've been working on packaging rules a lot lately, and there, the symlink-ness property does matter. That is, if the rule says symlink and you are building a zip file from windows and expect the symlink to be created in the zip file, you would be disappointed. But if the rule as called lightweight_copy I would not have that expectation. Does that make sense to you?

aiuto avatar Jun 01 '20 16:06 aiuto

Thanks! Yeah, that makes sense.

I'm leaning more towards a parameter on copy_file (allow_symlink might make it more obvious that a symlink can be created than allow_fast_copy).

Yannic avatar Jun 02 '20 15:06 Yannic

Sounds good to me. @laurentlb any thoughts?

aiuto avatar Jun 02 '20 17:06 aiuto

I believe this issue is closed by https://github.com/bazelbuild/bazel-skylib/pull/252?

adam-azarchs avatar Oct 07 '22 20:10 adam-azarchs