Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

dep init failed within another package and the error is unclear #270

Closed
elisarver opened this issue Feb 26, 2017 · 4 comments · May be fixed by sdboyer/gps#181
Closed

dep init failed within another package and the error is unclear #270

elisarver opened this issue Feb 26, 2017 · 4 comments · May be fixed by sdboyer/gps#181

Comments

@elisarver
Copy link

Included is the output from a dep init call on reach. It appears to be dying on a "self-dependency" of typewriter

package github.com/clipperhouse/typewriter depends on some other package within github.com/clipperhouse/typewriter with errors

I can build typewriter without error in its own directory. I can also run dep init in that directory and it does the "right thing" of generating a manifest and caching the vendor dependencies.

I don't know what the error is because the tool doesn't tell me, so I'm not sure how to resolve it.

Note: typewriter is a gen-dependency and its use is constrained to the _gen.go files, which aren't called during non-gen builds. I don't know if there's a plan to separate these concepts this in the future. I certainly don't need it to build.

(There's also a missing newline in the output between the 'solve error:' block and the 'No versions' block )

dep.txt

@sdboyer
Copy link
Member

sdboyer commented Mar 6, 2017

Sorry for the delayed response - busy week!

I haven't had time to reproduce this yet, but it's possible we could be falling into a hole here related to code generation. dep is built around the assumption that the dependencies you pull in need no post-processing (codegen) of any kind - that everything that might be needed is committed. This isn't necessarily the best assumption, but it's held so far.

I think we also may have changed our error reporting a bit since you experienced this issue - could you re-grab the latest version of dep and re-attempt, and report the results? I'll also try to replicate when I find some time.

@elisarver
Copy link
Author

The output isn't much different save for the fact the the command completes in half the time now.

dep2.txt

As far as design, the go tool by design ignores files starting with underscore, so I expected the same behavior here, for the sake of consistency. Might be worth considering, since underscore files are technically not required for builds. Maybe a flag to include them if you feel otherwise, or the opposite flag to exclude them. Though the thought of the latter would make dep feel incorrect relative to the go cmd.

@sdboyer
Copy link
Member

sdboyer commented Mar 6, 2017

The leading underscores thing is interesting, and isn't something we've accounted for in the parser thus far. Adding something to skip them would be trivial, but it could have complicated effects - if there's any circumstance in which you actually DO need those deps (e.g., to build a supporting tool for codegen), and at a reliable version, then they do need to be included in analysis. Computing deps has less in common with compiling than it might seem at first - as described in #291, compiling operates on a concrete input set, but we have to operate across potential source combinations.

In particular, CLI flags to control such behaviors is something we're trying hard to avoid. Having a flag to control this behavior doesn't change a compiled binary - it changes your lock file. Which means everyone on your team (and all of their supporting tooling) would have to know to pass the same flags, otherwise your computed results start changing for no apparent reason. (We could, of course, allow these options to be written into the manifest, which is more possible...but at that point, it's a question of trying to avoid adding complexity by giving configuration choices that don't add much value)

ALL that said...I'm not actually sure that reading underscore-led files has anything to do with this. The specific error being reported is that there's a problem with an internal package imported by typewriter...but that shouldn't be possible, because typewriter has no subpackages. That seems like the spot where I'll need to dive in.

@sdboyer
Copy link
Member

sdboyer commented Nov 15, 2017

i believe this is wrapped up now

@sdboyer sdboyer closed this as completed Nov 15, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants