bradleyfalzon / apicompat

apicompat checks recent changes to a Go project for backwards incompatible changes
https://abicheck.bradleyf.id.au
MIT License
179 stars 5 forks source link

apicompat not picking up subpackages on Travis CI #38

Closed leighmcculloch closed 7 years ago

leighmcculloch commented 7 years ago

I'm attempting to use apicompat on TravisCI. For some reason apicompat is not able to pickup the subpackages within the project directory.

Example: https://travis-ci.org/lionelbarrow/braintree-go/jobs/248235954

bradleyfalzon commented 7 years ago

Thanks, a quick look and it looks OK, i'll need to reproduce and check further. Leave it with me for a while.

leighmcculloch commented 7 years ago

Thanks. I'm unable to reproduce this locally, and it only happens to occur on TravisCI, but this only happens with apicompat. I'm using other tools and the builtin go tools with this package structure without problems. Let me know if you want to try anything.

bradleyfalzon commented 7 years ago

What version of git are you running locally?

I can reproduce with:

docker run -it --rm  -v /home/bradleyf/go/src/github.com/bradleyfalzon/apicompat:/go/src/github.com/bradleyfalzon/apicompat golang:1.7 bash
mkdir -p src/github.com/
git clone --depth=50 https://github.com/lionelbarrow/braintree-go.git lionelbarrow/braintree-go
cd lionelbarrow/braintree-go
git fetch origin +refs/pull/174/merge:
git checkout -qf FETCH_HEAD
go install github.com/bradleyfalzon/apicompat/cmd/apicompat
apicompat -v

Git version: 2.1.4 and 2.9.3 Go: 1.8 or 1.7.4

I just don't understand:

Braintree-go package:

[bradleyf@srv4 braintree-go ((f111c66...))]$ apicompat -v
import path: "github.com/lionelbarrow/braintree-go" before: "HEAD~1" after: "HEAD" recursive: false
Parsing revision: HEAD~1 path: github.com/lionelbarrow/braintree-go recurse: false
building paths: [github.com/lionelbarrow/braintree-go]
go/types error: HEAD~1:customer.go:4:2: could not import github.com/lionelbarrow/braintree-go/customfields (can't find import: github.com/lionelbarrow/braintree-go/customfields)

GopherCI package:

[bradleyf@srv4 gopherci (error-handling)]$ apicompat -v
import path: "github.com/bradleyfalzon/gopherci" before: "HEAD~1" after: "HEAD" recursive: false
Parsing revision: HEAD~1 path: github.com/bradleyfalzon/gopherci recurse: false
building paths: [github.com/bradleyfalzon/gopherci]
Parsing revision: HEAD path: github.com/bradleyfalzon/gopherci recurse: false
building paths: [github.com/bradleyfalzon/gopherci]
Timing: parse: 10.83991ms, diff: 1.957µs, sort: 372ns, total: 10.842239ms
Changes detected: 0
bradleyfalzon commented 7 years ago

OK, I know what it is, but it's not expected. If you run go get . just before apicompat, it should work.

bradleyfalzon commented 7 years ago

apicompat needs a type checker, and this depends on having a buildable package with all its dependencies.

To check AST from other versions I've been providing functions to read files and directories using a VCS instead of file system - so it can read and check files from previous revisions without affecting your checked out workspace. See https://github.com/bradleyfalzon/apicompat/blob/5f916b1b6db7928fc26f43ca060831b27bf31abe/apicompat.go#L313-L318

That's good enough for most cases, but I've been using the default importer https://github.com/bradleyfalzon/apicompat/blob/5f916b1b6db7928fc26f43ca060831b27bf31abe/apicompat.go#L380 which reads files from the file system. I can't easily overwrite this, but I believe this will be a problem as old source could could be importing the new packages.

Eg, I can't do:

  394     conf := &types.Config{
  395         IgnoreFuncBodies:         true,
  396         DisableUnusedImportCheck: true,
~ 397         Importer:                 importer.For("gc", lookup),
+ 398         //Importer:                 importer.Default(),
  399     }
<snip>
+ 411 func lookup(path string) (io.ReadCloser, error) {
+ 412     log.Println("lookup path:", path)
+ 413     return os.Open(path) // swap for implementation the reads from VCS
+ 414 }

As this results in a panic panic: gc importer for custom import path lookup not yet implemented. Which is discussed https://github.com/golang/go/issues/13847.

It's not quite clear how to solve this, Go 1.9 brings a new srcimporter, https://github.com/golang/go/commit/459d061c99b8bcd0ab688e2536f5429c9f125a4b but I don't think I can use this.

I'll need to spend more time on this issue, and just don't have time right now. It should manifest itself when a backwards incompatible change in a library is introduced as the old code won't be able to be type checked. Until then I'll need to monitor https://github.com/golang/go/issues/13847 too.

bradleyfalzon commented 7 years ago

So, to confirm, can you run go get -t -u ./... before running apicompat in your project's director (else change path to github.com/lionelbarrow/braintree-go/...)

leighmcculloch commented 7 years ago

Ah, thanks for explaining. What I've done to work around it is run go install before running apicompat, that way all the packages I'm scanning are included in the pkg directory. e.g. lionelbarrow/braintree-go#174 I'm not sure it'll work in every case but it's working at the moment.

bradleyfalzon commented 7 years ago

Excellent, I'll close this for the moment and monitor the upstream bug, but that'll only resolve issues with dependencies in the same repository. If the old revision depends on a dependency no longer installed, I think we're going to have a bad time.

leighmcculloch commented 6 years ago

Maybe this worked for a while, but the problem came back unfortunately.

go install \
&& go get github.com/bradleyfalzon/apicompat/cmd/apicompat \
&& apicompat -before master ./...
go/types error: master:customer.go:4:2: could not import github.com/lionelbarrow/braintree-go/customfields (can't find import: "github.com/lionelbarrow/braintree-go/customfields")

https://travis-ci.org/lionelbarrow/braintree-go/jobs/406738840#L468-L473