vivekrajenderan / simplesamlphp

Automatically exported from code.google.com/p/simplesamlphp
Other
0 stars 0 forks source link

Introduce Composer and externalise the SAML2 library #592

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Sorry this took a bit longer than I'd hoped, I just wanted to test everything 
before I handed you this patch.

This patch does the following:
- rm -r lib/SAML2 lib/Auth
- Add composer.json (package description) and composer.lock (dependency locking)
- XMLSecLibs, SAML2 lib, and PHP-OpenID are now loaded from their respective 
locations with given versions.
- Changed autoloader to use Composers autoloader.
- Added Composer download and install to release building script.

Note that with this change a checkout of SSP will not just work, you MUST run 
'composer install'.

Original issue reported on code.google.com by boy@ibuildings.nl on 6 Nov 2013 at 9:57

Attachments:

GoogleCodeExporter commented 9 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

GoogleCodeExporter commented 9 years ago
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:

GoogleCodeExporter commented 9 years ago
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