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

farmOS theme gets disabled during install #272

Closed
mstenta opened this issue Apr 10, 2020 · 11 comments
Closed

farmOS theme gets disabled during install #272

mstenta opened this issue Apr 10, 2020 · 11 comments
Labels

Comments

@mstenta
Copy link
Member

mstenta commented Apr 10, 2020

@paul121 noticed a strange issue when setting up a new install of farmOS locally using the recommended Docker Compose configuration for development: https://github.com/farmOS/farmOS/blob/028d2266c5b15be1528f5425aa2db78307bc378a/docker/docker-compose.development.yml

The issue (which ended up being the tip of the iceberg) was that the farmOS map header block was not showing up on his farm dashboard.

We spent some time tracing it:

  • In the {block} database table, the farmOS map block was not being enabled.
  • Turns out that the farm_theme_block_info_alter() function was not running, which is what enables the block in the farmOS theme.
  • That led me to discover that the theme itself was not actually enabled.
  • However, the code that enabled the theme during farmOS installation WAS running, and was successfully enabling the theme.
  • Through testing of the installation process with XDebug, I found that the theme's record in the {system} table was being deleted after submitting the form on the "Configure site" install task step. So by the time you were brought to the "Configure farmOS" step (where you select optional modules to install), the theme record was already deleted.
  • Many more XDebug traces later, I found that it was happening during the exit call that happens in Drupal core's install_display_output() function: https://git.drupalcode.org/project/drupal/-/blob/460ff5dc3c577d6139b37bc3467ab5b742371626/includes/install.core.inc#L725
  • I started digging into _drupal_shutdown_function() (which runs via PHP's shutdown functions callbacks during exit) to see if I could pinpoint further, but needed to go to sleep.

In the morning, on a hunch, I decided to try changing the MariaDB version from mariadb:latest to mariadb:10.4 and that fixed it!

So next steps:

  • Continue to dig into _drupal_shutdown_function() (???)
  • And/or just set image: mariadb:10.4 in the recommended Docker Compose config.

I'd really like to figure out what's happening here - because I'm worried that the disabled theme might not be the only issue. That's just the one we noticed. But in order to understand the extent of this, it's important to understand exactly what is going wrong...

@mstenta mstenta added the bug label Apr 10, 2020
@mstenta
Copy link
Member Author

mstenta commented Apr 12, 2020

Updated the description because @paul121 tried to replicate the MariaDB version change fix, and it didn't work for him. So then I retried it and it also didn't work for me. I may have done a faulty test previously. Not really sure... in either case, all the rest is still true. So we can pick up where I left off in the shutdown functions and try to trace it.

@mstenta
Copy link
Member Author

mstenta commented Apr 13, 2020

Ok I've spent some more time tracing this and have a better idea what's happening now. Turns out that exit and shutdown functions were a red herring...

What's really happening is:

  • During interactive install (install.php in the browser), Drupal performs an AJAX request to admin/config/search/clean-urls/check to see if it's possible to enable clean URLs. This causes a normal Drupal request to happen through index.php. (The reason I didn't notice this is because it happens synchronously, so PHPStorm's debugger picks it up right after it finishes with the install.php steps, which is why I was stuck thinking this was a shutdown function issue, when in fact it was after the start of a NEW request.)
  • When this request to index.php executes, the Update module's implementation of hook_init() runs, which runs update_requirements() -> update_get_available() -> update_get_projects() -> system_rebuild_theme_data() -> _system_rebuild_theme_data() -> drupal_system_listing() -> drupal_get_profile()
  • The call to drupal_get_profile() returns standard, when it should return farm. This is because the install_profile variable is not set yet (it gets set at the end of the install process in install_finished()) - see https://api.drupal.org/api/drupal/includes%21common.inc/function/drupal_get_profile/7.x.
  • This then results in system_rebuild_theme_data() executing system_update_files_database() and THAT is where the DELETE query is actually run that removes farm_theme from the {system} table.

So it's because of this AJAX request to admin/config/search/clean-urls/check during the installation that the record gets removed from the database, because it thinks the install_profile is standard, when in fact it should be farm. So the themes that come with farmOS (bootstrap and farm_theme) are not found, and are thus removed from the {system} table.

If you manually add the install_profile variable to the {variable} table before submitting the "Configure site" form, then the theme record does not get deleted from the {system} table, and everything works as expected.

So... that's the cause. But there are still questions:

  • Why didn't we notice this before?
  • Is this affecting any modules as well? Why only themes?
  • Why aren't other Drupal 7 distributions reporting the same issue? Commerce Kickstart and Lightning both take the same approach to installing custom theme's in the install profile's hook_install(). I haven't actually tested those... but I don't see any mention of this issue in their issue queues.

Also worth mentioning: one of the perplexing things here was that this seemed to only be happening in local dev environments. It is NOT happening on Farmier-hosted instances of farmOS. However, once I learned that it was the install_profile variable that was to blame, I remembered: Farmier sets that variable in settings.php, so it is always farm. But also: Farmier does not install using the interactive web UI, so that AJAX request to admin/config/search/clean-urls/check would never run anyway.

Therefore, this may have been an issue for a long time - and we just didn't notice it until now. Except those remaining questions are still perplexing... there is more to understand here...

@mstenta
Copy link
Member Author

mstenta commented Apr 13, 2020

If you manually add the install_profile variable to the {variable} table before submitting the "Configure site" form, then the theme record does not get deleted from the {system} table, and everything works as expected.

So actually... there is still another issue: even after fixing the above issue, and the theme is enabled, the farm map header still doesn't display when you visit the dashboard after install. Thankfully, this was a little easier to figure out. What's happening is: the farmOS theme is being installed BEFORE the farm_map module. So farm_theme_block_info_alter() is running, but the block is not yet available to alter. So... the block doesn't show up again until farm_theme_block_info_alter() runs a SECOND time after the installation. This happens by visiting the Admin > Structure > Blocks page, or by clearing the cache.

So that will also need to be fixed.

@mstenta
Copy link
Member Author

mstenta commented Apr 13, 2020

Why didn't we notice this before?

I'm digging into this a bit more. Specifically: why does the theme still seem to work even though it is technically disabled in the {system} table?

Well, turns out... it's due to the way drupal_alter() works. MOST of the hook implementation is farm_theme are _alter() hooks - which apparently get run regardless of the enabled/disabled status of the theme in the {system} table:

https://git.drupalcode.org/project/drupal/-/blob/460ff5dc3c577d6139b37bc3467ab5b742371626/includes/module.inc#L1146

That code just looks at the theme variable, which would have been set to farm_theme when the theme was originally enabled. So all of the _alter() hooks in farm_theme still run, it seems.

The other hooks in farm_theme are _preprocess_* hooks - which maybe work similarly (don't require the theme to be enabled). Without digging further, that's my assumption.

So that explains why this went unnoticed.

@mstenta
Copy link
Member Author

mstenta commented Apr 13, 2020

Is this affecting any modules as well? Why only themes?

Well... one possible explanation is that system_rebuild_module_data() and system_rebuild_theme_data() work a little differently.

system_rebuild_module_data() uses drupal_static() to cache the module list during the page load. So maybe that is playing a role?

I'm skeptical, though, because technically the index.php request is a separate page load, and therefore would refresh that static cache. So I'm not 100% convinced.

In either case, it's obvious that modules are not affected by this. It seems to be only the themes. So I'm not going to dig further into that for now...

@mstenta mstenta changed the title farmOS theme gets disabled during install in Docker dev environments farmOS theme gets disabled during install Apr 13, 2020
@mstenta
Copy link
Member Author

mstenta commented Apr 13, 2020

Changed the name of this because it's not only Docker dev environments that are affected by this, presumably.

@mstenta
Copy link
Member Author

mstenta commented Apr 13, 2020

Ok so just in case it wasn't clear from the above notes...

The issue (which ended up being the tip of the iceberg) was that the farmOS map header block was not showing up on his farm dashboard.

That issue, which turned us on to this whole thing, is actually unrelated! Because...

even after fixing the above issue, and the theme is enabled, the farm map header still doesn't display when you visit the dashboard after install

See that comment above for the cause of the map header not showing: #272 (comment)

I may break that out to a separate issue, since it is technically not the same issue.

@mstenta
Copy link
Member Author

mstenta commented Apr 13, 2020

Spun off a separate issue for the map header specifically: #273

@mstenta
Copy link
Member Author

mstenta commented Apr 13, 2020

Why aren't other Drupal 7 distributions reporting the same issue?

Just tested this on Drupal Commerce, and it is in fact happening there as well. The theme is not enabled after installation completes. The difference with Commerce Kickstart, though, is that they are only enabling an admin theme, and still using Bartik as the default theme. So it's even easier to not notice this.

@mstenta
Copy link
Member Author

mstenta commented Apr 13, 2020

I tested installation with a variable_set('install_profile', 'farm') in farm_install(), and it fixes the issue - the farmOS theme is enabled after install.

I am going to try one other approach (moving the theme installation to a later step in the install tasks), to see if I can avoid strong-arming that variable.

@mstenta
Copy link
Member Author

mstenta commented Apr 14, 2020

I am going to try one other approach (moving the theme installation to a later step in the install tasks), to see if I can avoid strong-arming that variable.

That did not seem to work. And it still has the issue of the farm profile not being set during installation, which I'm worried could lead to other issues. So I think I'm just going to go ahead and add variable_set('install_profile', 'farm') to farm_install().

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

No branches or pull requests

1 participant