Closed GoogleCodeExporter closed 8 years ago
Hi,
I have tried to test this now, but I am unable to make it work, and as far as I
can tell, it is due to something with the way composer is inserted into
simpleSAMLphp.
In composer.json, you have:
"autoload": {
"psr-0": {
"SimpleSAML_": "lib/"
},
"files": ["lib/_autoload.php"]
},
This tells Composer to load the lib/_autoload.php-script. However, Composer is
loaded from that script. Since Composer is using require() instead of
require_once(), you end up loading lib/_autoload.php twice, causing a
redeclaration of SimpleSAML_autoload.php.
I.e. www/_include.php loads lib/_autoload.php, which loads Composer, which in
turn loads lib/_autoload.php.
Other things:
- I do not think the build-script should download composer.phar. Instead,
require it to be on the path.
- Is the various SSP_*-constants in _autoload.php really required? In general,
I think we should avoid "polluting" the global namespace as much as possible.
If it means repeating the same string concatenation twice, so be it. If any
constants are required, I think they should be prefixed with "SIMPLESAML_"
rather than "SSP_". (In line with what we prefix our classes and our
autoload-function.
- Any chance that you could update docs/simplesamlphp-subversion.txt to mention
the «composer install» step as well?
Original comment by olavmrk@gmail.com
on 7 Nov 2013 at 12:28
Hi,
Strange, I have it working here.
Though I did wonder why that trick worked, I've rewritten it to move the module
autoloader to it's own file, which is more clean anyway.
Regarding the other things:
- Okay, I modified it.
- Ah yes, I added those for libsimplesaml, no longer required so I removed it.
- Sure, now's the time to ask :). I modified it, let me know if you're okay
with it?
Original comment by boy@ibuildings.nl
on 7 Nov 2013 at 2:55
Attachments:
As I already mentioned in an email, this patch has been committed (in r3290).
Closing the issue.
Original comment by olavmrk@gmail.com
on 15 Nov 2013 at 10:28
Original issue reported on code.google.com by
boy@ibuildings.nl
on 6 Nov 2013 at 9:57Attachments: