Make sure that composer file exists before running

[Isinlor]

The source of issue in #39 appears to be custom RequireCommand in Symfony Flex.

More specifically:

$this->getComposer();

Making sure that composer file exists fixes the issue.

Should I add tests and document it or do you see some better solution?

I have tried disabling plugins in command that BinCommand is running, but I think plugins are already loaded at that time, so it has no effect.

[theofidry]

Thanks for looking into it! If that's enough to make it work I'm happy with it, but it looks like the tests are failing.

I would also add one test with Flex in the CI, for example creating a skeleton symfony app which will have flex, add bamarni/composer-bin-plugin and then require a package with it

[Isinlor]

Seems like I'm breaking assertions:

        // make sure the proxy command didn't instantiate Composer
        $this->assert->assertNull($this->getComposer(false));
        $this->assert->assertNull($this->getApplication()->getComposer(false));

What is exact meaning of these assertions? Can I remove them?

[theofidry]

Edit: I think they can be removed yes

[Isinlor]

I fixed the tests.

I would also add one test with Flex in the CI, for example creating a skeleton symfony app which will have flex, add bamarni/composer-bin-plugin and then require a package with it

It sounds like a good idea, but I don't really know how to do it. It would require installing composer-bin-plugin from my own branch, wouldn't it?

I added an assertion that makes sure that composer.json does exists.

[theofidry]

Many thanks for the work :)

It sounds like a good idea, but I don't really know how to do it. It would require installing composer-bin-plugin from my own branch, wouldn't it?

It would be something along the lines of:

composer create-project symfony/skeleton symfony-app
cd symfony-app
composer require --dev bamarni/composer-bin-plugin:dev-master
composer bin var-dump require symfony/var-dump

done directly in .travis.yml

[Isinlor]

I'm worried that this will be failing until we merge this pull request, because it will be requiring dev-master:

composer require --dev bamarni/composer-bin-plugin:dev-master
composer bin var-dump require symfony/var-dump

In other words now and in the future to test whether a fix related to Symfony Flex works you will need to merge it first. This also apply to finding regressions. For some unrelated, but breaking changes you will see CI failing only after a merge.

I can add it, just want to make sure that we both see the issue ;) .

[theofidry]

Indeed. I'm however not a big fan of having tests where you need to merge them to check if they works. Maybe we can find a way around it, what about something like:

composer require --dev bamarni/composer-bin-plugin:$TRAVIS_COMMIT

?

[Isinlor]

Yes, that should work - later today I will try to add it.

Do you think this should be also changed?
https://github.com/bamarni/composer-bin-plugin/blob/master/.travis.yml#L25

[theofidry]

yes please

[Isinlor]

It still won't work :/ . Packagist doesn't know about pull-request until it gets merged. The only solution I see is to depend on direct link to a repository from where the pull request originated. That requires manual composer.json modification and playing with git to pick the repository origin and the branch name because locking composer dependency on only commit hash does not work.

I guess it may be good "to do" for later.

[theofidry]

Let's try it then :) Thank you very much for looking into this

[theofidry]

Looks like the tests passed :)