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

x/vgo: use submodule's latest commit in pseudo versions #26196

Closed
myitcv opened this issue Jul 3, 2018 · 4 comments
Closed

x/vgo: use submodule's latest commit in pseudo versions #26196

myitcv opened this issue Jul 3, 2018 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@myitcv
Copy link
Member

myitcv commented Jul 3, 2018

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.10.3 linux/amd64

Does this issue reproduce with the latest release?

n/a - this is a git and other VCS-related issue.

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GORACE=""
GOROOT="/home/myitcv/dev/go"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/dev/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build592861713=/tmp/go-build -gno-record-gcc-switches"

What did you do?

At the time of writing, commit 5e591d073e90 is the HEAD of master for myitcv.io modules.

cd `mktemp -d`
export GOPATH=$PWD
mkdir hello
cd hello
vgo mod -init -module example.com/hello
vgo get myitcv.io/cmd/modpub

gives output:

vgo: finding myitcv.io/cmd/modpub latest
vgo: finding github.com/kr/fs v0.0.0-20131111012553-2788f0dbd169
vgo: downloading myitcv.io/cmd/modpub v0.0.0-20180702093358-5e591d073e90
vgo: downloading github.com/kr/fs v0.0.0-20131111012553-2788f0dbd169

We can see the pseudo version for myitcv.io/cmd/modpub, v0.0.0-20180702093358-5e591d073e90, corresponds to the HEAD of master.

But the latest commit within the cmd/modpub subdirectory (submodule) is actually 601ad1bf19:

cd `mktemp -d`
git clone https://github.com/myitcv/x
cd x
git checkout 5e591d073e90
git log -1 cmd/modpub/

gives:

commit 601ad1bf192257c6550c004a55b6f2c809f6f98b
Author: Paul Jolly <paul@myitcv.io>
Date:   Mon Jul 2 09:20:47 2018 +0100

    ci: use latest vgo and remove soon-to-be-updated packages (#30)

What did you expect to see?

It would be preferable to have the pseudo version for a submodule correspond to the commits relevant to that submodule. That way the pseudo version won't correspond to a commit that had nothing to do with that submodule (i.e. a commit that touched code elsewhere in the repo).

What did you see instead?

As above; the pseudo version is calculated from the repo-level commit log.

/cc @rsc @bcmills

@gopherbot gopherbot added this to the vgo milestone Jul 3, 2018
@bcmills
Copy link
Contributor

bcmills commented Jul 4, 2018

It seems tricky (and inefficient) to decide which commits affect the submodule: for example, should we count commits that touch README files, or only Go source files?

On top of that, it's not obvious to me that we would gain much by bumping the pseudo-version back by a few commits. Are there cases where this results in a significant semantic change, or is it mostly aesthetic?

@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Jul 4, 2018
@myitcv
Copy link
Member Author

myitcv commented Jul 5, 2018

It seems tricky (and inefficient) to decide which commits affect the submodule: for example, should we count commits that touch README files, or only Go source files?

I might be missing something here, but is it not as simple of running git log or equivalent with an argument of the subdirectory, per above? By definition therefore it applies to all git-controlled files beneath that subdirectory.

Are there cases where this results in a significant semantic change, or is it mostly aesthetic?

There is no semantic change, but I think it's a more useful/relevant reference for the package author and consumer. Because a quick glance at the commit will then show either what last changed in that submodule.

@rsc
Copy link
Contributor

rsc commented Jul 6, 2018

In general we never run git log to poke around and look for commits. We only use tagged commits, or if someone specifies a commit then we'll look it up directly. But we don't go searching for commits beyond tags. I do not want to redefine "latest" on a subdirectory to have to fetch a git log.

If you want more control over which commits are addressed, I think the answer is to tag them, not make this logic more sophisticated.

@myitcv
Copy link
Member Author

myitcv commented Jul 6, 2018

I do not want to redefine "latest" on a subdirectory to have to fetch a git log.

Ah, that's the bit I'm missing, thanks.

 If you want more control over which commits are addressed, I think the answer is to tag them, not make this logic more sophisticated

That makes sense, even for pre-releases. Sorry, feels like I missed this as an obvious solution to the problem.

@myitcv myitcv closed this as completed Jul 6, 2018
@golang golang locked and limited conversation to collaborators Jul 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

4 participants