🇺🇸United States @phenaproxima

phenaproxima → created an issue.

phenaproxima → created an issue.

An update here -- if we can get ✨ Add a directory to the PATH Active in, this becomes much more viable, and would immediately improve things for Drupal CMS users -- if Composer installed and vendor/composer/composer/bin/composer exists, Package Manager would point to that.

The vendor hardening plugin would still be an obstacle for some sites -- although Drupal CMS does not currently use it -- but my idea there is to change it to not delete Composer's binaries outright, but rather just chmod them to 655 so that they cannot be executed directly. Then Composer simply becomes another PHP script run by the PHP interpreter, rather than something that can be invoked as its own process. All that would need to happen in a separate issue, though.

Postponing on the related issue.

I think we're probably safe if we use the PhpExecutableFinder class that comes with Symfony's Process component. It does a pretty robust job of searching for the PHP interpreter, based on the current OS and server API, and is the standard tool for finding the PHP interpreter. It's a documented part of the Symfony Process API: https://symfony.com/doc/current/components/process.html#finding-the-exec...

Hmmm...we might need to think a little bit more about this, because of https://stackoverflow.com/questions/35460281/php-bindir-on-windows-incor...

PHP_BINARY is a value set on runtime (when the script is executed)

PHP_BINDIR is a value set on compile time, not on runtime.

That could get squirrelly in some situations, especially Windows, since people aren't compiling PHP on Windows. I wonder if dirname(PHP_BINARY) is a safer approach.

Ooooh, great catch. Fixed with a test (there's no Christmas-ey CI run for this one since it's not fixing a pre-existing bug in HEAD).

phenaproxima → created an issue.

The change proposed in ✨ Add a directory to the PATH Active would allow us to implement #5. It's a worthwhile change in any case, though.

Still needs an issue summary update, but the MR was pretty simple to write. This removes an internal class that was very clearly and explicitly marked as internal, and subject to change or removal at any time...so I feel okay about it.

The use of PHP_BINDIR should work even in cases where PHP is being run through, say, PHP-FPM (which would be the case with PHP_BINARY).

The test failure in !821 is preexisting in 2.0.x.

Here's a way we could include Composer in the cPanel package as a runtime dependency without risking exposing executables to the Big, Bad Internet:

• First, core would need to be changed so that Package Manager always runs Composer through the PHP interpreter, rather than as an executable of its own. There's already an issue for this (somewhere); and it works. The Drupal CMS launcher runs Composer this way. That's the blocker.
• Then we could include Composer as a runtime dependency, with drupal/core-vendor-hardening taking care of cleaning the executable out of vendor/bin. But Package Manager would not be trying to run it, so that'd pretty much take care of it. The only other thing to do would be for site builders to set Composer's path to the locally installed copy, which is already doable (although not in the UI, but there's an issue for that as well).

I've confirmed the current MR backports cleanly to 2.0.x. 🚀

This should be backported to 2.0.x.

Include Composer in the cpanel package?

From a technical perspective, this wouldn't be hard to do. But it would put the executable composer binary into a publicly-accessible directory (http://example.com/vendor/bin/composer), which could be a, ahem, security concern (read: potentially monumental catastrophe).

Definitely worth discussing the pros and cons here.

Assigning to @jurgenhaas, our resident ECA expert, to triage this.

phenaproxima → created an issue.

Adjusting credit.

Change record drafted: https://www.drupal.org/node/3533854 →

I like that a lot better. I'd love to see Editoria11y merge some "no results found" text into their view, as I agree that the lack of any text at all is totally unclear. I wonder if there's a class we could add to the button which would allow it to become readable?

It might be possible to do this stuff in the view without waiting on upstream, if we can figure out exactly which switches to pull in the dense jungle that is Views configuration.

Unless I'm missing something, we definitely cannot allow path_alias entities to be exported, at least not automatically. Why? Because the system path stored in the entity is going to include things like /node/3, which uses a non-portable serial ID.

So that's a dead end.

Therefore, I think the path field on entities needs to be exported as part of the entity. Since that is probably one of the only computed fields that needs to be exported with an entity -- if not the only such field, at least in core -- I would propose we add a normalizer specifically for this purpose to the path_alias module, which momentarily (non-persistently) marks the path field of an entity as being non-computed so it will be exported.

Drupal CMS Olivero only exists as a shim, so here's the plan that Pam and I agreed on:

• It will never receive a 2.x release.
• When Drupal CMS 2 comes out, we will pull Drupal CMS Olivero out of our subtree split and remove it from our codebase, turning it into an independent legacy project.
• We'll mark it as obsolete in the info file (to prevent new installations of it while keeping existing sites alive) and tag a new release with that change.
• We'll mark the project as minimally maintained.
• We'll continue to do compatibility-only changes to that sites which use it, can keep using it. We'll do this indefinitely, until/unless...
• ...a migration path to XB for legacy themes is ever created, in which case we'll implement or become compatible with that, and officially EOL Drupal CMS Olivero.

I tried this out and I think it could be better. Here's what I see in the dashboard after applying the recipe (in the Tugboat preview):

If I was a content author or editor, I would have no idea what this meant or what it was doing there. The lowercasing of "url" looks unprofessional (and is a technical word anyway). The button is unreadable. There's nothing in the block, and no explanatory text. Clicking the button brings me to an equally inscrutable page.

I'll commit this if @pameeela feels it would be beneficial, but I'm not sure we should release this in its current state.

I think that makes sense. Recipe Installer Kit will need to do it in a backwards-compatible way, but it could be done.

I propose this format:

recipes:
  optional:
    - name: Blog # this is translatable
      packages: ['drupal/drupal_cms_blog']
    - ...

phenaproxima → created an issue.

The dependency changes look right but the test should approach the dashboard from a functional end-user perspective. :)

