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: vgo does not fix the timestamp in a pseudo-version #24369

Closed
enocom opened this issue Mar 13, 2018 · 9 comments
Closed

x/vgo: vgo does not fix the timestamp in a pseudo-version #24369

enocom opened this issue Mar 13, 2018 · 9 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@enocom
Copy link

enocom commented Mar 13, 2018

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

go version go1.10 darwin/amd64 vgo:2018-02-20.1

Does this issue reproduce with the latest release?

Yes.

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/eno/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/eno/go"
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/bt/375v_zy93bx4qz00lrgw7mhh0000gn/T/go-build831165372=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Here is a go.mod which pulls in gorilla/mux:

module "app"

require "github.com/gorilla/mux" v0.0.0-00000000000000-07ba1fd60e210dc5cc6f86ec25dc9ecc7c95c295

What did you expect to see?

I would expect vgo build to fail to find the specified version of gorilla/mux since the timestamp does not match that of the commit.

In Defining Go Modules, it says:

Specifically, vgo understands the special pseudo-version v0.0.0-yyyymmddhhmmss-commit as referring to the given commit identifier, which is typically a shortened Git hash and which must have a commit time matching the (UTC) timestamp.

What did you see instead?

vgo build works without a problem and produces a functioning binary.

@gopherbot gopherbot added this to the vgo milestone Mar 13, 2018
@ALTree ALTree changed the title x/vgo: vgo does not validate the timestamp in a psuedo-version x/vgo: vgo does not validate the timestamp in a pseudo-version Mar 13, 2018
@NobbZ
Copy link

NobbZ commented Mar 27, 2018

Thank you!

Using that row of zeros makes it a lot easier to specify the date in advance.

I'm not even sure in which timezone I had to specify the string after I had extracted it from a given commit info.

So in my opinion the actual check should only be introduced after we have an easy and documented way to extract the pseudoversion from a given package/module without having to do the git-dance manually.

@rsc
Copy link
Contributor

rsc commented Mar 27, 2018

Run "go build" and then "cat go.mod". What does it say? It should be updated to have the right timestamp.

For that matter, even if you'd written:

require "github.com/gorilla/mux" 07ba1fd60

it's supposed to get updated to turn into the right pseudo-version. Can you let me know if that's what you're seeing?

@NobbZ
Copy link

NobbZ commented Mar 27, 2018

In my case it was github.com/go-sql-driver/mysql having problems. vgo has determined v1.3 as latest version and used v1.3.0 in the module, which produced errors due tag not found on github…

Then I searched for ways to force another git-ref.

So I tried require "github.com/go-sql-driver/mysql" 1a676ac6e4dc now and it worked. after a vgo build … it has been replaced with a proper pseudo-version.

But this behaviour is not documented (or at least not easily discoverable), therefore my confusion.

Of course I am aware of the fact, that vgo is in a very early state and documentation is in an early state.

But as we are talking about it, is it possible to use arbitrary refs (branch, tag, commit) in a human readable way?

How does this play with other VCS like SVN?

@enocom
Copy link
Author

enocom commented Mar 27, 2018

Here are the results of running vgo build and cat go.mod. To be clear, this is for go version go1.10 darwin/amd64 vgo:2018-02-20.1.

Before running the commands, here are go.mod and main.go:

module "app"

require "github.com/gorilla/mux" v0.0.0-00000000000000-07ba1fd60e210dc5cc6f86ec25dc9ecc7c95c295

and

package main

import (
        "net/http"

        "github.com/gorilla/mux"
)

func main() {
        r := mux.NewRouter()

        http.ListenAndServe(":8080", r)
}

Then, I run vgo build and here are the contents of go.mod:

module "app"

require "github.com/gorilla/mux" v0.0.0-00000000000000-07ba1fd60e210dc5cc6f86ec25dc9ecc7c95c295

Notably, if I instead write my go.mod like this:

module "app"

require "github.com/gorilla/mux" 07ba1fd60

then after running vgo build, the contents of go.mod are correct:

module "app"

require "github.com/gorilla/mux" v0.0.0-20180226051151-07ba1fd60e21

@rsc rsc changed the title x/vgo: vgo does not validate the timestamp in a pseudo-version x/vgo: vgo does not fix the timestamp in a pseudo-version Mar 30, 2018
@rsc
Copy link
Contributor

rsc commented Mar 30, 2018

Thanks @enocom. Vgo should be fixing the bad timestamp the same way it fixes the commit.

@rsc rsc added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 30, 2018
@enocom
Copy link
Author

enocom commented Apr 5, 2018

@rsc Are you interested in contributions for this issue? I imagine vgo is still under heavy development and I don't want to interfere with the work.

@rsc
Copy link
Contributor

rsc commented Apr 5, 2018

Feel free to send a CL.

@gopherbot
Copy link

Change https://golang.org/cl/106799 mentions this issue: x/vgo: fix the timestamp in a pseudo-version

@gopherbot
Copy link

Change https://golang.org/cl/107655 mentions this issue: cmd/go/internal/vgo: speed up go.mod parsing

gopherbot pushed a commit to golang/vgo that referenced this issue Apr 25, 2018
CL 106799 changed the mod file parser to be call
the fixer function for every module, version pair,
not just the ones with non-canonical semantic versions.
Of course, we don't want to hit the network for every line
of every go.mod, when most of them are fine.
Skip the network for the ones that are syntactically OK,
including proper matching between the module path and
major version.

For golang/go#24369.

Change-Id: Id101d8f3c10bccde8a755ae734dbacf4d0a36f8d
Reviewed-on: https://go-review.googlesource.com/107655
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@golang golang locked and limited conversation to collaborators Apr 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants