Feature export, dependencies not prefixed

looks good, this needs review!

Thanks for the draft patch!

We need to update the relevant test or tests to expect the prefixed form.

+++ b/src/FeaturesManager.php
@@ -913,12 +913,19 @@ class FeaturesManager implements FeaturesManagerInterface {
+    foreach ($package->getDependencies() as $dependency) {

In subsequent reads, will we be loading already prefixed dependencies, then adding a duplicate prefix? ::addInfoFile() doesn't seem like the right place to do this. Can we instead ensure the dependencies are always prefixed (for example, Package::getDependencies() returns prefixed dependencies)?

The patch assumes that any module without a "project" key in the info file is Drupal core module, but that's not true. Custom modules don't have the project key, so the patch incorrectly assigns them the "drupal" project. The project key is added by Drupal.org's packaging scripts for contrib modules only.

I'm not sure how else to determine the correct prefix here.

I believe for custom modules it can use the format:
modulename:modulename
This won't handle custom modules that have submodules, but there isn't any good way to easily determine the parent module of a submodule without searching parent directories for *.info.yml files.

Also, ideally Features would look for the existing dependency and not modify it if found. For example, if Features exported:
submodule:submodule
and then this was manually fixed in the Feature info.yml to be
parentmodule:submodule
then Features needs to see that "submodule" is already listed and NOT overwrite this with the previous "submodule:submodule" when exporting again.

The UpdateManager::getProjects() method does what we need here, using the core ProjectInfo class. We could either crib from there or introduce a dependency on the update module and call its method directly.

Here is a patch that handles all of this with normal core services. No dependency on update module needed. Even with `getProjects` there is no ideal way to determine if a module is part of "core" or not, other than looking at the path. The only case this doesn't handle is if the exported feature is placed as a submodule within a parent custom module. In that case the custom submodule should add a `project:` key to the info.yml file similar to if the module was packaged.

I'm actually using Features on a client project (Platform build) for the first time in years and needed this functionality. Tested it on that project.

Hmm, this doesn't preserve existing constraint strings on module dependencies. New patch coming.

Here is a patch that properly merges the dependencies during the export process itself in the mergeInfo method. It basically creates a helper function for constructing dependency strings that is missing from core and which handles the missing project while preserving constraints.

I'm getting the following error after applying

TypeError: Argument 8 passed to Drupal\features\FeaturesManager::__construct() must be an instance of Drupal\Core\Extension\ModuleExtensionList, none given, called in /web/core/lib/Drupal/Component/DependencyInjection/Container.php on line 259 in /web/modules/contrib/features/src/FeaturesManager.php on line 153 #0 /web/core/lib/Drupal/Component/DependencyInjection/Container.php(259): Drupal\features\FeaturesManager->__construct('/www/servers/vr...', Object(Drupal\Core\Entity\EntityTypeManager), Object(Drupal\Core\Config\ConfigFactory), Object(Drupal\Core\Config\CachedStorage), Object(Drupal\Core\Config\ConfigManager), Object(Drupal\Core\Extension\ModuleHandler), Object(Drupal\config_update\ConfigReverter))
#1 /web/core/lib/Drupal/Component/DependencyInjection/Container.php(173): Drupal\Component\DependencyInjection\Container->createService(Array, 'features.manage...')
#2 /web/core/lib/Drupal/Component/DependencyInjection/Container.php(434): Drupal\Component\DependencyInjection\Container->get('features.manage...', 1)
#3 /web/core/lib/Drupal/Component/DependencyInjection/Container.php(237): Drupal\Component\DependencyInjection\Container->resolveServicesAndParameters(Array)
...

To be applied after #12.

Thanks for the patches!

Updated with some fixes. Interdiff is from the combination of patches from #12 and #15.

Further updates to tests, minor tweak to test for core module to follow what's done in ProjectInfo::getProjectName().

Fix up mocking in unit test addition.

Applied, thx all!

Tests were passing on 9.1.x with PHP 7.4 but the default branch test on 8.x-3.x - 8.9.x with PHP 7.2 - is failing.

After upgrading to 3.12 and exporting an existing feature, the dependencies listed in the info file now are using the project prefix, but they are ALL using "drupal:" as the prefix instead of the correct contrib module. For example: "drupal:smtp" when it should be "smtp:smtp". Anyone else have this issue?

Interestingly enough, it doesn't seem to affect new installs of the feature module. The contrib modules are still enabled despite the wrong project prefix.

Nevermind, re-exporting fixed it after manually removing the bad "drupal"prefix. Not sure what happened there.