If that's the case, then maybe we add it to a one of the basic recipes -- I'm thinking either drupal_cms_admin_ui or drupal_cms_authentication, although neither one really fits the bill entirely. Maybe drupal_cms_starter would be the right place.

Remove `-v` from `drush recipe` command examples, and fix missing newlines in Composer configuration command snippets.

Mentioned that `drush recipe` requires Drush 13+

Also Adam Hoenich may know if it was intended that the recipe name was not pulled dynamically but rather an alternate name was possible to be provided here. That may be a loss of a feature to not allow that anymore. That said it would be harder to translate it then :D

Gábor is correct -- the recipe name is not meant to be pulled dynamically. The idea here is that you can combine multiple recipes under a single arbitrary label. So it is actually the info file itself that needs to be translated.

So unfortunately I cannot merge this into Recipe Installer Kit as written. :(

Updated the docs. I don't think we should call the recipe unpack plugin optional, just give people simple and consistent commands they can use to get going. The less cognitive load, the better.

We desperately need this to be up to date, so I'm hoping we can merge this sooner rather than later.

phenaproxima → made their first commit to this issue’s fork.

Minor rephrasing for brevity and a clarification.

Removed outdated instructions referencing oomphinc/composer-installers-extender in favor of a simple set of commands to set up unpacked recipes in an existing site.

That looks like a good start but it's missing a dependency and it needs light test coverage.

Although I'm not the last word here, I'm against adding modules just so that they'll be there for site templates to use; it's trivial for a template to include a dependency on a module that it wants to use. If we can do some kind of initial configuration or value-add on top of Role Assign, that's one thing and I'm for it, but it doesn't sound like that's what's being proposed here?

This will need framework manager review for the proposed changes to extant core APIs and subsystem maintainer sign-off (I would qualify except I wrote the patch).

Wasn't file usage kind of deprecated as a concept because it has never really worked? That's what I heard a few DrupalCons ago, anyway.

Let's do that. I'm gonna jump the gun a bit and assume this should be a priority for 11.3.0. It's kind of nuts that core doesn't already support this, and it shouldn't be a heavy lift at all to merge the module into the main Media module.

With my Media maintainer hat on:

I don't even think we should add this to Drupal CMS. This should be converted to a core issue and the functionality should be merged into core directly. It's mind-boggling that it's not there already.

More thoughts, which I shared with committers on Slack:

A few thoughts here, based on a point someone raised in a postponed follow-up — right now, ✨ Add a command-line utility to export content in YAML format Active includes some low-level but probably harmless hacks (like the do not export setting) to prevent certain properties/fields from being exported. I think that, in a follow-up — or really, probably a raft of follow-ups -- we might want to formalize the concept of “exportability” at the data layer level. Semantically, “exportable” could be similar to “internal” — it’s up to the calling code to decide what it means and what to do with it. But we might want to add a DataDefinitionInterface::isExportable() method — not sure how disruptive this would be?

The more I think about this, the more I think we probably want to add a new top-level exportable key to field configuration entities (i.e., field_config and base_field_override). Having it as a setting of the field type plugin is more complex and is asking for backwards compatibility and update path headaches. Adding a new top-level key is straightforward.

Adding ✨ Add a command-line utility to export content in YAML format Active as related because this would allow us to remove some of the special processing from the ContentExportNormalizer we're adding in that issue.

The implications of this issue are significant. I think we should do it, for sure -- if we do, it will allow us to remove a great deal of special processing from the ContentExportNormalizer we're adding in ✨ Add a command-line utility to export content in YAML format Active .

Postponed on the export command.

Adding ✨ [PP-1] Allow fields to be marked as non-exportable Active as related, which would implement field-level export access control in core.

phenaproxima → created an issue.

Good point about the circular dependencies, we should ensure we specifically test that.

I think allowing fields to opt out of being exported should be admin configurable and happen in a follow-up, as it might have some additional complexity best handled separately.

As for dependency depth control — why? What would the use case be for such a thing?

Adding a related issue that will absolutely impact this one -- or will be impacted by this one -- depending on which gets committed first.

Tagging this as a soft blocker because this very much affects site template creators.

phenaproxima → created an issue.

Opened ✨ [PP-1] Add a normalizer for component tree items Postponed to take advantage of this in Experience Builder.

phenaproxima → created an issue.

Tagging as a contributed project soft blocker because without this, we can't easily build site templates.

Filed the follow-ups from #18.

phenaproxima → created an issue.

phenaproxima → created an issue.

phenaproxima → created an issue.

phenaproxima → created an issue.

I'm not sure how to flag for infrastructure

Infra has its own issue queue: https://www.drupal.org/project/issues/infrastructure?categories=All →

A few follow-ups have been suggested to me privately by interested parties, and just ideas I have:

• Support exporting an entity and all of its dependencies into a folder structure on disk. This includes exporting physical files that are part of File entities -- we'll probably want to introduce the concept of an "attachment" to ExportMetadata to facilitate this.
• Support straight-up JSON when exporting and importing. The importer needs no changes for this (only a one-line adjustment to the Finder class) and it would be trivial to make the export command write JSON instead of YAML, maybe with a --format=json option.
• Allow the export to be done as a specific user, not just "whoever has the administrative role". The importer already supports this; the exporter should too. It'd be a pretty easy change, and we could add a --as-user=N option to the export command for that.
• Ambitious: convert demo_umami_content to use the default content system! That would be a really strong test of its capabilities and would exercise more nooks and crannies than just our comparatively sad little test fixture. :)

We could do that...but the problem is then that you don't have a robots.txt file if you either uninstall RobotsTxt, or never install it to begin with. If you're not comfortable at the command line (which is presumed to be the case in Drupal CMS's target audience), you're sort of screwed.

To me, this is something RobotsTxt should handle on its own. It could maybe delete the existing robots.txt when it gets installed (or try to), and then automatically restore the scaffold version when it gets uninstalled. It could even use Package Manager for this, although that might be a bit of a heavy lift.

Or maybe we could ship an ECA configuration which does the same thing (although it'd need write access to the filesystem, but it could just quietly fail with a log message if the web root is not writable).

It's marked final you can't extend it or decorate it

(emphasis added)

That's not true. You can decorate anything with an interface, and the export normalizer has an interface (NormalizerInterface). It is a decorator itself. Decoration is the correct way to add more things to final classes.

I am not going to die on the final/private hill in this issue; if a committer tells me to mark it non-final and make the private members protected, I'll do that. But I will insist that the class be marked internal with a clearly-worded warning, because it is part of an experimental subsystem. If someone extends an internal class and it breaks them, they deserve what they get. 😈

I assume this will not work with Experience builder either then

Not out of the box, but by hooking into the serialization system, it gives XB a way to become exportable: all it needs to do is implement a normalizer that can normalize its various data structures. This is the single biggest advantage of this approach over Default Content -- modules can handle exporting their own data.

a content exporter feels explicitly like the kind of thing you want to extend

The exporter should not be extensible. If you want to change how it operates, you should implement a normalizer. To me, that feels like the correct amount of API surface here; what would the use case be for directly extending the exporter itself?

This is now reviewable and has a passing test.

My deep journey into the Serialization module (which I've not used before) has shown me several things:

• Default Content's export format is actually very close -- in most cases, identical -- to what this MR produces, using the Serialization module API. There are some small differences, but those are usually because either the fixture files were incomplete as generated, or because Default Content does certain things a little differently, but with the same net effect (for example, always casting primitives, which results in empty strings for undefined path aliases, which can also be NULL -- the import will work the same either way.
• Layout Builder is a significant sore point, due to the fact that it unconditionally denies access to its data for serialization. That needs to be fixed in 📌 [PP-1] Expose Layout Builder data to REST and JSON:API Postponed , which is a long-running issue and appears to be quite the dragon, but need not block this feature.
• The test case is to import the default content we test our importer with (which includes all core entity types, and a translation, plus a couple of edge cases), immediately export it, and then confirm that the exported version matches the stuff that we imported. This proves that the exporter is producing material that our importer understands. This doesn't necessarily means all content will be importable in all cases, but it's a great foundation.
• The export does not write anything to disk (yet), or export dependencies. However, there is a mechanism here (ExportMetadata) which makes it easy for normalizers to flag other entities as dependencies.
• As is true with importing content, exporting also requires administrator access, so that you have maximum visibility of all fields, and also have the ability to include hashed passwords in exported users. This is the same as what Default Content does, so I presume it's okay from a security standpoint, but might be worth a sign-off anyway.

So...onward! Let's get this crucially important feature shipshape, and merged in.

Adding ✨ Add a command-line utility to export content in YAML format Active as related, because this issue completely blocks the ability to export Layout Builder data.

Updating API changes based on my current progress.

Just to give a little context, @larowlan linked me to https://www.previousnext.com.au/blog/we-could-add-default-content-drupal..., which is from almost a decade ago. It outlines four tricky problems that prevent the addition of default content capabilities to core. I want to quickly shoot these down.

Adding default content to standard profile would mean that none of the profile's configuration would have been imported yet.

Core's default content import is done by recipes, and works well. The concern about shipping content with modules is moot. Recipes' job is to put all necessary configuration in place before any content is created, and the recipe system's strong, straightfoward configuration handling means that default content can come in easily. This concern is definitely no longer applicable.

The second issue here is that the default content module relies on the Rest, HAL and serialization modules in core.

This was true for v1 of the Default Content module. We would still need to have Serialization enabled for export, that's true...but there are no special modules required for import, which is the end user-facing case.

There are shortcomings in core's normalizers. The main ones are around fields that resemble entity references but really aren't and fields with calculated values. And then there's normalizing files and images.

This is probably legitimate, although the picture probably significantly better now than it was at the time the blog post was written, thanks to the advent of JSON:API and the core improvements that it brought us. But yes, normalization is the meat and potatoes of doing default content export correctly.

Just because we could add default content to the standard profile - does that mean we should?

We already put default content in recipes, which puts profile support on the back burner (where it belongs). This is not a problem anymore.

So there you go. If you ask me, the time to do this is now!

phenaproxima → created an issue.

Synced the branch with 0.x, restoring RTBC.

Opened follow-up to actually fix this in core: ✨ EntityReferenceItem should support a virtual `target_uuid` property Active

phenaproxima → created an issue.

Having a test makes of ton of sense and catches the bug you found.

If core/scripts/drupal is setting a bad base URL, that sounds like something that should be fixed in core as well. I'll open a follow-up for that.

This is now in a much better place thanks to @justafish's targeted simplification of the code that applies recipes to the test site.

It's still failing, but for a completely clear reason:

     /builds/issue/experience_builder-3532268/_drupal/web/../web/modules/contrib  
      /experience_builder/tests/fixtures/recipes/test_site/content/node/c66664af-  
      53b9-42f4-a0ca-8ecc9edacb8c.yml: field_xb_demo.1.inputs.348bfa10-af72-49cd-  
      900b-084d617c87df.image.src=Invalid URL format.    

If I go look at that spot, I see this (truncated for brevity):

inputs: '{"image":{"sourceType":"static:field_item:entity_reference","value":{"target_id":"1"} ...

I don't think we can rely on images by serial ID -- only by UUID. That would be my first guess as to why this isn't working.

Is this a known issue? Fixed in some way? Out of scope here? I could use some input from Wim or another elder XB wizard.

Trying to credit @justafish for help debugging this in a back channel.

phenaproxima → made their first commit to this issue’s fork.

The Trash module is already included with Drupal CMS and has been since the beginning. Should we close this issue as fixed?