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

Look for vendor/autoload.php in the current working directory first #1757

Closed
greg0ire opened this issue Jun 19, 2015 · 17 comments
Closed

Look for vendor/autoload.php in the current working directory first #1757

greg0ire opened this issue Jun 19, 2015 · 17 comments

Comments

@greg0ire
Copy link
Contributor

Some developers like to install phpunit globally and not add it as a development dependency because of the high number of dependency phpunit has.

When in that case, the lookup for an autoload file returns ~/.composer/vendor/phpunit/phpunit/../../autoload.php rather than the project's autoload file. This could be easily solved if we assume that phpunit is always run from the project root, by looking first in the current working directory. The original discussion is here

@sebastianbergmann
Copy link
Owner

@Seldaek Is there a best practice for how to handle this? Issues like this keep coming up and I would like to know a canonical way for dealing with them.

@greg0ire
Copy link
Contributor Author

The more I think of it, the more I think there should be 2 composer packages : one for the phpunit application, and one for the phpunit public api that people consume when they write their tests. People would only really need to require the second one in their projects.

@sebastianbergmann
Copy link
Owner

Separating the framework from the runner(s) is on the roadmap but has nothing to do with the problem at hand. There needs to be a standard way for finding the Composer-generated autoloader in a CLI bootstrap script.

@greg0ire
Copy link
Contributor Author

Yeah, I was referring to the problem that led to this one : people are often using a globally installed binary because the number of dependencies of phpunit is too high for them. But glad to learn that this separation is planned!

@Seldaek
Copy link
Contributor

Seldaek commented Jun 19, 2015

@sebastianbergmann well there are two problems, one is finding the autoloader in a CLI bootstrap, and that's fairly easy (look in ./vendor/autoload.php and ../../autoload.php). You need to do that anyway I guess in case phpunit isn't required in the project.

The other issue here is that you are a CLI script looking for the autoloader in whatever app you're currently running in, and that is defined as far as I know by:

  • cwd
  • the phpunit config, if it sets a bootstrap or maybe you can set a basedir/cwd in there?
  • a config param on the CLI?
  • ...

Once you have this base location (I guess looking up in both CWD and dirname($phpunitConfigXml) sounds reasonable) you can look up for ./vendor/autoload.php, although strictly speaking you should open composer.json if it's there, look for a $c['config']['vendor-dir'] in it, and use that or default to ./vendor/.

You should load the phpunit one first, and then the local one if it's different, because since they are prepended by default the local one will then take precedence over the phpunit one, just in case the user has global dependencies that are in different versions than local ones.

I'm sorry I don't have a shorter answer, and it sounds like it's worth turning into a tiny lib maybe because I am sure other tools could benefit from that (looking up the first one is easy and you'd need to do that before you can use the lib anyway but the rest is a bit tricky).

The other solution is to just fail hard if people don't define a bootstrap or run ./vendor/bin/phpunit (which solves it too since there is only one autoloader needed then) I guess you want to support and promote the phar still though so that's probably not a good option.

@sebastianbergmann
Copy link
Owner

@Seldaek The PHAR is my preferred option for using PHPUnit when no extensions are needed, yes. But personal preference aside, any mode of installation supported by Composer must work.

@sebastianbergmann
Copy link
Owner

Am I understanding SymfonyTest/SymfonyDependencyInjectionTest#48 correctly that the phpunit script of a global PHPUnit installation (installed via Composer) is used to execute the tests of a project that has a local installation of PHPUnit in its vendor directory and that this somehow causes a problem?

@greg0ire
Copy link
Contributor Author

Yes, that's it precisely! This PR wants to harmonize the travis configuration of the bundle with that of other Sonata bundles, switching from using the local installation to a global installation. As a workaround, I suggested to use "vendor/autoload.php" as a bootstrap file. The bug can be seen in this travis build.

@sebastianbergmann
Copy link
Owner

When you invoke phpunit then the globally installed PHPUnit is used (assuming ~/.composer/vendor/bin is on your $PATH) and it will loads its autoloader and not the one in your project's vendor directory with the extensions you need for that project.

When you use ./vendor/bin/phpunit in your project's root then the project-local installation of PHPUnit is used that has the extensions you need for the project.

You, however, want phpunit (from the globally installed PHPUnit) to invoke the project-local installation of PHPUnit? This should be doable (by using the CWD as was outlined above). However, I think that this may lead to rather obscure problems in situations I cannot imagine right now. At least it will be unintuitive. I, for one, would assume phpunit from the globally installed PHPUnit to always invoke the globally installed PHPUnit.

