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

x/build/cmd/coordinator: version.sh check for custom version fails when upstream isn't set #29929

Open
dmitshur opened this Issue Jan 25, 2019 · 2 comments

Comments

Projects
None yet
2 participants
@dmitshur
Copy link
Member

dmitshur commented Jan 25, 2019

The version.sh file attempts to append -$USER-$(date -u +%Y-%m-%dT%H:%M:%SZ) to the VERSION environment variable when the working directory is dirty, or when the current commit doesn't match the remote commit:

if [ -n "$dirty" ] || [ -n "$(git rev-list '@{upstream}..HEAD')" ]; then
  VERSION=$VERSION-$USER-$(date -u +%Y-%m-%dT%H:%M:%SZ)
fi

(Source: https://github.com/golang/build/blob/96be844d/cmd/coordinator/version.sh#L12-L14)

When the current branch doesn't have an upstream set (as can be the case if checking out a CL locally), git rev-list '@{upstream}..HEAD' exits with a non-zero exit code:

$ git rev-list '@{upstream}..HEAD'
fatal: no upstream configured for branch 'cl-155463'

As a result, the version reported at https://farmer.golang.org/ ends up being something like 93bae7b41a9a410dc6ca905034d3a49c59e10390 instead of 93bae7b41a9a410dc6ca905034d3a49c59e10390-dmitshur-2019-01-25T01:16:25Z.

Pretty harmless, but should be improved, if this can be done without adding significant complexity to the bash file. Help is welcome.

@dmitshur dmitshur added this to the Unreleased milestone Jan 25, 2019

@dmitshur dmitshur added the Builders label Jan 25, 2019

saitarunreddy added a commit to saitarunreddy/build that referenced this issue Jan 26, 2019

build/cmd/coordinator/version.sh: added check incase of no upstream set
Added " [-n $(git remote -v)] " to check whether any remote upstream exists. If exists it moves forward to next check else it appends USER and DATE to VERSION.
Fixes golang/go#29929
@dmitshur

This comment has been minimized.

Copy link
Member Author

dmitshur commented Jan 29, 2019

CL 159699 mentioned this issue (without the "golang/go" prefix, that's why it wasn't recognized by @gopherbot).

I don't think this issue is resolved yet. That CL added a check for git remote -v output being zero. git remote -v would be zero if the repository did not have any remotes. That's not what happens when you checkout a different branch. The branch doesn't have an upstream remote branch that it's tracking.

Consider the following reproduce steps:

$ cd /tmp
$ git clone https://go.googlesource.com/build
$ cd build
$ git fetch https://go.googlesource.com/build refs/changes/99/159699/6 && git checkout FETCH_HEAD
$ git checkout -b cl-159699
$ git remote -v
origin	https://go.googlesource.com/build (fetch)
origin	https://go.googlesource.com/build (push)
$ git rev-list '@{upstream}..HEAD'
fatal: no upstream configured for branch 'cl-159699'

Note that git remote -v still produces non-empty output, yet git rev-list '@{upstream}..HEAD' fails.

@bradfitz

This comment has been minimized.

Copy link
Member

bradfitz commented Jan 29, 2019

Ah! Now I understand this bug. I missed that before and assumed the CL author knew what they were doing more than me and approved the CL because it was at least harmless in the worst case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment