cue icon indicating copy to clipboard operation
cue copied to clipboard

all: to what extent should CUE support symlinks?

Open myitcv opened this issue 1 year ago • 21 comments
trafficstars

"Inspired" by https://github.com/cue-lang/cue/issues/3299, I don't think we have been explicit about the extent to which modules support (or otherwise) symbolic links.

Questions not limited to:

  • Should module loading follow symlinks at all? We already document that published modules don't support them: https://cuelang.org/docs/reference/modules/#zip-path-size-constraints
  • Could/should we distinguish between the main module or otherwise?
  • ...

This issue might generate further issues for bugs/implementation.

End goal is that we should be clear in our documentation (exact location TBC) on the extent to which module loading does/doesn't support symbolic links.

See also #3298

myitcv avatar Jul 17 '24 15:07 myitcv

In my case—the basis for the aforementioned #3299—I'm not creating these symbolic links deliberately in my primary source repository. Rather, I'm using my seh/rules_cue Bazel ruleset to evaluate the CUE files within Bazel's actions, and Bazel's normal sandbox preparation behavior involves creating a directory tree full of symbolic links to regular files sitting elsewhere, as an optimization against copying all of those files.

seh avatar Jul 17 '24 17:07 seh

@seh - thanks for adding this context, extremely useful.

myitcv avatar Jul 18 '24 04:07 myitcv

As a data point, I checked the go command's behaviour in this regard:

  • Go accepts and reads symlinks locally, including when using a directory replace directive and in vendor directories.
  • Go silently drops symlinks when consuming from a network source, such as the Go proxy or a remote VCS source

rogpeppe avatar Jul 18 '24 12:07 rogpeppe

Regarding support for Bazel's sandboxes, it wouldn't be much trouble to require that callers of the cue tool pass a command-line flag to enable tolerance for symbolic links, if you didn't feel that it was safe or fair to use that as the default behavior. If there was such a command-line flag, I'd have my Bazel ruleset pass it to the cue tool when invoking it inside of Bazel actions.

seh avatar Jul 18 '24 13:07 seh

Should module loading follow symlinks at all?

Perhaps I jumped the gun by approving the patch for https://github.com/cue-lang/cue/issues/3298, which makes the loader follow symlinks locally. The way that issue was raised before this one, in the format of a bug report, didn't make it clear that it was a slightly open question.

Given that Go does this locally, doing the same seems fine to me. But given that symlinks cannot be published or consumed via a GOPROXY (nor a CUE registry), I think it would also be fine to leave them as unsupported in a consistent way.

mvdan avatar Jul 19 '24 12:07 mvdan

Making a decision here should also close https://github.com/cue-lang/cue/issues/1672, hopefully.

Note that some people in that thread were wondering about symlink support for loading local packages, but it's not clear to me that any of them actually relied on it in the past.

mvdan avatar Jul 19 '24 12:07 mvdan

But given that symlinks cannot be published or consumed via a GOPROXY (nor a CUE registry), I think it would also be fine to leave them as unsupported in a consistent way.

With the caveat that we would then need to understand/answer how things could/should work with Bazel. @seh how familiar are you with what Go does with respect to Bazel? Anything we should be learning from there?

myitcv avatar Jul 19 '24 12:07 myitcv

Note that some people in that thread were wondering about symlink support for loading local packages, but it's not clear to me that any of them actually relied on it in the past.

I've been relying on it since first using CUE with Bazel three and a half years ago.

seh avatar Jul 19 '24 13:07 seh

how familiar are you with what Go does with respect to Bazel? Anything we should be learning from there?

As far as I can tell from inspecting the directory trees created by the rules_go ruleset when I build Go programs with Bazel, every file presented to the Go toolchain is a symbolic link.

If we happen to have Bazel generate any files used as input to compiling or linking action, that artifact might wind up as a regular file sitting within that same sandbox's directory tree, but not necessarily.

seh avatar Jul 19 '24 13:07 seh

My sense is that we keep things working as-is, by first tightening up what we mean by "as-is", and adding any missing tests.

So here is an attempt to define what "as-is" is/should be:

CUE does drops symlinks when writing module zip files (per https://cuelang.org/docs/reference/modules/#zip-path-size-constraints) and when reading them.

That second part of the sentence is worth us making sure with a test. Because it's possible to construct a "rogue" zip file that does include symlinks using the regular Unix zip command. Being really sure how CUE treats such a zip is important.

Note that the rule would also apply in the case of:

cue export $pkg@$version

As such, an embed "read" will only ever happen within the CUE module cache for a zip-based dependency.

That way we can say that if the CUE code did not come from a zip archive (working with a main module on disk, a directory-based replace directive, vendor, cue.mod/{pkg,gen,usr}, ...) then symlinks will work. This should ensure we leave the existing support for Bazel in place, and indeed make it more official via that definition.

@embed directives will never follow symlinks.

myitcv avatar Jul 22 '24 04:07 myitcv

by first tightening up what we mean by "as-is", and adding any missing tests.

When we have those tests, we should also verify how older CUE versions behaved, for example v0.5.0.

mvdan avatar Jul 22 '24 09:07 mvdan

Following my comment in https://github.com/cue-lang/cue/issues/3299#issuecomment-2244581796, I have updated my comment above to state clearly that:

@embed directives will never follow symlinks.

This brings CUE inline with Go's behaviour with respect to embed.

@seh - please could I ask you to take a look at my conclusion above and raise any concerns you might have from a Bazel perspective?

myitcv avatar Jul 23 '24 09:07 myitcv

please could I ask you to take a look at my conclusion above and raise any concerns you might have from a Bazel perspective?

The treatment of files in modules proposed above sounds like it would work fine with Bazel's sandbox technique, at least for files in modules that come from the same Bazel workspace. That would cover my use of modules thus far.

The restriction on embed may turn out to be onerous, in that one could write a CUE file that embeds a JSON file sitting right next to it on disk, but when Bazel prepares a sandbox for an action that involves these two files as input, it will create symbolic links to both. When the cue tool reads the CUE file—traversing the symbolic link to do so—it will find an @embed attribute mentioning a file that's present on disk as another symbolic link. If the cue tool then refuses to read that file, it appears to be behaving differently within the sandbox. Of course, the cue tool would have refused to read the symbolic link outside of the sandbox too, but that's not what the author intended to arrange.

I can imagine a softer but more complicated rule, something like: If we read the CUE file containing the @embed attribute through a symbolic link, then we'll look for the file mentioned in the @embed attribute via relative path from the target of the CUE file's symbolic link. In other words, for relative paths, the resolution base would be the CUE file's actual path, meaning the final target of any symbolic links that pointed to it.

seh avatar Jul 23 '24 12:07 seh

@seh

The restriction on embed may turn out to be onerous, in that one could write a CUE file that embeds a JSON file sitting right next to it on disk, but when Bazel prepares a sandbox for an action that involves these two files as input, it will create symbolic links to both.

Given that Go seems to have exactly the same rule, do you know how Go manages to avoid that difficulty, by any chance?

rogpeppe avatar Jul 23 '24 17:07 rogpeppe

do you know how Go manages to avoid that difficulty, by any chance?

In the rules_go Bazel ruleset, both the go_library and go_binary rules accept an "embedsrcs" attribute to designate the input files that are candidates for being embedded. The rules restrict the nominated dependencies to sit within the same Bazel package's root directory, which is a logical restriction; it's possible to mix both primary source files and files generated by other Bazel targets, so long as the targets produce output files that sit in that same directory.

rules_go prepares a so-called embedcfg file that it then supplies to the Go compiler via the -embedcfg command-line flag. You can see how the compiler uses that file in the readEmbedCfg function.

There's a lot of bundling up the files to be embedded and figuring out their relative paths. It's hard to follow what's going on, but I think it's mainly about working around the restriction on following symbolic links.

seh avatar Jul 23 '24 17:07 seh

With the tests from https://review.gerrithub.io/c/cue-lang/cue/+/1198252 merged, I'm pushing the rest of the work here to alpha.3, as we are releasing alpha.2 today.

mvdan avatar Jul 24 '24 08:07 mvdan

@seh

do you know how Go manages to avoid that difficulty, by any chance?

In the rules_go Bazel ruleset, both the go_library and go_binary rules accept an "embedsrcs" attribute to designate the input files that are candidates for being embedded. The rules restrict the nominated dependencies to sit within the same Bazel package's root directory, which is a logical restriction; it's possible to mix both primary source files and files generated by other Bazel targets, so long as the targets produce output files that sit in that same directory.

rules_go prepares a so-called embedcfg file that it then supplies to the Go compiler via the -embedcfg command-line flag. You can see how the compiler uses that file in the readEmbedCfg function.

There's a lot of bundling up the files to be embedded and figuring out their relative paths. It's hard to follow what's going on, but I think it's mainly about working around the restriction on following symbolic links.

That's all super helpful, thanks! I'm totally unfamiliar with Bazel so it's hard for me to know how much of this applies only to Go and how much might apply to CUE too, but it seems like we will probably need a similar mechanism to Go's -embedcfg so we can make embed directives work even when the embed targets are in another directory. Or... does Bazel know enough to be able to make symlinks to the embedded files too (and hence just supporting embedded files that are symlinks might be OK) ?

rogpeppe avatar Jul 31 '24 10:07 rogpeppe

does Bazel know enough to be able to make symlinks to the embedded files too (and hence just supporting embedded files that are symlinks might be OK) ?

Bazel will create symbolic links to all the files nominated as "inputs" for an action that it's going to run, regardless of where the source files sit relative to each other either in the primary repository or as generated outputs from other actions. So long as we mention that the embedded files need to be inputs to the action that runs cue def or cue export, we'll get symbolic links to them in the sandbox that Bazel creates for the action.

The reason that rules_go wants the embedded source files specified in a separate attribute on the go_library or go_binary rule—that is, "embedsrcs" alongside the usual "srcs"—is so that it can arrange for telling compiler about them differently, assuaging its concerns about reading symbolic links.

seh avatar Aug 04 '24 17:08 seh

I've retitled this issue to the much broader "to what extent should CUE support symlinks?"

With a view to this issue being closed with a holistic explanation of where CUE does/doesn't support symlinks, along with brief rationale for each relevant point.

@rogpeppe is going to follow up with that summary.

We will also create further issues as needs be to capture outstanding bugs/work. e.g. if our aim is to support symlinks in such a way that things work for Bazel, we should have tests that lock that in.

myitcv avatar Oct 10 '24 08:10 myitcv

@seh Do I have it right that you're happy with CUE's current behaviour with respect to symbolic links?

rogpeppe avatar Oct 10 '24 13:10 rogpeppe

Do I have it right that you're happy with CUE's current behaviour with respect to symbolic links?

From what I've read, it sounds workable for use with Bazel. I'd like to try an example involving a mixture of static source and generated files that I can have Bazel run through cue export.

seh avatar Oct 12 '24 14:10 seh

For the record, the executive summary of current (and proposed future) behaviour with respect to symlinks is as follows:

We allow and respect symbolic links except that we reject/ignore them whenever they become the contents of a module in a registry.

As such, symbolic links will not be unarchived from the contents of a module archive and will not appear in the module cache. As another consequence of this behaviour, when we implement directory replace for modules (#2956), symbolic links will function inside the directory holding the replacement module. Note: this behaviour covers all files within a module, including files embedded with the @embed directive.

rogpeppe avatar Oct 22 '24 14:10 rogpeppe

Great. Thanks, @rogpeppe. This I think concludes the discussion. We follow up on a possible FAQ for detailed points implied by this conclusion in https://github.com/cue-lang/docs-and-content/issues/187. Publishing such an FAQ to cuelang.org will allow for the more easy surfacing of answers/results. FYI @jpluscplusm

myitcv avatar Oct 22 '24 14:10 myitcv

We allow and respect symbolic links except that we reject/ignore them whenever they become the contents of a module in a registry.

Does this mean that symbolic links are only supported in local modules, but not registry modules?

This seems reasonable, I'm just a bit confused by the phrasing. 🙏

infogulch avatar Dec 18 '24 16:12 infogulch

@infogulch correct. https://cuelang.org/docs/reference/modules/ says:

Symbolic links and other irregular files are ignored when creating zip files, since they aren’t portable across operating systems and file systems, and there’s no portable way to represent them in the zip file format.

so any symbolic links present are ignored when using cue mod publish. But since loading CUE files and packages does follow symlinks, as long as you load a module which wasn't fetched from a CUE registry (e.g. via git clone), they will work.

mvdan avatar Dec 18 '24 16:12 mvdan

This seems reasonable, I'm just a bit confused by the phrasing. 🙏

We recently published https://cuelang.org/docs/concept/faq/symbolic-link-support/ - does its wording feel less confusing and more useful to you, @infogulch?

jpluscplusm avatar Dec 18 '24 16:12 jpluscplusm

Thanks for clarifying. Yes the docs are better.

Maybe it would be helpful to have a name to positively identify and distinguish the two different kinds/sources of modules -- like "registry module" vs "local module" -- then defining the behavior by reference to these names. I suspect an explanation of this form would be faster to grok and make it easier to derive the answers to such FAQ for oneself.

Is this the only difference between "local" and "remote" modules? If there are more features that distinguish them then maybe it would be worth naming.

infogulch avatar Dec 18 '24 16:12 infogulch