-
Notifications
You must be signed in to change notification settings - Fork 40.1k
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
Speedups for precommit hooks #11841
Speedups for precommit hooks #11841
Conversation
GCE e2e build/test failed for commit 1bc3b2e587cb94c538f8952812f4eb398ab0d52e. |
So this now even more assumes that your local build is up to date, it looks like? i.e. if you haven't done a |
find "${tmpdir}" -exec rsync -pt {} "${dest}" \; >/dev/null | ||
files=($(find "${tmpdir}" -type f)) | ||
if [[ "${#files[@]}" -ne 0 ]]; then | ||
cp -f "${files[@]}" "${dest}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this does the right thing. Consider:
thockin@freakshow:/tmp$ rm -rf a z
thockin@freakshow:/tmp$ mkdir -p a/b/c/d
thockin@freakshow:/tmp$ touch a/b/c/d/file-in-a-b-c-d
thockin@freakshow:/tmp$ touch a/b/c/file-in-a-b-c
thockin@freakshow:/tmp$ mkdir -p z/b/c/d
thockin@freakshow:/tmp$ find a -type f
a/b/c/d/file-in-a-b-c-d
a/b/c/file-in-a-b-c
thockin@freakshow:/tmp$ find z -type f
thockin@freakshow:/tmp$ f=($(find a -type f))
thockin@freakshow:/tmp$ cp -f "${f[@]}" z
thockin@freakshow:/tmp$ find z -type f
z/file-in-a-b-c
z/file-in-a-b-c-d
Eric, Are you still working on this. It's driving me batty a little more each day... |
i keep hoping to get back to it every afternoon. today looks ok. |
Using the hack/ version is probably a little slower, but it still only takes about .2 seconds. So probably worth the reduction in code.
6bce02d
to
1212aab
Compare
GCE e2e build/test passed for commit 1212aab78bdf50e4492fdd932e9e2e8c43a659ac. |
hopefully other comments have been addressed. PTAL. |
GCE e2e build/test passed for commit f97e17680e8a6fd3ef709e0f9d7bfb631ba9ec7e. |
GCE e2e build/test failed for commit 81ab88cbedab2c1519c528323bb70985c242a255. |
LGTM |
@k8s-bot test this |
GCE e2e build/test passed for commit 81ab88cbedab2c1519c528323bb70985c242a255. |
@eparis Do you want to squash commits before I merge? |
We were setting etcd values, but we started etcd before we set them. Thankfully we were just setting them to etcd defaults, so it all worked
It isn't used anywhere, why have it?
Hooks seems like they should be hooks. Not sure hack makes a lot more sense, but it has more stuff already.
Just to get everything in one place...
Right now some of the hack/* tools use `go run` and build almost every time. There are some which expect you to have already run `go install`. And in all cases the pre-commit hook, which runs a full build wouldn't want to do either, since it just built! This creates a new hack/after-build/ directory and has the scripts which REQUIRE that the binary already be built. It doesn't test and complain. It just fails miserably. Users should not be in this directory. Users should just use hack/verify-* which will just do the build and then call the "after-build" version. The pre-commit hook or anything which KNOWS the binaries have been built can use the fast version.
Instead of calling rsync over and over and over and over and over and over and over and over and over (and probably over) use one `cp` Before: real 0m5.247s user 0m2.294s sys 0m1.300s After: real 0m2.260s user 0m2.230s sys 0m0.936s
squashed as seemed appropriate... each commit should be its own thing now. |
GCE e2e build/test passed for commit efd71090799138feb3d949ea50e37f89e3fc6b4c. |
GCE e2e build/test passed for commit bf9c3cd. |
Thanks! |
This (along with the separate munger change) greatly reduces the pre-commit hook time. From like 30+ seconds down to 8.5 seconds to build everything, 2.5 seconds to get swagger (that stinks) and another 2 seconds to do everything else in the hook.
Notice this does make it easier for the gendeepcopy and genconversion to be wrong when run by hand. This is the exact same as gen* can be wrong when run by hand if you didn't build new copies. The git pre-commit hook builds everything, so it should always be right (as would travis et al once you open a PR)