exposing enough runfiles data to Skylark to implement correct packaging rules

107 views
Skip to first unread message

Benjamin Peterson

unread,
Nov 7, 2017, 1:05:49 AM11/7/17
to baze...@googlegroups.com
Background
----------

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

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 Skylark. See
https://github.com/bazelbuild/bazel/issues/3880.

I have prepared to two options for how to expose the necessary
information to
Skylark. I slightly prefer option 1. However, I mostly just want to
write a
correct packaging rule, so I'll let the Skylark brain trust pick between
them.

Option 1
--------

Expose Runfiles.SymlinkEntry.path and Runfiles.SymlinkEntry.artifact
attributes
to Skylark. 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
Skylark.

I have implemented this option in
https://bazel-review.googlesource.com/c/bazel/+/19390.

Option 2
--------

In a conversation at the conference today, it was pointed out that
having the
runfiles manifest artifact and the set of all the artifacts in it would
also be
sufficient. Since FilesToRunProvider already exposes the
runfiles_manifest, all
we would need is the Runfiles.getAllArtifacts() method exposed to
Skylark. I
would propose naming it "runfiles.all_files".

Benjamin Peterson

unread,
Nov 10, 2017, 2:17:39 AM11/10/17
to baze...@googlegroups.com


On Mon, Nov 6, 2017, at 22:05, Benjamin Peterson wrote:
>
> Option 2
> --------
>
> In a conversation at the conference today, it was pointed out that
> having the
> runfiles manifest artifact and the set of all the artifacts in it would
> also be
> sufficient. Since FilesToRunProvider already exposes the
> runfiles_manifest, all
> we would need is the Runfiles.getAllArtifacts() method exposed to
> Skylark. I
> would propose naming it "runfiles.all_files".

It occurs to me that depending on the runfiles input manifest is
problematic because it contains absolute paths. This inhibits both
remote caching and proper sandboxing.

So, option 1 is looking better.

Damien Martin-Guillerez

unread,
Nov 23, 2017, 4:06:51 AM11/23/17
to Benjamin Peterson, dsl...@google.com, laur...@google.com, baze...@googlegroups.com
+Dmitry Lomov +Laurent Le Brun could you update the thread with the status of this proposal?

--
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/1510298257.241072.1167877808.43183D0B%40webmail.messagingengine.com.
For more options, visit https://groups.google.com/d/optout.

Benjamin Peterson

unread,
Nov 28, 2017, 2:47:13 AM11/28/17
to baze...@googlegroups.com
We could salvage something like option 2 by exposing the final runfiles
tree layout as a map:

{(runfiles path) -> artifact}

This has the virtue of simplicity, since rule authors wouldn't have to
poke through files, symlinks, and empty_files.

On, the other hand it's less efficient because the the entire runfiles
tree map would have to realized at analysis time.

Benjamin Peterson

unread,
Apr 25, 2018, 1:23:17 AM4/25/18
to baze...@googlegroups.com
Does anyone have opinions on this subject? I'm still interested in seeing https://cr.bazel.build/19390 land.
Reply all
Reply to author
Forward
0 new messages