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

Support importing from govend #998

Closed
carolynvs opened this issue Aug 12, 2017 · 5 comments · Fixed by #1040
Closed

Support importing from govend #998

carolynvs opened this issue Aug 12, 2017 · 5 comments · Fixed by #1040

Comments

@carolynvs
Copy link
Collaborator

Let's add support for importing from https://github.com/govend/govend. You can find instructions for how to add an importer here: #186 (comment).

Note: It looks like govend's config only lists revisions, so a good importer to use as an example would be the godepImporter.

@RaviTezu
Copy link
Contributor

@carolynvs Can I take this up?

@carolynvs
Copy link
Collaborator Author

@RaviTezu Of course! Let me know if you have any questions.

@RaviTezu
Copy link
Contributor

Hi @carolynvs

I have the following questions:

  1. Looks like govend supports multiple formats like json, xml and yaml with the help of -f option. Of which yaml, is default, Do we need to consider the other formats or just the yaml is fine?

  2. After reading about govend, I believe, it creates the vendor.yml only when the -l option is passed to the govend command. So, by default, each dependency is locked using a revision. We can just convert this configuration into the lock file? Please correct me, if I am wrong.

  3. I see, --hold option where govend will ignore the dependency, but let it live in the vendor folder, even though it is not imported. I am not sure, how to handle this while converting to depconfigurations.

Thanks.

@carolynvs
Copy link
Collaborator Author

Do we need to consider the other formats or just the yaml is fine?

dep should look for all the various formats, in some preferred order (ideally the same order that govend uses). It would be fine however to just add support for one of the formats first, yaml, and add support for the others in a separate PR.

We can just convert this configuration into the lock file

This is what I was referring to in my note in at the top of this issue. godep does something similar where it only locks to revisions, and doesn't list version or branch constraints. If you dig into

if pkg.Comment == "" {
// When there's no comment, try to get corresponding version for the Rev
// and fill Comment.
pi := gps.ProjectIdentifier{
ProjectRoot: gps.ProjectRoot(pkg.ImportPath),
}
revision := gps.Revision(pkg.Rev)
version, err := lookupVersionForLockedProject(pi, nil, revision, g.sm)
if err != nil {
// Only warn about the problem, it is not enough to warrant failing
g.logger.Println(err.Error())
} else {
pp := getProjectPropertiesFromVersion(version)
if pp.Constraint != nil {
pkg.Comment = pp.Constraint.String()
}
}
, you'll see that what we do is lock to the revisions from the original tool's config, but then we also try to guess at sensible constraints and add them to the dep manifest when possible.

--hold option where govend will ignore the dependency, but let it live in the vendor folder, even though it is not imported.

From the govend lock file, you can't tell that --hold was used to generate the lock... So there's no way for the importer to know about it. We will just have to import whatever we find in the govend lock, and then dep will later remove unused dependencies. I don't see any better way of handling this.

If someone really wants to vendor a unused dependency, they will have to realize that dep is removing it, and then manually add a [[required]] entry to their Gopkg.toml. Which is really outside the scope of the importer.

@RaviTezu
Copy link
Contributor

Thanks for the reply @carolynvs

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