modules: to what extent should modules support symlinks?

[myitcv]

"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

[seh]

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/rulescue 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.

[myitcv]

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

[rogpeppe]

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

[seh]

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.

[mvdan]

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]

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.

[myitcv]

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?

[seh]

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]

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 _rulesgo 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.

[myitcv]

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.

[mvdan]

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.

[myitcv]

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?

[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?

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.

[rogpeppe]

@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?

[seh]

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

In the _rulesgo 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.

_rulesgo 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.

[mvdan]

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.

[rogpeppe]

@seh

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

In the _rulesgo 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.

_rulesgo 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) ?

[seh]

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 _rulesgo 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.