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

docker: simplify git ref resolution, thanks @chriscool #2754

Merged
merged 1 commit into from May 31, 2016

Conversation

ghost
Copy link

@ghost ghost commented May 23, 2016

git rev-parse simply needed .git/packed-refs and an empty .git/objects/ directory.

@Kubuxu Kubuxu added the RFM label May 23, 2016
# This saves us quite a bit of image size.
&& ref=$(cat .git/HEAD | grep ref | cut -d' ' -f2) \
&& commit=$(if [ -z "$ref" ]; then cat .git/HEAD; else cat ".git/$ref"; fi | head -c 7) \
&& mkdir .git/objects && commit=$(git rev-parse HEAD | head -c 7) \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you use git rev-parse --short HEAD instead of git rev-parse HEAD | head -c 7?

There is the following about --short in the git rev-parse documentation:

--short
--short=number

    Instead of outputting the full SHA-1 values of object names try to abbreviate them to a shorter unique name. When no length is specified 7 is used. The minimum length is 4.

so it does the right thing by default. It prints a 7 character long name. (See: https://git-scm.com/docs/git-rev-parse/2.8.3)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point, thanks! Changed to --short.

@Kubuxu Kubuxu added need/author-input Needs input from the original author and removed RFM labels May 23, 2016
@ghost ghost force-pushed the fix-dockerfile-currentcommit branch from 772b189 to 4a21f31 Compare May 23, 2016 22:25
License: MIT
Signed-off-by: Lars Gierth <larsg@systemli.org>
@ghost ghost force-pushed the fix-dockerfile-currentcommit branch from 4a21f31 to 0eed330 Compare May 23, 2016 22:27
@chriscool
Copy link
Contributor

It looks great to me now!

@Kubuxu Kubuxu added RFM and removed need/author-input Needs input from the original author labels May 24, 2016
@whyrusleeping whyrusleeping merged commit 82c0a3b into master May 31, 2016
@whyrusleeping whyrusleeping deleted the fix-dockerfile-currentcommit branch May 31, 2016 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants