diff --git a/.github/ISSUE_TEMPLATE.md b/.github/ISSUE_TEMPLATE.md new file mode 100644 index 0000000000..7a3ef43a0f --- /dev/null +++ b/.github/ISSUE_TEMPLATE.md @@ -0,0 +1,23 @@ + + +### What version of Go (`go version`) and `dep` (`git describe --tags`) are you using? + +### What `dep` command did you run? + + + +### What did you expect to see? + +### What did you see instead? + diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 0000000000..b77e62c15c --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,19 @@ +<-- +Work-in-progress PRs are welcome as a way to get early feedback - just prefix +the title with [WIP]. +--> + +### What does this do / why do we need it? + +### What should your reviewer look out for in this PR? + +### Do you need help or clarification on anything? + +### Which issue(s) does this PR fix? + +<-- + +fixes # +fixes # + +--> diff --git a/.gitignore b/.gitignore index e4236754a4..a7c3f4116e 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,6 @@ /dep /testdep /dep.exe +/licenseok +/profile.out +/coverage.txt diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 09a4bacaa4..3717fca9db 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,6 +1,6 @@ -# Contributing to Dep +# Contributing to `dep` -Dep is an open source project. +`dep` is an open source project. It is the work of hundreds of contributors. We appreciate your help! @@ -12,9 +12,8 @@ Please check the existing issues and [FAQ](FAQ.md) to see if your feedback has a When [filing an issue](https://github.com/golang/dep/issues/new), make sure to answer these five questions: -1. What version of Go are you using (`go version`)? -2. What operating system and processor architecture are you using? -3. What did you do? +1. What version of Go (`go version`) and `dep` (`git describe --tags`) are you using?? +3. What `dep` command did you run? 4. What did you expect to see? 5. What did you see instead? @@ -32,7 +31,7 @@ label highlights issues that are well-suited for folks to jump in on. The [good-first-pr](https://github.com/golang/dep/issues?q=is%3Aissue+is%3Aopen+label%3Agood-first-pr) label further identifies issues that are particularly well-sized for newcomers. -Unless otherwise noted, the Dep source files are distributed under +Unless otherwise noted, the `dep` source files are distributed under the BSD-style license found in the LICENSE file. All submissions, including submissions by project members, require review. We @@ -52,3 +51,49 @@ your current agreements on file or to sign a new one. You generally only need to submit a CLA once, so if you've already submitted one (even if it was for a different project), you probably don't need to do it again. + +## Maintainer's Guide + +`dep` has subsystem maintainers; this guide is intended for them in performing their work as a maintainer. + +### General guidelines + +* _Be kind, respectful, and inclusive_. Really live [that CoC](https://github.com/golang/dep/blob/master/CODE_OF_CONDUCT.md). We've developed a reputation as one of the most welcoming and supportive project environments in the Go community, and we want to keep that up! +* The lines of responsibility between maintainership areas can be fuzzy. Get to know your fellow maintainers - it's important to work _with_ them when an issue falls in this grey area. +* Remember, the long-term goal of `dep` is to disappear into the `go` toolchain. That's going to be a challenging process, no matter what. Minimizing that eventual difficulty should be a guiding light for all your decisions today. + * Try to match the toolchain's assumptions as closely as possible ([example](https://github.com/golang/dep/issues/564#issuecomment-300994599)), and avoid introducing new rules the toolchain would later have to incorporate. + * Every new flag or option in the metadata files is more exposed surface area that demands conversion later. Only add these with a clear design plan. + * `dep` is experimental, but increasingly only on a larger scale. Experiments need clear hypotheses and parameters for testing - nothing off-the-cuff. +* Being a maintainer doesn't mean you're always right. Admitting when you've made a mistake keeps the code flowing, the environment health, and the respect level up. +* It's fine if you need to step back from maintainership responsibilities - just, please, don't fade away! Let other maintainers know what's going on. + +### Issue management + +* We use [Zenhub](https://www.zenhub.com) to manage the queue, in addition to what we do with labels. + * Pipelines, and [the board](https://github.com/golang/dep#boards) are one thing we try to utilize: + * **New Issues Pipeline**: When someone creates a new issue, it goes here first. Keep an eye out for issues that fall into your area. Add labels to them, and if it's something we should do, put it in the `Backlog` pipeline. If you aren't sure, throw it in the `Icebox`. It helps to sort this pipeline by date. + * **Icebox Pipeline**: Issues that we aren't immediately closing but aren't really ready to be prioritized and started on. It's not a wontfix bucket, but a "not sure if we should/can fix right now" bucket. + * **Backlog Pipeline**: Issues that we know we want to tackle. You can drag/drop up and down to prioritize issues. + * Marking dependencies/blockers is also quite useful where appropriate; please do that. + * We use epics and milestones in roughly the same way (because OSS projects don't have real sprints). Epics should be duplicated as milestones; if there's a main epic issue, it should contain a checklist of the relevant issues to complete it. +* The `area:` labels correspond to maintainership areas. Apply yours to any issues or PRs that fall under your purview. It's to be expected that multiple `area:` labels may be applied to a single issue. +* The [`help-wanted`](https://github.com/golang/dep/issues?q=is%3Aissue+is%3Aopen+label%3Ahelp-wanted) and [`good-first-pr`](https://github.com/golang/dep/issues?q=is%3Aissue+is%3Aopen+label%3Agood-first-pr) labels are two of our most important tools for making the project accessible to newcomers - a key goal for our community. Here's how to use them well. + * `good-first-pr` should be applied when there's a very straightforward, self-contained task that is very unlikely to have any hidden complexity. The real purpose of these is to provide a "chink in the armor", providing newcomers a lens through which to start understanding the project. + * `help-wanted` should be applied to issues where there's a clear, stated goal, there is at most one significant question that needs answering, and it looks like the implementation won't be inordinately difficult, or disruptive to other parts of the system. + * `help-wanted` should also be applied to all `good-first-pr` issues - it's duplicative, but not doing so seems unfriendly. + + +### Pull Requests + +* Try to make, and encourage, smaller pull requests. +* [No is temporary. Yes is forever.](https://blog.jessfraz.com/post/the-art-of-closing/) +* Long-running feature branches should generally be avoided. Discuss it with other maintainers first. +* Unless it's trivial, don't merge your own PRs - ask another maintainer. +* Commit messages should follow [Tim Pope's rules](http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html). +* Checklist for merging PRs: + * Does the PR pass [the code review comments](https://github.com/golang/go/wiki/CodeReviewComments)? (internalize these rules!) + * Are there tests to cover new or changed behavior? Prefer reliable tests > no tests > flaky tests. + * Does the first post in the PR contain "Fixes #..." text for any issues it resolves? + * Are any necessary follow-up issues _already_ posted, prior to merging? + * Does this change entail the updating of any docs? + * For docs kept in the repo, e.g. FAQ.md, docs changes _must_ be submitted as part of the same PR. diff --git a/FAQ.md b/FAQ.md index 71db50d4d8..19e72e2fea 100644 --- a/FAQ.md +++ b/FAQ.md @@ -6,26 +6,59 @@ _The first rule of FAQ is don't bikeshed the FAQ, leave that for Please contribute to the FAQ! Found an explanation in an issue or pull request helpful? Summarize the question and quote the reply, linking back to the original comment. +* [Does `dep` replace `go get`?](#does-dep-replace-go-get) +* [Why is it `dep ensure` instead of `dep install`?](#why-is-it-dep-ensure-instead-of-dep-install) + * [What is the difference between Gopkg.toml (the "manifest") and Gopkg.lock (the "lock")?](#what-is-the-difference-between-gopkgtoml-the-manifest-and-gopkglock-the-lock) * [When should I use `constraint`, `override` `required`, or `ignored` in the Gopkg.toml?](#when-should-i-use-constraint-override-required-or-ignored-in-gopkgtoml) * [What is a direct or transitive dependency?](#what-is-a-direct-or-transitive-dependency) -* [Should I commit my vendor directory?](#should-i-commit-my-vendor-directory) -* [Why is it `dep ensure` instead of `dep install`?](#why-is-it-dep-ensure-instead-of-dep-install) -* [Does `dep` replace `go get`?](#does-dep-replace-go-get) +* [How does `dep` decide what version of a dependency to use?](#how-does-dep-decide-what-version-of-a-dependency-to-use) * [Why is `dep` ignoring a version constraint in the manifest?](#why-is-dep-ignoring-a-version-constraint-in-the-manifest) * [How do I constrain a transitive dependency's version?](#how-do-i-constrain-a-transitive-dependencys-version) +* [Why did `dep` use a different revision for package X instead of the revision in the lock file?](#why-did-dep-use-a-different-revision-for-package-x-instead-of-the-revision-in-the-lock-file) + +* [Should I commit my vendor directory?](#should-i-commit-my-vendor-directory) * [`dep` deleted my files in the vendor directory!](#dep-deleted-my-files-in-the-vendor-directory) * [Can I put the manifest and lock in the vendor directory?](#can-i-put-the-manifest-and-lock-in-the-vendor-directory) -* [Why did dep use a different revision for package X instead of the revision in the lock file?](#why-did-dep-use-a-different-revision-for-package-x-instead-of-the-revision-in-the-lock-file) +* [How can I test changes to a dependency?](#how-can-i-test-changes-to-a-dependency) + * [Why is `dep` slow?](#why-is-dep-slow) * [How does `dep` handle symbolic links?](#how-does-dep-handle-symbolic-links) + * [How do I roll releases that `dep` will be able to use?](#how-do-i-roll-releases-that-dep-will-be-able-to-use) -* [How does `dep` decide what version of a dependency to use?](#how-does-dep-decide-what-version-of-a-dependency-to-use) * [What semver version should I use?](#what-semver-version-should-i-use) * [Is it OK to make backwards-incompatible changes now?](#is-it-ok-to-make-backwards-incompatible-changes-now) * [My dependers don't use `dep` yet. What should I do?](#my-dependers-dont-use-dep-yet-what-should-i-do) -## What is the difference between Gopkg.toml (the "manifest") and Gopkg.lock (the "lock")? +## Does `dep` replace `go get`? + +No, `dep` is an experiment and is still in its infancy. Depending on how this +experiment goes, it may be considered for inclusion in the go project in some form +or another in the future but that is not guaranteed. + +Here are some suggestions for when you could use `dep` or `go get`: +> I would say that dep doesn't replace go get, but they both can do similar things. Here's how I use them: +> +> `go get`: I want to download the source code for a go project so that I can work on it myself, or to install a tool. This clones the repo under GOPATH for all to use. +> +> `dep ensure`: I have imported a new dependency in my code and want to download the dependency so I can start using it. My workflow is "add the import to the code, and then run dep ensure so that the manifest/lock/vendor are updated". This clones the repo under my project's vendor directory, and remembers the revision used so that everyone who works on my project is guaranteed to be using the same version of dependencies. +-[@carolynvs in #376](https://github.com/golang/dep/issues/376#issuecomment-293964655) + +> The long term vision is a sane, overall-consistent go tool. My general take is that `go get` +> is for people consuming Go code, and dep-family commands are for people developing it. +-[@sdboyer in #376](https://github.com/golang/dep/issues/376#issuecomment-294045873) + +## Why is it `dep ensure` instead of `dep install`? + +> Yeah, we went round and round on names. [A lot](https://gist.github.com/jessfraz/315db91b272441f510e81e449f675a8b). +> +> The idea of "ensure" is roughly, "ensure that all my local states - code tree, manifest, lock, and vendor - are in sync with each other." When arguments are passed, it becomes "ensure this argument is satisfied, along with synchronization between all my local states." +> +> We opted for this approach because we came to the conclusion that allowing the tool to perform partial work/exit in intermediate states ended up creating a tool that had more commands, had far more possible valid exit and input states, and was generally full of footguns. In this approach, the user has most of the same ultimate control, but exercises it differently (by modifying the code/manifest and re-running dep ensure). +-[@sdboyer in #371](https://github.com/golang/dep/issues/371#issuecomment-293246832) + + +## What is the difference between `Gopkg.toml` (the "manifest") and `Gopkg.lock` (the "lock")? > The manifest describes user intent, and the lock describes computed outputs. There's flexibility in manifests that isn't present in locks..., as the "branch": "master" constraint will match whatever revision master HAPPENS to be at right now, whereas the lock is nailed down to a specific revision. > @@ -35,60 +68,72 @@ Summarize the question and quote the reply, linking back to the original comment ## When should I use `constraint`, `override`, `required`, or `ignored` in `Gopkg.toml`? * Use `constraint` to constrain a [direct dependency](#what-is-a-direct-or-transitive-dependency) to a specific branch, version range, revision, or specify an alternate source such as a fork. -* Use `override` to constrain a [transitive dependency](#what-is-a-direct-or-transitive-dependency). See [How do I constrain a transitive dependency's version?](#how-do-i-constrain-a-transitive-dependencys-version) for more details on how overrides differ from dependencies. Overrides should be used cautiously, sparingly, and temporarily. +* Use `override` to constrain a [transitive dependency](#what-is-a-direct-or-transitive-dependency). See [How do I constrain a transitive dependency's version?](#how-do-i-constrain-a-transitive-dependencys-version) for more details on how overrides differ from constraints. Overrides should be used cautiously, sparingly, and temporarily. * Use `required` to explicitly add a dependency that is not imported directly or transitively, for example a development package used for code generation. * Use `ignored` to ignore a package and any of that package's unique dependencies. ## What is a direct or transitive dependency? * Direct dependencies are dependencies that are imported directly by your project: they appear in at least one import statement from your project. * Transitive dependencies are the dependencies of your dependencies. Necessary to compile but are not directly used by your code. +## How does `dep` decide what version of a dependency to use? -## Should I commit my vendor directory? - -It's up to you: - -**Pros** - -- it's the only way to get truly reproducible builds, as it guards against upstream renames and deletes -- you don't need an extra `dep ensure` step (to fetch dependencies) on fresh clones to build your repo - -**Cons** - -- your repo will be bigger, potentially a lot bigger -- PR diffs are more annoying - -## Why is it `dep ensure` instead of `dep install`? +The full algorithm is complex, but the most important thing to understand is +that `dep` tries versions in a [certain +order](https://godoc.org/github.com/golang/dep/internal/gps#SortForUpgrade), +checking to see a version is acceptable according to specified constraints. -> Yeah, we went round and round on names. [A lot](https://gist.github.com/jessfraz/315db91b272441f510e81e449f675a8b). -> -> The idea of "ensure" is roughly, "ensure that all my local states - code tree, manifest, lock, and vendor - are in sync with each other." When arguments are passed, it becomes "ensure this argument is satisfied, along with synchronization between all my local states." -> -> We opted for this approach because we came to the conclusion that allowing the tool to perform partial work/exit in intermediate states ended up creating a tool that had more commands, had far more possible valid exit and input states, and was generally full of footguns. In this approach, the user has most of the same ultimate control, but exercises it differently (by modifying the code/manifest and re-running dep ensure). --[@sdboyer in #371](https://github.com/golang/dep/issues/371#issuecomment-293246832) +- All semver versions come first, and sort mostly according to the semver 2.0 + spec, with one exception: + - Semver versions with a prerelease are sorted after *all* non-prerelease + semver. Within this subset they are sorted first by their numerical + component, then lexicographically by their prerelease version. +- The default branch(es) are next; the semantics of what "default branch" means + are specific to the underlying source type, but this is generally what you'd + get from a `go get`. +- All other branches come next, sorted lexicographically. +- All non-semver versions (tags) are next, sorted lexicographically. +- Revisions, if any, are last, sorted lexicographically. Revisions do not + typically appear in version lists, so the only invariant we maintain is + determinism - deeper semantics, like chronology or topology, do not matter. -## Does `dep` replace `go get`? +So, given a slice of the following versions: -No, `dep` is an experiment and is still in its infancy. Depending on how this -experiment goes, it may be considered for inclusion in the go project in some form -or another in the future but that is not guaranteed. +- Branch: `master` `devel` +- Semver tags: `v1.0.0` `v1.1.0` `v1.1.0-alpha1` +- Non-semver tags: `footag` +- Revision: `f6e74e8d` +Sorting for upgrade will result in the following slice. -Here are some suggestions for when you could use `dep` or `go get`: -> I would say that dep doesn't replace go get, but they both can do similar things. Here's how I use them: -> -> `go get`: I want to download the source code for a go project so that I can work on it myself, or to install a tool. This clones the repo under GOPATH for all to use. -> -> `dep ensure`: I have imported a new dependency in my code and want to download the dependency so I can start using it. My workflow is "add the import to the code, and then run dep ensure so that the manifest/lock/vendor are updated". This clones the repo under my project's vendor directory, and remembers the revision used so that everyone who works on my project is guaranteed to be using the same version of dependencies. --[@carolynvs in #376](https://github.com/golang/dep/issues/376#issuecomment-293964655) +`[v1.1.0 v1.0.0 v1.1.0-alpha1 footag devel master f6e74e8d]` -> The long term vision is a sane, overall-consistent go tool. My general take is that `go get` -> is for people consuming Go code, and dep-family commands are for people developing it. --[@sdboyer in #376](https://github.com/golang/dep/issues/376#issuecomment-294045873) +There are a number of factors that can eliminate a version from consideration, +the simplest of which is that it doesn't match a constraint. But if you're +trying to figure out why `dep` is doing what it does, understanding that its +basic action is to attempt versions in this order should help you to reason +about what's going on. ## Why is `dep` ignoring a version constraint in the manifest? -Only your project's directly imported dependencies are affected by a `dependencies` entry +Only your project's directly imported dependencies are affected by a `constraint` entry in the manifest. Transitive dependencies are unaffected. -Use an `overrides` entry for transitive dependencies. +Use an `override` entry for transitive dependencies. + +By default, when you specify a version without an operator, such as `~` or `=`, +`dep` automatically adds a caret operator, `^`. The caret operator pins the +left-most non-zero digit in the version. For example: +``` +^1.2.3 means 1.2.3 <= X < 2.0.0 +^0.2.3 means 0.2.3 <= X < 0.3.0 +^0.0.3 means 0.0.3 <= X < 0.0.4 +``` + +To pin a version of direct dependency in manifest, prefix the version with `=`. +For example: +``` +[[constraint]] + name = "github.com/pkg/errors" + version = "=0.8.0" +``` ## How do I constrain a transitive dependency's version? First, if you're wondering about this because you're trying to keep the version @@ -104,13 +149,13 @@ dependency, you have a couple of options: 2. Use an override. Overrides are a sledgehammer, and should only be used as a last resort. While -dependencies and overrides are declared in the same way in `Gopkg.toml`, they +constraints and overrides are declared in the same way in `Gopkg.toml`, they behave differently: -* Dependencies: +* Constraints: 1. Can be declared by any project's manifest, yours or a dependency 2. Apply only to direct dependencies of the project declaring the constraint - 3. Must not conflict with the `dependencies` declared in any other project's manifest + 3. Must not conflict with the `constraint` entries declared in any other project's manifest * Overrides: 1. Are only utilized from the current/your project's manifest 2. Apply globally, to direct and transitive dependencies @@ -118,20 +163,7 @@ behave differently: Overrides are also discussed with some visuals in [the gps docs](https://github.com/sdboyer/gps/wiki/gps-for-Implementors#overrides). -## `dep` deleted my files in the vendor directory! -If you just ran `dep init`, there should be a copy of your original vendor directory named `_vendor-TIMESTAMP` in your project root. The other commands do not make a backup before modifying the vendor directory. - -> dep assumes complete control of vendor/, and may indeed blow things away if it feels like it. --[@peterbourgon in #206](https://github.com/golang/dep/issues/206#issuecomment-277139419) - -## Can I put the manifest and lock in the vendor directory? -No. - -> Placing these files inside `vendor/` would concretely bind us to `vendor/` in the long term. -> We prefer to treat the `vendor/` as an implementation detail. --[@sdboyer on go package management list](https://groups.google.com/d/msg/go-package-management/et1qFUjrkP4/LQFCHP4WBQAJ) - -## Why did dep use a different revision for package X instead of the revision in the lock file? +## Why did `dep` use a different revision for package X instead of the revision in the lock file? Sometimes the revision specified in the lock file is no longer valid. There are a few ways this can occur: @@ -151,6 +183,43 @@ Unable to update checked out version: fatal: reference is not a tree: 4dfc6a8a7e > Under most circumstances, if those arguments don't change, then the lock remains fine and correct. You've hit one one of the few cases where that guarantee doesn't apply. The fact that you ran dep ensure and it DID a solve is a product of some arguments changing; that solving failed because this particular commit had become stale is a separate problem. -[@sdboyer in #405](https://github.com/golang/dep/issues/405#issuecomment-295998489) + +## Should I commit my vendor directory? + +It's up to you: + +**Pros** + +- it's the only way to get truly reproducible builds, as it guards against upstream renames and deletes +- you don't need an extra `dep ensure` step (to fetch dependencies) on fresh clones to build your repo + +**Cons** + +- your repo will be bigger, potentially a lot bigger +- PR diffs are more annoying + +## `dep` deleted my files in the vendor directory! +If you just ran `dep init`, there should be a copy of your original vendor directory named `_vendor-TIMESTAMP` in your project root. The other commands do not make a backup before modifying the vendor directory. + +> dep assumes complete control of vendor/, and may indeed blow things away if it feels like it. +-[@peterbourgon in #206](https://github.com/golang/dep/issues/206#issuecomment-277139419) + +## Can I put the manifest and lock in the vendor directory? +No. + +> Placing these files inside `vendor/` would concretely bind us to `vendor/` in the long term. +> We prefer to treat the `vendor/` as an implementation detail. +-[@sdboyer on go package management list](https://groups.google.com/d/msg/go-package-management/et1qFUjrkP4/LQFCHP4WBQAJ) + + +## How can I test changes to a dependency? + +> I would recommend against ever working in your vendor directory since dep will overwrite any changes. It’s too easy to lose work that way. +-[@carolynvs in #706](https://github.com/golang/dep/issues/706#issuecomment-305807261) + +If you have a fork, add a `[[constraint]]` entry for the project in `Gopkg.toml` and set `source` to the fork source. This will ensure that `dep` will fetch the project from the fork instead of the original source. +Otherwise, if you want to test changes locally, you can delete the package from `vendor/` and make changes directly in `GOPATH/src/*package*` so that your changes are picked up by the go tool chain. + ## Why is `dep` slow? There are two things that really slow `dep` down. One is unavoidable; for the other, we have a plan. @@ -209,43 +278,6 @@ names. We hope to develop documentation soon that describes this more precisely, but in the meantime, the [npm](https://docs.npmjs.com/misc/semver) docs match our patterns pretty well. -## How does `dep` decide what version of a dependency to use? - -The full algorithm is complex, but the most important thing to understand is -that `dep` tries versions in a [certain -order](https://godoc.org/github.com/golang/dep/internal/gps#SortForUpgrade), -checking to see a version is acceptable according to specified constraints. - -- All semver versions come first, and sort mostly according to the semver 2.0 - spec, with one exception: - - Semver versions with a prerelease are sorted after *all* non-prerelease - semver. Within this subset they are sorted first by their numerical - component, then lexicographically by their prerelease version. -- The default branch(es) are next; the semantics of what "default branch" means - are specific to the underlying source type, but this is generally what you'd - get from a `go get`. -- All other branches come next, sorted lexicographically. -- All non-semver versions (tags) are next, sorted lexicographically. -- Revisions, if any, are last, sorted lexicographically. Revisions do not - typically appear in version lists, so the only invariant we maintain is - determinism - deeper semantics, like chronology or topology, do not matter. - -So, given a slice of the following versions: - -- Branch: `master` `devel` -- Semver tags: `v1.0.0` `v1.1.0` `v1.1.0-alpha1` -- Non-semver tags: `footag` -- Revision: `f6e74e8d` -Sorting for upgrade will result in the following slice. - -`[v1.1.0 v1.0.0 v1.1.0-alpha1 footag devel master f6e74e8d]` - -There are a number of factors that can eliminate a version from consideration, -the simplest of which is that it doesn't match a constraint. But if you're -trying to figure out why `dep` is doing what it does, understanding that its -basic action is to attempt versions in this order should help you to reason -about what's going on. - ## What semver version should I use? This can be a nuanced question, and the community is going to have to work out diff --git a/MAINTAINERS.md b/MAINTAINERS.md new file mode 100644 index 0000000000..b5dc06c5bf --- /dev/null +++ b/MAINTAINERS.md @@ -0,0 +1,17 @@ + +General maintainers: + sam boyer (@sdboyer) + +* dep + * `init` command: Carolyn Van Slyck (@carolynvs) + * `ensure` command: Ibrahim AshShohail (@ibrasho) + * `status` command: Sunny (@darkowlzz) + * testing harness: (vacant) +* gps + * solver: (vacant) + * source manager: (vacant) + * root deduction: (vacant) + * source/vcs interaction: (vacant) + * caching: (vacant) + * pkgtree: (vacant) + * versions and constraints: (vacant) diff --git a/analyzer.go b/analyzer.go index a0a83a8036..2e4b6fa30a 100644 --- a/analyzer.go +++ b/analyzer.go @@ -12,6 +12,7 @@ import ( "github.com/golang/dep/internal/gps" ) +// Analyzer implements gps.ProjectAnalyzer. type Analyzer struct{} // HasDepMetadata determines if a dep manifest exists at the specified path. @@ -21,6 +22,8 @@ func (a Analyzer) HasDepMetadata(path string) bool { return err == nil && fileOK } +// DeriveManifestAndLock reads and returns the manifest at path/ManifestName or nil if one is not found. +// The Lock is always nil for now. func (a Analyzer) DeriveManifestAndLock(path string, n gps.ProjectRoot) (gps.Manifest, gps.Lock, error) { if !a.HasDepMetadata(path) { return nil, nil, nil @@ -42,6 +45,10 @@ func (a Analyzer) DeriveManifestAndLock(path string, n gps.ProjectRoot) (gps.Man return m, nil, nil } -func (a Analyzer) Info() (string, int) { - return "dep", 1 +// Info returns Analyzer's name and version info. +func (a Analyzer) Info() gps.ProjectAnalyzerInfo { + return gps.ProjectAnalyzerInfo{ + Name: "dep", + Version: 1, + } } diff --git a/analyzer_test.go b/analyzer_test.go index f67fffa0d8..4e453620a4 100644 --- a/analyzer_test.go +++ b/analyzer_test.go @@ -105,9 +105,9 @@ func TestAnalyzerDeriveManifestAndLockInvalidManifest(t *testing.T) { func TestAnalyzerInfo(t *testing.T) { a := Analyzer{} - name, vers := a.Info() + info := a.Info() - if name != "dep" || vers != 1 { - t.Fatalf("expected name to be 'dep' and version to be 1: name -> %q vers -> %d", name, vers) + if info.Name != "dep" || info.Version != 1 { + t.Fatalf("expected name to be 'dep' and version to be 1: name -> %q vers -> %d", info.Name, info.Version) } } diff --git a/cmd/dep/ensure.go b/cmd/dep/ensure.go index c03ffc241f..65c33e5ded 100644 --- a/cmd/dep/ensure.go +++ b/cmd/dep/ensure.go @@ -104,7 +104,7 @@ type ensureCommand struct { func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error { if cmd.examples { - ctx.Loggers.Err.Println(strings.TrimSpace(ensureExamples)) + ctx.Err.Println(strings.TrimSpace(ensureExamples)) return nil } @@ -121,8 +121,8 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error { defer sm.Release() params := p.MakeParams() - if ctx.Loggers.Verbose { - params.TraceLogger = ctx.Loggers.Err + if ctx.Verbose { + params.TraceLogger = ctx.Err } params.RootPackageTree, err = pkgtree.ListPackages(p.AbsRoot, string(p.ImportRoot)) if err != nil { @@ -136,7 +136,7 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error { if cmd.update { applyUpdateArgs(args, ¶ms) } else { - err := applyEnsureArgs(ctx.Loggers.Err, args, cmd.overrides, p, sm, ¶ms) + err := applyEnsureArgs(ctx.Err, args, cmd.overrides, p, sm, ¶ms) if err != nil { return err } @@ -182,7 +182,7 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error { return err } if cmd.dryRun { - return sw.PrintPreparedActions(ctx.Loggers.Out) + return sw.PrintPreparedActions(ctx.Out) } return errors.Wrap(sw.Write(p.AbsRoot, sm, false), "grouped write of manifest, lock and vendor") diff --git a/cmd/dep/glide_importer.go b/cmd/dep/glide_importer.go index f6c29c9dd9..c53ff391ea 100644 --- a/cmd/dep/glide_importer.go +++ b/cmd/dep/glide_importer.go @@ -208,6 +208,9 @@ func (g *glideImporter) buildProjectConstraint(pkg glidePackage) (pc gps.Project pc.Ident = gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(pkg.Name), Source: pkg.Repository} pc.Constraint, err = deduceConstraint(pkg.Reference, pc.Ident, g.sm) + f := fb.NewConstraintFeedback(pc, fb.DepTypeImported) + f.LogFeedback(g.logger) + return } @@ -226,6 +229,10 @@ func (g *glideImporter) buildLockedProject(pkg glideLockedPackage) gps.LockedPro version = revision } - feedback(version, pi.ProjectRoot, fb.DepTypeImported, g.logger) - return gps.NewLockedProject(pi, version, nil) + lp := gps.NewLockedProject(pi, version, nil) + + f := fb.NewLockedProjectFeedback(lp, fb.DepTypeImported) + f.LogFeedback(g.logger) + + return lp } diff --git a/cmd/dep/glide_importer_test.go b/cmd/dep/glide_importer_test.go index 03acf6b4f4..336840fb3a 100644 --- a/cmd/dep/glide_importer_test.go +++ b/cmd/dep/glide_importer_test.go @@ -21,16 +21,16 @@ import ( const testGlideProjectRoot = "github.com/golang/notexist" var ( - discardLogger = log.New(ioutil.Discard, "", 0) - discardLoggers = &dep.Loggers{Out: discardLogger, Err: discardLogger} + discardLogger = log.New(ioutil.Discard, "", 0) ) func newTestContext(h *test.Helper) *dep.Ctx { h.TempDir("src") pwd := h.Path(".") return &dep.Ctx{ - GOPATH: pwd, - Loggers: discardLoggers, + GOPATH: pwd, + Out: discardLogger, + Err: discardLogger, } } @@ -38,23 +38,21 @@ func TestGlideConfig_Import(t *testing.T) { h := test.NewHelper(t) defer h.Cleanup() - cacheDir := "gps-repocache" - h.TempDir(cacheDir) - h.TempDir("src") + ctx := newTestContext(h) + sm, err := ctx.SourceManager() + h.Must(err) + defer sm.Release() + h.TempDir(filepath.Join("src", testGlideProjectRoot)) h.TempCopy(filepath.Join(testGlideProjectRoot, glideYamlName), "glide/glide.yaml") h.TempCopy(filepath.Join(testGlideProjectRoot, glideLockName), "glide/glide.lock") - projectRoot := h.Path(testGlideProjectRoot) - sm, err := gps.NewSourceManager(h.Path(cacheDir)) - h.Must(err) - defer sm.Release() // Capture stderr so we can verify output verboseOutput := &bytes.Buffer{} - logger := log.New(verboseOutput, "", 0) + ctx.Err = log.New(verboseOutput, "", 0) - g := newGlideImporter(logger, false, sm) // Disable verbose so that we don't print values that change each test run + g := newGlideImporter(ctx.Err, false, sm) // Disable verbose so that we don't print values that change each test run if !g.HasDepMetadata(projectRoot) { t.Fatal("Expected the importer to detect the glide configuration files") } @@ -85,22 +83,19 @@ func TestGlideConfig_Import(t *testing.T) { } func TestGlideConfig_Import_MissingLockFile(t *testing.T) { - h := test.NewHelper(t) defer h.Cleanup() - cacheDir := "gps-repocache" - h.TempDir(cacheDir) - h.TempDir("src") + ctx := newTestContext(h) + sm, err := ctx.SourceManager() + h.Must(err) + defer sm.Release() + h.TempDir(filepath.Join("src", "glidetest")) h.TempCopy(filepath.Join("glidetest", glideYamlName), "glide/glide.yaml") - projectRoot := h.Path("glidetest") - sm, err := gps.NewSourceManager(h.Path(cacheDir)) - h.Must(err) - defer sm.Release() - g := newGlideImporter(discardLogger, true, sm) + g := newGlideImporter(ctx.Err, true, sm) if !g.HasDepMetadata(projectRoot) { t.Fatal("The glide importer should gracefully handle when only glide.yaml is present") } @@ -118,18 +113,17 @@ func TestGlideConfig_Import_MissingLockFile(t *testing.T) { } func TestGlideConfig_Convert_Project(t *testing.T) { - h := test.NewHelper(t) defer h.Cleanup() - pkg := "github.com/sdboyer/deptest" - repo := "https://github.com/sdboyer/deptest.git" - ctx := newTestContext(h) sm, err := ctx.SourceManager() h.Must(err) defer sm.Release() + pkg := "github.com/sdboyer/deptest" + repo := "https://github.com/sdboyer/deptest.git" + g := newGlideImporter(ctx.Err, true, sm) g.yaml = glideYaml{ Imports: []glidePackage{ @@ -195,7 +189,7 @@ func TestGlideConfig_Convert_Project(t *testing.T) { } wantRev := "ff2948a2ac8f538c4ecd55962e919d1e13e74baf" - gotRev := lpv.Underlying().String() + gotRev := lpv.Revision().String() if gotRev != wantRev { t.Fatalf("Expected locked revision to be %s, got %s", wantRev, gotRev) } @@ -211,13 +205,13 @@ func TestGlideConfig_Convert_TestProject(t *testing.T) { h := test.NewHelper(t) defer h.Cleanup() - pkg := "github.com/sdboyer/deptest" - ctx := newTestContext(h) sm, err := ctx.SourceManager() h.Must(err) defer sm.Release() + pkg := "github.com/sdboyer/deptest" + g := newGlideImporter(ctx.Err, true, sm) g.yaml = glideYaml{ TestImports: []glidePackage{ @@ -256,9 +250,17 @@ func TestGlideConfig_Convert_TestProject(t *testing.T) { } func TestGlideConfig_Convert_Ignore(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + + ctx := newTestContext(h) + sm, err := ctx.SourceManager() + h.Must(err) + defer sm.Release() + pkg := "github.com/sdboyer/deptest" - g := newGlideImporter(discardLogger, true, nil) + g := newGlideImporter(ctx.Err, true, sm) g.yaml = glideYaml{ Ignores: []string{pkg}, } @@ -278,7 +280,15 @@ func TestGlideConfig_Convert_Ignore(t *testing.T) { } func TestGlideConfig_Convert_ExcludeDir(t *testing.T) { - g := newGlideImporter(discardLogger, true, nil) + h := test.NewHelper(t) + defer h.Cleanup() + + ctx := newTestContext(h) + sm, err := ctx.SourceManager() + h.Must(err) + defer sm.Release() + + g := newGlideImporter(ctx.Err, true, sm) g.yaml = glideYaml{ ExcludeDirs: []string{"samples"}, } @@ -298,7 +308,15 @@ func TestGlideConfig_Convert_ExcludeDir(t *testing.T) { } func TestGlideConfig_Convert_ExcludeDir_IgnoresMismatchedPackageName(t *testing.T) { - g := newGlideImporter(discardLogger, true, nil) + h := test.NewHelper(t) + defer h.Cleanup() + + ctx := newTestContext(h) + sm, err := ctx.SourceManager() + h.Must(err) + defer sm.Release() + + g := newGlideImporter(ctx.Err, true, sm) g.yaml = glideYaml{ Name: "github.com/golang/mismatched-package-name", ExcludeDirs: []string{"samples"}, @@ -326,18 +344,27 @@ func TestGlideConfig_Convert_WarnsForUnusedFields(t *testing.T) { for wantWarning, pkg := range testCases { t.Run(wantWarning, func(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + pkg.Name = "github.com/sdboyer/deptest" pkg.Reference = "v1.0.0" + ctx := newTestContext(h) + sm, err := ctx.SourceManager() + h.Must(err) + defer sm.Release() + // Capture stderr so we can verify warnings verboseOutput := &bytes.Buffer{} - logger := log.New(verboseOutput, "", 0) - g := newGlideImporter(logger, true, nil) + ctx.Err = log.New(verboseOutput, "", 0) + + g := newGlideImporter(ctx.Err, true, sm) g.yaml = glideYaml{ Imports: []glidePackage{pkg}, } - _, _, err := g.convert(testGlideProjectRoot) + _, _, err = g.convert(testGlideProjectRoot) if err != nil { t.Fatal(err) } @@ -351,12 +378,20 @@ func TestGlideConfig_Convert_WarnsForUnusedFields(t *testing.T) { } func TestGlideConfig_Convert_BadInput_EmptyPackageName(t *testing.T) { - g := newGlideImporter(discardLogger, true, nil) + h := test.NewHelper(t) + defer h.Cleanup() + + ctx := newTestContext(h) + sm, err := ctx.SourceManager() + h.Must(err) + defer sm.Release() + + g := newGlideImporter(ctx.Err, true, sm) g.yaml = glideYaml{ Imports: []glidePackage{{Name: ""}}, } - _, _, err := g.convert(testGlideProjectRoot) + _, _, err = g.convert(testGlideProjectRoot) if err == nil { t.Fatal("Expected conversion to fail because the package name is empty") } diff --git a/cmd/dep/godep_importer.go b/cmd/dep/godep_importer.go new file mode 100644 index 0000000000..101582f7b6 --- /dev/null +++ b/cmd/dep/godep_importer.go @@ -0,0 +1,199 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "encoding/json" + "io/ioutil" + "log" + "os" + "path/filepath" + + "github.com/golang/dep" + fb "github.com/golang/dep/internal/feedback" + "github.com/golang/dep/internal/gps" + "github.com/pkg/errors" +) + +const godepJSONName = "Godeps.json" + +type godepImporter struct { + json godepJSON + + logger *log.Logger + verbose bool + sm gps.SourceManager +} + +func newGodepImporter(logger *log.Logger, verbose bool, sm gps.SourceManager) *godepImporter { + return &godepImporter{ + logger: logger, + verbose: verbose, + sm: sm, + } +} + +type godepJSON struct { + Imports []godepPackage `json:"Deps"` +} + +type godepPackage struct { + ImportPath string `json:"ImportPath"` + Rev string `json:"Rev"` + Comment string `json:"Comment"` +} + +func (g *godepImporter) Name() string { + return "godep" +} + +func (g *godepImporter) HasDepMetadata(dir string) bool { + y := filepath.Join(dir, "Godeps", godepJSONName) + if _, err := os.Stat(y); err != nil { + return false + } + + return true +} + +func (g *godepImporter) Import(dir string, pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) { + err := g.load(dir) + if err != nil { + return nil, nil, err + } + + return g.convert(pr) +} + +func (g *godepImporter) load(projectDir string) error { + g.logger.Println("Detected godep configuration files...") + j := filepath.Join(projectDir, "Godeps", godepJSONName) + if g.verbose { + g.logger.Printf(" Loading %s", j) + } + jb, err := ioutil.ReadFile(j) + if err != nil { + return errors.Wrapf(err, "Unable to read %s", j) + } + err = json.Unmarshal(jb, &g.json) + if err != nil { + return errors.Wrapf(err, "Unable to parse %s", j) + } + + return nil +} + +func (g *godepImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) { + g.logger.Println("Converting from Godeps.json ...") + + manifest := &dep.Manifest{ + Constraints: make(gps.ProjectConstraints), + } + lock := &dep.Lock{} + + for _, pkg := range g.json.Imports { + // ImportPath must not be empty + if pkg.ImportPath == "" { + err := errors.New("Invalid godep configuration, ImportPath is required") + return nil, nil, err + } + + // Obtain ProjectRoot. Required for avoiding sub-package imports. + ip, err := g.sm.DeduceProjectRoot(pkg.ImportPath) + if err != nil { + return nil, nil, err + } + pkg.ImportPath = string(ip) + + // Check if it already existing in locked projects + if projectExistsInLock(lock, pkg.ImportPath) { + continue + } + + // Rev must not be empty + if pkg.Rev == "" { + err := errors.New("Invalid godep configuration, Rev is required") + return nil, nil, err + } + + if pkg.Comment == "" { + // When there's no comment, try to get corresponding version for the Rev + // and fill Comment. + pi := gps.ProjectIdentifier{ + ProjectRoot: gps.ProjectRoot(pkg.ImportPath), + } + revision := gps.Revision(pkg.Rev) + version, err := lookupVersionForRevision(revision, pi, g.sm) + if err != nil { + warn := errors.Wrapf(err, "Unable to lookup the version represented by %s in %s. Falling back to locking the revision only.", pkg.Rev, pi.ProjectRoot) + g.logger.Printf(warn.Error()) + version = revision + } + + pp := getProjectPropertiesFromVersion(version) + if pp.Constraint != nil { + pkg.Comment = pp.Constraint.String() + } + } + + if pkg.Comment != "" { + // If there's a comment, use it to create project constraint + pc, err := g.buildProjectConstraint(pkg) + if err != nil { + return nil, nil, err + } + manifest.Constraints[pc.Ident.ProjectRoot] = gps.ProjectProperties{Constraint: pc.Constraint} + } + + lp := g.buildLockedProject(pkg) + lock.P = append(lock.P, lp) + } + + return manifest, lock, nil +} + +// buildProjectConstraint uses the provided package ImportPath and Comment to +// create a project constraint +func (g *godepImporter) buildProjectConstraint(pkg godepPackage) (pc gps.ProjectConstraint, err error) { + pc.Ident = gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(pkg.ImportPath)} + pc.Constraint, err = deduceConstraint(pkg.Comment, pc.Ident, g.sm) + + f := fb.NewConstraintFeedback(pc, fb.DepTypeImported) + f.LogFeedback(g.logger) + + return +} + +// buildLockedProject uses the package Rev and Comment to create lock project +func (g *godepImporter) buildLockedProject(pkg godepPackage) gps.LockedProject { + pi := gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(pkg.ImportPath)} + + var version gps.Version + + if pkg.Comment != "" { + ver := gps.NewVersion(pkg.Comment) + version = ver.Pair(gps.Revision(pkg.Rev)) + } else { + version = gps.Revision(pkg.Rev) + } + + lp := gps.NewLockedProject(pi, version, nil) + f := fb.NewLockedProjectFeedback(lp, fb.DepTypeImported) + f.LogFeedback(g.logger) + + return lp +} + +// projectExistsInLock checks if the given import path already existing in +// locked projects. +func projectExistsInLock(l *dep.Lock, ip string) bool { + for _, lp := range l.P { + if ip == string(lp.Ident().ProjectRoot) { + return true + } + } + + return false +} diff --git a/cmd/dep/godep_importer_test.go b/cmd/dep/godep_importer_test.go new file mode 100644 index 0000000000..5a2e1eb8d4 --- /dev/null +++ b/cmd/dep/godep_importer_test.go @@ -0,0 +1,356 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "bytes" + "log" + "path/filepath" + "testing" + + "github.com/golang/dep" + "github.com/golang/dep/internal/gps" + "github.com/golang/dep/internal/test" + "github.com/pkg/errors" +) + +const testGodepProjectRoot = "github.com/golang/notexist" + +func TestGodepConfig_Import(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + + cacheDir := "gps-repocache" + h.TempDir(cacheDir) + h.TempDir("src") + h.TempDir(filepath.Join("src", testGodepProjectRoot)) + h.TempCopy(filepath.Join(testGodepProjectRoot, "Godeps", godepJSONName), "godep/Godeps.json") + + projectRoot := h.Path(testGodepProjectRoot) + sm, err := gps.NewSourceManager(h.Path(cacheDir)) + h.Must(err) + defer sm.Release() + + // Capture stderr so we can verify output + verboseOutput := &bytes.Buffer{} + logger := log.New(verboseOutput, "", 0) + + g := newGodepImporter(logger, false, sm) // Disable verbose so that we don't print values that change each test run + if !g.HasDepMetadata(projectRoot) { + t.Fatal("Expected the importer to detect godep configuration file") + } + + m, l, err := g.Import(projectRoot, testGodepProjectRoot) + h.Must(err) + + if m == nil { + t.Fatal("Expected the manifest to be generated") + } + + if l == nil { + t.Fatal("Expected the lock to be generated") + } + + goldenFile := "godep/expected_import_output.txt" + got := verboseOutput.String() + want := h.GetTestFileString(goldenFile) + if want != got { + if *test.UpdateGolden { + if err := h.WriteTestFile(goldenFile, got); err != nil { + t.Fatalf("%+v", errors.Wrapf(err, "Unable to write updated golden file %s", goldenFile)) + } + } else { + t.Fatalf("expected %s, got %s", want, got) + } + } +} + +func TestGodepConfig_JsonLoad(t *testing.T) { + // This is same as cmd/dep/testdata/Godeps.json + wantJSON := godepJSON{ + Imports: []godepPackage{ + { + ImportPath: "github.com/sdboyer/deptest", + Rev: "3f4c3bea144e112a69bbe5d8d01c1b09a544253f", + }, + { + ImportPath: "github.com/sdboyer/deptestdos", + Rev: "5c607206be5decd28e6263ffffdcee067266015e", + Comment: "v2.0.0", + }, + }, + } + + h := test.NewHelper(t) + defer h.Cleanup() + + ctx := newTestContext(h) + + h.TempCopy(filepath.Join(testGodepProjectRoot, "Godeps", godepJSONName), "godep/Godeps.json") + + projectRoot := h.Path(testGodepProjectRoot) + + g := newGodepImporter(ctx.Err, true, nil) + err := g.load(projectRoot) + if err != nil { + t.Fatalf("Error while loading... %v", err) + } + + if !equalImports(g.json.Imports, wantJSON.Imports) { + t.Fatalf("Expected imports to be equal. \n\t(GOT): %v\n\t(WNT): %v", g.json.Imports, wantJSON.Imports) + } +} + +func TestGodepConfig_ConvertProject(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + + ctx := newTestContext(h) + sm, err := ctx.SourceManager() + h.Must(err) + defer sm.Release() + + g := newGodepImporter(discardLogger, true, sm) + g.json = godepJSON{ + Imports: []godepPackage{ + { + ImportPath: "github.com/sdboyer/deptest", + // This revision has 2 versions attached to it, v1.0.0 & v0.8.0. + Rev: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", + Comment: "v0.8.0", + }, + }, + } + + manifest, lock, err := g.convert("") + if err != nil { + t.Fatal(err) + } + + d, ok := manifest.Constraints["github.com/sdboyer/deptest"] + if !ok { + t.Fatal("Expected the manifest to have a dependency for 'github.com/sdboyer/deptest' but got none") + } + + v := d.Constraint.String() + if v != "^0.8.0" { + t.Fatalf("Expected manifest constraint to be ^0.8.0, got %s", v) + } + + p := lock.P[0] + if p.Ident().ProjectRoot != "github.com/sdboyer/deptest" { + t.Fatalf("Expected the lock to have a project for 'github.com/sdboyer/deptest' but got '%s'", p.Ident().ProjectRoot) + } + + lv := p.Version() + lpv, ok := lv.(gps.PairedVersion) + if !ok { + t.Fatalf("Expected locked version to be PairedVersion but got %T", lv) + } + + rev := lpv.Revision() + if rev != "ff2948a2ac8f538c4ecd55962e919d1e13e74baf" { + t.Fatalf("Expected locked revision to be 'ff2948a2ac8f538c4ecd55962e919d1e13e74baf', got %s", rev) + } + + ver := lpv.String() + if ver != "v0.8.0" { + t.Fatalf("Expected locked version to be 'v0.8.0', got %s", ver) + } +} + +func TestGodepConfig_ConvertProject_EmptyComment(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + h.TempDir("src") + + ctx := newTestContext(h) + sm, err := ctx.SourceManager() + h.Must(err) + defer sm.Release() + + g := newGodepImporter(discardLogger, true, sm) + g.json = godepJSON{ + Imports: []godepPackage{ + { + ImportPath: "github.com/sdboyer/deptest", + // This revision has 2 versions attached to it, v1.0.0 & v0.8.0. + Rev: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", + }, + }, + } + + manifest, lock, err := g.convert("") + if err != nil { + t.Fatal(err) + } + + d, ok := manifest.Constraints["github.com/sdboyer/deptest"] + if !ok { + t.Fatal("Expected the manifest to have a dependency for 'github.com/sdboyer/deptest' but got none") + } + + v := d.Constraint.String() + if v != "^1.0.0" { + t.Fatalf("Expected manifest constraint to be ^1.0.0, got %s", v) + } + + p := lock.P[0] + if p.Ident().ProjectRoot != "github.com/sdboyer/deptest" { + t.Fatalf("Expected the lock to have a project for 'github.com/sdboyer/deptest' but got '%s'", p.Ident().ProjectRoot) + } + + lv := p.Version() + lpv, ok := lv.(gps.PairedVersion) + if !ok { + t.Fatalf("Expected locked version to be PairedVersion but got %T", lv) + } + + rev := lpv.Revision() + if rev != "ff2948a2ac8f538c4ecd55962e919d1e13e74baf" { + t.Fatalf("Expected locked revision to be 'ff2948a2ac8f538c4ecd55962e919d1e13e74baf', got %s", rev) + } + + ver := lpv.String() + if ver != "^1.0.0" { + t.Fatalf("Expected locked version to be '^1.0.0', got %s", ver) + } +} + +func TestGodepConfig_Convert_BadInput_EmptyPackageName(t *testing.T) { + g := newGodepImporter(discardLogger, true, nil) + g.json = godepJSON{ + Imports: []godepPackage{{ImportPath: ""}}, + } + + _, _, err := g.convert("") + if err == nil { + t.Fatal("Expected conversion to fail because the ImportPath is empty") + } +} + +func TestGodepConfig_Convert_BadInput_EmptyRev(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + h.TempDir("src") + + ctx := newTestContext(h) + sm, err := ctx.SourceManager() + h.Must(err) + defer sm.Release() + + g := newGodepImporter(discardLogger, true, sm) + g.json = godepJSON{ + Imports: []godepPackage{ + { + ImportPath: "github.com/sdboyer/deptest", + }, + }, + } + + _, _, err = g.convert("") + if err == nil { + t.Fatal("Expected conversion to fail because the Rev is empty") + } +} + +func TestGodepConfig_Convert_SubPackages(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + h.TempDir("src") + + ctx := newTestContext(h) + sm, err := ctx.SourceManager() + h.Must(err) + defer sm.Release() + + g := newGodepImporter(discardLogger, true, sm) + g.json = godepJSON{ + Imports: []godepPackage{ + { + ImportPath: "github.com/sdboyer/deptest", + Rev: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", + }, + { + ImportPath: "github.com/sdboyer/deptest/foo", + Rev: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", + }, + }, + } + + manifest, lock, err := g.convert("") + if err != nil { + t.Fatal(err) + } + + if _, present := manifest.Constraints["github.com/sdboyer/deptest"]; !present { + t.Fatal("Expected the manifest to have a dependency for 'github.com/sdboyer/deptest'") + } + + if _, present := manifest.Constraints["github.com/sdboyer/deptest/foo"]; present { + t.Fatal("Expected the manifest to not have a dependency for 'github.com/sdboyer/deptest/foo'") + } + + if len(lock.P) != 1 { + t.Fatalf("Expected lock to have 1 project, got %v", len(lock.P)) + } + + if string(lock.P[0].Ident().ProjectRoot) != "github.com/sdboyer/deptest" { + t.Fatal("Expected lock to have 'github.com/sdboyer/deptest'") + } +} + +func TestGodepConfig_ProjectExistsInLock(t *testing.T) { + lock := &dep.Lock{} + pi := gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/sdboyer/deptest")} + ver := gps.NewVersion("v1.0.0") + lock.P = append(lock.P, gps.NewLockedProject(pi, ver, nil)) + + cases := []struct { + importPath string + want bool + }{ + { + importPath: "github.com/sdboyer/deptest", + want: true, + }, + { + importPath: "github.com/golang/notexist", + want: false, + }, + } + + for _, c := range cases { + result := projectExistsInLock(lock, c.importPath) + + if result != c.want { + t.Fatalf("projectExistsInLock result is not as expected: \n\t(GOT) %v \n\t(WNT) %v", result, c.want) + } + } +} + +// Compares two slices of godepPackage and checks if they are equal. +func equalImports(a, b []godepPackage) bool { + + if a == nil && b == nil { + return true + } + + if a == nil || b == nil { + return false + } + + if len(a) != len(b) { + return false + } + + for i := range a { + if a[i] != b[i] { + return false + } + } + + return true +} diff --git a/cmd/dep/gopath_scanner.go b/cmd/dep/gopath_scanner.go index a5824be924..f6ca1d157e 100644 --- a/cmd/dep/gopath_scanner.go +++ b/cmd/dep/gopath_scanner.go @@ -46,7 +46,7 @@ func newGopathScanner(ctx *dep.Ctx, directDeps map[string]bool, sm gps.SourceMan func (g *gopathScanner) InitializeRootManifestAndLock(rootM *dep.Manifest, rootL *dep.Lock) error { var err error - g.ctx.Loggers.Err.Println("Searching GOPATH for projects...") + g.ctx.Err.Println("Searching GOPATH for projects...") g.pd, err = g.scanGopathForDependencies() if err != nil { return err @@ -91,7 +91,12 @@ func (g *gopathScanner) overlay(rootM *dep.Manifest, rootL *dep.Lock) { } rootM.Constraints[pkg] = prj v := g.pd.ondisk[pkg] - feedback(v, pkg, fb.DepTypeDirect, g.ctx.Err) + + pi := gps.ProjectIdentifier{ProjectRoot: pkg, Source: prj.Source} + f := fb.NewConstraintFeedback(gps.ProjectConstraint{Ident: pi, Constraint: v}, fb.DepTypeDirect) + f.LogFeedback(g.ctx.Err) + f = fb.NewLockedProjectFeedback(gps.NewLockedProject(pi, v, nil), fb.DepTypeDirect) + f.LogFeedback(g.ctx.Err) } // Keep track of which projects have been locked @@ -109,8 +114,8 @@ func (g *gopathScanner) overlay(rootM *dep.Manifest, rootL *dep.Lock) { lockedProjects[pkg] = true if _, isDirect := g.directDeps[string(pkg)]; !isDirect { - v := g.pd.ondisk[pkg] - feedback(v, pkg, fb.DepTypeTransitive, g.ctx.Err) + f := fb.NewLockedProjectFeedback(lp, fb.DepTypeTransitive) + f.LogFeedback(g.ctx.Err) } } @@ -123,7 +128,7 @@ func (g *gopathScanner) overlay(rootM *dep.Manifest, rootL *dep.Lock) { unlockedProjects = append(unlockedProjects, string(pr)) } if len(unlockedProjects) > 0 { - g.ctx.Loggers.Err.Printf("Following dependencies were not found in GOPATH. "+ + g.ctx.Err.Printf("Following dependencies were not found in GOPATH. "+ "Dep will use the most recent versions of these projects.\n %s", strings.Join(unlockedProjects, "\n ")) } @@ -211,7 +216,7 @@ func (g *gopathScanner) scanGopathForDependencies() (projectData, error) { var syncDepGroup sync.WaitGroup syncDep := func(pr gps.ProjectRoot, sm gps.SourceManager) { if err := sm.SyncSourceFor(gps.ProjectIdentifier{ProjectRoot: pr}); err != nil { - g.ctx.Loggers.Err.Printf("%+v", errors.Wrapf(err, "Unable to cache %s", pr)) + g.ctx.Err.Printf("%+v", errors.Wrapf(err, "Unable to cache %s", pr)) } syncDepGroup.Done() } diff --git a/cmd/dep/gopath_scanner_test.go b/cmd/dep/gopath_scanner_test.go index 9b7679dfa0..d40e35b310 100644 --- a/cmd/dep/gopath_scanner_test.go +++ b/cmd/dep/gopath_scanner_test.go @@ -151,11 +151,11 @@ func TestGetProjectPropertiesFromVersion(t *testing.T) { want: wantSemver, }, { - version: gps.NewBranch("foo-branch").Is("some-revision"), + version: gps.NewBranch("foo-branch").Pair("some-revision"), want: gps.NewBranch("foo-branch"), }, { - version: gps.NewVersion("foo-version").Is("some-revision"), + version: gps.NewVersion("foo-version").Pair("some-revision"), want: gps.NewVersion("foo-version"), }, { @@ -163,7 +163,7 @@ func TestGetProjectPropertiesFromVersion(t *testing.T) { want: nil, }, { - version: gps.NewVersion("v1.0.0").Is("some-revision"), + version: gps.NewVersion("v1.0.0").Pair("some-revision"), want: wantSemver, }, } diff --git a/cmd/dep/hash_in.go b/cmd/dep/hash_in.go index d2af94d969..5b3e8e9f09 100644 --- a/cmd/dep/hash_in.go +++ b/cmd/dep/hash_in.go @@ -51,6 +51,6 @@ func (hashinCommand) Run(ctx *dep.Ctx, args []string) error { if err != nil { return errors.Wrap(err, "prepare solver") } - ctx.Loggers.Out.Println(gps.HashingInputsAsString(s)) + ctx.Out.Println(gps.HashingInputsAsString(s)) return nil } diff --git a/cmd/dep/init.go b/cmd/dep/init.go index 6f0f83221f..2cfe19017d 100644 --- a/cmd/dep/init.go +++ b/cmd/dep/init.go @@ -8,7 +8,6 @@ import ( "flag" "os" "path/filepath" - "strings" "time" "github.com/golang/dep" @@ -136,8 +135,8 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error { ProjectAnalyzer: rootAnalyzer, } - if ctx.Loggers.Verbose { - params.TraceLogger = ctx.Loggers.Err + if ctx.Verbose { + params.TraceLogger = ctx.Err } s, err := gps.Prepare(params, sm) @@ -170,7 +169,7 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error { return err } if vendorbak != "" { - ctx.Loggers.Err.Printf("Old vendor backed up to %v", vendorbak) + ctx.Err.Printf("Old vendor backed up to %v", vendorbak) } sw, err := dep.NewSafeWriter(m, nil, l, dep.VendorAlways) @@ -204,10 +203,3 @@ func getDirectDependencies(root, cpr string) (pkgtree.PackageTree, map[string]bo // in handling them and informing the user appropriately func handleAllTheFailuresOfTheWorld(err error) { } - -func hasImportPathPrefix(s, prefix string) bool { - if s == prefix { - return true - } - return strings.HasPrefix(s, prefix+"/") -} diff --git a/cmd/dep/main.go b/cmd/dep/main.go index dfd960abf5..4094f2e152 100644 --- a/cmd/dep/main.go +++ b/cmd/dep/main.go @@ -12,6 +12,7 @@ import ( "io" "log" "os" + "path/filepath" "strings" "text/tabwriter" @@ -46,11 +47,10 @@ func main() { // A Config specifies a full configuration for a dep execution. type Config struct { - // Args hold the command-line arguments, starting with the program name. - Args []string - Stdout, Stderr io.Writer - WorkingDir string - Env []string + WorkingDir string // Where to execute + Args []string // Command-line arguments, starting with the program name. + Env []string // Environment variables + Stdout, Stderr io.Writer // Log output } // Run executes a configuration and returns an exit code. @@ -135,23 +135,23 @@ func (c *Config) Run() (exitCode int) { } // Parse the flags the user gave us. - // flag package automaticly prints usage and error message in err != nil + // flag package automatically prints usage and error message in err != nil // or if '-h' flag provided if err := fs.Parse(c.Args[2:]); err != nil { exitCode = 1 return } - loggers := &dep.Loggers{ + // Set up the dep context. + ctx := &dep.Ctx{ Out: log.New(c.Stdout, "", 0), Err: errLogger, Verbose: *verbose, } - - // Set up the dep context. - ctx, err := dep.NewContext(c.WorkingDir, c.Env, loggers) + gopaths := filepath.SplitList(getEnv(c.Env, "GOPATH")) + err := ctx.SetPaths(c.WorkingDir, gopaths...) if err != nil { - loggers.Err.Println(err) + errLogger.Printf("%q not in any GOPATH: %s\n", c.WorkingDir, err) exitCode = 1 return } @@ -174,6 +174,18 @@ func (c *Config) Run() (exitCode int) { return } +// getEnv returns the last instance of the environment variable. +func getEnv(env []string, key string) string { + pre := key + "=" + for i := len(env) - 1; i >= 0; i-- { + v := env[i] + if strings.HasPrefix(v, pre) { + return strings.TrimPrefix(v, pre) + } + } + return "" +} + func resetUsage(logger *log.Logger, fs *flag.FlagSet, name, args, longHelp string) { var ( hasFlags bool diff --git a/cmd/dep/prune.go b/cmd/dep/prune.go index 89e61403fc..201c09e672 100644 --- a/cmd/dep/prune.go +++ b/cmd/dep/prune.go @@ -59,8 +59,8 @@ func (cmd *pruneCommand) Run(ctx *dep.Ctx, args []string) error { params := p.MakeParams() params.RootPackageTree = ptree - if ctx.Loggers.Verbose { - params.TraceLogger = ctx.Loggers.Err + if ctx.Verbose { + params.TraceLogger = ctx.Err } s, err := gps.Prepare(params, sm) @@ -77,8 +77,8 @@ func (cmd *pruneCommand) Run(ctx *dep.Ctx, args []string) error { } var pruneLogger *log.Logger - if ctx.Loggers.Verbose { - pruneLogger = ctx.Loggers.Err + if ctx.Verbose { + pruneLogger = ctx.Err } return dep.PruneProject(p, sm, pruneLogger) } diff --git a/cmd/dep/root_analyzer.go b/cmd/dep/root_analyzer.go index 9868cede4e..f93af66f24 100644 --- a/cmd/dep/root_analyzer.go +++ b/cmd/dep/root_analyzer.go @@ -5,14 +5,12 @@ package main import ( - "encoding/hex" + "io/ioutil" + "log" "github.com/golang/dep" - fb "github.com/golang/dep/internal/feedback" "github.com/golang/dep/internal/gps" "github.com/pkg/errors" - "io/ioutil" - "log" ) // importer handles importing configuration from other dependency managers into @@ -73,12 +71,16 @@ func (a *rootAnalyzer) importManifestAndLock(dir string, pr gps.ProjectRoot, sup importers := []importer{ newGlideImporter(logger, a.ctx.Verbose, a.sm), + newGodepImporter(logger, a.ctx.Verbose, a.sm), } for _, i := range importers { if i.HasDepMetadata(dir) { - a.ctx.Loggers.Err.Printf("Importing configuration from %s. These are only initial constraints, and are further refined during the solve process.", i.Name()) + a.ctx.Err.Printf("Importing configuration from %s. These are only initial constraints, and are further refined during the solve process.", i.Name()) m, l, err := i.Import(dir, pr) + if err != nil { + return nil, nil, err + } a.removeTransitiveDependencies(m) return m, l, err } @@ -139,57 +141,16 @@ func (a *rootAnalyzer) FinalizeRootManifestAndLock(m *dep.Manifest, l *dep.Lock) } } -func (a *rootAnalyzer) Info() (string, int) { +func (a *rootAnalyzer) Info() gps.ProjectAnalyzerInfo { name := "dep" version := 1 if !a.skipTools { name = "dep+import" } - return name, version -} - -// feedback logs project constraint as feedback to the user. -func feedback(v gps.Version, pr gps.ProjectRoot, depType string, logger *log.Logger) { - rev, version, branch := gps.VersionComponentStrings(v) - - // Check if it's a valid SHA1 digest and trim to 7 characters. - if len(rev) == 40 { - if _, err := hex.DecodeString(rev); err == nil { - // Valid SHA1 digest - rev = rev[0:7] - } + return gps.ProjectAnalyzerInfo{ + Name: name, + Version: version, } - - // Get LockedVersion - var ver string - if version != "" { - ver = version - } else if branch != "" { - ver = branch - } - - cf := &fb.ConstraintFeedback{ - LockedVersion: ver, - Revision: rev, - ProjectPath: string(pr), - DependencyType: depType, - } - - // Get non-revision constraint if available - if c := getProjectPropertiesFromVersion(v).Constraint; c != nil { - cf.Version = c.String() - } - - // Attach ConstraintType for direct/imported deps based on locked version - if cf.DependencyType == fb.DepTypeDirect || cf.DependencyType == fb.DepTypeImported { - if cf.LockedVersion != "" { - cf.ConstraintType = fb.ConsTypeConstraint - } else { - cf.ConstraintType = fb.ConsTypeHint - } - } - - cf.LogFeedback(logger) } func lookupVersionForRevision(rev gps.Revision, pi gps.ProjectIdentifier, sm gps.SourceManager) (gps.Version, error) { @@ -201,7 +162,7 @@ func lookupVersionForRevision(rev gps.Revision, pi gps.ProjectIdentifier, sm gps gps.SortPairedForUpgrade(versions) // Sort versions in asc order for _, v := range versions { - if v.Underlying() == rev { + if v.Revision() == rev { return v, nil } } diff --git a/cmd/dep/root_analyzer_test.go b/cmd/dep/root_analyzer_test.go index baca305d9f..e4b45ee41e 100644 --- a/cmd/dep/root_analyzer_test.go +++ b/cmd/dep/root_analyzer_test.go @@ -13,7 +13,7 @@ func TestRootAnalyzer_Info(t *testing.T) { } for skipTools, want := range testCases { a := rootAnalyzer{skipTools: skipTools} - got, _ := a.Info() + got := a.Info().Name if got != want { t.Errorf("Expected the name of the importer with skipTools=%t to be '%s', got '%s'", skipTools, want, got) } diff --git a/cmd/dep/status.go b/cmd/dep/status.go index 45ca58fdb5..c522b50706 100644 --- a/cmd/dep/status.go +++ b/cmd/dep/status.go @@ -215,22 +215,22 @@ func (cmd *statusCommand) Run(ctx *dep.Ctx, args []string) error { } } - digestMismatch, hasMissingPkgs, err := runStatusAll(ctx.Loggers, out, p, sm) + digestMismatch, hasMissingPkgs, err := runStatusAll(ctx, out, p, sm) if err != nil { return err } if digestMismatch { if hasMissingPkgs { - ctx.Loggers.Err.Printf("Lock inputs-digest mismatch due to the following packages missing from the lock:\n\n") - ctx.Loggers.Out.Print(buf.String()) - ctx.Loggers.Err.Printf("\nThis happens when a new import is added. Run `dep ensure` to install the missing packages.\n") + ctx.Err.Printf("Lock inputs-digest mismatch due to the following packages missing from the lock:\n\n") + ctx.Out.Print(buf.String()) + ctx.Err.Printf("\nThis happens when a new import is added. Run `dep ensure` to install the missing packages.\n") } else { - ctx.Loggers.Err.Printf("Lock inputs-digest mismatch. This happens when Gopkg.toml is modified.\n" + + ctx.Err.Printf("Lock inputs-digest mismatch. This happens when Gopkg.toml is modified.\n" + "Run `dep ensure` to regenerate the inputs-digest.") } } else { - ctx.Loggers.Out.Print(buf.String()) + ctx.Out.Print(buf.String()) } return nil @@ -253,7 +253,7 @@ type MissingStatus struct { MissingPackages []string } -func runStatusAll(loggers *dep.Loggers, out outputter, p *dep.Project, sm gps.SourceManager) (bool, bool, error) { +func runStatusAll(ctx *dep.Ctx, out outputter, p *dep.Project, sm gps.SourceManager) (bool, bool, error) { var digestMismatch, hasMissingPkgs bool if p.Lock == nil { @@ -276,8 +276,8 @@ func runStatusAll(loggers *dep.Loggers, out outputter, p *dep.Project, sm gps.So Manifest: p.Manifest, // Locks aren't a part of the input hash check, so we can omit it. } - if loggers.Verbose { - params.TraceLogger = loggers.Err + if ctx.Verbose { + params.TraceLogger = ctx.Err } s, err := gps.Prepare(params, sm) @@ -328,7 +328,7 @@ func runStatusAll(loggers *dep.Loggers, out outputter, p *dep.Project, sm gps.So bs.Revision = tv case gps.PairedVersion: bs.Version = tv.Unpair() - bs.Revision = tv.Underlying() + bs.Revision = tv.Revision() } // Check if the manifest has an override for this project. If so, @@ -364,7 +364,7 @@ func runStatusAll(loggers *dep.Loggers, out outputter, p *dep.Project, sm gps.So // upgrade, the first version we encounter that // matches our constraint will be what we want. if c.Constraint.Matches(v) { - bs.Latest = v.Underlying() + bs.Latest = v.Revision() break } } @@ -412,9 +412,9 @@ func runStatusAll(loggers *dep.Loggers, out outputter, p *dep.Project, sm gps.So // TODO this is just a fix quick so staticcheck doesn't complain. // Visually reconciling failure to deduce project roots with the rest of // the mismatch output is a larger problem. - loggers.Err.Printf("Failed to deduce project roots for import paths:\n") + ctx.Err.Printf("Failed to deduce project roots for import paths:\n") for _, fail := range errs { - loggers.Err.Printf("\t%s: %s\n", fail.ex, fail.err.Error()) + ctx.Err.Printf("\t%s: %s\n", fail.ex, fail.err.Error()) } return digestMismatch, hasMissingPkgs, errors.New("address issues with undeducible import paths to get more status information") diff --git a/cmd/dep/testdata/glide/expected_import_output.txt b/cmd/dep/testdata/glide/expected_import_output.txt index 7b599e9570..d828374e57 100644 --- a/cmd/dep/testdata/glide/expected_import_output.txt +++ b/cmd/dep/testdata/glide/expected_import_output.txt @@ -1,7 +1,7 @@ Detected glide configuration files... Converting from glide.yaml and glide.lock... - Using ^0.8.1 as initial constraint for imported dep github.com/sdboyer/deptest - Trying v0.8.1 (3f4c3be) as initial lock for imported dep github.com/sdboyer/deptest + Using master as initial constraint for imported dep github.com/sdboyer/deptest Using ^2.0.0 as initial constraint for imported dep github.com/sdboyer/deptestdos + Using master as initial constraint for imported dep github.com/golang/lint + Trying v0.8.1 (3f4c3be) as initial lock for imported dep github.com/sdboyer/deptest Trying v2.0.0 (5c60720) as initial lock for imported dep github.com/sdboyer/deptestdos - Using cb00e56 as initial hint for imported dep github.com/golang/lint diff --git a/cmd/dep/testdata/godep/Godeps.json b/cmd/dep/testdata/godep/Godeps.json new file mode 100644 index 0000000000..15126ac12d --- /dev/null +++ b/cmd/dep/testdata/godep/Godeps.json @@ -0,0 +1,16 @@ +{ + "ImportPath": "github.com/golang/notexist", + "GoVersion": "go1.8", + "GodepVersion": "vXYZ", + "Deps": [ + { + "ImportPath": "github.com/sdboyer/deptest", + "Rev": "3f4c3bea144e112a69bbe5d8d01c1b09a544253f" + }, + { + "ImportPath": "github.com/sdboyer/deptestdos", + "Comment": "v2.0.0", + "Rev": "5c607206be5decd28e6263ffffdcee067266015e" + } + ] +} diff --git a/cmd/dep/testdata/godep/expected_import_output.txt b/cmd/dep/testdata/godep/expected_import_output.txt new file mode 100644 index 0000000000..b023ee471a --- /dev/null +++ b/cmd/dep/testdata/godep/expected_import_output.txt @@ -0,0 +1,6 @@ +Detected godep configuration files... +Converting from Godeps.json ... + Using ^0.8.1 as initial constraint for imported dep github.com/sdboyer/deptest + Trying ^0.8.1 (3f4c3be) as initial lock for imported dep github.com/sdboyer/deptest + Using ^2.0.0 as initial constraint for imported dep github.com/sdboyer/deptestdos + Trying v2.0.0 (5c60720) as initial lock for imported dep github.com/sdboyer/deptestdos diff --git a/cmd/dep/testdata/harness_tests/init/godep/case1/final/Gopkg.lock b/cmd/dep/testdata/harness_tests/init/godep/case1/final/Gopkg.lock new file mode 100644 index 0000000000..0e62baa0cc --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/godep/case1/final/Gopkg.lock @@ -0,0 +1,21 @@ +# This file is autogenerated, do not edit; changes may be undone by the next 'dep ensure'. + + +[[projects]] + name = "github.com/sdboyer/deptest" + packages = ["."] + revision = "3f4c3bea144e112a69bbe5d8d01c1b09a544253f" + version = "master" + +[[projects]] + name = "github.com/sdboyer/deptestdos" + packages = ["."] + revision = "5c607206be5decd28e6263ffffdcee067266015e" + version = "v2.0.0" + +[solve-meta] + analyzer-name = "dep" + analyzer-version = 1 + inputs-digest = "1ed417a0bec57ffe988fae1cba8f3d49994fb893394d61844e0b3c96d69573fe" + solver-name = "gps-cdcl" + solver-version = 1 diff --git a/cmd/dep/testdata/harness_tests/init/godep/case1/final/Gopkg.toml b/cmd/dep/testdata/harness_tests/init/godep/case1/final/Gopkg.toml new file mode 100644 index 0000000000..aaf78303fa --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/godep/case1/final/Gopkg.toml @@ -0,0 +1,4 @@ + +[[constraint]] + name = "github.com/sdboyer/deptestdos" + version = "2.0.0" diff --git a/cmd/dep/testdata/harness_tests/init/godep/case1/initial/Godeps/Godeps.json b/cmd/dep/testdata/harness_tests/init/godep/case1/initial/Godeps/Godeps.json new file mode 100644 index 0000000000..ee87370e5d --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/godep/case1/initial/Godeps/Godeps.json @@ -0,0 +1,17 @@ +{ + "ImportPath": "github.com/golang/notexist", + "GoVersion": "go1.8", + "GodepVersion": "vXYZ", + "Deps": [ + { + "ImportPath": "github.com/sdboyer/deptest", + "Comment": "master", + "Rev": "3f4c3bea144e112a69bbe5d8d01c1b09a544253f" + }, + { + "ImportPath": "github.com/sdboyer/deptestdos", + "Comment": "v2.0.0", + "Rev": "5c607206be5decd28e6263ffffdcee067266015e" + } + ] +} diff --git a/cmd/dep/testdata/harness_tests/init/godep/case1/initial/main.go b/cmd/dep/testdata/harness_tests/init/godep/case1/initial/main.go new file mode 100644 index 0000000000..2b2c7c396e --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/godep/case1/initial/main.go @@ -0,0 +1,16 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "fmt" + + "github.com/sdboyer/deptestdos" +) + +func main() { + var x deptestdos.Bar + fmt.Println(x) +} diff --git a/cmd/dep/testdata/harness_tests/init/godep/case1/testcase.json b/cmd/dep/testdata/harness_tests/init/godep/case1/testcase.json new file mode 100644 index 0000000000..017dc4cd55 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/godep/case1/testcase.json @@ -0,0 +1,13 @@ +{ + "commands": [ + ["init", "-no-examples"] + ], + "error-expected": "", + "gopath-initial": { + "github.com/sdboyer/deptest": "3f4c3bea144e112a69bbe5d8d01c1b09a544253f" + }, + "vendor-final": [ + "github.com/sdboyer/deptest", + "github.com/sdboyer/deptestdos" + ] +} diff --git a/context.go b/context.go index 0aa96afbd4..d2e8cafdc6 100644 --- a/context.go +++ b/context.go @@ -5,6 +5,7 @@ package dep import ( + "fmt" "log" "os" "path/filepath" @@ -17,60 +18,79 @@ import ( "github.com/pkg/errors" ) -// Ctx defines the supporting context of the tool. +/* +Ctx defines the supporting context of the tool. +A properly initialized Ctx has a GOPATH containing WorkingDir, and non-nil Loggers. + + ctx := &dep.Ctx{ + WorkingDir: gopath + "/src/project/root", + GOPATH: gopath, + Out: log.New(os.Stdout, "", 0), + Err: log.New(os.Stderr, "", 0), + } + +SetPaths assists with setting consistent path fields. + + ctx := &dep.Ctx{ + Out: log.New(os.Stdout, "", 0), + Err: log.New(os.Stderr, "", 0), + } + err := ctx.SetPaths(projectRootPath, filepath.SplitList(os.Getenv("GOPATH")) + if err != nil { + // projectRootPath not in any GOPATH + } + +*/ type Ctx struct { - GOPATH string // Selected Go path - GOPATHS []string // Other Go paths - WorkingDir string - *Loggers + WorkingDir string // Where to execute. + GOPATH string // Selected Go path, containing WorkingDir. + GOPATHS []string // Other Go paths. + Out, Err *log.Logger // Required loggers. + Verbose bool // Enables more verbose logging. } -// Loggers holds standard loggers and a verbosity flag. -type Loggers struct { - Out, Err *log.Logger - // Whether verbose logging is enabled. - Verbose bool -} +/* +SetPaths sets the WorkingDir, GOPATH, and GOPATHS fields. +It selects the GOPATH containing WorkingDir, or returns an error if none is found. + + err := ctx.SetPaths(projectRootPath, filepath.SplitList(os.Getenv("GOPATH")) + if err != nil { + // project root not in any GOPATH + } + +The default GOPATH is checked when none are provided. -// NewContext creates a struct with the project's GOPATH. It assumes -// that of your "GOPATH"'s we want the one we are currently in. -func NewContext(wd string, env []string, loggers *Loggers) (*Ctx, error) { - ctx := &Ctx{WorkingDir: wd, Loggers: loggers} + err := ctx.SetPaths(projectRootPath) + if err != nil { + // project root not in default GOPATH, or none available + } - GOPATH := getEnv(env, "GOPATH") - if GOPATH == "" { - GOPATH = defaultGOPATH() +*/ +func (c *Ctx) SetPaths(workingDir string, gopaths ...string) error { + c.WorkingDir = workingDir + if len(gopaths) == 0 { + d := defaultGOPATH() + if d == "" { + return errors.New("no default GOPATH available") + } + gopaths = []string{d} } - for _, gp := range filepath.SplitList(GOPATH) { + wd := filepath.FromSlash(workingDir) + for _, gp := range gopaths { gp = filepath.FromSlash(gp) - if fs.HasFilepathPrefix(filepath.FromSlash(wd), gp) { - ctx.GOPATH = gp + if fs.HasFilepathPrefix(wd, gp) { + c.GOPATH = gp } - ctx.GOPATHS = append(ctx.GOPATHS, gp) + c.GOPATHS = append(c.GOPATHS, gp) } - if ctx.GOPATH == "" { - return nil, errors.New("project not in a GOPATH") + if c.GOPATH == "" { + return fmt.Errorf("%q not in any GOPATH", wd) } - return ctx, nil -} - -// getEnv returns the last instance of an environment variable. -func getEnv(env []string, key string) string { - for i := len(env) - 1; i >= 0; i-- { - v := env[i] - kv := strings.SplitN(v, "=", 2) - if kv[0] == key { - if len(kv) > 1 { - return kv[1] - } - return "" - } - } - return "" + return nil } // defaultGOPATH gets the default GOPATH that was added in 1.8 @@ -142,7 +162,7 @@ func (c *Ctx) LoadProject() (*Project, error) { var warns []error p.Manifest, warns, err = readManifest(mf) for _, warn := range warns { - c.Loggers.Err.Printf("dep: WARNING: %v\n", warn) + c.Err.Printf("dep: WARNING: %v\n", warn) } if err != nil { return nil, errors.Errorf("error while parsing %s: %s", mp, err) @@ -270,7 +290,7 @@ func (c *Ctx) VersionInWorkspace(root gps.ProjectRoot) (gps.Version, error) { if contains(tags, ver) { // Assume semver if it starts with a v. if strings.HasPrefix(ver, "v") { - return gps.NewVersion(ver).Is(gps.Revision(rev)), nil + return gps.NewVersion(ver).Pair(gps.Revision(rev)), nil } return nil, errors.Errorf("version for root %s does not start with a v: %q", pr, ver) @@ -283,7 +303,7 @@ func (c *Ctx) VersionInWorkspace(root gps.ProjectRoot) (gps.Version, error) { } // Try to match the current version to a branch. if contains(branches, ver) { - return gps.NewBranch(ver).Is(gps.Revision(rev)), nil + return gps.NewBranch(ver).Pair(gps.Revision(rev)), nil } return gps.Revision(rev), nil diff --git a/context_test.go b/context_test.go index 9709538031..506da0ad13 100644 --- a/context_test.go +++ b/context_test.go @@ -19,30 +19,44 @@ import ( ) var ( - discardLogger = log.New(ioutil.Discard, "", 0) - discardLoggers = &Loggers{Out: discardLogger, Err: discardLogger} + discardLogger = log.New(ioutil.Discard, "", 0) ) -func TestNewContextNoGOPATH(t *testing.T) { +func TestCtx_SetGOPATH(t *testing.T) { h := test.NewHelper(t) defer h.Cleanup() - h.TempDir("src") - h.Cd(h.Path(".")) - wd, err := os.Getwd() - if err != nil { - t.Fatal("failed to get work directory:", err) - } + wd := h.Path(".") + + t.Run("slash", func(t *testing.T) { + var c Ctx + err := c.SetPaths(wd, filepath.ToSlash(wd)) + if err != nil { + t.Error(err) + } + }) + + t.Run("separator", func(t *testing.T) { + var c Ctx + err := c.SetPaths(wd, filepath.FromSlash(wd)) + if err != nil { + t.Error(err) + } + }) +} - c, err := NewContext(wd, os.Environ(), nil) +func TestCtx_SetGOPATH_empty(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + + h.TempDir("src") + var c Ctx + + err := c.SetPaths(h.Path("."), "") if err == nil { t.Fatal("error should not have been nil") } - - if c != nil { - t.Fatalf("expected context to be nil, got: %#v", c) - } } func TestSplitAbsoluteProjectRoot(t *testing.T) { @@ -142,7 +156,7 @@ func TestVersionInWorkspace(t *testing.T) { checkout bool }{ "github.com/pkg/errors": { - rev: gps.NewVersion("v0.8.0").Is("645ef00459ed84a119197bfb8d8205042c6df63d"), // semver + rev: gps.NewVersion("v0.8.0").Pair("645ef00459ed84a119197bfb8d8205042c6df63d"), // semver checkout: true, }, "github.com/Sirupsen/logrus": { @@ -150,7 +164,7 @@ func TestVersionInWorkspace(t *testing.T) { checkout: true, }, "github.com/rsc/go-get-default-branch": { - rev: gps.NewBranch("another-branch").Is("8e6902fdd0361e8fa30226b350e62973e3625ed5"), + rev: gps.NewBranch("another-branch").Pair("8e6902fdd0361e8fa30226b350e62973e3625ed5"), }, } @@ -198,7 +212,12 @@ func TestLoadProject(t *testing.T) { for _, testcase := range testcases { start := testcase.start - ctx := &Ctx{GOPATH: tg.Path("."), WorkingDir: tg.Path(start), Loggers: discardLoggers} + ctx := &Ctx{ + GOPATH: tg.Path("."), + WorkingDir: tg.Path(start), + Out: discardLogger, + Err: discardLogger, + } proj, err := ctx.LoadProject() tg.Must(err) @@ -261,7 +280,12 @@ func TestLoadProjectManifestParseError(t *testing.T) { t.Fatal("failed to get working directory", err) } - ctx := &Ctx{GOPATH: tg.Path("."), WorkingDir: wd, Loggers: discardLoggers} + ctx := &Ctx{ + GOPATH: tg.Path("."), + WorkingDir: wd, + Out: discardLogger, + Err: discardLogger, + } _, err = ctx.LoadProject() if err == nil { @@ -287,7 +311,12 @@ func TestLoadProjectLockParseError(t *testing.T) { t.Fatal("failed to get working directory", err) } - ctx := &Ctx{GOPATH: tg.Path("."), WorkingDir: wd, Loggers: discardLoggers} + ctx := &Ctx{ + GOPATH: tg.Path("."), + WorkingDir: wd, + Out: discardLogger, + Err: discardLogger, + } _, err = ctx.LoadProject() if err == nil { diff --git a/doc.go b/doc.go new file mode 100644 index 0000000000..63226ce4c2 --- /dev/null +++ b/doc.go @@ -0,0 +1,6 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package dep is a prototype dependency management library. +package dep diff --git a/internal/feedback/feedback.go b/internal/feedback/feedback.go index 17462097ac..166c8395f8 100644 --- a/internal/feedback/feedback.go +++ b/internal/feedback/feedback.go @@ -5,8 +5,11 @@ package feedback import ( + "encoding/hex" "fmt" "log" + + "github.com/golang/dep/internal/gps" ) // Constraint types @@ -25,31 +28,60 @@ const DepTypeImported = "imported dep" // ConstraintFeedback holds project constraint feedback data type ConstraintFeedback struct { - Version, LockedVersion, Revision, ConstraintType, DependencyType, ProjectPath string + Constraint, LockedVersion, Revision, ConstraintType, DependencyType, ProjectPath string } -// LogFeedback logs the feedback +// NewConstraintFeedback builds a feedback entry for a constraint in the manifest. +func NewConstraintFeedback(pc gps.ProjectConstraint, depType string) *ConstraintFeedback { + cf := &ConstraintFeedback{ + Constraint: pc.Constraint.String(), + ProjectPath: string(pc.Ident.ProjectRoot), + DependencyType: depType, + } + + if _, ok := pc.Constraint.(gps.Revision); ok { + cf.ConstraintType = ConsTypeHint + } else { + cf.ConstraintType = ConsTypeConstraint + } + + return cf +} + +// NewLockedProjectFeedback builds a feedback entry for a project in the lock. +func NewLockedProjectFeedback(lp gps.LockedProject, depType string) *ConstraintFeedback { + cf := &ConstraintFeedback{ + ProjectPath: string(lp.Ident().ProjectRoot), + DependencyType: depType, + } + + switch vt := lp.Version().(type) { + case gps.PairedVersion: + cf.LockedVersion = vt.String() + cf.Revision = vt.Revision().String() + case gps.UnpairedVersion: // Logically this should never occur, but handle for completeness sake + cf.LockedVersion = vt.String() + case gps.Revision: + cf.Revision = vt.String() + } + + return cf +} + +// LogFeedback logs feedback on changes made to the manifest or lock. func (cf ConstraintFeedback) LogFeedback(logger *log.Logger) { - // "Using" feedback for direct dep - if cf.DependencyType == DepTypeDirect || cf.DependencyType == DepTypeImported { - ver := cf.Version - // revision as version for hint - if cf.ConstraintType == ConsTypeHint { - ver = cf.Revision - } - logger.Printf(" %v", GetUsingFeedback(ver, cf.ConstraintType, cf.DependencyType, cf.ProjectPath)) + if cf.Constraint != "" { + logger.Printf(" %v", GetUsingFeedback(cf.Constraint, cf.ConstraintType, cf.DependencyType, cf.ProjectPath)) } - // No "Locking" feedback for hints. "Locking" feedback only for constraint - // and transitive dep - if cf.ConstraintType != ConsTypeHint { + if cf.LockedVersion != "" && cf.Revision != "" { logger.Printf(" %v", GetLockingFeedback(cf.LockedVersion, cf.Revision, cf.DependencyType, cf.ProjectPath)) } } -// GetUsingFeedback returns dependency using feedback string. -// Example: -// Using ^1.0.0 as constraint for direct dep github.com/foo/bar -// Using 1b8edb3 as hint for direct dep github.com/bar/baz +// GetUsingFeedback returns a dependency "using" feedback message. For example: +// +// Using ^1.0.0 as constraint for direct dep github.com/foo/bar +// Using 1b8edb3 as hint for direct dep github.com/bar/baz func GetUsingFeedback(version, consType, depType, projectPath string) string { if depType == DepTypeImported { return fmt.Sprintf("Using %s as initial %s for %s %s", version, consType, depType, projectPath) @@ -57,11 +89,20 @@ func GetUsingFeedback(version, consType, depType, projectPath string) string { return fmt.Sprintf("Using %s as %s for %s %s", version, consType, depType, projectPath) } -// GetLockingFeedback returns dependency locking feedback string. -// Example: -// Locking in v1.1.4 (bc29b4f) for direct dep github.com/foo/bar -// Locking in master (436f39d) for transitive dep github.com/baz/qux +// GetLockingFeedback returns a dependency "locking" feedback message. For +// example: +// +// Locking in v1.1.4 (bc29b4f) for direct dep github.com/foo/bar +// Locking in master (436f39d) for transitive dep github.com/baz/qux func GetLockingFeedback(version, revision, depType, projectPath string) string { + // Check if it's a valid SHA1 digest and trim to 7 characters. + if len(revision) == 40 { + if _, err := hex.DecodeString(revision); err == nil { + // Valid SHA1 digest + revision = revision[0:7] + } + } + if depType == DepTypeImported { return fmt.Sprintf("Trying %s (%s) as initial lock for %s %s", version, revision, depType, projectPath) } diff --git a/internal/feedback/feedback_test.go b/internal/feedback/feedback_test.go index 1986cfc459..51190e4099 100644 --- a/internal/feedback/feedback_test.go +++ b/internal/feedback/feedback_test.go @@ -5,47 +5,82 @@ package feedback import ( + "bytes" + log2 "log" + "strings" "testing" + + "github.com/golang/dep/internal/gps" ) -func TestGetConstraintString(t *testing.T) { +func TestFeedback_Constraint(t *testing.T) { + ver, _ := gps.NewSemverConstraint("^1.0.0") + rev := gps.Revision("1b8edb3") + pi := gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/foo/bar")} + cases := []struct { - feedback string + feedback *ConstraintFeedback want string }{ { - feedback: GetUsingFeedback("^1.0.0", ConsTypeConstraint, DepTypeDirect, "github.com/foo/bar"), + feedback: NewConstraintFeedback(gps.ProjectConstraint{Constraint: ver, Ident: pi}, DepTypeDirect), want: "Using ^1.0.0 as constraint for direct dep github.com/foo/bar", }, { - feedback: GetUsingFeedback("^1.0.0", ConsTypeConstraint, DepTypeImported, "github.com/foo/bar"), + feedback: NewConstraintFeedback(gps.ProjectConstraint{Constraint: ver, Ident: pi}, DepTypeImported), want: "Using ^1.0.0 as initial constraint for imported dep github.com/foo/bar", }, { - feedback: GetUsingFeedback("1b8edb3", ConsTypeHint, DepTypeDirect, "github.com/bar/baz"), - want: "Using 1b8edb3 as hint for direct dep github.com/bar/baz", + feedback: NewConstraintFeedback(gps.ProjectConstraint{Constraint: rev, Ident: pi}, DepTypeDirect), + want: "Using 1b8edb3 as hint for direct dep github.com/foo/bar", }, { - feedback: GetUsingFeedback("1b8edb3", ConsTypeHint, DepTypeImported, "github.com/bar/baz"), - want: "Using 1b8edb3 as initial hint for imported dep github.com/bar/baz", + feedback: NewConstraintFeedback(gps.ProjectConstraint{Constraint: rev, Ident: pi}, DepTypeImported), + want: "Using 1b8edb3 as initial hint for imported dep github.com/foo/bar", }, + } + + for _, c := range cases { + buf := &bytes.Buffer{} + log := log2.New(buf, "", 0) + c.feedback.LogFeedback(log) + got := strings.TrimSpace(buf.String()) + if c.want != got { + t.Errorf("Feedbacks are not expected: \n\t(GOT) '%s'\n\t(WNT) '%s'", got, c.want) + } + } +} + +func TestFeedback_LockedProject(t *testing.T) { + v := gps.NewVersion("v1.1.4").Pair("bc29b4f") + b := gps.NewBranch("master").Pair("436f39d") + pi := gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/foo/bar")} + + cases := []struct { + feedback *ConstraintFeedback + want string + }{ { - feedback: GetLockingFeedback("v1.1.4", "bc29b4f", DepTypeDirect, "github.com/foo/bar"), + feedback: NewLockedProjectFeedback(gps.NewLockedProject(pi, v, nil), DepTypeDirect), want: "Locking in v1.1.4 (bc29b4f) for direct dep github.com/foo/bar", }, { - feedback: GetLockingFeedback("v1.1.4", "bc29b4f", DepTypeImported, "github.com/foo/bar"), + feedback: NewLockedProjectFeedback(gps.NewLockedProject(pi, v, nil), DepTypeImported), want: "Trying v1.1.4 (bc29b4f) as initial lock for imported dep github.com/foo/bar", }, { - feedback: GetLockingFeedback("master", "436f39d", DepTypeTransitive, "github.com/baz/qux"), - want: "Locking in master (436f39d) for transitive dep github.com/baz/qux", + feedback: NewLockedProjectFeedback(gps.NewLockedProject(pi, b, nil), DepTypeTransitive), + want: "Locking in master (436f39d) for transitive dep github.com/foo/bar", }, } for _, c := range cases { - if c.want != c.feedback { - t.Errorf("Feedbacks are not expected: \n\t(GOT) %v\n\t(WNT) %v", c.feedback, c.want) + buf := &bytes.Buffer{} + log := log2.New(buf, "", 0) + c.feedback.LogFeedback(log) + got := strings.TrimSpace(buf.String()) + if c.want != got { + t.Errorf("Feedbacks are not expected: \n\t(GOT) '%s'\n\t(WNT) '%s'", got, c.want) } } } diff --git a/internal/fs/fs.go b/internal/fs/fs.go index cd526f6401..a79032017b 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -374,7 +374,6 @@ func IsNonEmptyDir(name string) (bool, error) { // IsRegular determines if the path given is a regular file or not. func IsRegular(name string) (bool, error) { - // TODO: lstat? fi, err := os.Stat(name) if os.IsNotExist(err) { return false, nil @@ -382,8 +381,9 @@ func IsRegular(name string) (bool, error) { if err != nil { return false, err } - if fi.IsDir() { - return false, errors.Errorf("%q is a directory, should be a file", name) + mode := fi.Mode() + if mode&os.ModeType != 0 { + return false, errors.Errorf("%q is a %v, expected a file", name, mode) } return true, nil } diff --git a/internal/gps/constraint_test.go b/internal/gps/constraint_test.go index 0aaf5f1709..d3619bfde2 100644 --- a/internal/gps/constraint_test.go +++ b/internal/gps/constraint_test.go @@ -12,7 +12,7 @@ import ( // gu - helper func for stringifying what we assume is a VersionPair (otherwise // will panic), but is given as a Constraint func gu(v Constraint) string { - return fmt.Sprintf("%q at rev %q", v, v.(PairedVersion).Underlying()) + return fmt.Sprintf("%q at rev %q", v, v.(PairedVersion).Revision()) } func TestBranchConstraintOps(t *testing.T) { @@ -47,7 +47,7 @@ func TestBranchConstraintOps(t *testing.T) { // Add rev to one snuffster := Revision("snuffleupagus") - v3 := v1.Is(snuffster).(versionPair) + v3 := v1.Pair(snuffster).(versionPair) if v2.Matches(v3) { t.Errorf("%s should not match %s", v2, gu(v3)) } @@ -70,7 +70,7 @@ func TestBranchConstraintOps(t *testing.T) { } // Add different rev to the other - v4 := v2.Is(Revision("cookie monster")).(versionPair) + v4 := v2.Pair(Revision("cookie monster")).(versionPair) if v4.Matches(v3) { t.Errorf("%s should not match %s", gu(v4), gu(v3)) } @@ -96,7 +96,7 @@ func TestBranchConstraintOps(t *testing.T) { // TODO(sdboyer) this might not actually be a good idea, when you consider the // semantics of floating versions...matching on an underlying rev might be // nice in the short term, but it's probably shit most of the time - v5 := v2.Is(Revision("snuffleupagus")).(versionPair) + v5 := v2.Pair(Revision("snuffleupagus")).(versionPair) if !v5.Matches(v3) { t.Errorf("%s should match %s", gu(v5), gu(v3)) } @@ -122,9 +122,9 @@ func TestBranchConstraintOps(t *testing.T) { cookie := Revision("cookie monster") o1 := NewVersion("master").(plainVersion) o2 := NewVersion("1.0.0").(semVersion) - o3 := o1.Is(cookie).(versionPair) - o4 := o2.Is(cookie).(versionPair) - v6 := v1.Is(cookie).(versionPair) + o3 := o1.Pair(cookie).(versionPair) + o4 := o2.Pair(cookie).(versionPair) + v6 := v1.Pair(cookie).(versionPair) if v1.Matches(o1) { t.Errorf("%s (branch) should not match %s (version) across types", v1, o1) @@ -231,7 +231,7 @@ func TestVersionConstraintOps(t *testing.T) { // Add rev to one snuffster := Revision("snuffleupagus") - v3 := v1.Is(snuffster).(versionPair) + v3 := v1.Pair(snuffster).(versionPair) if v2.Matches(v3) { t.Errorf("%s should not match %s", v2, gu(v3)) } @@ -254,7 +254,7 @@ func TestVersionConstraintOps(t *testing.T) { } // Add different rev to the other - v4 := v2.Is(Revision("cookie monster")).(versionPair) + v4 := v2.Pair(Revision("cookie monster")).(versionPair) if v4.Matches(v3) { t.Errorf("%s should not match %s", gu(v4), gu(v3)) } @@ -277,7 +277,7 @@ func TestVersionConstraintOps(t *testing.T) { } // Now add same rev to different versions, and things should line up - v5 := v2.Is(Revision("snuffleupagus")).(versionPair) + v5 := v2.Pair(Revision("snuffleupagus")).(versionPair) if !v5.Matches(v3) { t.Errorf("%s should match %s", gu(v5), gu(v3)) } @@ -303,9 +303,9 @@ func TestVersionConstraintOps(t *testing.T) { cookie := Revision("cookie monster") o1 := NewBranch("master").(branchVersion) o2 := NewVersion("1.0.0").(semVersion) - o3 := o1.Is(cookie).(versionPair) - o4 := o2.Is(cookie).(versionPair) - v6 := v1.Is(cookie).(versionPair) + o3 := o1.Pair(cookie).(versionPair) + o4 := o2.Pair(cookie).(versionPair) + v6 := v1.Pair(cookie).(versionPair) if v1.Matches(o1) { t.Errorf("%s (version) should not match %s (branch) across types", v1, o1) @@ -412,7 +412,7 @@ func TestSemverVersionConstraintOps(t *testing.T) { // Add rev to one snuffster := Revision("snuffleupagus") - v3 := v1.Is(snuffster).(versionPair) + v3 := v1.Pair(snuffster).(versionPair) if v2.Matches(v3) { t.Errorf("%s should not match %s", v2, gu(v3)) } @@ -435,7 +435,7 @@ func TestSemverVersionConstraintOps(t *testing.T) { } // Add different rev to the other - v4 := v2.Is(Revision("cookie monster")).(versionPair) + v4 := v2.Pair(Revision("cookie monster")).(versionPair) if v4.Matches(v3) { t.Errorf("%s should not match %s", gu(v4), gu(v3)) } @@ -458,7 +458,7 @@ func TestSemverVersionConstraintOps(t *testing.T) { } // Now add same rev to different versions, and things should line up - v5 := v2.Is(Revision("snuffleupagus")).(versionPair) + v5 := v2.Pair(Revision("snuffleupagus")).(versionPair) if !v5.Matches(v3) { t.Errorf("%s should match %s", gu(v5), gu(v3)) } @@ -484,9 +484,9 @@ func TestSemverVersionConstraintOps(t *testing.T) { cookie := Revision("cookie monster") o1 := NewBranch("master").(branchVersion) o2 := NewVersion("ab123").(plainVersion) - o3 := o1.Is(cookie).(versionPair) - o4 := o2.Is(cookie).(versionPair) - v6 := v1.Is(cookie).(versionPair) + o3 := o1.Pair(cookie).(versionPair) + o4 := o2.Pair(cookie).(versionPair) + v6 := v1.Pair(cookie).(versionPair) if v1.Matches(o1) { t.Errorf("%s (semver) should not match %s (branch) across types", v1, o1) @@ -586,9 +586,9 @@ func TestSemverConstraintOps(t *testing.T) { v3 := NewVersion("1.0.0").(semVersion) fozzie := Revision("fozzie bear") - v4 := v1.Is(fozzie).(versionPair) - v5 := v2.Is(fozzie).(versionPair) - v6 := v3.Is(fozzie).(versionPair) + v4 := v1.Pair(fozzie).(versionPair) + v5 := v2.Pair(fozzie).(versionPair) + v6 := v3.Pair(fozzie).(versionPair) // TODO(sdboyer) we can't use the same range as below b/c semver.rangeConstraint is // still an incomparable type @@ -702,9 +702,9 @@ func TestVersionUnion(t *testing.T) { rev := Revision("flooboofoobooo") v1 := NewBranch("master") v2 := NewBranch("test") - v3 := NewVersion("1.0.0").Is(rev) + v3 := NewVersion("1.0.0").Pair(rev) v4 := NewVersion("1.0.1") - v5 := NewVersion("v2.0.5").Is(Revision("notamatch")) + v5 := NewVersion("v2.0.5").Pair(Revision("notamatch")) uv1 := versionTypeUnion{v1, v4, rev} uv2 := versionTypeUnion{v2, v3} @@ -877,7 +877,7 @@ func TestTypedConstraintString(t *testing.T) { // Also tests typedVersionString(), as this nests down into that rev := Revision("flooboofoobooo") v1 := NewBranch("master") - v2 := NewBranch("test").Is(rev) + v2 := NewBranch("test").Pair(rev) v3 := NewVersion("1.0.1") v4 := NewVersion("v2.0.5") v5 := NewVersion("2.0.5.2") diff --git a/internal/gps/example.go b/internal/gps/example.go index 5a4c2d4f54..e91db30a79 100644 --- a/internal/gps/example.go +++ b/internal/gps/example.go @@ -70,6 +70,9 @@ func (a NaiveAnalyzer) DeriveManifestAndLock(path string, n gps.ProjectRoot) (gp // Reports the name and version of the analyzer. This is used internally as part // of gps' hashing memoization scheme. -func (a NaiveAnalyzer) Info() (name string, version int) { - return "example-analyzer", 1 +func (a NaiveAnalyzer) Info() gps.ProjectAnalyzerInfo { + return gps.ProjectAnalyzerInfo{ + Name: "example-analyzer", + Version: 1, + } } diff --git a/internal/gps/hash.go b/internal/gps/hash.go index 61c7b5fcba..43e84c711d 100644 --- a/internal/gps/hash.go +++ b/internal/gps/hash.go @@ -106,9 +106,9 @@ func (s *solver) writeHashingInputs(w io.Writer) { } writeString(hhAnalyzer) - an, av := s.rd.an.Info() - writeString(an) - writeString(strconv.Itoa(av)) + ai := s.rd.an.Info() + writeString(ai.Name) + writeString(strconv.Itoa(ai.Version)) } // bytes.Buffer wrapper that injects newlines after each call to Write(). diff --git a/internal/gps/lock.go b/internal/gps/lock.go index af728dd18f..db71c694e9 100644 --- a/internal/gps/lock.go +++ b/internal/gps/lock.go @@ -145,7 +145,7 @@ func (lp LockedProject) Version() Version { return lp.r } - return lp.v.Is(lp.r) + return lp.v.Pair(lp.r) } // Eq checks if two LockedProject instances are equal. diff --git a/internal/gps/lock_test.go b/internal/gps/lock_test.go index 23bb7f88a5..8b6ef6f99e 100644 --- a/internal/gps/lock_test.go +++ b/internal/gps/lock_test.go @@ -36,7 +36,7 @@ func TestLockedProjectsEq(t *testing.T) { NewLockedProject(mkPI("github.com/sdboyer/gps"), NewVersion("v0.10.0"), []string{"gps", "flugle"}), NewLockedProject(mkPI("foo"), NewVersion("nada"), []string{"foo"}), NewLockedProject(mkPI("github.com/sdboyer/gps"), NewVersion("v0.10.0"), []string{"flugle", "gps"}), - NewLockedProject(mkPI("github.com/sdboyer/gps"), NewVersion("v0.10.0").Is("278a227dfc3d595a33a77ff3f841fd8ca1bc8cd0"), []string{"gps"}), + NewLockedProject(mkPI("github.com/sdboyer/gps"), NewVersion("v0.10.0").Pair("278a227dfc3d595a33a77ff3f841fd8ca1bc8cd0"), []string{"gps"}), NewLockedProject(mkPI("github.com/sdboyer/gps"), NewVersion("v0.11.0"), []string{"gps"}), NewLockedProject(mkPI("github.com/sdboyer/gps"), Revision("278a227dfc3d595a33a77ff3f841fd8ca1bc8cd0"), []string{"gps"}), } @@ -81,9 +81,9 @@ func TestLockedProjectsEq(t *testing.T) { } func TestLocksAreEq(t *testing.T) { - gpl := NewLockedProject(mkPI("github.com/sdboyer/gps"), NewVersion("v0.10.0").Is("278a227dfc3d595a33a77ff3f841fd8ca1bc8cd0"), []string{"gps"}) + gpl := NewLockedProject(mkPI("github.com/sdboyer/gps"), NewVersion("v0.10.0").Pair("278a227dfc3d595a33a77ff3f841fd8ca1bc8cd0"), []string{"gps"}) svpl := NewLockedProject(mkPI("github.com/Masterminds/semver"), NewVersion("v2.0.0"), []string{"semver"}) - bbbt := NewLockedProject(mkPI("github.com/beeblebrox/browntown"), NewBranch("master").Is("63fc17eb7966a6f4cc0b742bf42731c52c4ac740"), []string{"browntown", "smoochies"}) + bbbt := NewLockedProject(mkPI("github.com/beeblebrox/browntown"), NewBranch("master").Pair("63fc17eb7966a6f4cc0b742bf42731c52c4ac740"), []string{"browntown", "smoochies"}) l1 := solution{ hd: []byte("foo"), diff --git a/internal/gps/manager_test.go b/internal/gps/manager_test.go index 9bf5a1c415..d505846677 100644 --- a/internal/gps/manager_test.go +++ b/internal/gps/manager_test.go @@ -27,8 +27,11 @@ func (naiveAnalyzer) DeriveManifestAndLock(string, ProjectRoot) (Manifest, Lock, return nil, nil, nil } -func (a naiveAnalyzer) Info() (name string, version int) { - return "naive-analyzer", 1 +func (a naiveAnalyzer) Info() ProjectAnalyzerInfo { + return ProjectAnalyzerInfo{ + Name: "naive-analyzer", + Version: 1, + } } func mkNaiveSM(t *testing.T) (*SourceMgr, func()) { @@ -148,13 +151,13 @@ func TestSourceInit(t *testing.T) { t.Errorf("Expected seven version results from the test repo, got %v", len(pvl)) } else { expected := []PairedVersion{ - NewVersion("v2.0.0").Is(Revision("4a54adf81c75375d26d376459c00d5ff9b703e5e")), - NewVersion("v1.1.0").Is(Revision("b2cb48dda625f6640b34d9ffb664533359ac8b91")), - NewVersion("v1.0.0").Is(Revision("bf85021c0405edbc4f3648b0603818d641674f72")), - newDefaultBranch("master").Is(Revision("bf85021c0405edbc4f3648b0603818d641674f72")), - NewBranch("v1").Is(Revision("e3777f683305eafca223aefe56b4e8ecf103f467")), - NewBranch("v1.1").Is(Revision("f1fbc520489a98306eb28c235204e39fa8a89c84")), - NewBranch("v3").Is(Revision("4a54adf81c75375d26d376459c00d5ff9b703e5e")), + NewVersion("v2.0.0").Pair(Revision("4a54adf81c75375d26d376459c00d5ff9b703e5e")), + NewVersion("v1.1.0").Pair(Revision("b2cb48dda625f6640b34d9ffb664533359ac8b91")), + NewVersion("v1.0.0").Pair(Revision("bf85021c0405edbc4f3648b0603818d641674f72")), + newDefaultBranch("master").Pair(Revision("bf85021c0405edbc4f3648b0603818d641674f72")), + NewBranch("v1").Pair(Revision("e3777f683305eafca223aefe56b4e8ecf103f467")), + NewBranch("v1.1").Pair(Revision("f1fbc520489a98306eb28c235204e39fa8a89c84")), + NewBranch("v3").Pair(Revision("4a54adf81c75375d26d376459c00d5ff9b703e5e")), } // SourceManager itself doesn't guarantee ordering; sort them here so we @@ -186,13 +189,13 @@ func TestSourceInit(t *testing.T) { t.Errorf("Expected seven version results from the test repo, got %v", len(vl)) } else { expected := []Version{ - NewVersion("v2.0.0").Is(Revision("4a54adf81c75375d26d376459c00d5ff9b703e5e")), - NewVersion("v1.1.0").Is(Revision("b2cb48dda625f6640b34d9ffb664533359ac8b91")), - NewVersion("v1.0.0").Is(Revision("bf85021c0405edbc4f3648b0603818d641674f72")), - newDefaultBranch("master").Is(Revision("bf85021c0405edbc4f3648b0603818d641674f72")), - NewBranch("v1").Is(Revision("e3777f683305eafca223aefe56b4e8ecf103f467")), - NewBranch("v1.1").Is(Revision("f1fbc520489a98306eb28c235204e39fa8a89c84")), - NewBranch("v3").Is(Revision("4a54adf81c75375d26d376459c00d5ff9b703e5e")), + NewVersion("v2.0.0").Pair(Revision("4a54adf81c75375d26d376459c00d5ff9b703e5e")), + NewVersion("v1.1.0").Pair(Revision("b2cb48dda625f6640b34d9ffb664533359ac8b91")), + NewVersion("v1.0.0").Pair(Revision("bf85021c0405edbc4f3648b0603818d641674f72")), + newDefaultBranch("master").Pair(Revision("bf85021c0405edbc4f3648b0603818d641674f72")), + NewBranch("v1").Pair(Revision("e3777f683305eafca223aefe56b4e8ecf103f467")), + NewBranch("v1.1").Pair(Revision("f1fbc520489a98306eb28c235204e39fa8a89c84")), + NewBranch("v3").Pair(Revision("4a54adf81c75375d26d376459c00d5ff9b703e5e")), } for k, e := range expected { @@ -273,9 +276,9 @@ func TestDefaultBranchAssignment(t *testing.T) { brev := Revision("fda020843ac81352004b9dca3fcccdd517600149") mrev := Revision("9f9c3a591773d9b28128309ac7a9a72abcab267d") expected := []PairedVersion{ - NewBranch("branchone").Is(brev), - NewBranch("otherbranch").Is(brev), - NewBranch("master").Is(mrev), + NewBranch("branchone").Pair(brev), + NewBranch("otherbranch").Pair(brev), + NewBranch("master").Pair(mrev), } SortPairedForUpgrade(v) diff --git a/internal/gps/maybe_source.go b/internal/gps/maybe_source.go index 9b2927ae7d..9151f17b1f 100644 --- a/internal/gps/maybe_source.go +++ b/internal/gps/maybe_source.go @@ -76,15 +76,19 @@ func (sf sourceFailures) Error() string { return buf.String() } +// sourceCachePath returns a url-sanitized source cache dir path. +func sourceCachePath(cacheDir, sourceURL string) string { + return filepath.Join(cacheDir, "sources", sanitizer.Replace(sourceURL)) +} + type maybeGitSource struct { url *url.URL } func (m maybeGitSource) try(ctx context.Context, cachedir string, c singleSourceCache, superv *supervisor) (source, sourceState, error) { ustr := m.url.String() - path := filepath.Join(cachedir, "sources", sanitizer.Replace(ustr)) - r, err := newCtxRepo(vcs.Git, ustr, path) + r, err := newCtxRepo(vcs.Git, ustr, sourceCachePath(cachedir, ustr)) if err != nil { return nil, 0, unwrapVcsErr(err) @@ -140,7 +144,7 @@ func (m maybeGopkginSource) try(ctx context.Context, cachedir string, c singleSo // We don't actually need a fully consistent transform into the on-disk path // - just something that's unique to the particular gopkg.in domain context. // So, it's OK to just dumb-join the scheme with the path. - path := filepath.Join(cachedir, "sources", sanitizer.Replace(m.url.Scheme+"/"+m.opath)) + path := sourceCachePath(cachedir, m.url.Scheme+"/"+m.opath) ustr := m.url.String() r, err := newCtxRepo(vcs.Git, ustr, path) @@ -189,9 +193,8 @@ type maybeBzrSource struct { func (m maybeBzrSource) try(ctx context.Context, cachedir string, c singleSourceCache, superv *supervisor) (source, sourceState, error) { ustr := m.url.String() - path := filepath.Join(cachedir, "sources", sanitizer.Replace(ustr)) - r, err := newCtxRepo(vcs.Bzr, ustr, path) + r, err := newCtxRepo(vcs.Bzr, ustr, sourceCachePath(cachedir, ustr)) if err != nil { return nil, 0, unwrapVcsErr(err) @@ -231,9 +234,8 @@ type maybeHgSource struct { func (m maybeHgSource) try(ctx context.Context, cachedir string, c singleSourceCache, superv *supervisor) (source, sourceState, error) { ustr := m.url.String() - path := filepath.Join(cachedir, "sources", sanitizer.Replace(ustr)) - r, err := newCtxRepo(vcs.Hg, ustr, path) + r, err := newCtxRepo(vcs.Hg, ustr, sourceCachePath(cachedir, ustr)) if err != nil { return nil, 0, unwrapVcsErr(err) diff --git a/internal/gps/result.go b/internal/gps/result.go index 03948ff5bd..eb9d6ad0e1 100644 --- a/internal/gps/result.go +++ b/internal/gps/result.go @@ -35,11 +35,8 @@ type solution struct { // The hash digest of the input opts hd []byte - // The analyzer name - analyzerName string - - // The analyzer version - analyzerVersion int + // The analyzer info + analyzerInfo ProjectAnalyzerInfo // The solver used in producing this solution solv Solver @@ -95,11 +92,11 @@ func (r solution) InputHash() []byte { } func (r solution) AnalyzerName() string { - return r.analyzerName + return r.analyzerInfo.Name } func (r solution) AnalyzerVersion() int { - return r.analyzerVersion + return r.analyzerInfo.Version } func (r solution) SolverName() string { diff --git a/internal/gps/result_test.go b/internal/gps/result_test.go index da73b47b34..cc96a8e2cb 100644 --- a/internal/gps/result_test.go +++ b/internal/gps/result_test.go @@ -26,22 +26,22 @@ func init() { p: []LockedProject{ pa2lp(atom{ id: pi("github.com/sdboyer/testrepo"), - v: NewBranch("master").Is(Revision("4d59fb584b15a94d7401e356d2875c472d76ef45")), + v: NewBranch("master").Pair(Revision("4d59fb584b15a94d7401e356d2875c472d76ef45")), }, nil), pa2lp(atom{ id: pi("github.com/Masterminds/VCSTestRepo"), - v: NewVersion("1.0.0").Is(Revision("30605f6ac35fcb075ad0bfa9296f90a7d891523e")), + v: NewVersion("1.0.0").Pair(Revision("30605f6ac35fcb075ad0bfa9296f90a7d891523e")), }, nil), }, } - basicResult.analyzerName, basicResult.analyzerVersion = (naiveAnalyzer{}).Info() + basicResult.analyzerInfo = (naiveAnalyzer{}).Info() // Just in case something needs punishing, kubernetes offers a complex, // real-world set of dependencies, and this revision is known to work. /* _ = atom{ id: pi("github.com/kubernetes/kubernetes"), - v: NewVersion("1.0.0").Is(Revision("528f879e7d3790ea4287687ef0ab3f2a01cc2718")), + v: NewVersion("1.0.0").Pair(Revision("528f879e7d3790ea4287687ef0ab3f2a01cc2718")), } */ } @@ -66,15 +66,15 @@ func testWriteDepTree(t *testing.T) { p: []LockedProject{ pa2lp(atom{ id: pi("github.com/sdboyer/testrepo"), - v: NewBranch("master").Is(Revision("4d59fb584b15a94d7401e356d2875c472d76ef45")), + v: NewBranch("master").Pair(Revision("4d59fb584b15a94d7401e356d2875c472d76ef45")), }, nil), pa2lp(atom{ id: pi("launchpad.net/govcstestbzrrepo"), - v: NewVersion("1.0.0").Is(Revision("matt@mattfarina.com-20150731135137-pbphasfppmygpl68")), + v: NewVersion("1.0.0").Pair(Revision("matt@mattfarina.com-20150731135137-pbphasfppmygpl68")), }, nil), pa2lp(atom{ id: pi("bitbucket.org/sdboyer/withbm"), - v: NewVersion("v1.0.0").Is(Revision("aa110802a0c64195d0a6c375c9f66668827c90b4")), + v: NewVersion("v1.0.0").Pair(Revision("aa110802a0c64195d0a6c375c9f66668827c90b4")), }, nil), }, } diff --git a/internal/gps/selection.go b/internal/gps/selection.go index 2e464447bb..e29d83fe53 100644 --- a/internal/gps/selection.go +++ b/internal/gps/selection.go @@ -7,7 +7,7 @@ package gps type selection struct { projects []selected deps map[ProjectRoot][]dependency - vu versionUnifier + vu *versionUnifier } type selected struct { diff --git a/internal/gps/solve_basic_test.go b/internal/gps/solve_basic_test.go index f36dc7ee51..2728c44145 100644 --- a/internal/gps/solve_basic_test.go +++ b/internal/gps/solve_basic_test.go @@ -113,7 +113,7 @@ func mkAtom(info string) atom { } if rev != "" { - v = v.(UnpairedVersion).Is(rev) + v = v.(UnpairedVersion).Pair(rev) } return atom{ @@ -164,7 +164,7 @@ func mkPCstrnt(info string) ProjectConstraint { // Of course, this *will* panic if the predicate is a revision or a // semver constraint, neither of which implement UnpairedVersion. This // is as intended, to prevent bad data from entering the system. - c = c.(UnpairedVersion).Is(rev) + c = c.(UnpairedVersion).Pair(rev) } return ProjectConstraint{ @@ -293,7 +293,7 @@ func mkrevlock(pairs ...string) fixLock { l := make(fixLock, 0) for _, s := range pairs { pa := mkAtom(s) - l = append(l, NewLockedProject(pa.id, pa.v.(PairedVersion).Underlying(), nil)) + l = append(l, NewLockedProject(pa.id, pa.v.(PairedVersion).Revision(), nil)) } return l @@ -1419,7 +1419,7 @@ func (sm *depspecSourceManager) ExternalReach(id ProjectIdentifier, v Version) ( func (sm *depspecSourceManager) ListPackages(id ProjectIdentifier, v Version) (pkgtree.PackageTree, error) { pid := pident{n: ProjectRoot(id.normalizedSource()), v: v} - if pv, ok := v.(PairedVersion); ok && pv.Underlying() == "FAKEREV" { + if pv, ok := v.(PairedVersion); ok && pv.Revision() == "FAKEREV" { // An empty rev may come in here because that's what we produce in // ListVersions(). If that's what we see, then just pretend like we have // an unpaired. @@ -1482,7 +1482,7 @@ func (sm *depspecSourceManager) ListVersions(id ProjectIdentifier) ([]PairedVers case UnpairedVersion: // Dummy revision; if the fixture doesn't provide it, we know // the test doesn't need revision info, anyway. - pvl = append(pvl, tv.Is(Revision("FAKEREV"))) + pvl = append(pvl, tv.Pair(Revision("FAKEREV"))) default: panic(fmt.Sprintf("unreachable: type of version was %#v for spec %s", ds.v, id.errString())) } @@ -1568,7 +1568,7 @@ func (b *depspecBridge) listVersions(id ProjectIdentifier) ([]Version, error) { // remove the underlying component. vl := make([]Version, 0, len(pvl)) for _, v := range pvl { - if v.Underlying() == "FAKEREV" { + if v.Revision() == "FAKEREV" { vl = append(vl, v.Unpair()) } else { vl = append(vl, v) diff --git a/internal/gps/solve_test.go b/internal/gps/solve_test.go index 29e4b619fa..b2ccc087d3 100644 --- a/internal/gps/solve_test.go +++ b/internal/gps/solve_test.go @@ -176,7 +176,7 @@ func fixtureSolveSimpleChecks(fix specfix, soln Solution, err error, t *testing. pv := func(v Version) string { if pv, ok := v.(PairedVersion); ok { - return fmt.Sprintf("%s (%s)", pv.Unpair(), pv.Underlying()) + return fmt.Sprintf("%s (%s)", pv.Unpair(), pv.Revision()) } return v.String() } diff --git a/internal/gps/solver.go b/internal/gps/solver.go index 6d6ae2c54d..119586ac55 100644 --- a/internal/gps/solver.go +++ b/internal/gps/solver.go @@ -126,7 +126,7 @@ type solver struct { // A versionUnifier, to facilitate cross-type version comparison and set // operations. - vUnify versionUnifier + vUnify *versionUnifier // A stack containing projects and packages that are currently "selected" - // that is, they have passed all satisfiability checks, and are part of the @@ -301,7 +301,7 @@ func Prepare(params SolveParameters, sm SourceManager) (Solver, error) { if err != nil { return nil, err } - s.vUnify = versionUnifier{ + s.vUnify = &versionUnifier{ b: s.b, } @@ -405,7 +405,7 @@ func (s *solver) Solve() (Solution, error) { att: s.attempts, solv: s, } - soln.analyzerName, soln.analyzerVersion = s.rd.an.Info() + soln.analyzerInfo = s.rd.an.Info() soln.hd = s.HashInputs() // Convert ProjectAtoms into LockedProjects diff --git a/internal/gps/source.go b/internal/gps/source.go index 85cdb23eb4..d687a1b687 100644 --- a/internal/gps/source.go +++ b/internal/gps/source.go @@ -276,7 +276,7 @@ func (sg *sourceGateway) getManifestAndLock(ctx context.Context, pr ProjectRoot, return nil, nil, err } - m, l, has := sg.cache.getManifestAndLock(r, an) + m, l, has := sg.cache.getManifestAndLock(r, an.Info()) if has { return m, l, nil } @@ -286,8 +286,7 @@ func (sg *sourceGateway) getManifestAndLock(ctx context.Context, pr ProjectRoot, return nil, nil, err } - name, vers := an.Info() - label := fmt.Sprintf("%s:%s.%v", sg.src.upstreamURL(), name, vers) + label := fmt.Sprintf("%s:%s", sg.src.upstreamURL(), an.Info()) err = sg.suprvsr.do(ctx, label, ctGetManifestAndLock, func(ctx context.Context) error { m, l, err = sg.src.getManifestAndLock(ctx, pr, r, an) return err @@ -317,7 +316,7 @@ func (sg *sourceGateway) getManifestAndLock(ctx context.Context, pr ProjectRoot, return nil, nil, err } - sg.cache.setManifestAndLock(r, an, m, l) + sg.cache.setManifestAndLock(r, an.Info(), m, l) return m, l, nil } diff --git a/internal/gps/source_cache.go b/internal/gps/source_cache.go index 3b46948d16..b0b5e073aa 100644 --- a/internal/gps/source_cache.go +++ b/internal/gps/source_cache.go @@ -16,11 +16,11 @@ import ( type singleSourceCache interface { // Store the manifest and lock information for a given revision, as defined by // a particular ProjectAnalyzer. - setManifestAndLock(Revision, ProjectAnalyzer, Manifest, Lock) + setManifestAndLock(Revision, ProjectAnalyzerInfo, Manifest, Lock) // Get the manifest and lock information for a given revision, as defined by // a particular ProjectAnalyzer. - getManifestAndLock(Revision, ProjectAnalyzer) (Manifest, Lock, bool) + getManifestAndLock(Revision, ProjectAnalyzerInfo) (Manifest, Lock, bool) // Store a PackageTree for a given revision. setPackageTree(Revision, pkgtree.PackageTree) @@ -63,7 +63,7 @@ type singleSourceCache interface { type singleSourceCacheMemory struct { mut sync.RWMutex // protects all maps - infos map[ProjectAnalyzer]map[Revision]projectInfo + infos map[ProjectAnalyzerInfo]map[Revision]projectInfo ptrees map[Revision]pkgtree.PackageTree vMap map[UnpairedVersion]Revision rMap map[Revision][]UnpairedVersion @@ -71,7 +71,7 @@ type singleSourceCacheMemory struct { func newMemoryCache() singleSourceCache { return &singleSourceCacheMemory{ - infos: make(map[ProjectAnalyzer]map[Revision]projectInfo), + infos: make(map[ProjectAnalyzerInfo]map[Revision]projectInfo), ptrees: make(map[Revision]pkgtree.PackageTree), vMap: make(map[UnpairedVersion]Revision), rMap: make(map[Revision][]UnpairedVersion), @@ -83,12 +83,12 @@ type projectInfo struct { Lock } -func (c *singleSourceCacheMemory) setManifestAndLock(r Revision, an ProjectAnalyzer, m Manifest, l Lock) { +func (c *singleSourceCacheMemory) setManifestAndLock(r Revision, pai ProjectAnalyzerInfo, m Manifest, l Lock) { c.mut.Lock() - inner, has := c.infos[an] + inner, has := c.infos[pai] if !has { inner = make(map[Revision]projectInfo) - c.infos[an] = inner + c.infos[pai] = inner } inner[r] = projectInfo{Manifest: m, Lock: l} @@ -100,11 +100,11 @@ func (c *singleSourceCacheMemory) setManifestAndLock(r Revision, an ProjectAnaly c.mut.Unlock() } -func (c *singleSourceCacheMemory) getManifestAndLock(r Revision, an ProjectAnalyzer) (Manifest, Lock, bool) { +func (c *singleSourceCacheMemory) getManifestAndLock(r Revision, pai ProjectAnalyzerInfo) (Manifest, Lock, bool) { c.mut.Lock() defer c.mut.Unlock() - inner, has := c.infos[an] + inner, has := c.infos[pai] if !has { return nil, nil, false } @@ -149,7 +149,7 @@ func (c *singleSourceCacheMemory) storeVersionMap(versionList []PairedVersion, f for _, v := range versionList { pv := v.(PairedVersion) - u, r := pv.Unpair(), pv.Underlying() + u, r := pv.Unpair(), pv.Revision() c.vMap[u] = r c.rMap[r] = append(c.rMap[r], u) } @@ -174,7 +174,7 @@ func (c *singleSourceCacheMemory) getVersionsFor(r Revision) ([]UnpairedVersion, func (c *singleSourceCacheMemory) getAllVersions() []PairedVersion { vlist := make([]PairedVersion, 0, len(c.vMap)) for v, r := range c.vMap { - vlist = append(vlist, v.Is(r)) + vlist = append(vlist, v.Pair(r)) } return vlist } @@ -191,7 +191,7 @@ func (c *singleSourceCacheMemory) toRevision(v Version) (Revision, bool) { case Revision: return t, true case PairedVersion: - return t.Underlying(), true + return t.Revision(), true case UnpairedVersion: c.mut.Lock() r, has := c.vMap[t] diff --git a/internal/gps/source_manager.go b/internal/gps/source_manager.go index 0119fca472..6f60c6dad6 100644 --- a/internal/gps/source_manager.go +++ b/internal/gps/source_manager.go @@ -87,8 +87,19 @@ type ProjectAnalyzer interface { // expected files containing Manifest and Lock data are merely absent. DeriveManifestAndLock(path string, importRoot ProjectRoot) (Manifest, Lock, error) - // Report the name and version of this ProjectAnalyzer. - Info() (name string, version int) + // Info reports this project analyzer's info. + Info() ProjectAnalyzerInfo +} + +// ProjectAnalyzerInfo indicates a ProjectAnalyzer's name and version. +type ProjectAnalyzerInfo struct { + Name string + Version int +} + +// String returns a string like: "." +func (p ProjectAnalyzerInfo) String() string { + return fmt.Sprintf("%s.%d", p.Name, p.Version) } // SourceMgr is the default SourceManager for gps. diff --git a/internal/gps/source_test.go b/internal/gps/source_test.go index 591ece626a..b5679761a8 100644 --- a/internal/gps/source_test.go +++ b/internal/gps/source_test.go @@ -81,10 +81,10 @@ func testSourceGateway(t *testing.T) { } else { SortPairedForUpgrade(vlist) evl := []PairedVersion{ - NewVersion("v1.0.0").Is(Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf")), - NewVersion("v0.8.1").Is(Revision("3f4c3bea144e112a69bbe5d8d01c1b09a544253f")), - NewVersion("v0.8.0").Is(Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf")), - newDefaultBranch("master").Is(Revision("3f4c3bea144e112a69bbe5d8d01c1b09a544253f")), + NewVersion("v1.0.0").Pair(Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf")), + NewVersion("v0.8.1").Pair(Revision("3f4c3bea144e112a69bbe5d8d01c1b09a544253f")), + NewVersion("v0.8.0").Pair(Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf")), + newDefaultBranch("master").Pair(Revision("3f4c3bea144e112a69bbe5d8d01c1b09a544253f")), } if !reflect.DeepEqual(vlist, evl) { t.Fatalf("Version list was not what we expected:\n\t(GOT): %s\n\t(WNT): %s", vlist, evl) diff --git a/internal/gps/vcs_source.go b/internal/gps/vcs_source.go index 5533f5e843..397cfd1b32 100644 --- a/internal/gps/vcs_source.go +++ b/internal/gps/vcs_source.go @@ -229,7 +229,7 @@ func (s *gitSource) listVersions(ctx context.Context) (vlist []PairedVersion, er v = branchVersion{ name: n, isDefault: isdef, - }.Is(rev).(PairedVersion) + }.Pair(rev).(PairedVersion) vlist[uniq] = v uniq++ @@ -247,7 +247,7 @@ func (s *gitSource) listVersions(ctx context.Context) (vlist []PairedVersion, er // version first. Which should be impossible, but this // covers us in case of weirdness, anyway. } - v = NewVersion(vstr).Is(Revision(pair[:40])).(PairedVersion) + v = NewVersion(vstr).Pair(Revision(pair[:40])).(PairedVersion) smap[vstr] = true vlist[uniq] = v uniq++ @@ -265,7 +265,7 @@ func (s *gitSource) listVersions(ctx context.Context) (vlist []PairedVersion, er if bv, ok := pv.Unpair().(branchVersion); ok { if bv.name != "master" && bv.isDefault { bv.isDefault = false - vlist[k] = bv.Is(pv.Underlying()) + vlist[k] = bv.Pair(pv.Revision()) } } } @@ -341,7 +341,7 @@ func (s *gopkginSource) listVersions(ctx context.Context) ([]PairedVersion, erro vlist[dbranch] = branchVersion{ name: dbv.v.(branchVersion).name, isDefault: true, - }.Is(dbv.r) + }.Pair(dbv.r) } return vlist, nil @@ -386,13 +386,13 @@ func (s *bzrSource) listVersions(ctx context.Context) ([]PairedVersion, error) { idx := bytes.IndexByte(line, 32) // space v := NewVersion(string(line[:idx])) r := Revision(bytes.TrimSpace(line[idx:])) - vlist = append(vlist, v.Is(r)) + vlist = append(vlist, v.Pair(r)) } // Last, add the default branch, hardcoding the visual representation of it // that bzr uses when operating in the workflow mode we're using. v := newDefaultBranch("(default)") - vlist = append(vlist, v.Is(Revision(string(branchrev)))) + vlist = append(vlist, v.Pair(Revision(string(branchrev)))) return vlist, nil } @@ -443,7 +443,7 @@ func (s *hgSource) listVersions(ctx context.Context) ([]PairedVersion, error) { } idx := bytes.IndexByte(pair[0], 32) // space - v := NewVersion(string(pair[0][:idx])).Is(Revision(pair[1])).(PairedVersion) + v := NewVersion(string(pair[0][:idx])).Pair(Revision(pair[1])).(PairedVersion) vlist = append(vlist, v) } @@ -475,9 +475,9 @@ func (s *hgSource) listVersions(ctx context.Context) ([]PairedVersion, error) { var v PairedVersion if str == "@" { magicAt = true - v = newDefaultBranch(str).Is(Revision(pair[1])).(PairedVersion) + v = newDefaultBranch(str).Pair(Revision(pair[1])).(PairedVersion) } else { - v = NewBranch(str).Is(Revision(pair[1])).(PairedVersion) + v = NewBranch(str).Pair(Revision(pair[1])).(PairedVersion) } vlist = append(vlist, v) } @@ -504,9 +504,9 @@ func (s *hgSource) listVersions(ctx context.Context) ([]PairedVersion, error) { // "default" branch, then mark it as default branch var v PairedVersion if !magicAt && str == "default" { - v = newDefaultBranch(str).Is(Revision(pair[1])).(PairedVersion) + v = newDefaultBranch(str).Pair(Revision(pair[1])).(PairedVersion) } else { - v = NewBranch(str).Is(Revision(pair[1])).(PairedVersion) + v = NewBranch(str).Pair(Revision(pair[1])).(PairedVersion) } vlist = append(vlist, v) } diff --git a/internal/gps/vcs_source_test.go b/internal/gps/vcs_source_test.go index 8fff2d7899..f819c5baeb 100644 --- a/internal/gps/vcs_source_test.go +++ b/internal/gps/vcs_source_test.go @@ -105,13 +105,13 @@ func testGitSourceInteractions(t *testing.T) { } else { SortForUpgrade(vlist) evl := []Version{ - NewVersion("v2.0.0").Is(Revision("4a54adf81c75375d26d376459c00d5ff9b703e5e")), - NewVersion("v1.1.0").Is(Revision("b2cb48dda625f6640b34d9ffb664533359ac8b91")), - NewVersion("v1.0.0").Is(Revision("bf85021c0405edbc4f3648b0603818d641674f72")), - newDefaultBranch("master").Is(Revision("bf85021c0405edbc4f3648b0603818d641674f72")), - NewBranch("v1").Is(Revision("e3777f683305eafca223aefe56b4e8ecf103f467")), - NewBranch("v1.1").Is(Revision("f1fbc520489a98306eb28c235204e39fa8a89c84")), - NewBranch("v3").Is(Revision("4a54adf81c75375d26d376459c00d5ff9b703e5e")), + NewVersion("v2.0.0").Pair(Revision("4a54adf81c75375d26d376459c00d5ff9b703e5e")), + NewVersion("v1.1.0").Pair(Revision("b2cb48dda625f6640b34d9ffb664533359ac8b91")), + NewVersion("v1.0.0").Pair(Revision("bf85021c0405edbc4f3648b0603818d641674f72")), + newDefaultBranch("master").Pair(Revision("bf85021c0405edbc4f3648b0603818d641674f72")), + NewBranch("v1").Pair(Revision("e3777f683305eafca223aefe56b4e8ecf103f467")), + NewBranch("v1.1").Pair(Revision("f1fbc520489a98306eb28c235204e39fa8a89c84")), + NewBranch("v3").Pair(Revision("4a54adf81c75375d26d376459c00d5ff9b703e5e")), } if !reflect.DeepEqual(vlist, evl) { t.Errorf("Version list was not what we expected:\n\t(GOT): %s\n\t(WNT): %s", vlist, evl) @@ -193,7 +193,7 @@ func testGopkginSourceInteractions(t *testing.T) { } // check that an expected rev is present - rev := evl[0].(PairedVersion).Underlying() + rev := evl[0].(PairedVersion).Revision() is, err := src.revisionPresentIn(rev) if err != nil { t.Errorf("Unexpected error while checking revision presence: %s", err) @@ -246,31 +246,31 @@ func testGopkginSourceInteractions(t *testing.T) { wg.Add(4) go func() { tfunc("gopkg.in/sdboyer/gpkt.v1", "github.com/sdboyer/gpkt", 1, []Version{ - NewVersion("v1.1.0").Is(Revision("b2cb48dda625f6640b34d9ffb664533359ac8b91")), - NewVersion("v1.0.0").Is(Revision("bf85021c0405edbc4f3648b0603818d641674f72")), - newDefaultBranch("v1.1").Is(Revision("f1fbc520489a98306eb28c235204e39fa8a89c84")), - NewBranch("v1").Is(Revision("e3777f683305eafca223aefe56b4e8ecf103f467")), + NewVersion("v1.1.0").Pair(Revision("b2cb48dda625f6640b34d9ffb664533359ac8b91")), + NewVersion("v1.0.0").Pair(Revision("bf85021c0405edbc4f3648b0603818d641674f72")), + newDefaultBranch("v1.1").Pair(Revision("f1fbc520489a98306eb28c235204e39fa8a89c84")), + NewBranch("v1").Pair(Revision("e3777f683305eafca223aefe56b4e8ecf103f467")), }) wg.Done() }() go func() { tfunc("gopkg.in/sdboyer/gpkt.v2", "github.com/sdboyer/gpkt", 2, []Version{ - NewVersion("v2.0.0").Is(Revision("4a54adf81c75375d26d376459c00d5ff9b703e5e")), + NewVersion("v2.0.0").Pair(Revision("4a54adf81c75375d26d376459c00d5ff9b703e5e")), }) wg.Done() }() go func() { tfunc("gopkg.in/sdboyer/gpkt.v3", "github.com/sdboyer/gpkt", 3, []Version{ - newDefaultBranch("v3").Is(Revision("4a54adf81c75375d26d376459c00d5ff9b703e5e")), + newDefaultBranch("v3").Pair(Revision("4a54adf81c75375d26d376459c00d5ff9b703e5e")), }) wg.Done() }() go func() { tfunc("github.com/sdboyer/gpkt2.v1-unstable", "github.com/sdboyer/gpkt2", 1, []Version{ - newDefaultBranch("v1-unstable").Is(Revision("24de0be8f4a0b8a44321562117749b257bfcef69")), + newDefaultBranch("v1-unstable").Pair(Revision("24de0be8f4a0b8a44321562117749b257bfcef69")), }) wg.Done() }() @@ -336,8 +336,8 @@ func testBzrSourceInteractions(t *testing.T) { t.Errorf("Expected %s as source URL, got %s", un, src.upstreamURL()) } evl := []Version{ - NewVersion("1.0.0").Is(Revision("matt@mattfarina.com-20150731135137-pbphasfppmygpl68")), - newDefaultBranch("(default)").Is(Revision("matt@mattfarina.com-20150731135137-pbphasfppmygpl68")), + NewVersion("1.0.0").Pair(Revision("matt@mattfarina.com-20150731135137-pbphasfppmygpl68")), + newDefaultBranch("(default)").Pair(Revision("matt@mattfarina.com-20150731135137-pbphasfppmygpl68")), } // check that an expected rev is present @@ -501,20 +501,20 @@ func testHgSourceInteractions(t *testing.T) { donech := make(chan struct{}) go func() { tfunc("bitbucket.org/sdboyer/withbm", []Version{ - NewVersion("v1.0.0").Is(Revision("aa110802a0c64195d0a6c375c9f66668827c90b4")), - newDefaultBranch("@").Is(Revision("b10d05d581e5401f383e48ccfeb84b48fde99d06")), - NewBranch("another").Is(Revision("b10d05d581e5401f383e48ccfeb84b48fde99d06")), - NewBranch("default").Is(Revision("3d466f437f6616da594bbab6446cc1cb4328d1bb")), - NewBranch("newbranch").Is(Revision("5e2a01be9aee942098e44590ae545c7143da9675")), + NewVersion("v1.0.0").Pair(Revision("aa110802a0c64195d0a6c375c9f66668827c90b4")), + newDefaultBranch("@").Pair(Revision("b10d05d581e5401f383e48ccfeb84b48fde99d06")), + NewBranch("another").Pair(Revision("b10d05d581e5401f383e48ccfeb84b48fde99d06")), + NewBranch("default").Pair(Revision("3d466f437f6616da594bbab6446cc1cb4328d1bb")), + NewBranch("newbranch").Pair(Revision("5e2a01be9aee942098e44590ae545c7143da9675")), }) close(donech) }() tfunc("bitbucket.org/sdboyer/nobm", []Version{ - NewVersion("v1.0.0").Is(Revision("aa110802a0c64195d0a6c375c9f66668827c90b4")), - newDefaultBranch("default").Is(Revision("3d466f437f6616da594bbab6446cc1cb4328d1bb")), - NewBranch("another").Is(Revision("b10d05d581e5401f383e48ccfeb84b48fde99d06")), - NewBranch("newbranch").Is(Revision("5e2a01be9aee942098e44590ae545c7143da9675")), + NewVersion("v1.0.0").Pair(Revision("aa110802a0c64195d0a6c375c9f66668827c90b4")), + newDefaultBranch("default").Pair(Revision("3d466f437f6616da594bbab6446cc1cb4328d1bb")), + NewBranch("another").Pair(Revision("b10d05d581e5401f383e48ccfeb84b48fde99d06")), + NewBranch("newbranch").Pair(Revision("5e2a01be9aee942098e44590ae545c7143da9675")), }) <-donech diff --git a/internal/gps/version.go b/internal/gps/version.go index 060c5b6971..29b71334df 100644 --- a/internal/gps/version.go +++ b/internal/gps/version.go @@ -47,8 +47,8 @@ type Version interface { type PairedVersion interface { Version - // Underlying returns the immutable Revision that identifies this Version. - Underlying() Revision + // Revision returns the immutable Revision that identifies this Version. + Revision() Revision // Unpair returns the surface-level UnpairedVersion that half of the pair. // @@ -64,9 +64,9 @@ type PairedVersion interface { // VersionPair by indicating the version's corresponding, underlying Revision. type UnpairedVersion interface { Version - // Is takes the underlying Revision that this UnpairedVersion corresponds + // Pair takes the underlying Revision that this UnpairedVersion corresponds // to and unites them into a PairedVersion. - Is(Revision) PairedVersion + Pair(Revision) PairedVersion // Ensures it is impossible to be both a PairedVersion and an // UnpairedVersion _pair(bool) @@ -267,7 +267,7 @@ func (v branchVersion) Intersect(c Constraint) Constraint { return none } -func (v branchVersion) Is(r Revision) PairedVersion { +func (v branchVersion) Pair(r Revision) PairedVersion { return versionPair{ v: v, r: r, @@ -348,7 +348,7 @@ func (v plainVersion) Intersect(c Constraint) Constraint { return none } -func (v plainVersion) Is(r Revision) PairedVersion { +func (v plainVersion) Pair(r Revision) PairedVersion { return versionPair{ v: v, r: r, @@ -439,7 +439,7 @@ func (v semVersion) Intersect(c Constraint) Constraint { return none } -func (v semVersion) Is(r Revision) PairedVersion { +func (v semVersion) Pair(r Revision) PairedVersion { return versionPair{ v: v, r: r, @@ -460,14 +460,14 @@ func (v versionPair) ImpliedCaretString() string { } func (v versionPair) typedString() string { - return fmt.Sprintf("%s-%s", v.Unpair().typedString(), v.Underlying().typedString()) + return fmt.Sprintf("%s-%s", v.Unpair().typedString(), v.Revision().typedString()) } func (v versionPair) Type() VersionType { return v.v.Type() } -func (v versionPair) Underlying() Revision { +func (v versionPair) Revision() Revision { return v.r } @@ -786,7 +786,7 @@ func VersionComponentStrings(v Version) (revision string, branch string, version case Revision: revision = tv.String() case PairedVersion: - revision = tv.Underlying().String() + revision = tv.Revision().String() } switch v.Type() { diff --git a/internal/gps/version_queue_test.go b/internal/gps/version_queue_test.go index 5a0684280f..0bcbea5053 100644 --- a/internal/gps/version_queue_test.go +++ b/internal/gps/version_queue_test.go @@ -16,11 +16,11 @@ type fakeBridge struct { } var fakevl = []Version{ - NewVersion("v2.0.0").Is("200rev"), - NewVersion("v1.1.1").Is("111rev"), - NewVersion("v1.1.0").Is("110rev"), - NewVersion("v1.0.0").Is("100rev"), - NewBranch("master").Is("masterrev"), + NewVersion("v2.0.0").Pair("200rev"), + NewVersion("v1.1.1").Pair("111rev"), + NewVersion("v1.1.0").Pair("110rev"), + NewVersion("v1.0.0").Pair("100rev"), + NewBranch("master").Pair("masterrev"), } func init() { diff --git a/internal/gps/version_test.go b/internal/gps/version_test.go index 4489d4e4b5..3c68fe5882 100644 --- a/internal/gps/version_test.go +++ b/internal/gps/version_test.go @@ -8,16 +8,16 @@ import "testing" func TestVersionSorts(t *testing.T) { rev := Revision("flooboofoobooo") - v1 := NewBranch("master").Is(rev) - v2 := NewBranch("test").Is(rev) - v3 := NewVersion("1.0.0").Is(rev) - v4 := NewVersion("1.0.1").Is(rev) - v5 := NewVersion("v2.0.5").Is(rev) - v6 := NewVersion("2.0.5.2").Is(rev) - v7 := newDefaultBranch("unwrapped").Is(rev) - v8 := NewVersion("20.0.5.2").Is(rev) - v9 := NewVersion("v1.5.5-beta.4").Is(rev) - v10 := NewVersion("v3.0.1-alpha.1").Is(rev) + v1 := NewBranch("master").Pair(rev) + v2 := NewBranch("test").Pair(rev) + v3 := NewVersion("1.0.0").Pair(rev) + v4 := NewVersion("1.0.1").Pair(rev) + v5 := NewVersion("v2.0.5").Pair(rev) + v6 := NewVersion("2.0.5.2").Pair(rev) + v7 := newDefaultBranch("unwrapped").Pair(rev) + v8 := NewVersion("20.0.5.2").Pair(rev) + v9 := NewVersion("v1.5.5-beta.4").Pair(rev) + v10 := NewVersion("v3.0.1-alpha.1").Pair(rev) start := []Version{ v1, diff --git a/internal/gps/version_unifier.go b/internal/gps/version_unifier.go index d9cfb2a9ef..71657559ef 100644 --- a/internal/gps/version_unifier.go +++ b/internal/gps/version_unifier.go @@ -146,14 +146,14 @@ func (vu versionUnifier) createTypeUnion(id ProjectIdentifier, v Version) versio case Revision: return versionTypeUnion(vu.pairRevision(id, tv)) case PairedVersion: - return versionTypeUnion(vu.pairRevision(id, tv.Underlying())) + return versionTypeUnion(vu.pairRevision(id, tv.Revision())) case UnpairedVersion: pv := vu.pairVersion(id, tv) if pv == nil { return versionTypeUnion{tv} } - return versionTypeUnion(vu.pairRevision(id, pv.Underlying())) + return versionTypeUnion(vu.pairRevision(id, pv.Revision())) } return nil diff --git a/internal/gps/version_unifier_test.go b/internal/gps/version_unifier_test.go index cc6bec2084..f044347188 100644 --- a/internal/gps/version_unifier_test.go +++ b/internal/gps/version_unifier_test.go @@ -20,16 +20,16 @@ func init() { rev3 := Revision("revision-three") lvfb1 = lvFixBridge{ - NewBranch("master").Is(rev1), - NewBranch("test").Is(rev2), - NewVersion("1.0.0").Is(rev1), - NewVersion("1.0.1").Is("other1"), - NewVersion("v2.0.5").Is(rev3), - NewVersion("2.0.5.2").Is(rev3), - newDefaultBranch("unwrapped").Is(rev3), - NewVersion("20.0.5.2").Is(rev1), - NewVersion("v1.5.5-beta.4").Is("other2"), - NewVersion("v3.0.1-alpha.1").Is(rev2), + NewBranch("master").Pair(rev1), + NewBranch("test").Pair(rev2), + NewVersion("1.0.0").Pair(rev1), + NewVersion("1.0.1").Pair("other1"), + NewVersion("v2.0.5").Pair(rev3), + NewVersion("2.0.5.2").Pair(rev3), + newDefaultBranch("unwrapped").Pair(rev3), + NewVersion("20.0.5.2").Pair(rev1), + NewVersion("v1.5.5-beta.4").Pair("other2"), + NewVersion("v3.0.1-alpha.1").Pair(rev2), } } @@ -96,7 +96,7 @@ func TestTypeUnionIntersect(t *testing.T) { } gotc = vu.intersect(id, c, rev3) - if gotc != NewVersion("v2.0.5").Is(rev3) { + if gotc != NewVersion("v2.0.5").Pair(rev3) { t.Fatalf("wanted v2.0.5, got %s from intersect", gotc.typedString()) } } diff --git a/lock.go b/lock.go index 7c63d8921d..e5c3ac4c46 100644 --- a/lock.go +++ b/lock.go @@ -95,9 +95,9 @@ func fromRawLock(raw rawLock) (*Lock, error) { if ld.Branch != "" { return nil, errors.Errorf("lock file specified both a branch (%s) and version (%s) for %s", ld.Branch, ld.Version, ld.Name) } - v = gps.NewVersion(ld.Version).Is(r) + v = gps.NewVersion(ld.Version).Pair(r) } else if ld.Branch != "" { - v = gps.NewBranch(ld.Branch).Is(r) + v = gps.NewBranch(ld.Branch).Pair(r) } else if r == "" { return nil, errors.Errorf("lock file has entry for %s, but specifies no branch or version", ld.Name) } diff --git a/lock_test.go b/lock_test.go index c15a1e6a62..344f442c38 100644 --- a/lock_test.go +++ b/lock_test.go @@ -34,7 +34,7 @@ func TestReadLock(t *testing.T) { P: []gps.LockedProject{ gps.NewLockedProject( gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/golang/dep/internal/gps")}, - gps.NewBranch("master").Is(gps.Revision("d05d5aca9f895d19e9265839bffeadd74a2d2ecb")), + gps.NewBranch("master").Pair(gps.Revision("d05d5aca9f895d19e9265839bffeadd74a2d2ecb")), []string{"."}, ), }, @@ -60,7 +60,7 @@ func TestReadLock(t *testing.T) { P: []gps.LockedProject{ gps.NewLockedProject( gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/golang/dep/internal/gps")}, - gps.NewVersion("0.12.2").Is(gps.Revision("d05d5aca9f895d19e9265839bffeadd74a2d2ecb")), + gps.NewVersion("0.12.2").Pair(gps.Revision("d05d5aca9f895d19e9265839bffeadd74a2d2ecb")), []string{"."}, ), }, @@ -85,7 +85,7 @@ func TestWriteLock(t *testing.T) { P: []gps.LockedProject{ gps.NewLockedProject( gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/golang/dep/internal/gps")}, - gps.NewBranch("master").Is(gps.Revision("d05d5aca9f895d19e9265839bffeadd74a2d2ecb")), + gps.NewBranch("master").Pair(gps.Revision("d05d5aca9f895d19e9265839bffeadd74a2d2ecb")), []string{"."}, ), }, @@ -116,7 +116,7 @@ func TestWriteLock(t *testing.T) { P: []gps.LockedProject{ gps.NewLockedProject( gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/golang/dep/internal/gps")}, - gps.NewVersion("0.12.2").Is(gps.Revision("d05d5aca9f895d19e9265839bffeadd74a2d2ecb")), + gps.NewVersion("0.12.2").Pair(gps.Revision("d05d5aca9f895d19e9265839bffeadd74a2d2ecb")), []string{"."}, ), }, diff --git a/manifest.go b/manifest.go index d152dc8f05..4e223ae0c0 100644 --- a/manifest.go +++ b/manifest.go @@ -20,6 +20,12 @@ import ( // ManifestName is the manifest file name used by dep. const ManifestName = "Gopkg.toml" +// Errors +var errInvalidConstraint = errors.New("\"constraint\" must be a TOML array of tables") +var errInvalidOverride = errors.New("\"override\" must be a TOML array of tables") +var errInvalidRequired = errors.New("\"required\" must be a TOML list of strings") +var errInvalidIgnored = errors.New("\"ignored\" must be a TOML list of strings") + // Manifest holds manifest file data and implements gps.RootManifest. type Manifest struct { Constraints gps.ProjectConstraints @@ -44,11 +50,11 @@ type rawProject struct { } func validateManifest(s string) ([]error, error) { - var errs []error + var warns []error // Load the TomlTree from string tree, err := toml.Load(s) if err != nil { - return errs, errors.Wrap(err, "Unable to load TomlTree from string") + return warns, errors.Wrap(err, "Unable to load TomlTree from string") } // Convert tree to a map manifest := tree.ToMap() @@ -61,46 +67,83 @@ func validateManifest(s string) ([]error, error) { case "metadata": // Check if metadata is of Map type if reflect.TypeOf(val).Kind() != reflect.Map { - errs = append(errs, errors.New("metadata should be a TOML table")) + warns = append(warns, errors.New("metadata should be a TOML table")) } case "constraint", "override": + valid := true // Invalid if type assertion fails. Not a TOML array of tables. if rawProj, ok := val.([]interface{}); ok { - // Iterate through each array of tables - for _, v := range rawProj { - // Check the individual field's key to be valid - for key, value := range v.(map[string]interface{}) { - // Check if the key is valid - switch key { - case "name", "branch", "version", "source": - // valid key - case "revision": - if valueStr, ok := value.(string); ok { - if abbrevRevHash.MatchString(valueStr) { - errs = append(errs, fmt.Errorf("revision %q should not be in abbreviated form", valueStr)) + // Check element type. Must be a map. Checking one element would be + // enough because TOML doesn't allow mixing of types. + if reflect.TypeOf(rawProj[0]).Kind() != reflect.Map { + valid = false + } + + if valid { + // Iterate through each array of tables + for _, v := range rawProj { + // Check the individual field's key to be valid + for key, value := range v.(map[string]interface{}) { + // Check if the key is valid + switch key { + case "name", "branch", "version", "source": + // valid key + case "revision": + if valueStr, ok := value.(string); ok { + if abbrevRevHash.MatchString(valueStr) { + warns = append(warns, fmt.Errorf("revision %q should not be in abbreviated form", valueStr)) + } } + case "metadata": + // Check if metadata is of Map type + if reflect.TypeOf(value).Kind() != reflect.Map { + warns = append(warns, fmt.Errorf("metadata in %q should be a TOML table", prop)) + } + default: + // unknown/invalid key + warns = append(warns, fmt.Errorf("Invalid key %q in %q", key, prop)) } - case "metadata": - // Check if metadata is of Map type - if reflect.TypeOf(value).Kind() != reflect.Map { - errs = append(errs, fmt.Errorf("metadata in %q should be a TOML table", prop)) - } - default: - // unknown/invalid key - errs = append(errs, fmt.Errorf("Invalid key %q in %q", key, prop)) } } } } else { - errs = append(errs, fmt.Errorf("%v should be a TOML array of tables", prop)) + valid = false + } + + if !valid { + if prop == "constraint" { + return warns, errInvalidConstraint + } + if prop == "override" { + return warns, errInvalidOverride + } } case "ignored", "required": + valid := true + if rawList, ok := val.([]interface{}); ok { + // Check element type of the array. TOML doesn't let mixing of types in + // array. Checking one element would be enough. Empty array is valid. + if len(rawList) > 0 && reflect.TypeOf(rawList[0]).Kind() != reflect.String { + valid = false + } + } else { + valid = false + } + + if !valid { + if prop == "ignored" { + return warns, errInvalidIgnored + } + if prop == "required" { + return warns, errInvalidRequired + } + } default: - errs = append(errs, fmt.Errorf("Unknown field in manifest: %v", prop)) + warns = append(warns, fmt.Errorf("Unknown field in manifest: %v", prop)) } } - return errs, nil + return warns, nil } // readManifest returns a Manifest read from r and a slice of validation warnings. diff --git a/manifest_test.go b/manifest_test.go index 8413c2bf60..80b4c03593 100644 --- a/manifest_test.go +++ b/manifest_test.go @@ -124,14 +124,80 @@ func TestReadManifestErrors(t *testing.T) { func TestValidateManifest(t *testing.T) { cases := []struct { tomlString string - want []error + wantWarn []error + wantError error }{ { tomlString: ` - [[constraint]] - name = "github.com/foo/bar" + required = ["github.com/foo/bar"] + `, + wantWarn: []error{}, + wantError: nil, + }, + { + tomlString: ` + required = "github.com/foo/bar" + `, + wantWarn: []error{}, + wantError: errInvalidRequired, + }, + { + tomlString: ` + required = [] + `, + wantWarn: []error{}, + wantError: nil, + }, + { + tomlString: ` + required = [1, 2, 3] + `, + wantWarn: []error{}, + wantError: errInvalidRequired, + }, + { + tomlString: ` + [[required]] + name = "foo" + `, + wantWarn: []error{}, + wantError: errInvalidRequired, + }, + { + tomlString: ` + ignored = ["foo"] + `, + wantWarn: []error{}, + wantError: nil, + }, + { + tomlString: ` + ignored = "foo" + `, + wantWarn: []error{}, + wantError: errInvalidIgnored, + }, + { + tomlString: ` + ignored = [] + `, + wantWarn: []error{}, + wantError: nil, + }, + { + tomlString: ` + ignored = [1, 2, 3] + `, + wantWarn: []error{}, + wantError: errInvalidIgnored, + }, + { + tomlString: ` + [[ignored]] + name = "foo" `, - want: []error{}, + wantWarn: []error{}, + wantError: errInvalidIgnored, }, { tomlString: ` @@ -139,7 +205,8 @@ func TestValidateManifest(t *testing.T) { authors = "foo" version = "1.0.0" `, - want: []error{}, + wantWarn: []error{}, + wantError: nil, }, { tomlString: ` @@ -153,11 +220,12 @@ func TestValidateManifest(t *testing.T) { name = "github.com/foo/bar" version = "" `, - want: []error{ + wantWarn: []error{ errors.New("Unknown field in manifest: foo"), errors.New("Unknown field in manifest: bar"), errors.New("Unknown field in manifest: version"), }, + wantError: nil, }, { tomlString: ` @@ -166,17 +234,66 @@ func TestValidateManifest(t *testing.T) { [[constraint]] name = "github.com/foo/bar" `, - want: []error{errors.New("metadata should be a TOML table")}, + wantWarn: []error{errors.New("metadata should be a TOML table")}, + wantError: nil, + }, + { + tomlString: ` + [[constraint]] + name = "github.com/foo/bar" + `, + wantWarn: []error{}, + wantError: nil, + }, + { + tomlString: ` + [[constraint]] + `, + wantWarn: []error{}, + wantError: nil, }, { tomlString: ` constraint = "foo" + `, + wantWarn: []error{}, + wantError: errInvalidConstraint, + }, + { + tomlString: ` + constraint = ["foo", "bar"] + `, + wantWarn: []error{}, + wantError: errInvalidConstraint, + }, + { + tomlString: ` + [[override]] + name = "github.com/foo/bar" + `, + wantWarn: []error{}, + wantError: nil, + }, + { + tomlString: ` + [[override]] + `, + wantWarn: []error{}, + wantError: nil, + }, + { + tomlString: ` override = "bar" `, - want: []error{ - errors.New("constraint should be a TOML array of tables"), - errors.New("override should be a TOML array of tables"), - }, + wantWarn: []error{}, + wantError: errInvalidOverride, + }, + { + tomlString: ` + override = ["foo", "bar"] + `, + wantWarn: []error{}, + wantError: errInvalidOverride, }, { tomlString: ` @@ -189,12 +306,13 @@ func TestValidateManifest(t *testing.T) { [[override]] nick = "foo" `, - want: []error{ + wantWarn: []error{ errors.New("Invalid key \"location\" in \"constraint\""), errors.New("Invalid key \"link\" in \"constraint\""), errors.New("Invalid key \"nick\" in \"override\""), errors.New("metadata in \"constraint\" should be a TOML table"), }, + wantError: nil, }, { tomlString: ` @@ -204,7 +322,8 @@ func TestValidateManifest(t *testing.T) { [constraint.metadata] color = "blue" `, - want: []error{}, + wantWarn: []error{}, + wantError: nil, }, { tomlString: ` @@ -212,7 +331,8 @@ func TestValidateManifest(t *testing.T) { name = "github.com/foo/bar" revision = "b86ad16" `, - want: []error{errors.New("revision \"b86ad16\" should not be in abbreviated form")}, + wantWarn: []error{errors.New("revision \"b86ad16\" should not be in abbreviated form")}, + wantError: nil, }, { tomlString: ` @@ -220,7 +340,8 @@ func TestValidateManifest(t *testing.T) { name = "foobar.com/hg" revision = "8d43f8c0b836" `, - want: []error{errors.New("revision \"8d43f8c0b836\" should not be in abbreviated form")}, + wantWarn: []error{errors.New("revision \"8d43f8c0b836\" should not be in abbreviated form")}, + wantError: nil, }, } @@ -236,19 +357,21 @@ func TestValidateManifest(t *testing.T) { for _, c := range cases { errs, err := validateManifest(c.tomlString) - if err != nil { - t.Fatal(err) + + // compare validation errors + if err != c.wantError { + t.Fatalf("Manifest errors are not as expected: \n\t(GOT) %v \n\t(WNT) %v", err, c.wantError) } // compare length of error slice - if len(errs) != len(c.want) { - t.Fatalf("Number of manifest errors are not as expected: \n\t(GOT) %v errors(%v)\n\t(WNT) %v errors(%v).", len(errs), errs, len(c.want), c.want) + if len(errs) != len(c.wantWarn) { + t.Fatalf("Number of manifest errors are not as expected: \n\t(GOT) %v errors(%v)\n\t(WNT) %v errors(%v).", len(errs), errs, len(c.wantWarn), c.wantWarn) } // check if the expected errors exist in actual errors slice for _, er := range errs { - if !contains(c.want, er) { - t.Fatalf("Manifest errors are not as expected: \n\t(MISSING) %v\n\t(FROM) %v", er, c.want) + if !contains(c.wantWarn, er) { + t.Fatalf("Manifest errors are not as expected: \n\t(MISSING) %v\n\t(FROM) %v", er, c.wantWarn) } } } diff --git a/project.go b/project.go index 93a0ad3b1e..309316fb34 100644 --- a/project.go +++ b/project.go @@ -39,13 +39,14 @@ func findProjectRoot(from string) (string, error) { } } +// A Project holds a Manifest and optional Lock for a project. type Project struct { // AbsRoot is the absolute path to the root directory of the project. AbsRoot string // ImportRoot is the import path of the project's root directory. ImportRoot gps.ProjectRoot Manifest *Manifest - Lock *Lock + Lock *Lock // Optional } // MakeParams is a simple helper to create a gps.SolveParameters without setting diff --git a/project_test.go b/project_test.go index 889282c570..2629a279d9 100644 --- a/project_test.go +++ b/project_test.go @@ -80,30 +80,6 @@ func TestProjectMakeParams(t *testing.T) { } } -func TestSlashedGOPATH(t *testing.T) { - h := test.NewHelper(t) - defer h.Cleanup() - h.TempDir("src") - - wd, err := os.Getwd() - if err != nil { - t.Fatal("failed to get work directory:", err) - } - env := os.Environ() - - h.Setenv("GOPATH", filepath.ToSlash(h.Path("."))) - _, err = NewContext(wd, env, nil) - if err != nil { - t.Fatal(err) - } - - h.Setenv("GOPATH", filepath.FromSlash(h.Path("."))) - _, err = NewContext(wd, env, nil) - if err != nil { - t.Fatal(err) - } -} - func TestBackupVendor(t *testing.T) { h := test.NewHelper(t) defer h.Cleanup() diff --git a/test_project_context_test.go b/test_project_context_test.go index 3ae86ec320..09ffda6111 100644 --- a/test_project_context_test.go +++ b/test_project_context_test.go @@ -37,7 +37,11 @@ func NewTestProjectContext(h *test.Helper, projectName string) *TestProjectConte // Set up a Source Manager var err error - pc.Context = &Ctx{GOPATH: pc.tempDir, Loggers: discardLoggers} + pc.Context = &Ctx{ + GOPATH: pc.tempDir, + Out: discardLogger, + Err: discardLogger, + } pc.SourceManager, err = pc.Context.SourceManager() h.Must(errors.Wrap(err, "Unable to create a SourceManager")) @@ -64,7 +68,7 @@ func (pc *TestProjectContext) Load() { var warns []error m, warns, err = readManifest(mf) for _, warn := range warns { - pc.Context.Loggers.Err.Printf("dep: WARNING: %v\n", warn) + pc.Context.Err.Printf("dep: WARNING: %v\n", warn) } pc.h.Must(errors.Wrapf(err, "Unable to read manifest at %s", mp)) }