Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add missing functionality to build_tar #100

Open
ixdy opened this issue Jan 31, 2019 · 8 comments
Open

add missing functionality to build_tar #100

ixdy opened this issue Jan 31, 2019 · 8 comments
Labels
lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@ixdy
Copy link
Member

ixdy commented Jan 31, 2019

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" (bazelbuild/bazel@25d202f)
  • deb support - tracked in support debs in tools/build_tar #61

cc @mikedanese

@ixdy
Copy link
Member Author

ixdy commented Jan 31, 2019

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.

@mikedanese
Copy link
Member

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

@ixdy
Copy link
Member Author

ixdy commented Feb 1, 2019

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.

@fejta
Copy link
Contributor

fejta commented Mar 29, 2019

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

@ixdy
Copy link
Member Author

ixdy commented Mar 29, 2019

@fejta the built-in python one is slooooow

@fejta
Copy link
Contributor

fejta commented Mar 29, 2019

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

@ixdy
Copy link
Member Author

ixdy commented Mar 29, 2019

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 bazelbuild/bazel@72ab932, 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.

@fejta-bot
Copy link

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

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 27, 2019
@mikedanese mikedanese added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
None yet
Development

No branches or pull requests

5 participants