Skip to content
This repository has been archived by the owner on Feb 3, 2018. It is now read-only.

Introduce 'requires' #114

Merged
merged 6 commits into from
Dec 13, 2016
Merged

Introduce 'requires' #114

merged 6 commits into from
Dec 13, 2016

Conversation

sdboyer
Copy link
Owner

@sdboyer sdboyer commented Dec 8, 2016

This introduces the notion of a 'required' package:

  • Required packages must be present in any complete solution returned by the solver.
  • Requiring a package that is a logical child of the project root is not an error, but it is a no-op.
  • Requiring and ignoring the same package from a manifest is an error.

This is a root-only capability: listing required packages is, thus, a RootManifest method.

  • Base type updates - interface and structs
  • Set of bimodal tests
  • Prepare() validation tests
  • Add/update fail types as needed
  • New logic in the solver itself
  • Docs

Addresses #42

Also add rudimentary implementations and error handling in Prepare().
@sdboyer sdboyer self-assigned this Dec 8, 2016
mklp("baz 1.0.0", "baz/qux"),
),
},
"require impossible subpackage": {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This brings us back to another variant of a familiar question - if an unimported dep is constrained in the manifest, does that constraint still come into effect?

I think it's pretty clear in this case, though: having the requires list is basically equivalent to having an import path, and these are both emerging from the same manifest (so we can assume the same user has control over it), thus the constraint should be applied. And because deps can't express requires, we don't have the same transitivity problem as with normal imports.

Also touchup docs and remove project root prefix from mklp() call.
This was far easier than I expected. Requires really are the equivalent
of imports, just taken from a manifest instead of static analysis.
@codecov-io
Copy link

codecov-io commented Dec 8, 2016

Current coverage is 76.92% (diff: 82.75%)

Merging #114 into master will increase coverage by 0.13%

@@             master       #114   diff @@
==========================================
  Files            23         23          
  Lines          3400       3428    +28   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           2611       2637    +26   
- Misses          599        600     +1   
- Partials        190        191     +1   

Powered by Codecov. Last update 71648f3...7adc457

@sdboyer
Copy link
Owner Author

sdboyer commented Dec 8, 2016

This doesn't introduce any new failure types, or change any of the existing ones. That means we're leaving it on the tool to figure out if a given package-related failure is due to a real import, or a require.

This is fine for now, though in the future if we end up wanting to do any kind of lineage tracking, we might need more.

@sdboyer sdboyer merged commit aa95bd0 into master Dec 13, 2016
@sdboyer sdboyer deleted the requires branch January 5, 2017 03:15
krisnova pushed a commit to krisnova/dep that referenced this pull request Apr 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants