glide icon indicating copy to clipboard operation
glide copied to clipboard

Glide fails to detect VCS even though explicitly specified in glide.yaml

Open albrow opened this issue 8 years ago • 23 comments

We have some private repositories on GitHub Enterprise. I have a glide.yaml which contains the following:

- package: github.plaid.com/plaid/go-plogger-client
  version: 6d7a49971a9273ca885599004f850b4a6486010c
  repo: [email protected]:plaid/go-plogger-client.git
  vcs: git
  subpackages:
  - plog

When I run glide install, it starts getting all the other packages and then exits with following error message:

[ERROR] Failed to set version on github.plaid.com/plaid/go-plogger-client/plog to : Cannot detect VCS

There is also a warning further up:

[WARN] github.plaid.com/plaid/go-plogger-client/plog appears to be a vendored package. Unable to update. Consider the '--update-vendored' flag.

Since I have specified the VCS and repository to use, I would expect Glide to work here. Perhaps I'm misunderstanding what these yaml keys do? I'm also not sure how Glide determined that this is a "vendored" package and what that means exactly.

I have tried a few different versions and as far as I can tell, the error message started happening in version 0.9.x. In 0.8.3 (and perhaps previous versions) it was a warning instead, which probably still indicates the same underlying problem.

I'm on OS X 10.11.2. I've also observed the same problems on our CI and build servers which run Ubuntu.

albrow avatar Mar 01 '16 19:03 albrow

I think I have an idea what's going on. Glide follows the same package name syntax that the go tool does. If a package can't also be fetched with go get it will have trouble in Glide.

GitHub, Bitbucket, Launchpad, and a few others have special cases built into the toolchains. Once you move outside those you need to use package names like github.plaid.com/plaid/go-plogger-client.git and github.plaid.com/plaid/go-plogger-client.git/example/pkg in the import statements and the glide.yaml file.

If you switch that does it work?

mattfarina avatar Mar 02 '16 14:03 mattfarina

@mattfarina thanks for the quick reply.

Adding .git does work, but I thought the point of the vcs and repo fields in glide.yaml was to, well.. specify the VCS and repo to use for the package? If that's not the point, then what do those fields actually do?

Also, in order to get glide install to work, I actually had to change my source code to use github.plaid.com/plaid/go-plogger-client.git in the import statement. Updating glide.yaml alone was not sufficient. Is this intended behavior? IMO glide.yaml should be the source of truth and Glide should not override what I specified in glide.yaml with guesses based on source code analysis.

albrow avatar Mar 03 '16 19:03 albrow

There are a few things going on here. This may help add context.

First, Go is opinionated on the import path. Putting .git in an import like github.plaid.com/plaid/go-plogger-client.git is a Go proper thing. You have to have things like that for go get to work. If you want to poke at the source you can look here or even at the specific external resolution expressions.

When I first encountered this I was annoyed. We try to hold to the syntax used by the Go tool because breaking from that appears to cause more questions and issues than using it.

Second, Glide has a goal to get the dependencies right for a system. We strive for a complete dependency environment needed for an application. Prior to scanning the code that was the number 1 request. Adding it met the requestors needs and allowed us to get the whole tree.

The glide.yaml file allows us to have version information and details on repos when that's required. Details that are additive to what can be worked out automatically.

Third, vcs and repo are only there when the name doesn't give enough detail or the right detail. For example, if there is a fork and you want to use the fork. There are cases where the repo location won't contain enough detail to detect it. Did you know some hosts, like SVN hosts (and Go supports SVN), don't have an extension on their repo locations? We need a way to capture it when it can't be detected.

In your case this should work:

- package: github.plaid.com/plaid/go-plogger-client.git
  version: 6d7a49971a9273ca885599004f850b4a6486010c
  subpackages:
  - plog

This will be fetched over https://.

Does that help?

mattfarina avatar Mar 04 '16 13:03 mattfarina

@mattfarina thank you for clarifying. I think I have a good understanding of how things currently work and why they work that way.

I still think that the way Glide currently behaves violates the principle of least astonishment and is likely to cause more confusion for users in the future. I'm okay with mimicking the behavior of go get for cases where vcs and repo are unspecified. However. If I manually specify a VCS and repo to use, Glide should use that. The current behavior (for this package, at least) is to spit out an error message complaining that Glide could not detect the VCS. Can you understand why that is confusing?

I would like to see Glide behave more like this (in pseudocode):

if vcs != nil && repo != nil {
    // Use the VCS and repo specified in glide.yaml
} else {
    // Attempt to detect VCS and repo using the same conventions as the go tool
    if err != nil {
        // Return an error indicating that the VCS could not be detected.
    }
}

Do you think the behavior I've outlined would somehow be more confusing than the current behavior? If so, I would love to understand your reasoning. If you think this sounds like too much work or should be lower priority than other issues, I would be more than happy to take a look and submit a PR.

albrow avatar Mar 07 '16 19:03 albrow

This was astonishing to me as well this past week when including a reference to a privately hosted go lib. Despite the opinionated approach of golang I've been using glide specifically to mitigate the issues of dependency management and this case (and output) were both frustrating and confusing.

It is reasonable to expect that when vcs and repo are specified that glide should be able to manage the configuration appropriately.

brendangibat avatar Mar 08 '16 15:03 brendangibat

+1 For private repositories user shouldn't be adding a .git in the import statement when he has already specified vcs: git. This is indeed confusing.

saswatp avatar Apr 29 '16 18:04 saswatp

This request makes sense to me without violating compatibility with existing go toolchain or being "surprising".

In my case glide is the ONLY go tool that can correctly vendor a complicated sub-packaged repo with dependencies on things like coreos/rkt which is itself a complicated subpackaged repo with many transitive deps vendored with Godep...

While the current behaviour does mirror the rest of the Go toolchain, nothing else until now has required me to have .git extensions everywhere. Not just in imports in the code but also ensure that all my colleagues remember to checkout local versions as <package>.git.

However without doing all that I end up with a total mess of Cannot detect VCS, glide vendoring the current package into its own vendor dir at current HEAD and same libs being vendored as both and <package.git> possibly at different versions...

Without thinking too hard it seems the request here would solve this without changing default behaviour or being "surprising":

  • Allow vcs in glide.yml to override the URL.
    • With this in place we can manually specify git as VCS in YAML for our own Github Enterprise hosted repos and not have to rename with .git extensions (there are other minor downsides to not doing that in the go toolchain but they have never been blockers to getting things working).

banks avatar Jun 28 '16 09:06 banks

One thing that's confusing me in this thread - GH enterprise does implement the custom go-import header go get itself uses to figure out vcs type, and has for at least a couple years. That should obviate this issue, or at least the issue @albrow is having. I'm not saying that's the best or even a sufficient solution, but reading over the comments, it seems like one basic step here is to establish whether or not glide is correctly capturing something like github.plaid.com/plaid/go-plogger-client correctly by checking that HTML meta.

Or, maybe I read the comments too quickly and missed something...

sdboyer avatar Jun 28 '16 14:06 sdboyer

@sdboyer good question.

I can confirm that in our case the repos in question do have correct go-import meta (Github Enterprise).

I can go get a repo that contains a single package just fine (e.g. github.example.com/foo/bar)

The problem is that Github Enterprise does NOT handle sub-packages in the same way. When go get tries to fetch github.example.com/foo/bar/pkg/baz?go-get=1 that is a 404 in Github enterprise as on Github.com since the URL scheme would place the browse URL as github.example.com/foo/bar/tree/master/pkg/baz.

The reason (I suspect) that subpackages work on github.com is that it's one of the magical exceptions where to go tooling knows about the url scheme and quirks.

For subpackages to work correctly with go-import meta, Github Enterprise would have to have a special case for it's routing which detects ?go-get=1 or something and uses alternateive routing at least to output a page with the correct meta tag.

banks avatar Jun 28 '16 16:06 banks

@banks ahh, yes, that explains it entirely. Very clear breakdown, and prompt - thanks!

IMO, a reasonable approach to addressing this is allowing the user to declare some additional patterns for matching paths. Not as a cli opt - that'd be nuts - but in the manifest, or the global config. It'd take some doing, but I think it ought to be feasible to make a generic handler that can operate on regexes declared in a yaml file. Perhaps with a little micro-API around specific named capture groups...

sdboyer avatar Jun 28 '16 19:06 sdboyer

@mattfarina this is still causing us a lot of frustration at my place of work, and appears to be affecting other people as well. I don't think you have addressed my most recent comment. Do you have any thoughts to share?

albrow avatar Jun 28 '16 23:06 albrow

I think we can do some magic to detect GitHub Enterprise.

  1. We know the layout for projects in it
  2. Responses return the X-GitHub-Request-Id header
  3. The top level works for ?go-get=1

mattfarina avatar Jun 30 '16 16:06 mattfarina

@mattfarina I'm not sure that adding some magic to detect GitHub Enterprise is the best solution here. What about people that are using GitLab Enterprise, Bitbucket Enterprise, or any number of other code hosting solutions? Are we going to create special cases for those as well?

In my opinion, it would be better if Glide did not have any special casing and instead used the VCS and repo field in glide.yaml as I originally suggested. As a developer, I would greatly prefer adding this information to glide.yaml compared to adding the ".git" suffix all over the place. Since Glide already has a way to specify this information in glide.yaml, it seems strange to me that it is not being used here.

If there is a way to detect X-GitHub-Request-Id that is not specifically tied to GitHub Enterprise and will work for other providers, then I have no opposition to it. However, I still wonder if that kind of solution is more complicated than it needs to be?

Package management is hard, and I know there's a lot on your plate. I would be happy to submit a PR if you're open to my idea.

albrow avatar Jul 01 '16 02:07 albrow

@albrow There are two things...

  1. We need to have a detailed discussion on breaking with the semantics of go get and its rules. The reason for some of the pain is supporting go get compatibility. I opened #498 to have this conversation.
  2. Because GitHub Enterprise uses ?go-get=1 in some places I think the idea would be to help that along a little more which would benefit others doing the same thing (e.g., GitLab on prem). Both Glide and Go have special rules for some domains and services so we're already prepared for them. I would expect this to be an extension of that.

I would be happy to see a PR. I'm OK with some service specific things in there since we (and Go) already do that. Special case logic is ok. This may also affect the Masterminds/vcs package as well.

mattfarina avatar Jul 01 '16 14:07 mattfarina

I left some thoughts on https://github.com/Masterminds/glide/issues/498. I have more to add on this specific issue...

We need to have a detailed discussion on breaking with the semantics of go get and its rules.

go get does not work with nested packages on GitHub Enterprise. Any solution that we come up with is going to break compatibility with go get. While I understand there are reasons to not want to break compatibility, if we do break it, the solution I originally proposed seems more straightforward than doing magic to detect GitHub Enterprise.

There are cases where the repo location won't contain enough detail to detect it. Did you know some hosts, like SVN hosts (and Go supports SVN), don't have an extension on their repo locations? We need a way to capture it when it can't be detected.

I'm not familiar with how the go tools work with custom hosting solutions for SVN. Can you elaborate? If there is not enough information in the package name and Glide supports this, haven't we already broken compatibility with go get?

albrow avatar Jul 01 '16 19:07 albrow

The hope is to merge gps in during the 0.13 release cycle. This issue has an impact there so I'm moving to 0.13 for discussion.

mattfarina avatar Aug 19 '16 13:08 mattfarina

Same issue here, added more info on associated ticket

ghost avatar Aug 23 '16 16:08 ghost

+1 I have the same issue if you can add support for private git repos that would be a huge advantage over go get

dcu avatar Sep 13 '16 19:09 dcu

We're using github enterprise too. Any workaround to solve the "Cannot detect VCS" issue when using sub-packages in glide.yaml? Thanks!

alanma avatar Jun 12 '17 18:06 alanma

@alanma just set repo: git@.... it works well

dcu avatar Jun 12 '17 18:06 dcu

@dcu thanks David!

alanma avatar Jun 13 '17 00:06 alanma

Resolved the issue. After going though the Glide code understood the root cause. Its not the glide issue. Glide must have a flag to ignore server certificates. Solution: Get Common CA certificates Ubuntu - https://launchpad.net/ubuntu/xenial/+package/ca-certificates Similarly search for other OS and install and try.

subbu05 avatar Jun 06 '18 18:06 subbu05

Also do run following: git config --global http.sslVerify "false"

add vcs: git for every package in glide.yaml

subbu05 avatar Jun 12 '18 21:06 subbu05