search

cloudfoundry / go-buildpack

Cloud Foundry buildpack for the Go Language
http://docs.cloudfoundry.org/buildpacks/
Apache License 2.0
82 stars 119 forks source link

go-buildpack incorrectly reports 'GB package manger' error just because of `src/` directory existing #57

Closed dnwe closed 5 years ago

dnwe commented 6 years ago

What version of Cloud Foundry and CF CLI are you using? (i.e. What is the output of running cf curl /v2/info && cf version?

v2.92.0 && v6.32

What version of the buildpack you are using?

Latest (1.8.17 at time of writing)

If you were attempting to accomplish a task, what was it you were attempting to do?

Deploy a Go-based application using a glide.yaml, but a src/ directory at the top-level

What did you expect to happen?

Successful deployment.

What was the actual behavior?

Similar to https://github.com/cloudfoundry/go-buildpack/issues/51

!!    
!!    Error: Cloud Foundry does not support the GB package manager
!!    We currently only support the Godep and Glide package managers for go apps
!!    For support please file an issue: https://github.com/cloudfoundry/go-buildpack/issues
!!

A quick glance at the SelectVendorTool() func in supply/supply.go#L78-L143 code shows an unusual ordering of the logic whereby it will error out as soon as it encounters a src/ directory (which it assumes to mean someone is attempting to use the old GB package manager) despite the existence of a glide.yaml, because it doesn't check that until later.

The fix for this behaviour would be to move the code around in SelectVendorTool such that glide.yaml/Godeps.json/Gopkg.toml are checked for up-front and used if available, and any other error reporting is performed afterwards.

Please confirm where necessary:

cf-gitbot commented 6 years ago

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/154881427

The labels on this github issue will be updated when the story is started.

sclevine commented 6 years ago

@dgodd We should not fail staging immediately if src is detected. I'm prioritizing this as a bug.

chhhavi commented 5 years ago

@dnwe Sorry for the delay in action. We were wondering if you are still having this issue? And if you are, could you give us a sample app?

kardolus commented 5 years ago

@dnwe We were able to reproduce this issue using one of our fixtures. When you push the app from the with glide root directory using cf p -b go_buildpack you get the error you describe. The tricky part here is that GB apps are detected based on the existence of a src dir. We were able to resolve your issue by pushing with the following command instead cf p -b go_buildpack -p src/with_glide/.

Let us know if this works for you, in the meantime we will work on a more descriptive error message.

dnwe commented 5 years ago

@kardolus I'd raised on behalf of a customer, but I can try it out. However, I'm not sure why you'd improve the error message rather than just re-ordering the detection so that you check for the more likely package managers first like I suggested in the original post