Allow manual installation about php-redmine-api

[Art4]

What about creating a autoload.php file with all require_once() calls inside?

Example: lib/autoload.php (untested!)

<?php

// require this file if you aren't using composer, e.g.
// require_once(dirname(__FILE__) . '/php-redmine-api/lib/autoload.php');

require_once(dirname(__FILE__) . '/Redmine/Api/AbstractApi.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Attachment.php');
require_once(dirname(__FILE__) . '/Redmine/Api/CustomField.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Group.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Issue.php');
require_once(dirname(__FILE__) . '/Redmine/Api/IssueCategory.php');
require_once(dirname(__FILE__) . '/Redmine/Api/IssuePriority.php');
require_once(dirname(__FILE__) . '/Redmine/Api/IssueRelation.php');
require_once(dirname(__FILE__) . '/Redmine/Api/IssueStatus.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Membership.php');
require_once(dirname(__FILE__) . '/Redmine/Api/News.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Project.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Query.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Role.php');
require_once(dirname(__FILE__) . '/Redmine/Api/SimpleXMLElement.php');
require_once(dirname(__FILE__) . '/Redmine/Api/TimeEntry.php');
require_once(dirname(__FILE__) . '/Redmine/Api/TimeEntryActivity.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Tracker.php');
require_once(dirname(__FILE__) . '/Redmine/Api/User.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Version.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Wiki.php');
require_once(dirname(__FILE__) . '/Redmine/Client.php');

[kbsali]

@mjorlitzky i'm with @Art4 : could you give his solution a try?
Thanks

[orlitzky]

That'll work for me personally, but won't be so nice for our end users if I package it for Gentoo.

1. Users who just want to use the library will need to know about the modification we've made. If they just read the documentation, they're going to try to do require('Redmine/Client.php'); and it won't work.
2. Any application depending on the library needs to be patched (in how many places?) to run autoload.php before it tries to use the library. These packages might not be maintained by myself, so it's a lot to ask of someone else to patch potentially hundreds of PHP files.
3. I have to maintain a patch set on top of whatever the current version(s) are, and update the patch every time a new file is added to the API.

Given those downsides, I'd probably opt to just patch the source files like I've done above. I would still have to update the patch every once in a while, but at least it wouldn't bother anyone else. Of course I would rather do nothing, though =)

[osroot25]

Perhaps too simple-minded, but: Can't you just copy the lib-folder wherever you want it in your project-dir and than, in your project, register the namespace \Redmine to the corresponding dir?

[orlitzky]

First of all, copy/pasting code shouldn't be considered a solution to anything =)

The problems it causes are familiar-enough that I don't need to repeat them here, and they're the reason I'd like to be able to create a distro package for the library and install it once globally.

None of the local fixes address the real problem (please excuse the pejorative): the library appears broken for end-users who aren't me. I can't install and maintain a mangled version that requires secret voodoo for our web developers at work, and I can't ship the same to our end-users on Gentoo either. But as is, it just doesn't work.

For example, Api/Attachment.php requires the AbstractApi class. It doesn't require('AbstractApi.php');, though, so the source essentially doesn't "compile." It's missing a dependency. If you're fortunate enough to use Attachment.php in some other file that has already included AbstractApi.php, it will appear to work. But it's serendipity in that case: AbstractApi.php is still missing a dependency.

It's like if you had a Foo class inherited from Bar, and you expected everyone who uses Foo to do require('bar.php'); before they could do require('foo.php'); it's silly. Composer works around the missing dependency with the nuke-it-from-outer-space approach, but it has its own downsides. It's a bazillion times slower than just specifying the dependencies, since it has to go look around every time you use a class. And it requires an unrelated third-party build tool to be installed in order to run the library, which doesn't make any sense. You don't need GNU Make installed to run cat, do you?

[funivan]

@orlitzky if author add require it will be an overhead. It`s not good. This package optimized for php project. All use composer and this is like standard.

You can simple create bootstrap file or use composer in any system that you want. But composer you will have a lot of benefits.

You don't need GNU Make installed to run cat, do you?

Yes. you don`t But remember there are a lot of contributors to make cat work in all systems *nix. This is other story and this is hard to create php project optimized for web and for command line scripts in any platform.

[osroot25]

So, in your entire project, you don't use ANY classloader (to whom you can register the namespace Redmine, which effectively gives you the required require)?

There is NO chance you can add the three lines from http://php.net/manual/de/language.oop5.autoload.php to some script that uses this api?

[orlitzky]

@funivan Have you actually tried it? Autoload is many times slower than just adding the necessary require directives. Why? Because that's all autoload.php is doing! Except it's doing it on demand, and it doesn't know where the files are.

PHP is not a clever language. When all is said and done, the interpreter sees a big linear blob of code. If the require directive isn't there, the code won't work. So before the interpreter actually runs Attachment.php, you can be sure that it's already run AbstractApi.php. The guy who wrote the code knows where AbstractApi.php is, so the best you can do is take the unavoidable performance hit of require('AbstractApi.php') which is not even noticeable. Autoload, on the other hand, has to stop and go look for the file containing the AbstractApi class every time you use it. Try it before you tell me I'm wrong.

It's not hard to create PHP scripts that work on the web and on the command-line, and PHP is no different than C or any other language. If you do things right, they work. If you don't, they don't. Right now the library crashes out-of-the-box both on the web and the command line. With the proper require statements, it works on both, and we can package it and make it installable globally and everything is great. It's like 15 lines of requires that need to be there anyway. Not what I would call hard.

[orlitzky]

@osroot25 You're looking at this all wrong. I would like to be able to create a distribution package for php-redmine-api that works out of the box. I don't know who's going to install it, and I can't SSH into their homes and add any number of lines of code to whatever projects they're working on.

I am using this for a personal project, and I am able to work around the problem by patching the source or registering an autoloader or whatever. But I'd like to have the library installed globally and use the package manager to make sure that no one e.g. uninstalls php-redmine-api. To do that, I need to create a package for php-redmine-api. To do that, I need to kill the dependency on composer.

[funivan]

@orlitzky )) Yes i try it. If you use multiple libraries in project autoloader is best solution. If you use only 1 library and you require all classes of it - possible require is better.
But. I use redmine in several projects. They have multiple libraries with many classes. All of them use autoloader and it is much faster then require all classes of all libraries.
Read it http://blog.ircmaxell.com/2012/07/is-autoloading-good-solution.html

In general: current installation method is very good.

[orlitzky]

Loading All The Classes
The first and simplest test loads all 1000 classes. Not surprisingly, the autoload solution is
significantly slower than the hard-coded version. Shockingly over 42% slower. The hard coded test
ran in -on average- 0.0210 seconds. The autoloaded solution ran in -on average- 0.0300 seconds.
So it's clear that if you're loading all of your classes, hard-coding is clearly a better solution.

This is true not only when you "load all your classes," but whenever autoload will load the same number of classes as the manual require. In this case, I've added the exact require statements necessary, and autoload.php would load all of them. So you can expect the manual require statements to be a whole lot faster.

In your own projects, you don't need to worry either way: I've added the require statements to the library, so you can remove require('autoload.php'); from your code. You don't, then, have to require anything else yourself.

[Art4]

How about add the autoload.php file to the source and mention it in the docs as an alternative installation method? This will (probably?) solve the issue and keeps the files clean from require.

The downside is that we have to think about the autoload.php if a new file is added. Or we add a simplified autoloader in autoload.php.

[orlitzky]

@Art4 Is the goal really to keep a few require statements out of the top of a file? If so, that's probably the best you can do. But:

• It still requires everyone to require('autoload.php'); in their client code before anything will work.
• It's much slower than the manual requires
• If you ever want to go back and require only what's needed in each file , you'll have to disentangle the mess in autoload.php.
• It's harder to regenerate autoload.php than it is to add a new require at the top of a file.

