rust_doc: strip directory prefix from archive names #474

wchargin · 2020-11-04T17:37:03Z

The Zip archives emitted by rust_doc rules used to contain a long
/bazel-bin/k8-fastbuild/bin/path/to/target prefix before each archive
entry name. This made it hard to work with the generated archive. This
patch adds a transformation to strip the bin-dir prefix, leaving only
the path to the target. We leave the path to the target so that multiple
rust_doc zip outputs within one workspace can be meaningfully expanded
into the same directory.

This could be written in Starlark using the DirectoryExpander API, but
that was only introduced in Bazel 3.4.0. Since we have a minimum Bazel
version of 0.17.1, we instead write the helper as a small Rust script.

Fixes #471.

Test Plan:
Unit tests included. As an end-to-end test, run

cd examples/ &&
bazel build //hello_world:hello_world_doc &&
unzip -l bazel-bin/hello_world/hello_world_doc.zip

to list the contents of the built hello_world_doc.zip archive. Before
this patch, the result was like:

Archive:  bazel-bin/hello_world/hello_world_doc.zip
  Length      Date    Time    Name
---------  ---------- -----   ----
        0  2010-01-01 00:00   bazel-out/k8-fastbuild/bin/hello_world/hello_world_doc/.lock
     1792  2010-01-01 00:00   bazel-out/k8-fastbuild/bin/hello_world/hello_world_doc/COPYRIGHT.txt
     4421  2010-01-01 00:00   bazel-out/k8-fastbuild/bin/hello_world/hello_world_doc/FiraSans-LICENSE.txt
...
     2132  2010-01-01 00:00   bazel-out/k8-fastbuild/bin/hello_world/hello_world_doc/storage.js
     1180  2010-01-01 00:00   bazel-out/k8-fastbuild/bin/hello_world/hello_world_doc/theme.js
     3764  2010-01-01 00:00   bazel-out/k8-fastbuild/bin/hello_world/hello_world_doc/wheel.svg
---------                     -------
   890555                     38 files

After this patch:

Archive:  bazel-bin/hello_world/hello_world_doc.zip
  Length      Date    Time    Name
---------  ---------- -----   ----
        0  2010-01-01 00:00   hello_world/hello_world_doc/.lock
     1792  2010-01-01 00:00   hello_world/hello_world_doc/COPYRIGHT.txt
     4421  2010-01-01 00:00   hello_world/hello_world_doc/FiraSans-LICENSE.txt
...
     2132  2010-01-01 00:00   hello_world/hello_world_doc/storage.js
     1180  2010-01-01 00:00   hello_world/hello_world_doc/theme.js
     3764  2010-01-01 00:00   hello_world/hello_world_doc/wheel.svg
---------                     -------
   890555                     38 files

wchargin-branch: rustdoc-strip-prefix

The Zip archives emitted by `rust_doc` rules used to contain a long `/bazel-bin/k8-fastbuild/path/to/target` prefix before each archive entry name. This made it hard to work with the generated archive. This patch adds a transformation to This could be written in Starlark using the `DirectoryExapnder` API, but that was only introduced in Bazel 3.4.0. Since we have a minimum Bazel version of 0.17.1, we instead write the helper as a small Python script. Fixes bazelbuild#471. Test Plan: Unit tests included. As an end-to-end test, run ``` cd examples/ && bazel build //hello_world:hello_world_doc && unzip -l bazel-bin/hello_world/hello_world_doc.zip ``` to list the contents of the built `hello_world_doc.zip` archive. Before this patch, the result was like: ``` Archive: bazel-bin/hello_world/hello_world_doc.zip Length Date Time Name --------- ---------- ----- ---- 0 2010-01-01 00:00 bazel-out/k8-fastbuild/bin/hello_world/hello_world_doc/.lock 1792 2010-01-01 00:00 bazel-out/k8-fastbuild/bin/hello_world/hello_world_doc/COPYRIGHT.txt 4421 2010-01-01 00:00 bazel-out/k8-fastbuild/bin/hello_world/hello_world_doc/FiraSans-LICENSE.txt ... 2132 2010-01-01 00:00 bazel-out/k8-fastbuild/bin/hello_world/hello_world_doc/storage.js 1180 2010-01-01 00:00 bazel-out/k8-fastbuild/bin/hello_world/hello_world_doc/theme.js 3764 2010-01-01 00:00 bazel-out/k8-fastbuild/bin/hello_world/hello_world_doc/wheel.svg --------- ------- 890555 38 files ``` After this patch: ``` Archive: bazel-bin/hello_world/hello_world_doc.zip Length Date Time Name --------- ---------- ----- ---- 0 2010-01-01 00:00 .lock 1792 2010-01-01 00:00 COPYRIGHT.txt 4421 2010-01-01 00:00 FiraSans-LICENSE.txt ... 2132 2010-01-01 00:00 storage.js 1180 2010-01-01 00:00 theme.js 3764 2010-01-01 00:00 wheel.svg --------- ------- 890555 38 files ``` wchargin-branch: rustdoc-strip-prefix wchargin-source: 29978cf3bdb7de6cee154f8c11d0251574c2b104

wchargin-branch: rustdoc-strip-prefix wchargin-source: 69b2e78f1f52c88609aff631c908471c7521c5f7

dfreese · 2020-11-04T21:44:20Z

util/dir_zipper/dir_zipper_lib.py

+        rel = f[len(strip_prefix) :]
+        spec = "%s=%s" % (rel, f)
+        args.append(spec)


