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

Add support for -unstable gopkg.in imports #587

Merged
merged 3 commits into from May 22, 2017

Conversation

Projects
None yet
3 participants
@sectioneight
Copy link
Contributor

sectioneight commented May 16, 2017

Fixes #425

dep-test - master! ❯ dep ensure -v gopkg.in/mgo.v2-unstable
dep: No constraint or alternate source specified for "gopkg.in/mgo.v2-unstable", omitting from manifest
Root project is "scratch/dep-test"
 1 transitively valid internal packages
 2 external packages imported from 2 projects
(0)   ✓ select (root)
(1)     ? attempt github.com/pkg/errors with 1 pkgs; at least 1 versions to try
(1)         try github.com/pkg/errors@v0.8.0
(1)     ✓ select github.com/pkg/errors@v0.8.0 w/1 pkgs
(2)     ? attempt gopkg.in/mgo.v2-unstable with 1 pkgs; 1 versions to try
(2)         try gopkg.in/mgo.v2-unstable@v2-unstable
(2)     ✓ select gopkg.in/mgo.v2-unstable@v2-unstable w/5 pkgs
  ✓ found solution with 6 packages from 2 projects

Solver wall times by segment:
         b-list-pkgs: 637.471037ms
              b-gmal: 398.518303ms
     b-source-exists: 156.043538ms
             satisfy:    550.358µs
         select-atom:    358.518µs
            new-atom:    240.444µs
         select-root:     83.551µs
  b-deduce-proj-root:     15.934µs
               other:     14.055µs
     b-list-versions:     11.312µs

  TOTAL: 1.19330705s

dep-test - master! ❯ cat Gopkg.lock
memo = "842e27492a249b7020fca265003afa2ef25e260ebaae9e0242ccb02decac74ef"

[[projects]]
  name = "github.com/pkg/errors"
  packages = ["."]
  revision = "645ef00459ed84a119197bfb8d8205042c6df63d"
  version = "v0.8.0"

[[projects]]
  branch = "v2-unstable"
  name = "gopkg.in/mgo.v2-unstable"
  packages = [".","bson","internal/json","internal/sasl","internal/scram"]
  revision = "9a2573d4ae52a2bf9f5b7900a50e2f8bcceeb774"

If this is the right approach, I'll have @sdboyer push an -unstable version to the gopkt testbed instead of using mgo, which was just an example from the wild.

@googlebot googlebot added the cla: yes label May 16, 2017

@sectioneight

This comment has been minimized.

Copy link
Contributor

sectioneight commented May 16, 2017

I'm not really sure of the semantics of "unstable", so this could be totally off. It does, however, fetch the appropriate code and allow you to dep ensure it in a reliable way. Note that there's no version tag in the .lock file, which again, I'm not sure if we should have or not.

It does, however, seem in line with how we treat "normal" gopkg.in imports:

dep-test - master! ❯ dep ensure -v gopkg.in/mgo.v2
dep: No constraint or alternate source specified for "gopkg.in/mgo.v2", omitting from manifest
Root project is "scratch/dep-test"
 1 transitively valid internal packages
 3 external packages imported from 3 projects
(0)   ✓ select (root)
(1)     ? attempt github.com/pkg/errors with 1 pkgs; at least 1 versions to try
(1)         try github.com/pkg/errors@v0.8.0
(1)     ✓ select github.com/pkg/errors@v0.8.0 w/1 pkgs
(2)     ? attempt gopkg.in/mgo.v2 with 1 pkgs; 2 versions to try
(2)         try gopkg.in/mgo.v2@v2
(2)     ✓ select gopkg.in/mgo.v2@v2 w/5 pkgs
(3)     ? attempt gopkg.in/mgo.v2-unstable with 1 pkgs; at least 1 versions to try
(3)         try gopkg.in/mgo.v2-unstable@v2-unstable
(3)     ✓ select gopkg.in/mgo.v2-unstable@v2-unstable w/5 pkgs
  ✓ found solution with 11 packages from 3 projects

Solver wall times by segment:
     b-source-exists: 803.425035ms
         b-list-pkgs: 725.002541ms
              b-gmal: 633.621053ms
             satisfy:    793.528µs
         select-atom:    602.696µs
            new-atom:        186µs
         select-root:     112.06µs
  b-deduce-proj-root:     36.509µs
     b-list-versions:     18.849µs
               other:     15.436µs

  TOTAL: 2.163813707s
dep-test - master! ❯ cat Gopkg.lock
memo = "6429cb405afab146a78d18ef92f73c1b1a706126f7264abfd6bd26faab2bd792"

[[projects]]
  name = "github.com/pkg/errors"
  packages = ["."]
  revision = "645ef00459ed84a119197bfb8d8205042c6df63d"
  version = "v0.8.0"

[[projects]]
  branch = "v2"
  name = "gopkg.in/mgo.v2"
  packages = [".","bson","internal/json","internal/sasl","internal/scram"]
  revision = "3f83fa5005286a7fe593b055f0d7771a7dce4655"

[[projects]]
  branch = "v2-unstable"
  name = "gopkg.in/mgo.v2-unstable"
  packages = [".","bson","internal/json","internal/sasl","internal/scram"]
  revision = "9a2573d4ae52a2bf9f5b7900a50e2f8bcceeb774"

@sectioneight sectioneight force-pushed the sectioneight:handle-gopkg-unstable branch from 374bb33 to 518d0a3 May 16, 2017

@sectioneight sectioneight force-pushed the sectioneight:handle-gopkg-unstable branch from 518d0a3 to ee9379e May 16, 2017

@sectioneight sectioneight changed the title [WIP] Add support for -unstable gopkg.in imports Add support for -unstable gopkg.in imports May 16, 2017

@sectioneight

This comment has been minimized.

Copy link
Contributor

sectioneight commented May 18, 2017

Friendly ping for review

sv, err := semver.NewVersion(tv.name)
if err != nil || sv.Major() != s.major {
// not a semver-shaped branch name at all, or not the same major
// version as specified in the import path constraint
continue
}

// Gopkg.in has a special "-unstable" suffix which we need to handle
// separately.
if s.unstable && !strings.HasSuffix(tv.name, gopkgUnstableSuffix) {

This comment has been minimized.

@sdboyer

sdboyer May 18, 2017

Member

Looking at these docs, I think the expected behavior for gopkg.in is that the set of versions for stable vs. unstable are disjoint. That would suggest that we probably need to check for the -unstable suffix in both cases, and only proceed if it lines up with the value of s.unstable.

This comment has been minimized.

@sectioneight

sectioneight May 18, 2017

Contributor

Good catch! Let me know if the latest commit looks right to you

@sectioneight

This comment has been minimized.

Copy link
Contributor

sectioneight commented May 19, 2017

Ping for review

@sdboyer

This comment has been minimized.

Copy link
Member

sdboyer commented May 19, 2017

Yeah, I think this looks good. So, what do you need in the way of a fixture repo for testing?

@sectioneight

This comment has been minimized.

Copy link
Contributor

sectioneight commented May 20, 2017

I think you just need to push an -unstable branch to github.com/sdboyer/gpkt (the package used in the rest of the tests), maybe v1-unstable, put a dummy commit on it to make sure it's disjoint. Or we can leave the test relying on mongo, but that seems ungood.

@sdboyer

This comment has been minimized.

Copy link
Member

sdboyer commented May 21, 2017

I never push changes to those repos after initial creation in order to prevent other tests from breaking. However, I've duplicated github.com/sdboyer/gpkt at github.com/sdboyer/gpkt2, and pushed a new branch named v1-unstable on a new commit. Will that work?

@sectioneight

This comment has been minimized.

Copy link
Contributor

sectioneight commented May 21, 2017

Sounds good, just wanted to make sure it was something you controlled and not some random repo. Pushing updated tests now.

@sectioneight

This comment has been minimized.

Copy link
Contributor

sectioneight commented May 21, 2017

OK, I think we're good to go, assuming the generated Gopkg.lock above looks right to you.

@sdboyer

This comment has been minimized.

Copy link
Member

sdboyer commented May 22, 2017

Yep, I think we're good to go here.

We may eventually want a follow-up where we DO allow semver tags when unstable is set, but only e.g. if they have a non-empty prerelease property. I'd rather not do that until an actual human runs into that problem, though.

thanks! 🎉

@sdboyer sdboyer merged commit e0c7ec3 into golang:master May 22, 2017

4 checks passed

cla/google All necessary CLAs are signed
codeclimate no new or fixed issues
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@sectioneight

This comment has been minimized.

Copy link
Contributor

sectioneight commented May 22, 2017

woo, my first useful PR. are you saying that v1.2-unstable is a valid semver tag? i assumed it was not given that the comment you left was only in the "branch" block

@sectioneight sectioneight deleted the sectioneight:handle-gopkg-unstable branch May 22, 2017

@sdboyer

This comment has been minimized.

Copy link
Member

sdboyer commented May 22, 2017

woo, my first useful PR.

i think they've all been useful 😄 maybe this one's just more visible 🎉

are you saying that v1.2-unstable is a valid semver tag?

It's not, but only because it has just the two numerical components. v1.2.3-unstable, OTOH, is valid - the -unstable bit is classified as a pre-release component.

@sectioneight

This comment has been minimized.

Copy link
Contributor

sectioneight commented May 22, 2017

Ah, TIL. Created #619 so we don't forget about it.

@sdboyer

This comment has been minimized.

Copy link
Member

sdboyer commented May 22, 2017

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment