Skip to content
This repository has been archived by the owner on Dec 5, 2019. It is now read-only.

Add support for custom version requirements with a predicate #42

Closed
wants to merge 8 commits into from

Conversation

jspahrsummers
Copy link
Member

This enables support for requirements like "pin to this named branch," where maybe a semantic version isn't available.

@jspahrsummers jspahrsummers changed the title [WIP] Add support for custom version requirements with a predicate Add support for custom version requirements with a predicate Sep 7, 2016
@jspahrsummers
Copy link
Member Author

@kastiglione @mdiep Can you please take a look at this and see if it makes any sense? The logic ended up a bit different from Carthage, because Arbiter doesn't inherently have knowledge of what kinds of versions should clobber others—instead, it can only check a version against each requirement.

* If this result is returned multiple times for different versions, it is
* unspecified which version will be selected.
*/
ArbiterRequirementSuitabilityBestPossibleChoice,
Copy link

Choose a reason for hiding this comment

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

Ideal might be a better name here.

@mdiep
Copy link

mdiep commented Sep 9, 2016

Can you please take a look at this and see if it makes any sense? The logic ended up a bit different from Carthage, because Arbiter doesn't inherently have knowledge of what kinds of versions should clobber others—instead, it can only check a version against each requirement.

Makes sense to me. But I do think that's going to prevent you from optimizing by pruning nodes upfront.

* The version should be considered the best possible choice for satisfying
* the requirement.
*
* This result is unique in that it _overrides other requirements_, which is
Copy link
Contributor

Choose a reason for hiding this comment

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

Not to get caught up on naming, but I like the word "override", or something that communicates that this has been hand picked.

@kastiglione
Copy link
Contributor

Makes sense, though that doesn't say much since my knowledge about the inner workings of Carthage, or dependency managers in general, pales compared to you two.

@mdiep
Copy link

mdiep commented Sep 9, 2016

The other thing to consider here is that Carthage has a bug currently where it loops endless in some cases because of these sort of overriding constraints with non-semantic-versions. Because they're compatible with everything, you can get into situations where a version is picked that doesn't meet your requirements, even though nothing explicitly selected that version.

@jspahrsummers
Copy link
Member Author

Because they're compatible with everything, you can get into situations where a version is picked that doesn't meet your requirements, even though nothing explicitly selected that version.

I think this implementation would behave differently, because versions determined to be the "best" are not just compatible with everything—they override everything, and other versions get pruned from the graph.

That said, I'm not very happy with how this turned out, and given that neither of you were very gung-ho either, this probably deserves more thought. 😔

@mdiep
Copy link

mdiep commented Sep 10, 2016

I think Arbiter should be opinionated about how to handle non-semantic versions.

@jspahrsummers
Copy link
Member Author

What should that mean here? It's hard to be opinionated when the library is this general. 😕

@mdiep
Copy link

mdiep commented Sep 11, 2016

I just suspect that there's 1 reasonable approach to versioning with unversioned dependencies (non-semantic tags, branches, commits, etc.). IMO Arbiter should implement that so there's 1 easy built-in way, even if it enables other approaches.

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.

None yet

3 participants