Dep: Glide importer does not infer constraint

Created on 18 Nov 2017  路  9Comments  路  Source: golang/dep

When a Glide.yaml doesn't provide the key version, dep will not attempt to infer a constraint from the revision (ie: looking for a tag).

The below example:

# glide.yaml
...
import:
- package: github.com/blang/vfs

# glide.lock
...
imports:
- name: github.com/blang/vfs
  version: c14afcac17253ce7418da751ec6b1988790cdc8f
  subpackages:
  - memfs

Yields:

[[constraint]]
  name = "github.com/blang/vfs"

Most helpful comment

@arbourd @darkowlzz defaultConstraintFromLock was set to false for glide because it is one of the external tools that actually supported setting a constraint in its config file. The others didn't have a way to specify a constraint (or lack of one) so we infer one. But with glide, they could set a constraint in the glide config if they desired, so we decided to be conservative and not put back in a constraint that they may have intentionally decided to not set.

All 9 comments

@arbourd thanks for opening an issue. Such empty constraints would soon be removed from Gopkg.toml and would only be added in Gopkg.lock. See #1373 .

Regarding inferring constraint from revision, IIRC we do it here. We use the revision to check if there's any version associated with the revision. Check lookupVersionForLockedProject() to see how we do it.

In your case, github.com/blang/vfs hasn't released any tag, due to which lookupVersionForLockedProject() ends up using a revision.

Hope this helps.

oh! just noticed that it's you who is working on #1373 馃槄

Thanks @darkowlzz. I should have made the ticket a bit more clearer, but I really appreciate your walkthrough. 馃榿

This is specific to Glide directly because of setting defaultConstraintFromLock to false in internal/importers/glide/importer.go#L186.

Why is defaultConstraintFromLock set false for Glide (the logic continues at internal/importers/base/importer.go#L226)?

oh! right I see it now. But I'm not sure why was defaultConstraintFromLock set to false for Glide only. 馃

@arbourd @darkowlzz defaultConstraintFromLock was set to false for glide because it is one of the external tools that actually supported setting a constraint in its config file. The others didn't have a way to specify a constraint (or lack of one) so we infer one. But with glide, they could set a constraint in the glide config if they desired, so we decided to be conservative and not put back in a constraint that they may have intentionally decided to not set.

@arbourd Do you have an example of how the current behavior is causing problems?

@carolynvs no problems -- just a future concern. With the proposed changes in #1373, I was thinking that the glide importer might ignore these empty constraints.

I assumed that:

# glide.yaml
import:
- package: github.com/arbourd/exampleA
- package: github.com/arbourd/exampleB
  version: master

Would become:

# Gopkg.toml
[[constraint]]
  branch = "master"
  name = "github.com/arbourd/exampleB"

This would not necessarily be the case because of import statement analysis.

So in the example above, even though ExampleA is no longer in the manifest, it will still be included in the lock (due to import analysis). The lock contains all the dependencies that a project is using, either because they imported the package, it is a dependency of a dependency (transitive) or because it was listed as required in manifest.

So the end result will be the same, exampleA is populated in vendor/, but the manifest stays as simple as possible so that only the really important stuff (something that required manual configuration) is in the manifest. Then the giant wall of text can stay hidden in the lock. 馃榾

I'm simplifying a bit but hopefully that helps provide some background on how some decisions are being made with respect to this feature and others. If it's not clear, please keep asking questions and we'll get it figured out.

It's clear :) Thanks so much to both of you for walking me through the process.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

sdboyer picture sdboyer  路  26Comments

davecheney picture davecheney  路  32Comments

mikkeloscar picture mikkeloscar  路  45Comments

wallrj picture wallrj  路  27Comments

avelino picture avelino  路  25Comments