Complex exclusion not parsed correctly about semver HOT 21 CLOSED

@Seldaek I am still uncertain about how this should be parsed (e.g. is it a matter of order or operator precedence?). It seems like a bug to me, but want to check with you first.

Outcome of the above constraint is:

class Composer\Semver\Constraint\MultiConstraint#10 (3) {
  protected $constraints =>
  array(2) {
    [0] =>
    class Composer\Semver\Constraint\Constraint#11 (3) {
      protected $operator =>
      int(4)
      protected $version =>
      string(12) "0.13.0.0-dev"
      protected $prettyString =>
      NULL
    }
    [1] =>
    class Composer\Semver\Constraint\Constraint#12 (3) {
      protected $operator =>
      int(1)
      protected $version =>
      string(22) "2.0.0.0-dev != 1.3.1.0"
      protected $prettyString =>
      NULL
    }
  }
  protected $prettyString =>
  string(18) "~0.13|~1.0 !=1.3.1"
  protected $conjunctive =>
  bool(true)
}

from semver.

it's parsed like that because it's collapsed
https://github.com/composer/semver/blob/master/src/VersionParser.php#L263-L277
That constraint still looks ok though? so maybe a solver bug?

from semver.

the issue here is that the upper bound is parsed as being 2.0.0.0-dev != 1.3.1.0, instead of using 2 different constraints here.

Can you try to add a comma between ~1.0 and != 1.3.1 ?

from semver.

doesn't help unfortunately stof.

from semver.

Looks like the regex splitting the AND constraints does not play well with the != operator

from semver.

looks ok to me...

Composer\Semver\Constraint\MultiConstraint::__set_state(array(
   'constraints' => 
  array (
    0 => 
    Composer\Semver\Constraint\Constraint::__set_state(array(
       'operator' => 4,
       'version' => '1.0.0.0-dev',
       'prettyString' => NULL,
    )),
    1 => 
    Composer\Semver\Constraint\Constraint::__set_state(array(
       'operator' => 1,
       'version' => '2.0.0.0-dev',
       'prettyString' => NULL,
    )),
    2 => 
    Composer\Semver\Constraint\Constraint::__set_state(array(
       'operator' => 5,
       'version' => '1.3.1.0',
       'prettyString' => NULL,
    )),
  ),
   'prettyString' => NULL,
   'conjunctive' => true,
))

[>= 1.0.0.0-dev < 2.0.0.0-dev != 1.3.1.0]

from semver.

well, in this case, yes. But try with ~1.0 !=1.3.1, which is what was reported here

from semver.

that is for ~1.0 !=1.3.1,

from semver.

I suspect it has to do with attempting to collapse the contiguous range (since ~0.x || ~1.x is a contiguous range) not properly taking into account the exclude.

from semver.

that's what i said 😉

from semver.

I am confused. I wrote some tests, but they all work as expected..

from semver.

Never mind. Added some more assertions. The string representation is identical. The actual count of constraints inside the multiconstraint is not.

from semver.

Here is a simple debugging session against the given constraint in the crucial part of the code.

from semver.

Due to the simple string comparison in there things do not end up as expected. Not sure what the most elegant solution would be here?

from semver.

I don't think it's an issue here tbh, the parsing looks fine, and it should work.. collapsing that range isn't wrong AFAICT, as >=0.13 AND <2.0 AND not 1.3.1 should result in the correct stuff being picked. I am wondering if it's not instead something going of in the solver or the evaluation of the constraint.. But it's kinda late here so maybe I'm missing something :)

from semver.

The problem is that the upper bound + the exclude are merged into one constraint, while they should be a multiconstraint (or separate constraints in the multiconstraint being generated). You cannot have a single constraint that represents both the upper bound and the exclusion because a constraint only has one operator.

from semver.

Now it actually translates into (>="0.13") AND (<"2.0 AND NOT 1.3.1") (quotes and brackets added for clarity). So it simply acts as an upper bound only, the "NOT" gets lost in conversion through collapsing.

from semver.

In #42 (comment) the operator => 5 contains the NOT, so I think that's fine.

from semver.

feel free to reopen the original composer/composer#5617 if you think it's a solver issue.

from semver.

@Seldaek that is proper parsing yes. But that only happens when there is not a contiguous range (the constraint used there was ~1.0 !=1.3.1, not ~0.13 || ~1.0 !=1.3.1).

from semver.

As you can see in my debug session, it initially creates two multi constraints, the first containing lower and upper bound, and the second containing lower and upper bound and the not constraint. When it merges them it ends up with only a lower and upper bound. See #42 (comment) for details.

from semver.