Hungary
Account created on 13 June 2008, about 17 years ago
#

Merge Requests

More

Recent comments

🇭🇺Hungary mxr576 Hungary

The patch saved the day today again, because it helped with identifying a missing dependency inside a recipe that was completely hidden before the patch, what we have seen is just 1 !== 0 and recipe application failed.

Process exit code mismatch.
Expected: 0
Actual: 1

STDOUT:
Error: Call to a member function getConfigDependencyName() on null in Drupal\Core\Entity\EntityDisplayBase->calculateDependencies() (line 302 of /var/www/html/build/web/core/lib/Drupal/Core/Entity/EntityDisplayBase.php).
🇭🇺Hungary mxr576 Hungary

Please test my fix from MR#2.

Went ahead, merged the MR, will release a tagged version soon. Please test it and close this issue if the issue you experienced got resolved. Thank you

🇭🇺Hungary mxr576 Hungary

Thanks for raising awareness of this problem.

In the route it's used without a dot.

Would not it be enough fixing the route definition then? I would be surprised if a dot in the permission key would cause a trouble... but hey, I am still open for surprises in Drupal :)

This bug also means that something is not covered by our current test suite, so a failing test would great.

🇭🇺Hungary mxr576 Hungary

Realized that the issue is way more generic than I thought originally

🇭🇺Hungary mxr576 Hungary

Something is definitely ongoing, but I do not have more information other than what can be seen in the GitHub Actions failures. There have been many recently, most of them with the reported error, and others with:

 RuntimeException: Failed to fetch page 261. Reason: "cURL error 56: Recv failure: Connection reset by peer (see https://curl.haxx.se/libcurl/c/libcurl-errors.html) for https://www.drupal.org/api-d7/node.json?type%5B0%5D=project_module&type%5B1%5D=project_theme&type%5B2%5D=project_core&field_project_has_releases=1&field_project_type=full&sort=field_project_machine_name&page=261". in /home/runner/work/ddqg/ddqg/src/Infrastructure/DrupalOrg/DrupalOrgApi/DrupalOrgApiRepository.php:187
Stack trace:

https://github.com/mxr576/ddqg/actions/workflows/generate-no-release-ver...

This can also be seen in the action logs when the action runs, so I assume some infrastructure/WAF logs could be analyzed for anomaly.

🇭🇺Hungary mxr576 Hungary

With original implementation it was extremely painful for us figuring out that part of the Drupal 11.2.0 update an assert breaks in our code since the assertion error was only visible on stdout. "Failed asserting that 1 is identical to 0." was also not really helpful, so it got also improved in the MR.

STDOUT:
[notice] A backup checkpoint was not created because nothing has changed since the "Backup before the 'Foo' recipe." checkpoint was created.
AssertionError: assert(property_exists($index, 'original')) in assert() (line 51 of /mnt/files/local_mount/drupal/modules/foo/src/Bar.php).


STDERR:
0/7 [░░░░░░░░░░░░░░░░░░░░░░░░░░░░]
Applying recipe

1/7 [▓▓▓▓░░░░░░░░░░░░░░░░░░░░░░░░]
Installed Image module.

2/7 [▓▓▓▓▓▓▓▓░░░░░░░░░░░░░░░░░░░░]
Installed Comment module.

3/7 [▓▓▓▓▓▓▓▓▓▓▓▓░░░░░░░░░░░░░░░░]
Installed Database Search module.

4/7 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓░░░░░░░░░░░░]
Installed Database Search Defaults module.


Failed asserting that 1 is identical to 0.
/mnt/files/local_mount/build/web/core/tests/Drupal/FunctionalTests/Core/Recipe/RecipeTestTrait.php:89
/mnt/files/local_mount/drupal/recipes/Foo/tests/src/Funtional/BarTest.php:54
🇭🇺Hungary mxr576 Hungary

Maybe it is easier to cover the issue with test than I thought... and the bug is not only impacting the scenario that I originally got caught and it could be reproduced by splitting the input_test recipe and submitting the /form-test/recipe-input.

🇭🇺Hungary mxr576 Hungary

For a more realistic comparison, I have removed idempotency check - so the second recipe application - from the GenericRecipeTestBase classes and captured the following with XHGui:

web/core/recipes/standard/tests/src/Kernel/GenericTest.php

