search

shurcooL / Go-Package-Store

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

Wrongly reports local revisions are ahead of remote revisions. #80

Closed mvdan closed 7 years ago

mvdan commented 7 years ago

I just got:

skipping "github.com/astaxie/bat" because:
        local revision "cba1c6bba4a613a5c14348479cd5e97820bdfdc5" is ahead of remote revision "30a7d43fe800eb26ad83af2aed5fac738b7a2574"
skipping "github.com/kardianos/govendor" because:
        local revision "c86c10d612bf08e847456ce91d495eb69ad87087" is ahead of remote revision "2aae20585716bf5ad74d5eb3bf5a660243a1ac0e"
skipping "github.com/pelletier/go-toml" because:
        local revision "fe7536c3dee2596cdd23ee9976a17c22bdaae286" is ahead of remote revision "4a000a21a414d139727f616a8bb97f847b1b310b"
skipping "github.com/spf13/cobra" because:
        local revision "99b5d838ca16c25cc4944e323684f8415e8b10ba" is ahead of remote revision "4d647c8944eb42504a714e57e97f244ed6344722"
skipping "golang.org/x/sys" because:
        local revision "fb4cac33e3196ff7f507ab9b2d2a44b0142f5b5a" is ahead of remote revision "c23410a886927bab8ca5e80b08af6a56faeb330d"
skipping "golang.org/x/net" because:
        local revision "057a25b06247e0c51ba15d8ae475feb2fcb72164" is ahead of remote revision "5f8847ae0d0e90b6a9dc8148e7ad616874625171"
skipping "gopkg.in/src-d/go-git.v4" because:
        local revision "7e249dfcf28765939bde8f38784b3274b522f880" is ahead of remote revision "ad02bf020460c210660db4fffda7f926b6aae95a"

Looked at bat, sure enough, cba... is older than 30a... as per https://github.com/astaxie/bat/commits/master.

What's wrong here? I went into the clone and was able to git pull into the new version. Haven't touched the rest yet.

mvdan commented 7 years ago

Also, if at all useful:

$ go version
go version devel +8ec7a39fec Sat Jun 24 00:54:01 2017 +0000 linux/amd64
$ git --version
git version 2.13.2
dmitshur commented 7 years ago

Thanks for reporting. I highly suspect this is a bug in a recent change I did to detect when branches have diverged in f0f8894a76cd8c20b23d054f167e82226907d138.

To confirm, which version of Go Package Store do you have installed?

mvdan commented 7 years ago

The latest, f0f8894a76cd8c20b23d054f167e82226907d138 in particular.

dmitshur commented 7 years ago

I also highly suspect the issue is that I'm not running git remote update (or equivalent) before I call repo.VCS.RemoteContains.

Can you try to manually run git remote update in one of the repos where you're getting this message incorrectly, and see if that fixes it?

mvdan commented 7 years ago

I just tried it with golang.org/x/sys, still get the same (different SHAs because it was just pushed to, but same error).

dmitshur commented 7 years ago

Thanks. I'll try to reproduce it and work on a fix.

dmitshur commented 7 years ago

Not really able to reproduce this as easily as I thought.

Can you cd $GOPATH/src/golang.org/x/sys and tell me the output of running this command there:

git branch --remotes --contains fb4cac33e3196ff7f507ab9b2d2a44b0142f5b5a origin/HEAD
mvdan commented 7 years ago

Assuming my $GOPATH is a single dir ;)

$ git branch --remotes --contains fb4cac33e3196ff7f507ab9b2d2a44b0142f5b5a origin/HEAD
  origin/HEAD -> origin/master
dmitshur commented 7 years ago

Thanks a lot, that's very helpful and I can see where the issue is right away (hint, it's here).

It looks like a behavior present in your newer git version. I'll look for the best solution, and push a fix.

mvdan commented 7 years ago

A better solution here would probably be to use a porcelain command, i.e. one of those whose output is to be consumed by other programs and whose format won't change between git versions. I don't know what the equivalent here would be, though.

dmitshur commented 7 years ago

I know, I always try to use those (low-level/plumbing) types of commands when possible, but I didn't find one so far, so I had to make do. If/when I find a viable replacement, I'll gladly switch to it.

dmitshur commented 7 years ago

I think I found a way to use git for-each-ref command, which is low-level plumbing command. 🎉

Can you give https://github.com/shurcooL/vcsstate/commit/8573312ec71605de7e92273d5512af7d1c32c3f0 a shot and see if it fixes the issue for you? I believe it should work on all versions of git 2.8+.

djadala commented 7 years ago

Hi, I try shurcooL/vcsstate@8573312, but it does not work, same result as before, debian jessie, git version 2.1.4, I switched to branch contains-for-each-ref, deleted vcsstate.a and Go-Package-Store, then rebuild Go-Package-Store, and results are same:

skipping "github.com/shurcooL/vcsstate" because:
    local branch "contains-for-each-ref" doesn't match remote branch "master"
skipping "golang.org/x/crypto" because:
    local revision "84f24dfdf3c414ed893ca1b318d0045ef5a1f607" is ahead of remote revision "5ef0053f77724838734b6945dd364d3847e5de1d"
skipping "golang.org/x/oauth2" because:
    local revision "f047394b6d14284165300fd82dad67edb3a4d7f6" is ahead of remote revision "5432cc9688e6250a0dd8f5a5f4c781d92b398be6"
skipping "golang.org/x/net" because:
    local revision "d4223d6710aad90f892de3ed4f1e7539ccc444a0" is ahead of remote revision "c81e7f25cb61200d8bf0ae971a0bac8cb638d5bc"
dmitshur commented 7 years ago

Interesting. @djadala, what's the output if you run these commands in your x/crypto repo?

git rev-parse refs/remotes/origin/HEAD
git for-each-ref --format=contains --count=1 --contains 84f24dfdf3c414ed893ca1b318d0045ef5a1f607 refs/remotes/origin/HEAD

git remote update

# Again after remote update.
git rev-parse refs/remotes/origin/HEAD
git for-each-ref --format=contains --count=1 --contains 84f24dfdf3c414ed893ca1b318d0045ef5a1f607 refs/remotes/origin/HEAD
djadala commented 7 years ago

Hi, may be debian(oldstable) git is too old ?

git for-each-ref --format=contains --count=1   --contains 84f24dfdf3c414ed893ca1b318d0045ef5a1f607 refs/remotes/origin/HEAD
error: unknown option `contains'
usage: git for-each-ref [options] [<pattern>]

    -s, --shell           quote placeholders suitably for shells
    -p, --perl            quote placeholders suitably for perl
    --python              quote placeholders suitably for python
    --tcl                 quote placeholders suitably for tcl

    --count <n>           show only <n> matched refs
    --format <format>     format to use for the output
    --sort <key>          field name to sort on
djadala commented 7 years ago 
git rev-parse refs/remotes/origin/HEAD
84f24dfdf3c414ed893ca1b318d0045ef5a1f607
dmitshur commented 7 years ago

Hi, may be debian(oldstable) git is too old ?

Oh, my bad, I misread your git version. That means git17 implementation will be used instead of git28. It should still be okay, but can you run this other command instead please:

git branch -r --contains 84f24dfdf3c414ed893ca1b318d0045ef5a1f607 origin/HEAD

What does that print?

mvdan commented 7 years ago

@shurcooL apologies for the late reply. Yes, that commit seems to fix it for me :)

djadala commented 7 years ago 
crypto$ git branch -r --contains 84f24dfdf3c414ed893ca1b318d0045ef5a1f607 origin/HEAD
  origin/HEAD -> origin/master

git log
commit 84f24dfdf3c414ed893ca1b318d0045ef5a1f607
Author: Yasuhiro Matsumoto <mattn.jp@gmail.com>
Date:   Fri Jun 23 19:03:27 2017 +0900

    ssh: signal incorrect private key passwords with x509.IncorrectPasswordError

    Fixes golang/go#20781

    Change-Id: Iae42fff3c9b0b9984509e44a92f9bc99a1a12470
    Reviewed-on: https://go-review.googlesource.com/46439
    Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
    Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
    TryBot-Result: Gobot Gobot <gobot@golang.org>

commit adbae1b6b6fb4b02448a0fc0dbbc9ba2b95b294d
Author: Dave Cheney <dave@cheney.net>
Date:   Mon Jun 19 16:03:41 2017 +1000

    all: gofmt ./...

    Change-Id: I8ffee4dc712091e424b83a9f5a3cc2a6724abefc
    Reviewed-on: https://go-review.googlesource.com/46050
    Reviewed-by: Matt Layher <mdlayher@gmail.com>
djadala commented 7 years ago

just for safety:

ls -lh `which Go-Package-Store`
-rwxr-xr-x 1 jambo jambo 13M Jun 29 08:19 /home/jambo/golang/bin/Go-Package-Store

ls -lh ~/golang/pkg/linux_amd64/github.com/shurcooL/vcsstate.a
-rw-r--r-- 1 jambo jambo 155K Jun 29 08:19 ~(edited)/golang/pkg/linux_amd64/github.com/shurcooL/vcsstate.a

I have 3 places in GOPATH

dmitshur commented 7 years ago

Thanks @djadala, that should be sufficient information for me to fix it for you. I'll let you know if it turns out I need more.

dmitshur commented 7 years ago

@djadala, I was able to setup an equivalent environment and reproduce your issue:

ubuntu@ubuntu:~/Dmitri/crypto$ git version
git version 2.1.4
ubuntu@ubuntu:~/Dmitri/crypto$ git branch -r --contains 84f24dfdf3c414ed893ca1b318d0045ef5a1f607 origin/HEAD
  origin/HEAD -> origin/master
ubuntu@ubuntu:~/Dmitri/crypto$

I've looked around and tried different things, and there was only one viable fix that I could find.

I've pushed a new commit shurcooL/vcsstate@54e5467a36c1fcce74eaee022aa897254e23774d to the same branch contains-for-each-ref. It should fix your issue. Do you want to give it a try before I merge it to master?

djadala commented 7 years ago

if you want for me to confirm - ok, i try tomorrow, but if you are sure how to fix this, go just now. if issue is that debian git is to old, im are ok to upgrade - FYI debian have git 2.11 in backports-oldstable.

dmitshur commented 7 years ago

You git version is old but it should be fine. vcsstate supports git 1.7 and newer.

I'm pretty confident in the fix, so I'll push it now. If you still have any issues after updating to the latest version, please don't hesitate to open a new issue!