Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 UTC on 18 March 2024, to get $100 off your ticket.
Hello.
Using your module in the installation profile, causes an error.
When i removed function simple_sitemap_install() from simple_sitemap.install, installation finished successfully.
Comments
Comment #2
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedCan you please post what the error is?
Comment #3
AlexTsy CreditAttribution: AlexTsy commentedInstallation just don't want to finish.
At server logs there no php errors.
At watchdog appears only warning. See attachment
Comment #4
zach.bimson CreditAttribution: zach.bimson as a volunteer and at Comic Relief commentedFull log here:
Comment #5
zach.bimson CreditAttribution: zach.bimson as a volunteer and at Comic Relief commentedI'm not as familiar with the batch processing system as i aught to be but to me, stepping though the code it feels like its not passing the correct flag once its finished processing the batch ? Going to take some time out tomorrow to try and patch this.
Comment #6
gbyte CreditAttribution: gbyte as a volunteer and commented@Sam This issue could have something to do with the issue where web tests did not pass. The batch initialized by simple_sitemap in hook_install may interfere with the drupal install batch as it presumably did with the web test batch. I have no experience with testing batch processes or how batch processes can run simultaneously, maybe one of you has an idea?
Comment #7
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedYeah, the web tests weren't working from the UI due to the batch API. From what I could see stepping through it, you can't run a batch while in a batch due to too much global state. Perhaps a simple drupal_installation_attempted() check before attempting to run a batch would be helpful?
Comment #8
gbyte CreditAttribution: gbyte as a volunteer and commentedYes but batch (even the 'backend'
batch_process()
one which is independent from form API is the only way to create sitemaps and we need one after module installation, even if it only creates an empty sitemap. Maybe we need agenerateSitemap('no_batch')
for these cases...Comment #9
IT-CruWe have also problems with our automated github + travis deployments, when we enable simple_sitemap module. If we replace hook_install to hook_enable we get this working.
Perhaps this help other ones.
Comment #10
AlexTsy CreditAttribution: AlexTsy commentedhook_enable() was removed in D8
https://www.drupal.org/node/2193013
Comment #11
IT-CruShame on me. On drupalcontrib.org it is existing in D8 :/
Then perhaps a complete remove of initial sitemap generation during hook_install could be better.
Comment #12
benelori CreditAttribution: benelori commentedI'm not sure that is a good idea. There are projects that are built around running full site installs with a profile installations. This includes creating menu items, content and so on.
If we really need to generate a sitemap on install, then I think it's necessary to provide the full functionality and not just generate an empty sitemap.
Comment #13
gbyte CreditAttribution: gbyte as a volunteer and commented@benelori There is no need to generate the sitemap during site installation; in fact this would be a huge waste of resources. This can be taken care of manually or at first cron run (providing the sitemap settings have been set or imported via configuration).
Please be patient, I will be refactoring a couple of classes and will address this issue.
Comment #14
benelori CreditAttribution: benelori commentedHello!
No problem, I was just looking over some of the issues at a code sprint, so I'm in no hurry :D.
What I was thinking of is maybe implementing hook_install_tasks() to give the user the option at install.
Comment #15
zach.bimson CreditAttribution: zach.bimson as a volunteer and at Comic Relief commented@gbyte id agree that taking this out of the install would be better...
Initial generation could be done manually or on first cron run if not done manually
Comment #17
gbyte CreditAttribution: gbyte as a volunteer and commented@zach.bimson Well we need to generate the XML or alternatively remove the /sitemap.xml path until first generation, otherwise a broken page will be indexed.
I decided to create the generateSitemap('nobatch') option which runs the generation process on install without using the batch API. The batch class will still need some tidying up in the future though.
Comment #18
gbyte CreditAttribution: gbyte as a volunteer and commentedComment #19
zach.bimson CreditAttribution: zach.bimson as a volunteer and at Comic Relief commentedThanks for the update dude
Comment #20
heddnThis doesn't fix the problem for me when using 8.x-2.x.
Comment #21
heddnBut #9 seems to fix the issue. And still applies cleanly.
If an install profile is going to install this module as part of its installation, then some accommodations are needed. Either go with #9 or refer to #2696593: Fix the module install during config import as an alternative possible path forward.
Comment #23
gbyte CreditAttribution: gbyte as a volunteer and commentedThis is a different error, and it looks like the sitemap is trying to gather taxonomy terms through an SQL query while the taxonomy table has not been created yet (probably taxonomy module not installed yet).
Since 8.x-2.5 however, the module gathers entity data through an entityQuery() after checking if an entity type actually exists. This should be enough to prevent such an error.
I'm going to close this issue, as the bug from #4 has been fixed. I would really appreciate it if you could try to reproduce your error on the current dev and get back to me with your findings. Please open a new issue in case you can reproduce this bug.