shurcooL / Go-Package-Store

An app that displays updates for the Go packages in your GOPATH.
MIT License
900 stars 29 forks source link

Confusing "local revision is empty" skip reason in some situations. #74

Closed dmitshur closed 7 years ago

dmitshur commented 7 years ago

Originally reported by @mvdan in https://github.com/shurcooL/Go-Package-Store/issues/72#issuecomment-292488220; forking into a new issue because it's not related to #72:

Now it also throws local revision is empty sometimes:

skipping "gopkg.in/sourcemap.v1" because of remote error:
        no HEAD branch
skipping "gopkg.in/src-d/go-git.v4" because:
        local revision is empty
mvdan commented 7 years ago

Thanks for looking into this. If you need more info or want me to do a tarball of the git clone for you, let me know.

dmitshur commented 7 years ago

I was able to reproduce the gopkg.in/src-d/go-git.v4 issue. I investigated it and figured out what's going on.

This behavior is expected, and it happens because of #73. However, I might want to do a better job of printing the cause, and improve some internal documentation. I'll use this issue to track resolving that.

What happens, at a high level, is that it incorrectly determines the default branch to be master rather than v4, because it uses origin instead of upstream remote. Then, it tries to determine the local revision of master branch, but that branch doesn't exist, so an error is returned by call to vcs.LocalRevision(r.Path, r.Remote.Branch), and local revision is left empty.

A better error message would say that local branch "v4" doesn't match remote branch "master", which would make the skip reason more clear. Once #73 itself is resolved, the remote branch will be determined accurately as v4 instead of master, and the issue will be fully resolved.

mvdan commented 7 years ago

Great, thanks for the explanation :)

dmitshur commented 7 years ago

For (my) reference, what happens at a lower level was:

package main

import (
    "fmt"
    "log"

    "github.com/shurcooL/go-goon"
    "github.com/shurcooL/vcsstate"
    "golang.org/x/tools/go/vcs"
)

func main() {
    err := run()
    if err != nil {
        log.Fatalln(err)
    }
}

func run() error {
    const path = "/tmp/emptygopath/src/gopkg.in/src-d/go-git.v4"
    vcsCmd, _, err := vcs.FromDir(path, "/tmp/emptygopath/src")
    if err != nil {
        return err
    }
    vcs, err := vcsstate.NewVCS(vcsCmd)
    if err != nil {
        return err
    }
    remoteBranch, remoteRevision, err := vcs.RemoteBranchAndRevision(path)
    if err != nil {
        return err
    }
    goon.DumpExpr(remoteBranch, remoteRevision)
    rev, err := vcs.LocalRevision(path, remoteBranch)
    if err != nil {
        return fmt.Errorf("vcs.LocalRevision: %v", err)
    }
    goon.DumpExpr(rev)
    return nil
}

Output:

remoteBranch = (string)("master")
remoteRevision = (string)("4eef16a98d093057f1e4c560da4ed3bbba67cd76")
2017/04/11 14:13:09 vcs.LocalRevision: exit status 128
exit status 1
dmitshur commented 7 years ago

After the recent investigation and decision in #73, I think this error can be improved even more:

1. Previously (before #75)

$ Go-Package-Store 
Using all Go packages in GOPATH.
Go Package Store server is running at http://localhost:7043/updates.
skipping "gopkg.in/src-d/go-git.v4" because:
    local revision is empty

This was a really poor explanation for skipping. Completely unhelpful.

2. Currently (after #75)

$ Go-Package-Store 
Using all Go packages in GOPATH.
Go Package Store server is running at http://localhost:7043/updates.
skipping "gopkg.in/src-d/go-git.v4" because:
    local branch "v4" doesn't match remote branch "master"

Better... but it's not printing the most important thing that's wrong. Fixing the local branch (i.e., checking out master) would simply expose the mismatched remote URL problem anyway.

3. Proposed

$ Go-Package-Store 
Using all Go packages in GOPATH.
Go Package Store server is running at http://localhost:7043/updates.
skipping "gopkg.in/src-d/go-git.v4" because:
    remote URL doesn't match repo URL inferred from import path:
          (actual) https://github.com/mvdan/go-git
        (expected) https://gopkg.in/src-d/go-git.v4

This gives the highest level problem first. Fixing it can resolve all other problems without the unnecessary game of whack-a-mole.


Basically, it makes more sense to point out the highest level problem first. They are, in order:

  1. the entire remote is wrong (this will affect things like default branch)
  2. the locally checked out branch is wrong
  3. working tree is dirty

Otherwise, user can waste time fixing the locally checked out branch, when the problem can be that the remote is wrong (and locally checked out branch could actually be right).