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

Prefer-lowest strict mode? #7849

Closed
dereuromark opened this Issue Dec 20, 2018 · 10 comments

Comments

Projects
None yet
4 participants
@dereuromark
Copy link

dereuromark commented Dec 20, 2018

So this is somewhat a feature request.
We have some constraints in libraries like ^3.6.0.
Now we do check both min and max in travis using

  - if [[ $PREFER_LOWEST != 1 ]]; then composer install --prefer-source --no-interaction ; fi
  - if [[ $PREFER_LOWEST == 1 ]]; then composer update --prefer-dist --no-interaction --prefer-lowest --prefer-stable ; fi

For 3.6.x, 3.7.x and 3.8.x available it at least checks 3.6.x lowest and 3.8.x highest as smoke tests.

The problem is that it silently uses 3.7.0 at some point only for prefer-lowest due to some other hidden constraint.
This makes the build pass even though our composer.json declares 3.6.0 compatibility - which now very well might be compromised.

There is the option to manually validate the composer.json and the generated lock file with some custom tooling on our end, to check if each minimum version is the one used - and fail with a meaningful message.

It would be super helpful however, if composer could provide some strict mode here to force using the lowest versions defined (at least the ones promised in "require").
If the package was declared as ^3.6 it would force 3.6.0 only now and then very early abort due to to normal composer resolvement errors.

@naderman

This comment has been minimized.

Copy link
Member

naderman commented Dec 20, 2018

The "prefer" options are meant to modify preference in the solver. We should not be adding options to composer update that essentially edit the constaints in composer.json.

Further this is not useful, because the purpose of prefer lowest is to pick the lowest actually solvable/usable version, no user would get a different version installed by default anyway.

@dereuromark

This comment has been minimized.

Copy link

dereuromark commented Dec 20, 2018

This enhancement/tool is only necessary/useful for CI checking of library code.
This has nothing to do now with projects. Also, modifying the composer.json would not be done, only on the fly just picked the lowest one actually defined.

The point is:
Yeah, it solves it, but it is not in sync with the promise of the composer.json
So the composer.json is either wrong (needs higher constraint), or you lose support for this min version (compromising existing apps relying on this).

As such this is useful to add.
If you do not want to do this, then fine, we will have to implement some hack on our own.
Would have been just nice to keep too much logic and operations out of thousands of travis files.

@IMSoP

This comment has been minimized.

Copy link

IMSoP commented Jan 2, 2019

I think Nils is right that this doesn't belong in composer update, but it might make sense as a separate command.

Composer will only choose a version above the minimum if your listed minimum constraints are inconsistent - that is, you've listed a package which requires foo/bar 3.7.x, but claimed compatibility with foo/bar 3.6.x; or indeed, you've listed 3.6.0, but listed some other package requiring 3.6.2. This in turn can only happen when you edit composer.json, not due to a third-party change - unless someone replaces the tag for 3.6.0, or you have constraints with no minimum, like "dev-master".

So the actual requirement is not to install the "strict minimum" versions (because you can't) but to test the consistency of composer.json.

What about adding something like composer check-minimums, or an extra argument for composer validate, which parsed the composer.json, replaced every constraint with its minimum, and then attempted to solve it? You could then have Travis run this as a separate test, and fix any inconsistencies before releasing a changed composer.json.

@dereuromark

This comment has been minimized.

Copy link

dereuromark commented Jan 2, 2019

I like it.
Super useful for all libraries (projects/apps dont care here) that need/want to prove that they can ship with the defined minimum, and that this is tested properly.
Prevents accidental incompatibility sneaking in.

@naderman

This comment has been minimized.

Copy link
Member

naderman commented Jan 2, 2019

I think you both have a naive concept of how dependencies work. Even with this separate command, what are you actually going to test?

Are you going to do it for each dependency individually? Maybe a lowest version is supported, but only if a later version of some other dependency is actually installed. And then when you try to verify it for that one, it also works but only if a later version of the former is installed. So it tells you that it works, but it doesn't actually work if all are the lowest. Or are you going to test it by setting all of them to the lowest version? But then what about a user who has a different version of one of the dependencies than the lowest, are you going to be able to still support all the lowest versions of everything else then? You still have no idea.

The point is, to be able to actually definitively make any of these statements you would need to test every possible permutation which is entirely infeasible. That's why we have "prefer-lowest" to give you a good bottom line example to test against, but it makes no guarantees. I think adding any command like what you're proposing to composer core would give people the incorrect idea that this can give them any actual guarantees of compatibility which it cannot. So I'd much rather not see this in Composer at all.

You're of course welcome to build any tools like this yourself if you think they give you any additional benefits, but I don't see much value in this for most projects/libraries.

@Incognito

This comment has been minimized.

Copy link

Incognito commented Jan 2, 2019

I think you both have a naive concept of how dependencies work.

From my perspective it's pretty clear they want a canonical authority of what constitutes "lowest", instead of every package in the graph (ie: the actual dependencies) having equal weight as the root schema.

It's perfectly valid to assume that "prefer lowest" means "I prefer the lowest versions as defined in my project's schema" and not "I prefer the lowest versions that happen to be compatible with every schema I depend on".

This is pretty reasonable as the project's de-facto minimum dependency must be the lowest version supported in the entire graph. The information to generate these warnings is already present after completion (ie: project schema requires foo-1.0.x and bar requires foo-1.1.0, lowest packages in project-schema are not installable, 1.1.0 is the lowest possible package).

@naderman

This comment has been minimized.

Copy link
Member

naderman commented Jan 2, 2019

@Incognito My point is that this gains you very little useful information. They're talking about doing this to test compatibility of libraries. But the user of the library likely uses some combination of different versions within the constraints of the library's composer.json. So while yes, you now know there is a possible solution to the library requirements if the project uses the exact minimum version of each of the library's direct dependencies, you very likely have no idea if this holds as soon as you use even a single non-lowest package version in the project. If you limit the test to direct dependencies of the library, you don't even know if this holds if a dependency of a dependency is used in a different version in the project than part of this new lowest solution for the library test.

e.g. lib 1.0 requires foo 1.*, bar 1.*, bar 1.0 requires baz 1.1, bar 1.1 requires baz 1.*.

project requires lib 1.0, foo 1.0, bar 1.0, baz 1.0, this results in a conflict because the library is not actually compatible with bar 1.0 if your project contains a baz 1.0 dependency.

edited to fix asterisks

@naderman

This comment has been minimized.

Copy link
Member

naderman commented Jan 2, 2019

Better example of why the proposed mechanism is not very useful:

  • lib requires foo ^1.0 and bar ^1.0
  • foo 1.0 conflicts with bar 1.0
  • foo and bar also exists as versions 1.1 without any conflicts

Your proposed command would error here because the lowest solution foo 1.0 and bar 1.0 at the same time leads to a conflict.

But the two constraints are actually independently true:

example 1

  • project requires lib and foo 1.0
  • composer installs bar 1.1
  • project and lib work fine

example 2

  • project requires lib and bar 1.0
  • composer installs foo 1.1
  • project and lib work fine

So the two constraints are both correct because the library works fine with all versions of foo and all versions of bar. And still you cannot install the lowest versions of both of them at the same time because they are in conflict with each other. This is not a problem for composer users at all currently, but your proposed command would look on this as if something was wrong with the library.

@dereuromark

This comment has been minimized.

Copy link

dereuromark commented Jan 2, 2019

I think my use case was for trivial and simple primary dependencies. Where it would be clear. And where there wouldnt necessarily be such conflicts. These kind of conflicts could still just abort the whole thing.
But I am OK with closing then, as this can be done with some custom scripting here, and we dont need to further discuss then in this group.
Thank you.

@dereuromark

This comment has been minimized.

Copy link

dereuromark commented Jan 3, 2019

For reference for others:
This is the super useful tool now: https://github.com/dereuromark/composer-prefer-lowest
It works perfectly for the case stated above: Are all required dependencies tested against the defined version or not.

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