@sebastianbergmann sebastianbergmann changed the title [Feature Request]Look for vendor/autoload.php in the current working directory first Look for vendor/autoload.php in the current working directory first Jun 20, 2015
@greg0ire
Copy link
Contributor Author

You, however, want phpunit (from the globally installed PHPUnit) to invoke the project-local installation of PHPUnit?

Not sure that is what I would like. For instance, the globally installed PHPUnit_Framework_TestCase can be used, this is not really the problem. The main problem is that I want phpunit take the project's autoloader into account when looking for non-phpunit classes. At the moment, the first class that is not found has to do with unit testing, but I guess I have the same problem for any class that is part of my project, since vendor/autoload.php is not used at all. I can force phpunit to do that by configuring the bootstrap file, but since there already is a mechanism that tries to find the project's autoloader, I think it could be improved.

@greg0ire
Copy link
Contributor Author

Now, for the sake of the argument, let's admit that global phpunit and local phpunit are not in the same version. To work, global phpunit would need to autoload a class that does not even exist in local phpunit, and to work, the test suite would need to be able to autoload the project's file. When looking for classes, I think phpunit should first ask the global autoloader, then ask the local autoloader. But then what if some other lib is installed globally, and is also used in the project, with a different version. This could lead to very obscure problems indeed.

Maybe this is not a good solution. Maybe phpunit should ask people to configure their bootstrap file when it does not find a class…

@sebastianbergmann
Copy link
Owner

There are three supported ways of installing: PHAR, project-local with Composer, and global with Composer. All three work fine. You only run into problems if you try to mix them. And while I understand your problem and see your point, I am not convinced (yet) that this is something that should be fixed.

@greg0ire
Copy link
Contributor Author

I understand. Feel free to close this if you don't think there is an easy solution that would not mess things up for other projects.

@sebastianbergmann
Copy link
Owner

I'll leave it open for now.

@Seldaek
Copy link
Contributor

Seldaek commented Jun 20, 2015

IMO if you require a local phpunit you shouldn't run it via the global command, or it's ok to do so (I do a lot out of habit..) but when you encounter a weird failure you should be aware that you are not using the local one and that's probably the reason (happens to me sometimes, I switch to ./vendor/bin/phpunit and it usually works out).

That said, there are quite a few tools in the JS/npm world that do what was mentioned above, they ask you to require a foo-cli binary globally, and then require the exact version of the tool you need in every project. That way the binary is a very simple wrapper that looks for the lib locally and bootstraps it, and it pretty much never has to change so you avoid version conflicts. On the other hand, it would merely serve as an alias phpunit=./vendor/bin/phpunit, which one could argue people should know how to handle themselves if they really need it.

@greg0ire
Copy link
Contributor Author

I'm hitting the same bug (or something very similar) with symfony itself and sismo (see last reference).
Here is what it says :

Cannot redeclare class Symfony_Component_Debug_Tests_Fixtures_PEARClass in /home/greg/.composer/vendor/symfony/debug/Tests/Fixtures/PEARClass.php

This time, I think classes from my global installation are autoloaded for some reason, while the PEARClass from the project was already required / autoloaded (I do not know what exactly).

I switch to ./vendor/bin/phpunit and it usually works out

I cannot apply that to symfony, symfony does not require phpunit. Symfony's phpunit. If I uninstall sismo, symfony/debug goes away, and the problem with it, but I guess I will rather use phpunit's PHAR as suggested by @sebastianbergmann

@sebastianbergmann
Copy link
Owner

Dear contributor,

let me start by apologizing for not commenting and/or working on the issue you have reported or merging the pull request you have sent sooner.

PHPUnit 5.0 was released today. And today I am closing all open bug reports and pull requests for PHPUnit and its dependencies that I maintain. Please do not interpret the closing of this ticket as an insult or a lack of interest in your problem. I am sorry for any inconvenience this may cause.

If the topic of this ticket is still relevant then please open a new ticket or send a new pull request. If your ticket or pull request is about a defect then please check whether the issue still exists in PHPUnit 4.8 (which will received bug fixes until August 2016). If your ticket or pull request is about a new feature then please port your patch PHPUnit 5.0 before sending a new pull request.

I hope that today's extreme backlog grooming will allow me to respond to bug reports and pull requests in a more timely manner in the future.

Thank you for your understanding,
Sebastian

Repository owner locked and limited conversation to collaborators Oct 2, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants