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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Alexander Tsybenko created an issue. See original summary.

Sam152’s picture

Status: Active » Postponed (maintainer needs more info)

Can you please post what the error is?

AlexTsy’s picture

FileSize
178.43 KB

Installation just don't want to finish.

At server logs there no php errors.

At watchdog appears only warning. See attachment

zach.bimson’s picture

Full log here:

Invalid argument supplied for foreach() in _batch_finished() (line 405 of core/includes/batch.inc). _batch_finished() (Line: 333)                                             [warning]
_batch_process() (Line: 865)
batch_process(Object, Object) (Line: 605)
install_run_task(Array, Array) (Line: 526)
install_run_tasks(Array) (Line: 116)
install_drupal(Object, Array) (Line: 726)
drush_call_user_func_array('install_drupal', Array) (Line: 711)
drush_op('install_drupal', Object, Array) (Line: 80)
drush_core_site_install_version('cr', Array) (Line: 247)
drush_core_site_install('cr', 'Campaign')
call_user_func_array('drush_core_site_install', Array) (Line: 366)
_drush_invoke_hooks(Array, Array) (Line: 217)
drush_command('cr', 'Campaign')
call_user_func_array('drush_command', Array) (Line: 185)
drush_dispatch(Array) (Line: 67)
Drush\Boot\BaseBoot->bootstrap_and_dispatch() (Line: 66)
drush_main() (Line: 12)
zach.bimson’s picture

I'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.

gbyte’s picture

Title: Installation profile » Adding as dependency in installation profile produces error
Status: Postponed (maintainer needs more info) » Active

@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?

Sam152’s picture

Yeah, 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?

gbyte’s picture

Yes 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 a generateSitemap('no_batch') for these cases...

IT-Cru’s picture

We 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.

AlexTsy’s picture

hook_enable() was removed in D8
https://www.drupal.org/node/2193013

IT-Cru’s picture

Shame 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.

benelori’s picture

Yes 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 a generateSitemap('no_batch') for these cases...

I'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.

gbyte’s picture

@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.

benelori’s picture

Hello!

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.

zach.bimson’s picture

@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

  • gbyte.co committed f0eca20 on 8.x-2.x
    Issue #2696373 by Alexander Tsybenko: Adding as dependency in...
gbyte’s picture

Status: Active » Fixed

@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.

gbyte’s picture

Version: 8.x-2.2 » 8.x-2.x-dev
zach.bimson’s picture

Thanks for the update dude

heddn’s picture

Status: Fixed » Needs work

This doesn't fix the problem for me when using 8.x-2.x.

You are about to DROP all tables in your 'drupal' database. Do you want to continue? (y/n): y
Starting Drupal installation. This takes a while. Consider using the --notify global option.                        [ok]
PDOException: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'drupal.taxonomy_term_field_data'        [error]
doesn't exist in /var/www/drupalvm/web/core/lib/Drupal/Core/Database/Statement.php:59
Stack trace:
#0 /var/www/drupalvm/web/core/lib/Drupal/Core/Database/Statement.php(59): PDOStatement->execute(Array)
#1 /var/www/drupalvm/web/core/lib/Drupal/Core/Database/Connection.php(610):
Drupal\Core\Database\Statement->execute(Array, Array)
#2 /var/www/drupalvm/web/core/lib/Drupal/Core/Database/Driver/mysql/Connection.php(81):
Drupal\Core\Database\Connection->query('SELECT t.tid AS...', Array, Array)
#3 /var/www/drupalvm/web/core/lib/Drupal/Core/Database/Query/Select.php(476):
Drupal\Core\Database\Driver\mysql\Connection->query('SELECT t.tid AS...', Array, Array)
#4 /var/www/drupalvm/web/modules/contrib/simple_sitemap/src/Batch.php(165):
Drupal\Core\Database\Query\Select->execute()
#5 [internal function]: Drupal\simple_sitemap\Batch::generateBundleUrls(Array, Array, Array, Array)
#6 /var/www/drupalvm/web/modules/contrib/simple_sitemap/src/Batch.php(78):
call_user_func_array('Drupal\\simple_s...', Array)
#7 /var/www/drupalvm/web/modules/contrib/simple_sitemap/src/SitemapGenerator.php(48):
Drupal\simple_sitemap\Batch->start()
#8 /var/www/drupalvm/web/modules/contrib/simple_sitemap/src/Simplesitemap.php(101):
Drupal\simple_sitemap\SitemapGenerator->startBatch()
#9 /var/www/drupalvm/web/modules/contrib/simple_sitemap/simple_sitemap.install(83):
Drupal\simple_sitemap\Simplesitemap->generateSitemap('nobatch')
#10 [internal function]: simple_sitemap_install()
#11 /var/www/drupalvm/web/core/lib/Drupal/Core/Extension/ModuleHandler.php(391):
call_user_func_array('simple_sitemap_...', Array)
#12 /var/www/drupalvm/web/core/lib/Drupal/Core/Extension/ModuleInstaller.php(287):
Drupal\Core\Extension\ModuleHandler->invoke('simple_sitemap', 'install')
#13 /var/www/drupalvm/web/core/lib/Drupal/Core/ProxyClass/Extension/ModuleInstaller.php(83):
Drupal\Core\Extension\ModuleInstaller->install(Array, false)
#14 /var/www/drupalvm/web/core/lib/Drupal/Core/Config/ConfigImporter.php(786):
Drupal\Core\ProxyClass\Extension\ModuleInstaller->install(Array, false)
#15 /var/www/drupalvm/web/core/lib/Drupal/Core/Config/ConfigImporter.php(551):
Drupal\Core\Config\ConfigImporter->processExtension('module', 'install', 'simple_sitemap')
#16 /var/www/drupalvm/web/core/lib/Drupal/Core/Config/ConfigImporter.php(488):
Drupal\Core\Config\ConfigImporter->processExtensions(Array)
#17 /var/www/drupalvm/web/profiles/contrib/config_installer/config_installer.profile(137):
Drupal\Core\Config\ConfigImporter->doSyncStep('processExtensio...', Array)
#18 /var/www/drupalvm/web/core/includes/batch.inc(252):
config_install_batch_process(Object(Drupal\Core\Config\ConfigImporter), 'processExtensio...', Array)
#19 /var/www/drupalvm/web/core/includes/form.inc(872): _batch_process()
#20 /var/www/drupalvm/web/core/includes/install.core.inc(609): batch_process(Object(Drupal\Core\Url),
Object(Drupal\Core\Url))
#21 /var/www/drupalvm/web/core/includes/install.core.inc(530): install_run_task(Array, Array)
#22 /var/www/drupalvm/web/core/includes/install.core.inc(115): install_run_tasks(Array)
#23 /var/www/drupalvm/vendor/drush/drush/includes/drush.inc(726):
install_drupal(Object(Composer\Autoload\ClassLoader), Array)
#24 /var/www/drupalvm/vendor/drush/drush/includes/drush.inc(711):
drush_call_user_func_array('install_drupal', Array)
#25 /var/www/drupalvm/vendor/drush/drush/commands/core/drupal/site_install.inc(80):
drush_op('install_drupal', Object(Composer\Autoload\ClassLoader), Array)
#26 /var/www/drupalvm/vendor/drush/drush/commands/core/site_install.drush.inc(247):
drush_core_site_install_version('config_installe...', Array)
#27 /var/www/drupalvm/vendor/drush/drush/includes/command.inc(366):
drush_core_site_install('config_installe...')
#28 /var/www/drupalvm/vendor/drush/drush/includes/command.inc(217): _drush_invoke_hooks(Array, Array)
#29 /var/www/drupalvm/vendor/drush/drush/includes/command.inc(185): drush_command('config_installe...')
#30 /var/www/drupalvm/vendor/drush/drush/lib/Drush/Boot/BaseBoot.php(67): drush_dispatch(Array)
#31 /var/www/drupalvm/vendor/drush/drush/includes/preflight.inc(66):
Drush\Boot\BaseBoot->bootstrap_and_dispatch()
#32 /var/www/drupalvm/vendor/drush/drush/drush.php(12): drush_main()
#33 {main}
heddn’s picture

Status: Needs work » Needs review

But #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.

Status: Needs review » Needs work
gbyte’s picture

Status: Needs work » Closed (fixed)

This 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.