repo-infra icon indicating copy to clipboard operation
repo-infra copied to clipboard

add missing functionality to build_tar

Open ixdy opened this issue 6 years ago • 8 comments

Our Go implementation of build_tar has fallen behind some of the features in the upstream Python implementation of build_tar:

  • --manifest: JSON manifest of contents to add to the layer
  • --empty_file: add an empty file
  • --empty_dir: add an empty dir (Kubernetes would use this, currently we create a dummy archive instead)
  • --empty_root_dir: this defaults to "./" upstream; we currently don't support this, but could add it for compatibility (and then set to "kubernetes/" or "" for our usage, similar to what's being discussed in https://github.com/bazelbuild/bazel/issues/7293)
  • --mtime: "Set mtime on tar file entries. May be an integer or the value "portable", to get the value 2000-01-01, which is usable with non *nix OSes" (https://github.com/bazelbuild/bazel/commit/25d202fc1545f4aaba821df75a8bf3fb57284da1)
  • deb support - tracked in https://github.com/kubernetes/repo-infra/issues/61

cc @mikedanese

ixdy avatar Jan 31 '19 21:01 ixdy

It'd also be good if we could reuse some of the tests for upstream build_tar and pkg_tar, or at least add some kind of testing.

ixdy avatar Jan 31 '19 21:01 ixdy

@ixdy is bazel_tools open to using go based tools for core rules? what are the roadblocks for getting our build_tar to replace build_tar.py?

mikedanese avatar Feb 01 '19 18:02 mikedanese

I haven't asked, but I don't think there's any precedent of having go in core bazelbuild/bazel. Maybe if pkg_tar were moved into bazelbuild/rules_pkg? Or at least we could see about hosting this binary there instead.

ixdy avatar Feb 01 '19 18:02 ixdy

we should kill this tool and use the built-in one

there is no reason for us to spend our time competing with bazel here. If we are unhappy the performance we should get them to improve the performance, not roll our own replacement

fejta avatar Mar 29 '19 01:03 fejta

@fejta the built-in python one is slooooow

ixdy avatar Mar 29 '19 01:03 ixdy

I know, and I'll take slow and functional over high maintenance costs every day of the week

fejta avatar Mar 29 '19 02:03 fejta

If I use the python implementation and bazel build //build/release-tars, then make a change to CONTRIBUTING.md and rebuild //build/release-tars, it takes 225s just to rebuild kubernetes-src.tar.gz and kubernetes-server-linux-amd64.tar.gz (with no compilation). That seems pretty unacceptable to me.

Another issue is that our implementation is intentionally not quite identical - we don't prepend ./ to the filenames in the tarfile. I attempted to fix this issue upstream in https://github.com/bazelbuild/bazel/commit/72ab9322ac4ea6b743bc304b7b5d05f8b5b56239, but it ended up breaking other users, because pkg_tar is used in a lot of places, and it's hard to track them down (especially since Google's uses are more opaque).

https://github.com/bazelbuild/bazel/issues/2380 and https://github.com/bazelbuild/bazel/issues/7293 are relevant issues which might solve our problems if addressed, but neither seem to be getting any traction.

ixdy avatar Mar 29 '19 06:03 ixdy

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

fejta-bot avatar Jun 27 '19 06:06 fejta-bot