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.

Sign up for GitHub

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

Jump to bottom

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

Closed
dmitshur opened this issue Jan 25, 2019 · 3 comments
Closed

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

dmitshur opened this issue Jan 25, 2019 · 3 comments
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@dmitshur
Copy link
Contributor

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 help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Jan 25, 2019
@dmitshur dmitshur added this to the Unreleased milestone Jan 25, 2019
@dmitshur dmitshur added the Builders x/build issues (builders, bots, dashboards) label Jan 25, 2019
saitarunreddy added a commit to saitarunreddy/build that referenced this issue Jan 26, 2019
@saitarunreddy
build/cmd/coordinator/version.sh: added check incase of no upstream set
2a466b1 
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
Copy link
Contributor Author

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
Copy link
Contributor

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.

@gopherbot
Copy link

Change https://golang.org/cl/164780 mentions this issue: cmd/coordinator: make version.sh check upstream configured

@golang golang locked and limited conversation to collaborators May 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@bradfitz @dmitshur @gopherbot