-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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: fix ipfs version --commit #2734
Conversation
d36bc3f
to
15ece2c
Compare
Fixed the signoff |
is this related to #2453 at all? |
Oh hrm yes very much actually. And failing tests. Let me take a look (at both). |
15ece2c
to
547c8dc
Compare
I don't know docker that much but LGTM. |
547c8dc
to
f4f61a5
Compare
ok now this one is fixed too |
@@ -37,6 +37,7 @@ ENV SRC_PATH /go/src/github.com/ipfs/go-ipfs | |||
# Get the go-ipfs sourcecode | |||
COPY . $SRC_PATH | |||
|
|||
RUN apk add --update git |
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.
Oops, pushing one more update to remove this stray line
License: MIT Signed-off-by: Lars Gierth <larsg@systemli.org>
License: MIT Signed-off-by: Lars Gierth <larsg@systemli.org>
f4f61a5
to
7145d30
Compare
&& ref="$(cat .git/HEAD | cut -d' ' -f2)" \ | ||
&& commit="$(cat .git/$ref | head -c 7)" \ | ||
&& 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) \ |
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.
To get the current commit it's better to use git rev-parse --short HEAD
and to get the current branch git symbolic-ref --short HEAD 2>/dev/null || echo "$commit"
(where $commit is the current commit).
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.
Hey thanks for wighing in! -- do you happen to know if rev-parse
and symbolic-ref
work without an actual repo, i.e. with only .git/HEAD
and .git/refs
or something similar present? The idea with this hack is that we don't have to copy all of .git
, and thus gain savings in image size.
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.
You need the following for rev-parse
and symbolic-ref
to work:
.git/HEAD
.git/refs/
.git/objects/
which can be empty
See: https://github.com/git/git/blob/master/setup.c#L266
So it might be a good idea to just mkdir .git/objects/
so that you can use git commands afterwards.
It could be also a good idea to copy .git/packed-refs
if it exists in case the refs are packed.
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 think this is working right? did we want the improvements @chriscool proposed?
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.
Sorry, nevermind, already improved: 0eed330 👍
The dockerfile assumes we're always on a branch, in which case it resolves the head ref in
.git/HEAD
via.git/refs/
. Solarnet deploys are headless though, i.e. not on a branch but on a specific commit. In this case, Git puts that commit ref directly into.git/HEAD
, so that the resolution step breaks.