Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

only get dependencies of given subpackage not working #274

Closed
advdv opened this issue Feb 21, 2016 · 9 comments
Closed

only get dependencies of given subpackage not working #274

advdv opened this issue Feb 21, 2016 · 9 comments
Labels

Comments

@advdv
Copy link

advdv commented Feb 21, 2016

According to the release notes, since 0.9 only dependencies of specified subpackages should be retrieved recursively, to reproduce:

cd /tmp
mkdir glide-test
glide get github.com/hashicorp/terraform/dag

instead of downloading the dependencies of the dag package it will download all dependencies of terraform. Is the because of Godeps being used? Any way to overwrite the behaviour?

@technosophos
Copy link
Member

Yeah, I just reproduced this. My glide.lock shows this for terraform:

- name: github.com/hashicorp/terraform
  version: 938ab99d51195f04fbde54d7bcc9f2e53e0ffcfc
  subpackages:
  - dag
  - command
  - helper/logging
  - plugin
  - terraform
  - config
  - config/module
  - state
  - state/remote
  - rpc
  - dot
  - flatmap

But dag doesn't seem to import any other subpackages. So something is wrong there.

The Godeps file is only supposed to be used for pinning versions, but I wonder if there's a lingering bug there.

@technosophos
Copy link
Member

I think I found the problem:

⇒  glide --debug get github.com/hashicorp/terraform/dag
[DEBUG] Creating vendor
[INFO] Preparing to install 1 package.
[WARN] Package "github.com/hashicorp/terraform/dag" is already in glide.yaml. Skipping
[INFO] Downloading dependencies. Please wait...
[INFO] Fetching updates for github.com/hashicorp/terraform.
[DEBUG] Attempting to find current branch for https://github.com/hashicorp/terraform
[DEBUG] Saving default branch for https://github.com/hashicorp/terraform
[DEBUG] Error saving https://github.com/hashicorp/terraform to cache - Error: open cache/info/https-github.com-hashicorp-terraform.json: no such file or directory
[INFO] Resolving imports
[DEBUG] Adding local Import /Users/mbutcher/Code/Go/src/technosophos.com/x/scratch/vendor/github.com/Masterminds/cookoo to queue
[INFO] Found Godeps.json file.
[DEBUG] Trying to open /Users/mbutcher/Code/Go/src/technosophos.com/x/scratch/vendor/github.com/hashicorp/terraform
[DEBUG] Package github.com/hashicorp/terraform imports github.com/hashicorp/go-checkpoint
[DEBUG] Missing github.com/hashicorp/go-checkpoint. Trying to resolve.
[INFO] Fetching github.com/hashicorp/go-checkpoint into /Users/mbutcher/Code/Go/src/technosophos.com/x/scratch/vendor
[DEBUG] Attempting to find current branch for https://github.com/hashicorp/go-checkpoint
[DEBUG] Saving default branch for https://github.com/hashicorp/go-checkpoint
[DEBUG] Error saving https://github.com/hashicorp/go-checkpoint to cache - Error: open cache/info/https-github.com-hashicorp-go-checkpoint.json: no such file or directory
[INFO] Setting version for github.com/hashicorp/go-checkpoint to e4b2dc34c0f698ee04750bf2035d8b9384233e1b.
[DEBUG] Package github.com/hashicorp/terraform imports github.com/hashicorp/hcl

It looks like we automatically import the package at the base of the repo:

[DEBUG] Trying to open /Users/mbutcher/Code/Go/src/technosophos.com/x/scratch/vendor/github.com/hashicorp/terraform
[DEBUG] Package github.com/hashicorp/terraform imports github.com/hashicorp/go-checkpoint

In the case of terraform, the base package imports tons of stuff.

In pre-0.9, it makes sense for us to have done that. But now, it doesn't. The problem is that the current glide.yaml file doesn't indicate whether or not I need to import the base package. I'll have to chat with @mattfarina about how we want to handle this.

@dmitris
Copy link

dmitris commented Feb 22, 2016

the problem sounds similar to #166 (comment) and #270 ("overzealous" pulling of dependencies from the base packages) - maybe it would be possible to make sure that the same resolution mechanism is used for both for consistency (if it's not the case already)?

@technosophos
Copy link
Member

Yeah, it does sound like that issue.

What if we only pull the base package if the subpackage . (or ./) is in the list? Example:

package: technosophos.com/x/scratch
import:
- package: github.com/hashicorp/terraform
  subpackages:    # Do NOT pull the base package
  - dag
- package: github.com/Masterminds/sprig
  - .   # Pull the base package

@technosophos
Copy link
Member

^^ @dmitris actually, looking at your earlier pull request, adding support for . would be a tiny change, right? You already cover the case where a package is specified, but no subpackages are.

So if I'm thinking about this correctly, what we want is to support two distinct cases. I think they go like this:

I don't want the base package:

package: technosophos.com/x/scratch
import:
- package: github.com/hashicorp/terraform
  subpackages:
  - dag

I do want the base package:

package: technosophos.com/x/scratch
import:
- package: github.com/hashicorp/terraform
  subpackages:
  - .
  - dag

@dmitris
Copy link

dmitris commented Feb 22, 2016

I find the subpackage . both confusing and unnecessary. It also seems unclear what you mean as 'I do want the base package' - you want the code of the base package? but normally you already have it due to the source code grouping by the repos, at least in the typical github.com/user/repo case. Or do you mean 'I want all the dependencies of the base package` - the question is then why if they are not used...

I would like glide to behave more like go get - if my code and the code that my code needs (and so on, transitively) does require dependencies of the base package, then by all means pull them (without the user having to specify that in some special way), but pull only what is required. If the user wants to pull more than required, there is already that --all-dependencies option:

--all-dependencies      This will resolve all dependencies for all packages, not just those directly used.

Here's how I would like to be able to test glide - it should pull all the repositories required to build the project declaring glide.yaml, but only those ones. (I guess the test dependencies are special cases - maybe there should be a special option like -t for go get to indicate whether to bring in dependencies that are used only for testing; for now I think it's ok to pull them always to avoid breaking builds relying on them). I would expect the set of repositories pulled with go get <project> be the same as what you find under vendor/ after running glide init && glide update. Does it make sense?

@mattfarina
Copy link
Member

I think this fixes it.

mattfarina added a commit that referenced this issue Feb 23, 2016
Fixed #274: Was fetching dependencies of top level repo even when not imported
@nickveenhof
Copy link

This issue drastically improved the speed of glide for us. Huge huge thanks for this.

@advdv
Copy link
Author

advdv commented Feb 25, 2016

Yea, it awesome! Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants