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

Already on GitHub? Sign in to your account

Branch aliases are broken for replacements #2626

Closed
stof opened this Issue Jan 23, 2014 · 18 comments

Comments

Projects
None yet
5 participants
Contributor

stof commented Jan 23, 2014

It looks like a replacement does not replace the aliased version anymore based on the branch alias: https://travis-ci.org/FriendsOfSymfony/FOSJsRoutingBundle/jobs/17469579

@stof stof referenced this issue in FriendsOfSymfony/FOSRestBundle Jan 27, 2014

Closed

fix master testing #656

@naderman naderman added the Bug label Feb 5, 2014

@naderman naderman added this to the Bugs milestone Feb 5, 2014

@stof stof added a commit to stof/composer that referenced this issue Feb 21, 2014

@stof stof Added a failing test for #2626 677ff6c

@stof stof added a commit to stof/composer that referenced this issue Feb 21, 2014

@stof stof Added a failing test for #2626 b62d236

lsmith77 commented Mar 5, 2014

ping

kbond commented Mar 10, 2014

Not sure if related but I have been having trouble requiring an exact package version based on the git hash (dev-master#hash) - this resolves as dev-master for me...

Contributor

stof commented Mar 10, 2014

@kbond This has nothing to do with this issue. the reference locking is not an alias at all. and it is logical that it uses the dev-master metadata. It is only a hack at the level of the git installation (which also means that a dist download would not use this reference locking). You should avoid using such locking whenever possible

kbond commented Mar 10, 2014

I see, so it only works correctly if I include the package as a "custom" repository?

Contributor

stof commented Mar 10, 2014

no, it only works fine if the dependencies of dev-master are the same (or at least compatible) with the dependencies you expect, and you install from source. The reference locking has no impact at all on the dependency resolution

Contributor

xabbuh commented Mar 11, 2014

This doesn't seem to be an issue at all (see FriendsOfSymfony/FOSRestBundle#656 and FriendsOfSymfony/FOSRestBundle#705).

Contributor

stof commented Mar 11, 2014

@xabbuh It is an issue. FOSRestBundle simply managed to find a workaround. but it is still broken. I even managed to write a testcase reproducing it (see the linked PR)

Contributor

stof commented Mar 11, 2014

@xabbuh to be clear, FOSRestBundle should not need to explicitly alias symfony/event-dispatcher master to 2.5. The replacement of self.version in symfony/symfony should take the branch alias into account (and it was taking it into accoutn previously)

Contributor

xabbuh commented Mar 11, 2014

@stof I understand what you mean. But as far as I can see, the problem is with the symfony/framework-bundle dependencies which don't require self.version but ~2.5.

Contributor

stof commented Mar 11, 2014

@xabbuh No. Requiring ~2.5 is perfectly valid. the issue is that the optimization done in Composer a few weeks ago broke the resolution of dependencies in this case. It is definitely a composer bug. If you use composer 1.0-alpha7, the testcase above will pass

Contributor

xabbuh commented Mar 11, 2014

@stof But how should that work? 2.5 isn't stable yet. So, the FrameworkBundle requires a dev dependency which doesn't work for FOSRestBundle where minimum-stability isn't set. The minimum-stability flag of the FrameworkBundle shouldn't be taken into account when it isn't the root project.

Contributor

stof commented Mar 11, 2014

@xabbuh the requirement on symfony/symfony allows to install symfony/symfony:dev-master. and the combination of branch-alias+replace should satisfy the match (if the bundle was depending on symfony/symfony: ~2.5 and you require dev-master, it would work even if 2.5 is not stable yet, if we forget the fact that it would be broken because of symfony replacing FrameworkBundle).
The issue is that the optimization done a few weeks ago to limit the packages set in the solver based on root requirements does not take care of aliases of selected packages.

Contributor

xabbuh commented Mar 11, 2014

@stof I was confused since FOSRestBundle doesn't require symfony/symfony. But now I understand the issue.

Contributor

stof commented Mar 11, 2014

@xabbuh hmm, in their case, they are indeed not requiring it (which also means that they are not really testing against Symfony 2.2 as the 2.4 packages will match the dependencies of FrameworkBundle 2.2 and be used). But there is still a bug in composer anyway

Contributor

xabbuh commented Mar 11, 2014

@stof Yes, I can confirm the bug. It's easily reproducable with a composer.json like this:

{
    "require": {
        "symfony/framework-bundle": "dev-master",
        "symfony/symfony": "dev-master"
    }
}

Works with Composer 1.0.0-alpha7 but not with 1.0.0-alpha8.

Contributor

stof commented Mar 11, 2014

@xabbuh yeah indeed. And it is exactly what the testcase I submitted is doing

Contributor

xabbuh commented Mar 11, 2014

@stof Ah, didn't see the test case yet. Sorry for the confusion.

@xabbuh xabbuh added a commit to xabbuh/composer that referenced this issue Mar 12, 2014

@stof @xabbuh stof + xabbuh Added a failing test for #2626 b639005
Contributor

xabbuh commented Mar 12, 2014

Please see #2805 for a potential fix.

@naderman naderman closed this in #2805 Mar 12, 2014

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