Is similar logic not possible within starlark?

The logic is possible in Starlark; the tricky bit is expanding the
directory to its list of contents. I haven’t found a way to do that
other than using the DirectoryExpander API added in Bazel 3.4.0.

If there is such a way, I would be very happy to switch to it instead of
calling out to this utility.

rust/repositories.bzl

rust/private/rustdoc.bzl

wchargin

Thanks for the quick review! Happy to discuss further.

wchargin · 2020-11-04T22:03:38Z

util/dir_zipper/dir_zipper_lib.py

+        rel = f[len(strip_prefix) :]
+        spec = "%s=%s" % (rel, f)
+        args.append(spec)


The logic is possible in Starlark; the tricky bit is expanding the
directory to its list of contents. I haven’t found a way to do that
other than using the DirectoryExpander API added in Bazel 3.4.0.

If there is such a way, I would be very happy to switch to it instead of
calling out to this utility.

rust/private/rustdoc.bzl

rust/repositories.bzl

wchargin-branch: rustdoc-strip-prefix wchargin-source: 40227180c7f910d9f76716cf42621e08e5f808a1

wchargin-branch: rustdoc-strip-prefix wchargin-source: 8c46290195fc0668f3126589fd07ee863df8749f

wchargin-branch: rustdoc-strip-prefix wchargin-source: 838287ca34eb69c27f62e4a0900dbb7ac6a3ebd0

UebelAndre · 2020-11-13T22:00:22Z

What's the status here? Is this ready for review again?

wchargin · 2020-11-15T02:57:15Z

No; I haven’t gotten around to addressing all comments. Will re-request
review through the GitHub UI once I have.

wchargin-branch: rustdoc-strip-prefix wchargin-source: ea88a0484df0608cb08521fb4cde2d4dc7904553

UebelAndre · 2020-12-03T05:56:57Z

Friendly ping here. This change sounds like a great addition 😃

wchargin-branch: rustdoc-strip-prefix wchargin-source: aba05e35e129db74d032bcbfdb7764233a2d94ce

wchargin · 2020-12-03T23:43:11Z

Okay: I’ve rebased master. This patch seems to work fine. I’m having
trouble with the build setup for #475, but I agree that this patch is
nice to have by itself, so feel free to merge if you like.

google-cla bot added the cla: yes label Nov 4, 2020

[update patch]

0bd884c

wchargin-branch: rustdoc-strip-prefix wchargin-source: 69b2e78f1f52c88609aff631c908471c7521c5f7

dfreese reviewed Nov 4, 2020

View reviewed changes

wchargin commented Nov 4, 2020

View reviewed changes

wchargin mentioned this pull request Nov 4, 2020

rust_doc: add web server for docs #475

Closed

wchargin added 3 commits November 6, 2020 12:10

[update patch]

24d6478

wchargin-branch: rustdoc-strip-prefix wchargin-source: 40227180c7f910d9f76716cf42621e08e5f808a1

[update patch]

5ab7c6e

wchargin-branch: rustdoc-strip-prefix wchargin-source: 8c46290195fc0668f3126589fd07ee863df8749f

[update diffbase]

2674343

wchargin-branch: rustdoc-strip-prefix wchargin-source: 838287ca34eb69c27f62e4a0900dbb7ac6a3ebd0

wchargin added 2 commits November 19, 2020 15:28

[update diffbase]

1c405e0

wchargin-branch: rustdoc-strip-prefix wchargin-source: ea88a0484df0608cb08521fb4cde2d4dc7904553

[update patch]

a2f58a8

wchargin-branch: rustdoc-strip-prefix wchargin-source: ea88a0484df0608cb08521fb4cde2d4dc7904553

[rustdoc-strip-prefix: update diffbase]

9cacb6e

wchargin-branch: rustdoc-strip-prefix wchargin-source: aba05e35e129db74d032bcbfdb7764233a2d94ce

wchargin requested a review from dfreese December 3, 2020 23:43

Merge branch 'master' into wchargin-rustdoc-strip-prefix

569c738

dfreese approved these changes Dec 12, 2020

View reviewed changes

dfreese merged commit 4780f35 into bazelbuild:master Dec 12, 2020

wchargin deleted the wchargin-rustdoc-strip-prefix branch December 13, 2020 20:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rust_doc: strip directory prefix from archive names #474

rust_doc: strip directory prefix from archive names #474

wchargin commented Nov 4, 2020 •

edited

Loading

dfreese Nov 4, 2020

wchargin Nov 4, 2020

wchargin left a comment

wchargin Nov 4, 2020

UebelAndre commented Nov 13, 2020

wchargin commented Nov 15, 2020

UebelAndre commented Dec 3, 2020

wchargin commented Dec 3, 2020

rust_doc: strip directory prefix from archive names #474

rust_doc: strip directory prefix from archive names #474

Conversation

wchargin commented Nov 4, 2020 • edited Loading

dfreese Nov 4, 2020

Choose a reason for hiding this comment

wchargin Nov 4, 2020

Choose a reason for hiding this comment

wchargin left a comment

Choose a reason for hiding this comment

wchargin Nov 4, 2020

Choose a reason for hiding this comment

UebelAndre commented Nov 13, 2020

wchargin commented Nov 15, 2020

UebelAndre commented Dec 3, 2020

wchargin commented Dec 3, 2020

wchargin commented Nov 4, 2020 •

edited

Loading