It would still be better than the current situation, though, where I have no way to generate autoload.php myself.

[Art4]

It still requires everyone to require('autoload.php'); in their client code before anything will work.

Sure, but this is the same with your solution. What's the different between require('lib/Redmine/Client.php') and require('lib/autoload.php')? How can you get around this require()?

It's much slower than the manual requires

Hmm, I'm not sure. If you call Client.php in your solution it will need almost the same time. I'm not sure it will be much slower. We could banchmark this. :-)

If you ever want to go back and require only what's needed in each file , you'll have to disentangle the mess in autoload.php.

I think it will be a greater mess to keep the dependency for each file through require() calls. If e.g. sometimes Client.php depends on ClientAbstract.php you have to remember to add the new require() call.

And this is a simple untested autoloader:

<?php

// lib/autoload.php

spl_autoload_register(function ($class) {
    if (file_exists(dirname(__FILE__) . 'Redmine/Api/' . $class . '.php'))
    {
        require_once dirname(__FILE__) . 'Redmine/Api/' . $class . '.php';
    }

    if (file_exists(dirname(__FILE__) . 'Redmine/' . $class . '.php'))
    {
        require_once dirname(__FILE__) . 'Redmine/' . $class . '.php';
    }
});

It's harder to regenerate autoload.php than it is to add a new require at the top of a file.

I don't get this point, sorry. :-/

[orlitzky]

Sure, but this is the same with your solution.

I guess if the autoloader is capable of loading the Client class, it's the same. I was thinking you'd have to do both,

require('lib/autoload.php');
require('lib/Redmine/Client.php');

About the speed, I could try to convince you from first principles that the sky is not pink, but I agree a benchmark is in order. The stupidest thing that could possibly work (benchmark.php):

<?php
//require('lib/php-redmine-api/lib/autoload.php');
require('lib/php-redmine-api/lib/Redmine/Client.php');

$start_time = microtime(TRUE);
$c = new Redmine\Client("foo","bar");
$end_time = microtime(TRUE);

echo $end_time - $start_time;
?>

At the top, there are two require statements, one commented out. The first works with your autoload.php, the second with my patched library. Even if we include the start-up time for the php process, the numbers I get are waaaayyy in my favor. Averaging 10 trials, I get 0.0006499767303466801s for autoload.php and 4.37974929809571e-05s for the manual require statements. That's a 15x slowdown for autoload.php, and it would be much worse if we didn't start a PHP process each time.

I think it will be a greater mess to keep the dependency for each file through require() calls. If e.g. sometimes Client.php depends on ClientAbstract.php you have to remember to add the new require() call.

You don't have to remember anything. If you add code to e.g. Client.php that uses some class without the associated require, then it will crash. So you add the require. This is how programming without autoload.php has worked forever =)

And this is a simple untested autoloader

You need to account for the Redmine\ prefix in the class names, but yeah it seems to work. Here's what I wound up with:

<?php
// lib/autoload.php

spl_autoload_register(function ($class) {
    $class = str_replace('\\', '/', $class);

    $path = dirname(__FILE__) . '/' . $class . '.php';
    if (file_exists($path)){
      require_once($path);
    }

    $path = dirname(__FILE__) . '/' . $class . '.php';
    if (file_exists($path)) {
      require_once($path);
    }
});
?>

I don't get this point, sorry. :-/

I thought you were suggesting generating an autoload.php using composer, and then committing it to the repo. If you write your own, there's no extra work to be done.

I think keeping track of the includes is better for one's long-term sanity, but I would be grateful for this solution as well.

[orlitzky]

I should also mention that lib/autoload.php would be a bad choice of path, in case some other project does the same thing (require('lib/autoload.php'); would then be ambiguous). Something like lib/Redmine/autoload.php would be less likely to cause problems, but if it comes down to it, the end user could sort that out himself.

[Art4]

I'm surprised about the benchmark results. I have assumed, that the result would differ for about 50%. The require() calls are the same but in different files. Maybe we should remove the dirname(__FILE__) in autoload.php.

At the top, there are two require statements, one commented out. The first works with your autoload.php, the second with my patched library.

Are you sure you don't mixed up the code? The benchmark.php looks to me it used the same code. If I'll have time I will test it today. benchmark.php should look like this:

<?php
//require('lib/php-redmine-api_art4/lib/autoload.php');
require('lib/php-redmine-api_orlitzky/lib/Redmine/Client.php');

$start_time = microtime(TRUE);
$c = new Redmine\Client("foo","bar");
$end_time = microtime(TRUE);

echo $end_time - $start_time;
?>

This is how programming without autoload.php has worked forever =)

But this is not how this library worked till today. All developers who have not chanced the code are using the library with composer. If we forget a require() call composer will catch this and autoload the class.

I recommend to keep the files clear of the require calls. But regardless of our discussion, the dessision is up to @kbsali.

[orlitzky]

I'm surprised about the benchmark results. I have assumed, that the result would differ for about 50%. The require() calls are the same but in different files. Maybe we should remove the dirname(__FILE__) in autoload.php.

The difference is much bigger than it looks. If you have two relatively close numbers, like say 5 and 6, you can say that 6 is 20% bigger. But if you add a big number to both of them -- say, 100, you get 105 and 106, and the latter looks not that much bigger anymore. The benchmark I did starts up a php process each time, and that has the same effect as adding 100 to both numbers: the PHP start-up time is a huge portion of the total runtime. But since it still showed a 15% slowdown, I didn't see a reason to do a more careful benchmark: it's obvious that the autoload is way slower.

Both methods are including the same files, eventually, but autoload has to register a callback, run the callback every time a class is instantiated, check if a file exists, check if it's been required already, etc.

The benchmark doesn't even use any of the features in Client.php, if you actually do something, autoload will need to load more classes and it will get even slower.

Are you sure you don't mixed up the code? The benchmark.php looks to me it used the same code. If I'll have time I will test it today. benchmark.php should look like this:

Yes, I was simply renaming the php-redmine-api directory.

[orlitzky]

But this is not how this library worked till today. All developers who have not chanced the code are using the library with composer. If we forget a require() call composer will catch this and autoload the class.

It occurs to me that, even if you use composer to install the library, nobody is going to require_once 'vendor/autoload.php'; if it isn't necessary. If the require statements are added to the files that need them, everyone can remove that line from their code. Then no autoloading would occur if you forget to add a new require statement.

[Art4]

@orlitzky I have benchmark all three possibilities. 1. my list of require calls, 2. your solution in the first post, and 3. the simple autoloader. I have used your benchmark.php with some changes.

1. require list

in lib/autoload.php

<?php

// require this file if you aren't using composer, e.g.
// require_once(dirname(__FILE__) . '/php-redmine-api/lib/autoload.php');

require_once(dirname(__FILE__) . '/Redmine/Api/AbstractApi.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Attachment.php');
require_once(dirname(__FILE__) . '/Redmine/Api/CustomField.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Group.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Issue.php');
require_once(dirname(__FILE__) . '/Redmine/Api/IssueCategory.php');
require_once(dirname(__FILE__) . '/Redmine/Api/IssuePriority.php');
require_once(dirname(__FILE__) . '/Redmine/Api/IssueRelation.php');
require_once(dirname(__FILE__) . '/Redmine/Api/IssueStatus.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Membership.php');
require_once(dirname(__FILE__) . '/Redmine/Api/News.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Project.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Query.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Role.php');
require_once(dirname(__FILE__) . '/Redmine/Api/SimpleXMLElement.php');
require_once(dirname(__FILE__) . '/Redmine/Api/TimeEntry.php');
require_once(dirname(__FILE__) . '/Redmine/Api/TimeEntryActivity.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Tracker.php');
require_once(dirname(__FILE__) . '/Redmine/Api/User.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Version.php');
require_once(dirname(__FILE__) . '/Redmine/Api/Wiki.php');
require_once(dirname(__FILE__) . '/Redmine/Client.php');

