Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow incomplete solutions in relevant commands #447

Closed
wants to merge 7 commits into from

Conversation

mosteo
Copy link
Member

@mosteo mosteo commented Jun 11, 2020

Goes on top of #440, fixes #377.

This PR completes the work on pinning and solutions/solver internal rework by removing the valid/invalid solution dichotomy for the user (it was internally removed since #429). With these changes, solutions can be instead complete (when all dependencies have a corresponding release or linked folder) or incomplete.

In practical terms, instead of stopping with "invalid solution" errors whenever the solution is incomplete, now alr still presents the solution to the user, identifying any missing dependencies. The user can still accept this (although the default is No) or use --force to default to Yes (so it can also be done non-interactively).

This PR applies these changes to get, pin, update and with. The user should gain flexibility when working with "fringe" crates that use missing dependencies/versions, or on unsupported distributions, and encounter less breaking situations when a solution becomes incomplete for any reason.

A side benefit for the publishing workflow is that, for unindexed dependencies, they can be included in the crate to be published without the need to also index the missing dependencies as externals. This way, no crate should be published with hidden dependencies (there's no cost to include them), nor we need to clutter the community index with "empty" externals.

Externals of course still are needed to allow detection of tools/system packages, and hint externals still are valuable when they include manual installation hints, but hint externals are no longer mandatory to be able to retrieve dependent crates as they were until now.

Because of this new flexibility, Solution.Is_Complete is now False for crates depending on undetected external hints, whereas Solution.Valid (deprecated by this PR) was True for these situations. This was necessary because otherwise no crate with undetected externals was retrievable, or could be worked with. Currently these can be retrieved/used after the initial user confirmation.

The bottom line is that now all Is_Complete solutions should always properly build, whereas in the past that was not true for Valid solutions unless external hints where provided by the user. (Linked dirs introduced in #439 still need to be correct for a complete solution to successfully build, of course). In other words, a complete solution after this PR has more guarantees than a valid solution had in the past.

New tests check and illustrate these changes.

In my consideration the initial problems raised in #240 are solved with #439 and this PR. There are remaining issues pointed out there, that have ties to the publishing workflow. I will address these in a new issue, since I intend to move into the publishing workflow after this and I've been thinking about it for some time, but I'd like to discuss before starting.

@mosteo mosteo marked this pull request as ready for review Jun 11, 2020
@mosteo mosteo force-pushed the feat/partial branch 3 times, most recently from b664c3e to 9b47920 Compare Jun 17, 2020
mosteo added 7 commits Jun 18, 2020
Up to now, trying to retrieve a release with non-solvable dependencies resulted
in an opaque "can't get a solution", leaving the only resort of doing an `alr
get --only`. With this patch, the best partial solution found is used and
displayed. The user can accept a partial deployment, understanding that the
build will fail unless missing dependencies are manually provided.

It would be relatively easy to allow the user to reject the proposal and
continue seeing other incomplete solutions (as, e.g., `aptitude` does) until
one of them is selected.
The user may want for some reason to specify versions that are not in the
index, for work-in-progress releases or when switching between several indexes.
Instead of blocking him entirely, the incomplete solution is displayed and he
can accept it regardless. Forcing is required when not interactive.
When the solution is already incomplete we emit a warning to remind the user
about this, but we need not to err, since there are no changes to the solution,
or we might find a less incomplete one.

Also added a test for the behavior when trying to update a pinned version
This is the last of the commands that previously erred out with "Cannot solve
dependencies". Now, it can be forced into accepting any dependency as long as
the user is OK with having an incomplete solution.
Instead, .Is_Complete or .Composition should be used for better granularity.
Is_Complete is more stringent than before (hints make the solution incomplete),
but the new flexibility to work with incomplete solutions makes the difference
secondary; the user is less restricted than before.
By making solutions with any external (including hints) incomplete, it is
necessary to force a few commands in the tests that before succeeded normally.

Some tests that checked for exact text were now failing due to the extra
information/interaction caused by incomplete solutions.

The stderr warning on incomplete solutions in setenv also requires fixing its
output.
doc/user-changes.md Outdated Show resolved Hide resolved
@Fabien-Chouteau
Copy link
Member

Fabien-Chouteau commented Jun 19, 2020

@mosteo It is difficult to review the PR with code from other branches. Maybe you can change the base branch of this PR to make more readable.

@mosteo
Copy link
Member Author

mosteo commented Jun 19, 2020

@mosteo It is difficult to review the PR with code from other branches. Maybe you can change the base branch of this PR to make more readable.

Yes, this is a problem for PRs coming from another repository since I cannot use base branches that don't exist in the target repo... I'll see what I can do.

@mosteo mosteo changed the base branch from master to fix/deploy Jun 19, 2020
@mosteo
Copy link
Member Author

mosteo commented Jun 19, 2020

I've pushed to this repo the base branch, so you should be seeing only this PR commits now.

@Fabien-Chouteau
Copy link
Member

Fabien-Chouteau commented Jun 19, 2020

Yes, this is a problem for PRs coming from another repository since I cannot use base branches that don't exist in the target repo... I'll see what I can do.

Why don't you push feat/ branches to the main Alire repo directly? That's what I do.

@mosteo
Copy link
Member Author

mosteo commented Jun 19, 2020

Why don't you push feat/ branches to the main Alire repo directly? That's what I do.

I thought that this way I lowered the noise for you with all the rebasing going on, but if you don't mind I might go back to working against the Alire repo directly.

@Fabien-Chouteau
Copy link
Member

Fabien-Chouteau commented Jun 19, 2020

I don't think it will create any extra noise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

alr update fails: Dependency resolution failed
2 participants