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

Add a simple bootstrap file to decouple the test suite from Composer. #48

Merged
merged 1 commit into from Dec 6, 2015
Merged

Conversation

orlitzky
Copy link
Contributor

@orlitzky orlitzky commented Dec 5, 2015

The test case files all depend on PSR-4 autoloading, but no autoloader
exists when the library is installed through (for example) a
distribution package manager. Fortunately the bootstrap process is
simple and this can be fixed with a single "require" statement.

This commit adds a new file, test/bootstrap.php, which loads the
Html2Text library. We then use this new file to bootstrap PHPUnit. The
end result is that the test suite now works out-of-the-box without
Composer. Running,

$ phpunit

should succeed from an unmodified tarball.

The test case files all depend on PSR-4 autoloading, but no autoloader
exists when the library is installed through (for example) a
distribution package manager. Fortunately the bootstrap process is
simple and this can be fixed with a single "require" statement.

This commit adds a new file, test/bootstrap.php, which loads the
Html2Text library. We then use this new file to bootstrap PHPUnit. The
end result is that the test suite now works out-of-the-box without
Composer. Running,

  $ phpunit

should succeed from an unmodified tarball.
@mtibben
Copy link
Owner

mtibben commented Dec 5, 2015

So you're also assuming phpunit is installed globally or some other way?

Is having a dependency on PSR-4 autoloading so bad? Seems PSR-4 is the thing to do these days with PHP libs, so what is the reasoning behind this? Can you provide a realistic use-case? What distribution package manager are you referring to? Are there other PHP libs using PSR-4 that do the same thing?

@orlitzky
Copy link
Contributor Author

orlitzky commented Dec 5, 2015

So you're also assuming phpunit is installed globally or some other way?

Yeah, either the user could have it installed himself, or my real use case: it's installed by the distribution as a dependency of html2text.

Is having a dependency on PSR-4 autoloading so bad?

There's nothing wrong with autoloading, but you need to have an autoloader class for it to work. If you don't ship one, then really, no one can use your library without Composer. You're OK for now since it's only the test suite that needs autoloading, but if you ever add another library class, the problem will creep into normal usage.

What distribution package manager are you referring to?

I'm using Gentoo but other package managers let you run test suites too. It's nice to be able to run the test suite as part of our installation procedure, just in case something goes wrong. If the test suite fails, we can bail out and tell the user to file a bug so we can investigate why. Maybe we messed up the dependencies or something like that.

But, we can't run the test suite without an autoloader class. And we're not doing the installation with composer, so vendor/autoload.php doesn't exist.

Are there other PHP libs using PSR-4 that do the same thing?

A few, for example:

https://github.com/PHPMailer/PHPMailer/blob/master/PHPMailerAutoload.php
https://github.com/google/recaptcha/blob/master/src/autoload.php

If you ever add more library classes, you'd need an autoloader like that to support non-composer installations. But since you only have the one class for now and it's only the test suite that autoloads anything, I made the simplest change that would work.

@mtibben
Copy link
Owner

mtibben commented Dec 5, 2015

The other things to think about is that composer manages all dependencies

  1. composer specifies the version of phpunit, so if we suddenly required phpunit 5, you'd also need to update packages and the like
  2. we'll possibly we'll be adding dependencies like https://github.com/symfony/polyfill-mbstring soon - how do you manage deps in your dist package?

So unless there's a better case than "it's nice to have", I can't see any real benefits to not using composer. Composer just makes dependency management and autoloading simple, so I'm inclined to simply say that composer is hard dependency.

So in your gentoo package, can you just run composer install && vendor/bin/phpunit instead of phpunit ?

@cdp1337
Copy link

cdp1337 commented Dec 5, 2015

I'd have to agree with orlitzky on this matter. I've personally had major issues getting composer working well in my environments, and find it easier just to manually grab the required versions of the necessary packages.

In my use-case of this project, I have bundled html2text as a library on another project that handles its own autoloading and dependency resolution, so vendor/autoload.php doesn't exist. +1 vote for test/bootstrap.php from me! :)

@orlitzky
Copy link
Contributor Author

orlitzky commented Dec 5, 2015

The other things to think about is that composer manages all dependencies

The distribution package managers actually do a much better job of this. Composer has a fundamental limitation: it can only manage PHP dependencies, and doesn't know what to do with dependencies on C libraries or anything written in another language. So for example, you can't require the local PHP installation to have mbstring support. The distro PM simply works better in this regard.

composer specifies the version of phpunit, so if we suddenly required phpunit 5, you'd also need to update packages and the like
we'll possibly we'll be adding dependencies like https://github.com/symfony/polyfill-mbstring soon - how do you manage deps in your dist package?

If you release a new version with new dependencies, we release a new package with those new dependencies. The old version keeps working fine with the old dependencies until then.

A Gentoo package is just a small text file and the dependencies get listed in a string -- nothing fancy. In practice, I'd just read your composer.json file and copy those dependencies into our package. But we can specify e.g. "dev-lang/php[mbstring]" to ensure that PHP (which is written in C) was compiled with the mbstring extension.

So unless there's a better case than "it's nice to have", I can't see any real benefits to not using composer. Composer just makes dependency management and autoloading simple, so I'm inclined to simply say that composer is hard dependency.

That's OK, it's your package, and it isn't critical for me. A lot of other packages have taken this stance, but it does mean that once you add some more classes, it won't be possible to package this for the major distros.

So in your gentoo package, can you just run composer install && vendor/bin/phpunit instead of phpunit ?

There are a lot of reasons this won't work. I don't want to beat a dead horse too much, but, what would that even do? Where would the library be installed?

The distribution package manager and Composer have different audiences. Composer makes it really easy for you to get set up if you're developing something written in pure PHP, but it doesn't help end users deploy that thing when you're done. Composer is really just a fancy interface to copy/paste. The things it installs are untraceable and if some library you once installed on a hundred websites has a SQL injection hole, you're out of luck. If you want to keep track of what it installs, you need to write down every place you've ever used composer, and visit those locations every day, and run composer update (or whatever it's called) in those hundred places. If you automate that process and remove all of the other limitations and find 100 other people to work on composer for ten years, well, the end result would look a lot like a distribution package manager because that's what they do =)

There are other problems with using composer to install. One is that there's no audit trail for what happens. All of our packages are GPG signed to make sure you install the right thing, but composer just grabs whatever is on the other end of the HTTP connection and blindly installs it. How do you do a global install to a fixed location? You can't... and if you could, you would be running some stranger's code as root as soon as you tried to run the test suite.

We also have a huge reliable mirror infrastructure... whatever is on the other end of composer install probably doesn't. And then there's network-less installation. Lots of our users build packages in a central location and ship them out to thousands of firewalled hosts. It's both policy and a practical matter that you should be able to install a package with your ethernet cable unplugged.

But really, what it comes down to, is that copy/paste is a bad way to deploy code. I'm a small fry compared to a lot of our users, but I have a few hundred websites to babysit and some tens of thousands more packages to keep secure and up-to-date across a bunch of servers. There's no way something like composer can do that for you. (What would happen if all 1,000 packages on your desktop system were installed with composer?) The distro package manager is the right way to do it -- do it once, make sure everything is consistent, and then keep track of what is installed and where.

@mtibben
Copy link
Owner

mtibben commented Dec 6, 2015

Thanks for the detailed explanation, you've made a good case. I wonder then if a generic PSR-4 autoloader can be a package dependency rather than relying on composer's - is that even possible?

As there are no plans for this library to be more than 1 file let's get this merged as it's fairly trivial and you've demonstrated some value in doing this

Thanks for the contribution!

mtibben added a commit that referenced this pull request Dec 6, 2015
Add a simple bootstrap file to decouple the test suite from Composer.
@mtibben mtibben merged commit dc82bfd into mtibben:master Dec 6, 2015
@orlitzky
Copy link
Contributor Author

orlitzky commented Dec 6, 2015

Thanks!

I wonder then if a generic PSR-4 autoloader can be a package dependency rather than relying on composer's - is that even possible?

It should be easy, but I'm not aware of one. PSR-4 specifies a naming convention that essentially lets you replace \\ by DIRECTORY_SEPARATOR and then load the resulting path:

When loading a file that corresponds to a fully qualified class name ...

  • A contiguous series of one or more leading namespace and sub-namespace names, not including the leading namespace separator, in the fully qualified class name (a "namespace prefix") corresponds to at least one "base directory".
  • The contiguous sub-namespace names after the "namespace prefix" correspond to a subdirectory within a "base directory", in which the namespace separators represent directory separators. The subdirectory name MUST match the case of the sub-namespace names.
  • The terminating class name corresponds to a file name ending in .php. The file name MUST match the case of the terminating class name.

Alas, composer doesn't enforce those rules, so a lot of package authors violate them. For example, the paths may be in all lowercase while the namespaces are mixed-case. Or the filenames look like Foo.class.php instead of the required Foo.php. If everyone followed the rules, it would be trivial to write a PSR-4 autoloader that worked for everyone in about 10 lines of code. Very frustrating.

Thanks again!

@stof
Copy link

stof commented Apr 26, 2016

Alas, composer doesn't enforce those rules, so a lot of package authors violate them

Composer applies these rules exactly as is when a project says it uses PSR-4.
The only difference is that developers running their project on case sensitive filesystems (which is the default on Windows and Mac but not Linux) won't see a failure if they don't respect the case, because the filesystem simply tells that the file exists.
The DebugClassLoader shipped in the Symfony Debug component implemented some complex logic to catch such cases, so that people developing on Mac don't see failures only when deploying to a Linux prod server. But this logic is very complex (Windows detection was simpler than Mac detection there btw) and has a non-negligeable perf impact, which is why Composer itself does not both about adding it.

The next thing is that Composer accepts other ways to configure the autoloader than PSR-0 and PSR-4. But then, your tool building the package could require these packages to ship an autoloader (or just build the autoloader on their own with a different logic, as it is also possible, as shown by what composer does)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants