a proposal to expose attributes of runfiles symlinks to Starlark

[Benjamin Peterson]

Greetings,
This is a rehash of my previous post on this topic: https://groups.google.com/d/msg/bazel-dev/sH9gXsL_99g/GW-ipg_uCgAJ dslomov suggested I resend, since the Starlark review process has changed owners since my first post.

To write packaging rules (i.e., pkg_*) in Starlark that exactly represent runfiles structure in the output package, all artifacts in a runfiles tree and their final paths need to be accessible to Starlark.

The contents of runfiles tree is composed from a number of sources:
- plain old artifacts ("unconditional artifacts" internally)
- symlinks
- root symlinks
- empty files (for __init__.py generation)
- pruning manifests (These will hereafter be ignored since they don't seem play a role in Bazel.)

All of these sources except for symlinks and root symlinks are currently introspectable by Starlark. See
https://github.com/bazelbuild/bazel/issues/3880.

To remedy this situation, I propose to expose Runfiles.SymlinkEntry.path and Runfiles.SymlinkEntry.artifact attributes to Starlark. This would complete the work of https://github.com/bazelbuild/bazel/commit/c85af31c5b0938a24cf454f437d766c65cb4c921 and https://github.com/bazelbuild/bazel/commit/20bd27412fa699854b58af2b372b4fc9d3c336b, which exposed the basic symlinks and root_symlinks attributes runfiles to Starlark. An implementation of my proposal is available at https://cr.bazel.build/19390.

Thank you for your consideration,
Benjamin

[Christopher Parsons]

Given that one can set information about symlinks and root symlinks in new runfiles objects, it seems completely reasonable that one should be able to access symlink information in the resulting object (and have the object not be completely opaque!)

To bikeshed a little, it's a little awkward that one can only *create* symlink entries using a {path_string, artifact} map to runfiles, but accesses these symlinks using a SymlinkEntry object. However, one wouldn't be able to

If anything, we might want to look into accepting SymlinkEntry as an alternative to the "symlinks" parameter of ctx.runfiles for consistency, but this seems low enough priority to not worry about for now.

Any contrary thoughts here? I believe there were some concerns that symlink entries are inherently non-Windows-compatible -- but I'd argue that this concern is out of scope for this proposal -- I would argue that if symlinks are writable in new runfiles objects, they should be readable, plain and simple.

--
You received this message because you are subscribed to the Google Groups "bazel-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bazel-dev+...@googlegroups.com.
To post to this group, send email to baze...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-dev/1539107401.1687847.1536204264.6CF6141E%40webmail.messagingengine.com.
For more options, visit https://groups.google.com/d/optout.

[Laurent Le Brun]

If we expose new fields, we have to make sure they are meaningful on all supported platforms.

Does runfiles.symlinks make sense on Windows?

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-dev/CACGd%3Db%2BMObcuDgNcs3BZ10HeFKMGm5nMSk3mvheT1x6K0zhSKw%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

Laurent

[Ulf Adams]

There are people who use symlinks on Windows, so generally yes.

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-dev/CAFtxR%2B%3DCw4bdOBBfe3YzCP4biNCscA1QHeq_naW7bz2AHT6sDg%40mail.gmail.com.

[László Csomor]

Benjamin's proposal makes sense to me.

Agreed with Laurent: the new fields should be meaningful on Windows.

Questions: what are runfiles.symlinks and what are root symlinks?

--
László Csomor | Software Engineer | laszlo...@google.com

Google Germany GmbH | Erika-Mann-Str. 33 | 80636 München | Germany
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado

[László Csomor]

And another question: how would a Starlark rule reading the SymlinkEntries tell apart empty files (__init__.py) from normal symlinks?

--
László Csomor | Software Engineer | laszlo...@google.com

Google Germany GmbH | Erika-Mann-Str. 33 | 80636 München | Germany
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado

[Benjamin Peterson]

On Wed, Oct 17, 2018, at 01:10, László Csomor wrote:
> Benjamin's proposal makes sense to me.
> Agreed with Laurent: the new fields should be meaningful on Windows.

These fields are only "new" in the sense a little more about them is exposed to Starlark under my proposal. One can already create runfiles objects with symlinks and root_symlinks from Starlark; the resulting objects are just not introspectable. This proposal does not change the operational semantics of runfiles on any platform.

>
> Questions: what are runfiles.symlinks and what are root symlinks?

The symlinks attribute lets you put an artifact in runfiles with a name different that its root-relative path. You can think of a "normal" runfile artifact as being a symlink of the form {artifact.short_path: artifact}. By varying the key, you can change the destination path.

root_symlinks are similar but more obscure. They let you place artifacts in the root of the runfiles tree rather than inside the artifact's repository directory. I think the android rules use them to put JNIs in the runfiles tree.

On Wed, Oct 17, 2018, at 01:19, László Csomor wrote:
> And another question: how would a Starlark rule reading the SymlinkEntries
> tell apart empty files (__init__.py) from normal symlinks?

There's a separate attribute on runfiles objects called "empty_files" which contains the empty files.

[László Csomor]

All clear now, thanks!

--
László Csomor | Software Engineer | laszlo...@google.com

Google Germany GmbH | Erika-Mann-Str. 33 | 80636 München | Germany
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado

[Benjamin Peterson]

Given that all questions have been answered, would anyone like to give my CL the birthday present of a review? https://cr.bazel.build/19390

On Wed, Oct 10, 2018, at 10:07, Christopher Parsons wrote:
> Given that one can set information about symlinks and root symlinks in new
> runfiles objects, it seems completely reasonable that one should be able to
> access symlink information in the resulting object (and have the object not
> be completely opaque!)
>
> To bikeshed a little, it's a little awkward that one can only *create*
> symlink entries using a {path_string, artifact} map to runfiles, but
> accesses these symlinks using a SymlinkEntry object. However, one wouldn't
> be able to
> If anything, we might want to look into accepting SymlinkEntry as an
> alternative to the "symlinks" parameter of ctx.runfiles

> <https://docs.bazel.build/versions/master/skylark/lib/ctx.html#runfiles> for

[Christopher Parsons]

Sorry for the late amendment to this thread -- but maybe we can change the names of this new Symlink type in the proposal?

Instead of symlink.path and symlink.artifact, perhaps symlink.name and symlink.target_file  ? 
(Indeed, at the very least we want to avoid 'artifact', as we use 'File' throughout the API)

[Benjamin Peterson]

On Tue, Oct 23, 2018, at 16:20, Christopher Parsons wrote:
> Sorry for the late amendment to this thread -- but maybe we can change the
> names of this new Symlink type in the proposal?
>
> Instead of symlink.path and symlink.artifact, perhaps symlink.name and
> symlink.target_file ?
> (Indeed, at the very least we want to avoid 'artifact', as we use 'File'
> throughout the API)

That sounds fine with me, though I'm inclined to shorten the later to just "target". That it's a Starlark "File" is clear from the type.

[Christopher Parsons]

My concern with using "target" is that we also have a Starlark type "Target":  https://docs.bazel.build/versions/master/skylark/lib/Target.html

[Benjamin Peterson]

Makes sense. "target_file" it is!

[Christopher Parsons]

.. upon further reflection, I think perhaps symlink.path is still appropriate, instead of name. 

So: path and target_file ?

Sorry for the back and forth -- it apparently needed a few hours to marinate. 

If anyone else doesn't object in by EOD, I'll approve with this name change :)

[Laurent Le Brun]

+1 for "path" and "target_file"

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-dev/CACGd%3DbJhpBgkn3KCixDvV2zOFg6Vv6ytGqCRVadFvaiPGcF0iw%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

Laurent