Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
The line
\Drupal::service('module_installer')->install(['social_api']);
In the requirements hook, just shouldn't be there. The dependency in the info.yml file is enough.
Right now, if you want to use social_auth in an install profile, this creates issues, since it then tries to install social_api way too early.
Patch will follow soon.
Comment | File | Size | Author |
---|---|---|---|
#15 | 2943924-15.patch | 866 bytes | gvso |
| |||
#10 | dependency-declaration-10.patch | 743 bytes | MaskyS |
#8 | dependency-declaration-8.patch | 1.09 KB | MaskyS |
#4 | 2943924-4-dependency_declaration.patch | 641 bytes | daften |
#3 | 2943924-3-dependency_declaration.patch | 575 bytes | daften |
Comments
Comment #2
daften CreditAttribution: daften at District09, Picabit commentedComment #3
daften CreditAttribution: daften at District09, Picabit commentedAdded a patch for this
Comment #4
daften CreditAttribution: daften at District09, Picabit commentedFormer patch didn't work, better patch in attachment. Installing requirements in hook_requirements is simply not done.
Comment #5
gvsoThe intention of that part of the code was not to install Social API because it is a dependency, but rather to check if the correct composer library was installed. But in order to call the checkLibrary method, Social API must be installed.
For reference, check #2847739: Provide method to check if correct composer library is installed
Comment #6
daften CreditAttribution: daften at District09, Picabit commentedIt's still the wrong method to do it. The patch checks if social_api is installed and doesn't mess with the normal flow of things. Since the requirements hook is triggered in several places, the check will happen at the right time too.
Comment #7
gvsoI see..
My concern with the patch's solution is that the library won't be checked if Social API is not installed. Yes, this is actually not a big issue since when installing the module, you do it with composer which automatically downloads the dependencies.
I believe this method was implemented in Social API when composer wasn't completely integrated and there were some reports that were related to a missing library, so this approach might be considered 'outdated' at this point.
Any thoughts on this?
Comment #8
MaskyS CreditAttribution: MaskyS at Google Code-In commentedI agree with the point daften is making, but gvso is right too. As the patch is right now it won't check the installed library if Social API itself isn't installed. I wouldn't consider this outdated because I think we should let users install Social API manually without going through composer.
I went through implementations of hook_requirements by core modules. Here's something similar in the attached patch. Would be great if it could be tested.
Comment #9
gvsoJust found out that there's no need to install nor to check if Social API is installed. However, I noticed that Social Auth can be installed even if PHP League OAuth2 is not downloaded.
Comment #10
MaskyS CreditAttribution: MaskyS at Google Code-In commentedNow that I think about it, you're right! Drupal itself would verify if Social API is installed and abort if not since the dependency is listed in .info file. So let's just remove the check, and see why Social Auth is installing without PHP OAuth2 in a separate follow-up issue?
Comment #11
gvsoYes, this is the right approach, and I agree we should work on the problem I mentioned in a separate issue. However, we might need to solve the problem before applying this patch.
Comment #12
gvsoComment #13
gvsoComment #14
gvsoComment #15
gvsoComment #17
gvso