2. require() on top of files

I have made the changes from your first post.

3. simple autoloader

in lib/autoload.php

<?php

spl_autoload_register(function ($class) {
    $class = str_replace('\\', '/', $class);

    $path = dirname(__FILE__) . '/' . $class . '.php';
    if (file_exists($path)){
      require_once($path);
    }
});

benchmark.php

I have used this file to benchmark all three possibilities. The results are micro seconds.

<?php

//require('php-redmine-api_art4/lib/autoload.php');             // 1
//require('php-redmine-api_orlitzky/lib/Redmine/Client.php');   // 2
require('php-redmine-api_autoload/lib/autoload.php');         // 3

$start_time = microtime(true);
$c = new Redmine\Client("foo","bar");
$end_time = microtime(true);

echo sprintf('%.10f', $end_time - $start_time) * 100000 . " \n";

Results

Results are in microseconds.

1 (Art4)	2 (orlitzky)	3 (Autoloader)
4,22001	3,50475	59,79538
4,69685	3,98159	49,90101
5,79357	3,60012	61,48815
3,79086	3,91006	57,60193
4,72069	3,50475	70,40501
6,60419	5,19753	58,29334
3,88622	4,91142	49,71027
5,19753	3,31402	50,30632
4,19617	4,00543	58,29334
5,81741	3,38554	57,29198
3,79086	3,60012	141,38222
4,60148	4,41074	49,4957
4,19617	5,6982	47,49298
5,60284	4,41074	47,58835
4,41074	4,29153	60,10532
3,79086	4,1008	64,39686
4,29153	3,38554	64,58759
4,69685	14,49585	57,29198
3,60012	3,60012	47,39761
3,79086	4,1008	51,59378
Average	Average	Average
4,5847905	4,5704825	60,220956

tomorrow more...

[orlitzky]

I have benchmark all three possibilities.

Thanks for taking the time to do that. Your solution and mine will benchmark roughly the same because right now Client is the only class that end-users interact with, and Client.php includes pretty much the entire list of files in the Redmine/Api folder. But, suppose some day another user-facing class gets added -- let's call it Server.php since this is all hypothetical and nothing matters =)

If we add ~20 new files under Redmine/Api called e.g.,

• Redmine/Api/ServerAbstractApi.php
• Redmine/Api/ServerAttachment.php

...

• Redmine/Api/ServerWiki.php

then the two benchmarks would begin to diverge. Ostensibly the Client class would only need half of the includes, and the Server class would need the other half. If the end-user just wants to make some client calls, he'd load up Client.php and pull in half of the API classes with the require directives at the top of Client.php. But with all of the requires in a separate file, he'd have to load everything -- twice as many files as he needs, since he won't be using the server stuff.

Note: I don't actually care about the performance, so that isn't a big deal, but it's part of the reason why I consider require-at-the-top to be a better design. I only started talking benchmarks because it was claimed that autoload was faster and that was given as a reason to leave things alone.

[Art4]

I've done this benchmark because I thought you benchmarked our first solutions (1 and 2) and I was surprised your solution 2 was 15x faster than mine. 😄 But at all we are talking about microseconds.

To sum everything up:

• Solution 1: fast, keeps the installation methods seperated, but need to be maintained.
• Solution 2: fastest, but need to be maintained
• Solution 3: keeps the installation methods seperated, need not be maintained, but is 15x and more slower than 1 or 2

I think, we discussed everything. @kbsali what do you think?

[kbsali]

@Art4 thanks a lot for the clear benchmark! 👍

As far as I'm concerned, Solution 1 and 3 are the only valid ones; and given that solution 3 appears to be much slower than the 2 others, I would go in favor @Art4 's!
Also, @orlitzky, we have not added any new files/classes in the past year, so maintaining a "static" autoload.php should not be an issue.

Feel free to create a PR to get this in.

Again, thank you all for trying to getting around all this!

[kbsali]

fixed in #96
thanks again @orlitzky !