Manipulate the classmap rather than the generated classmap file #46
Conversation
|
|
||
| if (!file_exists($factoryFile)) { | ||
| $filesystem = new Filesystem(); | ||
| $filesystem->dumpFile($factoryFile, '<?php class '.$factoryClass.' {}'); |
There was a problem hiding this comment.
Is this here to avoid a Class not found problem? If it is, I think there should be a comment explaining it as it seems we are creating an empty class, which is weird for a generated factory.
There was a problem hiding this comment.
Yeah, Composer will only find classes that exist. It will be overwritten by Puli later on; will add a note.
|
That's a great PR, thanks @thewilkybarkid! |
thewilkybarkid
force-pushed
the
classmap
branch
from
48bc0aa
to
6d54d9d
Compare
thewilkybarkid
changed the title
|
Tests should pass now. |
|
Unfortunately, I can only +1 once. Wait, no.... 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍 |
|
Ping @webmozart :) |
|
From what I can tell, this patch does not fix the issue on a cold install, when no |
|
@magnusnordlander can you provide some context? Any autoloading generated? Is it only the "first" generation that doesn't fix the problem? (You said something about the puli directory) Although I haven't tested this solution, the approach seems to be better to me regarding the integration into the composer workflow, so it would be good to know whether there is a bug, an edge case to cover, or too much cases when this solution does not work. |
|
@sagikazarmark Better yet, I fixed it :) I've put up a PR up against the branch this PR is for (thewilkybarkid#1). Turns out the problem was that the stub file generated did not include a namespace, just using the FQCN as the class name, causing the classmap to be wrongly generated whenever the stub file is used. |
|
Any news? |
|
@magnusnordlander are you able to fix the tests in thewilkybarkid#1? I'll sort it out otherwise. As Composer 1.1.0 is out, this should be sorted soon. |
|
I've made the updates, so I think this is ready to go. |
|
@webmozart Could you take a look? As Composer 1.1 has had a stable release the plugin is currently broken on PHP 7. |
|
@webmozart Any chance of this getting merged? composer is already at v1.2 now... |
| if (!file_exists($this->autoloadFile)) { | ||
| $filesystem = new Filesystem(); | ||
| // Avoid problems if using the runner before autoload.php has been | ||
| // generated |
There was a problem hiding this comment.
Yes. Puli CLI, while doing the version check, tries to include it, leading to a warning message.
There was a problem hiding this comment.
Hm, can you outline when this exactly happens? I haven't seen any warning during my tests
There was a problem hiding this comment.
It doesn't happen directly in this library. Include your PR branch in another project, make sure the vendor folder doesn't exist and do a composer install.
I get "Warning: Version check failed: PHP Warning: require_once(/path/to/project/vendor/autoload.php): failed to open stream: No such file or directory in phar:///usr/local/bin/puli/vendor/puli/manager/src/Api/Puli.php on line 313
PHP Fatal error: require_once(): Failed opening required '/path/to/project/vendor/autoload.php' (include_path='.:') in phar:///usr/local/bin/puli/vendor/puli/manager/src/Api/Puli.php on line 313".
|
LGTM, thanks! :) |
|
OK, here are my findings: If puli/cli is installed in the current project, then this puli binary (from the current project) is preferred to be used. In this scenario, this issue is unfortunately not solved with #46, because this binary is not usable during To properly solve this I need more information on how puli is intended to work. The puli runner is used to get the puli config for the factory file and class name. Is there any other way to obtain this information at that point? If calling the runner is the only reliable way, is it required that the puli binary from the current project is used? or can we exclude this? |
|
Could we have a new release with this fix? We currently have a workaround in place that I would like to get rid of. |
|
Hi @pkruithof! We are working on getting a new release out ASAP. |
|
Not trying to be pushy, but there does not seem to be a lot going on for a couple of months now. This seems to be the case for multiple Puli repositories. I like the idea of the project but this way I'm afraid it's beginning to look like abandonware. Any news on progress or collaboration for this project? |
This is an attempt to fix puli/issues#190 by manipulating the classmap for the root package, rather than manipulating the generated autoload files (which in turn causes the file to appear in the new
autoload_static.phpcorrectly). The constant is still added toautoload.php, but as this is the one autoload file that has to exist it's safe to use.I haven't changed the tests yet, so Travis will fail (having some failures locally that I haven't yet understood).