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

Fix incompatibility with --classmap-authoritative #27

Closed
wants to merge 2 commits into from

Conversation

enumag
Copy link
Contributor

@enumag enumag commented Jul 14, 2016

I tried to replace --optimize-autoloader with --classmap-authoritative for my CI and got this exception:

  Fatal error: Class 'PackageVersions\Versions' not found 

It seems that the Versions class was generated but is missing in the composer autoloading classmap because when I added the file manually in composer.json it works fine:

    "autoload": {
        //...
        "files": [
            "vendor/ocramius/package-versions/src/PackageVersions/Versions.php"
        ]
    },

@Ocramius
Copy link
Owner

Any test case/scenario that can be used to automate this failure and verify its correctness?

@Ocramius Ocramius added the bug label Jul 14, 2016
@Ocramius Ocramius added this to the 1.0.5 milestone Jul 14, 2016
@Ocramius Ocramius self-assigned this Jul 14, 2016
@enumag
Copy link
Contributor Author

enumag commented Jul 20, 2016

@Ocramius I tried to add --classmap-authoritative to .travis.install.sh (and removed the fix). It caused the build to fail with:

1) PackageVersionsTest\VersionsTest::testValidVersions
Error: Class 'PackageVersions\Versions' not found

Which is the error we wanted to test. So I'll just commit the fix along with the .travis.installer.sh change and that should be it.

@Ocramius
Copy link
Owner

The test should be an additional one, not a replacement for current installation...

@enumag
Copy link
Contributor Author

enumag commented Jul 20, 2016

I don't think it's necessary in this case (if something works with --classmap-authoritative I'm sure it will work without it as well). But alright, have it your way. It has to be a separate travis build though.

@Ocramius
Copy link
Owner

I don't think it's necessary

Well, people usually run it without --classmap-authoritative, so we have to test with a separate flag for this particular scenario :-)

Anyway, I will have to take a closer look at this patch, since I might be able to also fix #18 in a similar way.

@Ocramius Ocramius modified the milestones: 1.1.0, 1.0.5 Jul 22, 2016
@Ocramius Ocramius closed this in 922e618 Jul 22, 2016
@Ocramius
Copy link
Owner

Manually rebased and merged, thanks!

@@ -20,7 +20,10 @@
"autoload": {
"psr-4": {
"PackageVersions\\": "src/PackageVersions"
}
},
"files": [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using files is quite bad, because this forces loading the file all the time instead of using autoloading

@stof
Copy link
Contributor

stof commented Jul 22, 2016

btw, due to the change done in #33, it is useless, as the classmap generator will contain the class due to the fallback class

@Ocramius
Copy link
Owner

Yeah, can be reverted...

@Ocramius
Copy link
Owner

86f2636

@stof
Copy link
Contributor

stof commented Jul 22, 2016

The proper way to ensure the class made it in the classmap (forgetting #33 which creates the file all the time) would have been to generate the file on the ScriptEvents::PRE_AUTOLOAD_DUMP instead

@itscaro
Copy link

itscaro commented Aug 6, 2019

Hello,

With 1.5.1 and --classmap-authoritative, I encounter this issue, is there any fix? Thank you

@Ocramius
Copy link
Owner

Ocramius commented Aug 6, 2019

@itscaro please write a test case reproducing the failure first

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

Successfully merging this pull request may close these issues.

None yet

4 participants