tools / godep

dependency tool for go
http://godoc.org/github.com/tools/godep
BSD 3-Clause "New" or "Revised" License
5.55k stars 455 forks source link

Honor build.Context.BuildTags and build.Context.ReleaseTags (and possibly file suffixes) #553

Closed jlubawy closed 6 years ago

jlubawy commented 6 years ago

Expected behavior

1) Build Godep with go1.6 (or earlier than go1.9). 2) Build a project that imports package that has a // +build go1.9 release tag. 3) Expect Godep to skip parsing the file, ignore parser errors, or at least print a warning.

Actual behavior

Godep fails with a parser.ParseFile error when trying to parse the "type alias" feature added in go1.9. I think this behavior was introduced in #529 which was mistaken about how build tags work. See this article for how they're really used.

Steps to reproduce behavior

Build Godep with 1.6 and import the package golang.org/x/net/context as a dependency. Run godep save ./... and it will fail with the error godep: $GOPATH\golang.org\x\net\context\go19.go:15:14: expected type, found '=' (and 1 more errors)

godep version output

godep v79 (windows/amd64/go1.6.4)

go version output

go version go1.6.4 windows/amd64

Contents of Godeps.json file

N/A

jlubawy commented 6 years ago

I'll create a test case for this and take a stab at fixing it myself, but wanted to write down what I had in mind so far.

First I think we should address BuildTags since it's an easier problem, I think a solution would look something like this:

  1. Create a custom build.Context that contains user-specified fields to ignore/include files (context can be copied from build.Default). The user in #529 could have then specified mytag via a command-line flag to include files that have that specific build tag, and this could be saved into Godeps.json so it's also included in future runs.
  2. Run ImportDir using this Context and it will skip any files that don't match the build tags (as it used to). It should also skip // +build ignore flags.

It's a little more complicated for the ReleaseTags field since Godeps can be built with a version of Go other than what the project is using. I think there are two cases you would need to handle:

  1. If (GoVersion > Godep version) then log a warning since Godep might not be able to parse project dependencies, the user should then update Godep to be able to parse files that are from a newer version, or lower the version specified in Godeps.json
  2. Else if (GoVersion <= Godep version) then include all files that Godep can parse (using Default.ImportDir)
jlubawy commented 6 years ago

I created a short-term fix for my problem here if anyone needs it. I added a command-line flag to ignore parser errors (errors that won't come up at build due to release tags being used).

I'd like to help on a long-term solution but it seems like it will be very easy to break existing projects that rely on build tags being ignored, so before putting in too much work I'd like to hear other people's thoughts and if they're interested. Also not clear how long Godep will be needed if official language support for vendoring is added soon.

Also worth mentioning that issue #443 is also requesting that build tag exclusions work.

120318 commented 6 years ago

I encountered the same problem and you use your modified version, but this does not output an error message is not a good program and look forward to officially modify.(google translate :) )

freeformz commented 6 years ago

I am closing this issue because the repo is going to be archived.