web/core/recipes/standard/tests/src/Functional/GenericTest.php

🇭🇺Hungary mxr576 Hungary

Based on my investigation into the performance regression, I've identified several contributing factors to the slowness in Recipe Kernel tests:

Both Functional and Kernel tests use MemoryCacheFactory as the default cache factory, but Kernel tests appear to be disproportionately impacted by its non-persistent nature. The frequent cache invalidations during module installation compound this issue.

Related to 📌 RecipeRunner::processInstall() installs modules one by one Active - every module installation triggers multiple cache invalidations. Even when replacing the cache factory with a persistent one in Kernel tests, there's no significant performance improvement due to this cache churn.

Attempted mitigation in our Kernel test:

public function register(ContainerBuilder $container) {
  parent::register($container);
  // Change container to database cache backends.
  $container
    ->register('cache_factory', 'Drupal\Core\Cache\CacheFactory')
    ->addArgument(new Reference('settings'))
    ->addMethodCall('setContainer', [new Reference('service_container')]);
}

XHProf profiling reveals that attribute discovery is the most time-consuming operation during Recipe Kernel tests:

  • Majority of calls to getDefinitions() are pass-through calls from AttributeDiscoveryWithAnnotations::getDefinitions()
  • 80% of these calls originate from DerivativeDiscoveryDecorator::getDefinitions()
  • 80% of those calls come from DefaultPluginManager::findDefinitions()
🇭🇺Hungary mxr576 Hungary

I have just came across the same issue and the example from #4 proves the problem. The same happen with local type aliases and template definitions.

 * @template Foo of \Drupal\foo\Bar
....
   * @phpstan-param Foo $apiBaseUrl

Foo in @phpstan-param gets changed to FQCN.

🇭🇺Hungary mxr576 Hungary

Released 1.0.2 with this change.

🇭🇺Hungary mxr576 Hungary

I'll fix the deadline first so this simple, unrelated one becomes green. Thanks for the MR!

🇭🇺Hungary mxr576 Hungary

general projects don’t get packaged and a license file added automatically.

TIL :)

Could you create an MR with this?

Thanks for making this great looking recipe!

Thanks! :)

🇭🇺Hungary mxr576 Hungary

might be a bit risky extending the phpunit Assert class.

Well, TestCase has a required constructor parameter, which conflicts with the way how these test scripts are initiated.

🇭🇺Hungary mxr576 Hungary

Thanks for raising awareness for this major issue Yce and providing a fix for it!

🇭🇺Hungary mxr576 Hungary

mxr576 made their first commit to this issue’s fork.

🇭🇺Hungary mxr576 Hungary
    - Root composer.json requires drupal/core-dev 11.2.2 -> satisfiable by drupal/core-dev[11.2.2].
    - drupal/core-dev 11.2.2 requires mglaman/phpstan-drupal ^2.0.7 -> found mglaman/phpstan-drupal[2.0.7] but it conflicts with your root composer.json require (^1.2.12).
🇭🇺Hungary mxr576 Hungary
+          drupal_flush_all_caches();

Calling flush all caches in #25 is something that should be avoid 99,9% of the times, it usually means we have no idea which caches has stale data so we have to flush them all.

Since 3 years passed and it was opened for an older major, maybe it is better to re-validate if this issue still exists and in which branches are affected.

🇭🇺Hungary mxr576 Hungary

Drupal's ability to serve content in Markdown format appears to be unique among CMS platforms as highlighted in this Pronovix article about making developer portals AI-ready.

However, I haven't seen evidence that current LLM crawlers automatically fetch .md versions of content or interpret link rel attribute values. Most crawlers already have established methodologies for mass site scanning, and the llms.txt specification is still relatively new in terms of widespread adoption.

My recommendation:Explicitly include the Markdown version of pages in your llms.txt file whenever possible, rather than relying on crawlers to discover them automatically.

You're correct that the module's menu token doesn't currently expose URLs with enforced MD format. This connects to another requested feature for embedding entity links via tokens that point to MD versions Support dynamic entity linking using tokens Active , which would address this limitation.

🇭🇺Hungary mxr576 Hungary

I am glad to hear that. Let us know if you have further questions related to this topic or we can consider this closed.

🇭🇺Hungary mxr576 Hungary

I secondh a.dmitriiev regarding the module's capabilities. I'd also recommend exploring the LLM support recipe for additional implementation ideas.

It's important to note that llms.txt differs fundamentally from a traditional sitemap. Rather than being an auto-generated file listing all pages, llms.txt is curated content specifically designed to help Large Language Models understand and consume your site's information in the most effective way possible.

🇭🇺Hungary mxr576 Hungary
  /**
   * {@inheritdoc}
   */
  public function getBaseFormId() {
    // Assign ENTITY_TYPE_form as base form ID to invoke corresponding
    // hook_form_alter(), #validate, #submit, and #theme callbacks, but only if
    // it is different from the actual form ID, since callbacks would be invoked
    // twice otherwise.
    $base_form_id = $this->entity->getEntityTypeId() . '_form';
    if ($base_form_id == $this->getFormId()) {
      $base_form_id = NULL;
    }
    return $base_form_id;
  }

Just found this workaround in \Drupal\Core\Entity\EntityForm::getBaseFormId() so somebody might have tried to address this problem before.

🇭🇺Hungary mxr576 Hungary

I have also run into this problem that an error only appeared on STDOUT and not on STDERR but that has not been reported by Recipe installer.

🇭🇺Hungary mxr576 Hungary

Makes complete sense to me, Lester, could you create MR-s instead of patches, because that would speed up the merge of the proposed fixes.
Thank you!

🇭🇺Hungary mxr576 Hungary

Add LLM support recipe

🇭🇺Hungary mxr576 Hungary

Thanks, also for the feedback, we will try to keep up with the quality! 🙂

🇭🇺Hungary mxr576 Hungary

The scope of this conversation has widened in unexpected ways. Since many projects already have .ddev folders committed as mentioned in previous discussions, I assumed the main debate about whether this is a good practice had already been settled. However, I recognize there are valid concerns about different use cases and project types.

I'm partially surprised by the pushback. DDEV has become the de-facto development environment for Drupal projects and provides the essentials for reproducible development and QA environments. While it doesn't offer the same level of
reproducibility as Nix, it represents a significant step forward. The ddev-drupal-contrib addon is officially maintained by DDEV and provides the same flexibility and reliability for contrib development. It not only accelerates initial
project involvement but also ensures that every development environment shares identical dependency versions and tools for contribution.

I understand there are different perspectives on this, however I believe contrib modules can also benefit from standardized development environments that facilitate consistent testing and contribution workflows.

In an ideal world, Drupal.org would provide a single command to spin up DDEV development environments for contrib modules. The majority of contrib modules don't require special dependencies or third-party software for development or
testing, so the same script could cover approximately 95% of contrib projects. However, even if such a script existed - and I hope this comment catalyzes its creation - some projects would still need custom config.yaml files,
Docker files, or other overrides like version pinning committed to their repository. Therefore, the .ddev folder wouldn't disappear completely.

The idea about a single YAML file also makes sense, but I believe that would contradict DDEV's composable extensions concept, which is genuinely powerful. You can find extensions for various needs without maintaining your own custom solutions.

Having a .ddev folder also proves valuable when an extension's latest release needs tweaking or bugfixing. For example, changes from pending PRs could be committed to a module's .ddev folder today, and the
extension can be updated after the PR gets merged. This approach also enables immediate onboarding for new contributors who can run ddev start and begin contributing within minutes, rather than spending time configuring their local environment.

Perhaps we could explore a hybrid approach where the .ddev folder contains only the minimal, project-specific configuration needed, while leveraging the ddev-drupal-contrib addon for common functionality. This would reduce maintenance overhead while preserving the benefits of reproducible environments.

Ideally, in the future, boilerplate code for providing local development environments will be reduced. However, I still see significant benefits in leveraging DDEV's composability through extensions and maintaining project-specific
.ddev folders with explicit dependencies and their versions. Yes, this approach introduces minor maintenance overhead for contrib maintainers, but the benefits should outweigh the consequences.

Additionally, perhaps in the future GitLab CI could also leverage DDEV to run automated QA for modules, which would lead to truly reproducible builds across development and testing environments.

🇭🇺Hungary mxr576 Hungary

I'm glad and honored that Randy joined the conversation! I'm planning to write a comprehensive response to his comment, but I need at least one more day to put it together. If there's time to wait before closing this discussion, I would really appreciate it.

🇭🇺Hungary mxr576 Hungary

It seems to me the question got answered here. Please re-open if necessary.

🇭🇺Hungary mxr576 Hungary

I cannot approve the MR, but it looks good for me. After the patch was applied, the LLM support recipe's automated tests passed without the config schema checker bypass workaround.

🇭🇺Hungary mxr576 Hungary

Re-rolling patch to work with 11.1.x

It is better to update the existing MR to target 11.x and only upload patches for backports.

🇭🇺Hungary mxr576 Hungary

Having automated tests would also help spotting with config schema issues as it was proved in 🐛 Config schema is missing for markdownify_file_attachment.settings Active .

🇭🇺Hungary mxr576 Hungary

🐛 Config schema is missing for markdownify_file_attachment.settings Active could be fixed as part of this task too, or in scope of the dedicated ticket.

🇭🇺Hungary mxr576 Hungary

Created an MR that assumes these modules dependencies should have been there already.

🇭🇺Hungary mxr576 Hungary

Well the whole site and any content in it could be used for prompt injection to any LLM by definition, so I have removed this todo.

🇭🇺Hungary mxr576 Hungary

Well, I guess the answer is "it depends". Whoever uses that action should be awareof consequences. However without that it is complicated to set up an immediately usable demo env with recipes, where waiting for cron to index items may not be an option.

🇭🇺Hungary mxr576 Hungary

Two additional action suggestions:

  • Index items - with optional data source filtering. Also useful when default content import doesn't trigger indexing automatically - haven't checked to root cause but experienced this when I wrote a test for a recipe
  • Assign index to search backend - for recipes that bundle indexes without backends. Should filter by server type (e.g., Solr) with fallback option to prevent exception when a match not found.
🇭🇺Hungary mxr576 Hungary

I was recently looking for a way to append an item to a list property on a config entity. After some discussion on Slack and a bit of Googling, I found this issue and the proposed solution.

A few important questions came up that should be discussed and decided:

  1. Should the scope of this issue/MR be expanded to include list property manipulation on config entities? The current state of the MR explicitly excludes support for config entities.
  2. If config entities are included, how should the action be named? The current name, "simple config array", would no longer be accurate. Should we introduce a separate set of actions for simple config and config entities, or find a naming approach that can cover both?

For reference and potentially for consistency it might be worth mentioning that support for config entity updates was recently deprecated in simpleConfigUpdate: https://www.drupal.org/node/3515543 .

🇭🇺Hungary mxr576 Hungary

Add mention to Recipe Code Installer

🇭🇺Hungary mxr576 Hungary

And the answer was in front of my eyes after my new RCA... Gotta go to the corner to cry.

Thanks for saving me from another hours of debugging and sorry for the noise.

🇭🇺Hungary mxr576 Hungary

mxr576 changed the visibility of the branch 3526781-set-config-synching to hidden.

🇭🇺Hungary mxr576 Hungary

Okay, at this point, I am unsure where to catch this bug and which layers are impacted... of course setting is synching on the config installer is not a god idea since it triggers #3 by design (from \Drupal\Core\Config\ConfigInstaller::createConfiguration():

        // If we are syncing do not create configuration entities. Pluggable
        // configuration entities can have dependencies on modules that are
        // not yet enabled. This approach means that any code that expects
        // default configuration entities to exist will be unstable after the
        // module has been enabled and before the config entity has been
        // imported.
        if ($this->isSyncing()) {
          continue;
        }

setting the isSynching() on the config entity was an idea to bypass the dependency calculation in \Drupal\Core\Config\Entity\ConfigEntityBase::preSave():

    if (!$this->isSyncing()) {
      // Ensure the correct dependencies are present. If the configuration is
      // being written during a configuration synchronization then there is no
      // need to recalculate the dependencies.
      $this->calculateDependencies();
      // If the data is trusted we need to ensure that the dependencies are
      // sorted as per their schema. If the save is not trusted then the
      // configuration will be sorted by StorableConfigBase.
      if ($this->trustedData) {}

...but this changes Drupal core behavior globally, there is a way to scope it to recipes only, but still, would that be the approach? Unsure, especially even with this change other parts of the Recipe install fails when it tries to fetch the "datasource_foo" entity for different reasons (like because entity insert hooks are triggered).

Production build 0.71.5 2024