Define project dependencies in core module .info files [#2798891]

Problem/Motivation

The dependencies in *.info.yml files as a best practice should have the project specified https://www.drupal.org/node/2299747.

Currently Drupal core has a dependency on the Link project. This is because it specifies the link module as a dependency and projects with the same name as the module are preferred.

Currently the automated testing infrastructure (Project Dependency) assumes that Drupal core has no dependencies so the erroneous dependency on the Link project is ignored.

It is important for Drupal core to use best practices because contributed projects will follow the example set by core and specifying dependencies in an ambiguous manner may cause problems with automated testing. Also, if in the future Drupal core uses external dependencies it may not always be the case that Drupal core does not have dependencies.

Proposed resolution

Replace all dependencies that are not namespaced with their namespaced versions.

Remaining tasks

User interface changes

None.

API changes

None.

Data model changes

None.

Comment	File	Size	Author
#16	interdiff.txt	1.25 KB	ankitjain28may
#16	Add-namespaces-2798891-16.patch	70.09 KB	ankitjain28may
#16	8.6.x: PHP 7 & MySQL 5.5 Patch Failed to Apply
#11	Add-namespaces-2798891-11.patch	77.49 KB	ankitjain28may
#11	8.6.x: PHP 7 & MySQL 5.5 19,493 pass, 2,865 fail
#8	Add-namespaces-2798891.patch	69.33 KB	ankitjain28may
#8	8.6.x: PHP 7 & MySQL 5.5 23,803 pass
#2	add_project_namespaces-2798891-2.patch	53.79 KB	trobey
#2	8.3.x: PHP 5.5 & MySQL 5.5 20,940 pass
	Screen Shot 2016-09-12 at 2.32.42 PM.png	378.74 KB	trobey

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

12 September 2016 at 20:43

trobey created an issue. See original summary.

Comment #2

trobey CreditAttribution: trobey as a volunteer commented 12 September 2016 at 20:47

Status:

Active

» Needs review

File	Size
add_project_namespaces-2798891-2.patch	53.79 KB
8.3.x: PHP 5.5 & MySQL 5.5 20,940 pass

Comment #3

12 September 2016 at 20:47

Version:

8.3.x-dev

» 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Comment #4

12 September 2016 at 20:47

Version:

8.4.x-dev

» 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Comment #5

12 September 2016 at 20:47

Version:

8.5.x-dev

» 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Comment #6

borisson_

Dutch

Mechelen, 🇧🇪

CreditAttribution: borisson_ as a volunteer commented 31 March 2018 at 11:31

Issue summary:	View changes
Status:	Needs review	» Needs work
Issue tags:		+Needs reroll

Patch doesn't apply anymore for these files.

core/modules/hal/hal.info.yml
core/modules/block_place/block_place.info.yml
core/modules/content_moderation/tests/modules/content_moderation_test_views/content_moderation_test_views.info.yml...

I do think that this is a great idea, so it would be awesome to get this rerolled.

Comment #7

tstoeckler

he/him

German

Essen, Germany

CreditAttribution: tstoeckler at bio.logis Genetic Information Management GmbH commented 31 March 2018 at 16:30

Issue tags:

+Novice

Comment #8

ankitjain28may CreditAttribution: ankitjain28may as a volunteer and commented 1 April 2018 at 17:26

Status:

Needs work

» Needs review

File	Size
Add-namespaces-2798891.patch	69.33 KB
8.6.x: PHP 7 & MySQL 5.5 23,803 pass

Re-rolled

Comment #9

TR CreditAttribution: TR commented 2 April 2018 at 06:07

Status:	Needs review	» Needs work
Issue tags:	-Needs reroll

+1 for getting this done.

I applied the patch then examined all the .info.yml files in the Drupal distribution (using find, xargs, grep). It's almost entirely correct. However, I found a few problems:

diff --git a/core/modules/views/tests/modules/views_test_formatter/views_test_formatter.info.yml b/core/modules/views/tests/modules/views_test_formatter/views_test_formatter.info.yml
index 91658d8..cd5e527 100644
--- a/core/modules/views/tests/modules/views_test_formatter/views_test_formatter.info.yml
+++ b/core/modules/views/tests/modules/views_test_formatter/views_test_formatter.info.yml
@@ -5,4 +5,4 @@ package: Testing
 version: VERSION
 core: 8.x
 dependencies:
-  - views
+  - druapl:views

Should be drupal:views, not druapl:views (spelling).

2) None of the profiles in core/profiles were fixed, they all still need namespaces in their .info.yml files.

3) core/modules/system/tests/modules/module_handler_test_multiple/module_handler_test_multiple_child/module_handler_test_multiple_child.info.yml was not fixed.

Comment #10

ankitjain28may CreditAttribution: ankitjain28may as a volunteer and commented 2 April 2018 at 06:25

@TR Thanks for the feedback, I will make all the necessary changes.

Comment #11

ankitjain28may CreditAttribution: ankitjain28may as a volunteer and commented 2 April 2018 at 06:35

Status:

Needs work

» Needs review

File	Size
Add-namespaces-2798891-11.patch	77.49 KB
8.6.x: PHP 7 & MySQL 5.5 19,493 pass, 2,865 fail

Made the changes as per comment #9

Comment #12

2 April 2018 at 06:55

Status:

Needs review

» Needs work

The last submitted patch, 11: Add-namespaces-2798891-11.patch, failed testing. View results

Comment #13

TR CreditAttribution: TR commented 2 April 2018 at 06:58

Hi @ankitjain28may:

Please create an interdiff to show the differences between #8 and #11. See https://www.drupal.org/documentation/git/interdiff

This is especially important with big patches like yours - I don't want to have to review the entire patch again, I just want to review the changes you made, and I can't do that without an interdiff.

Comment #14

TR CreditAttribution: TR commented 2 April 2018 at 07:20

Ah, it seems I may have led you astray. See #2855026: Installation profiles do not support project:module format for dependencies. I guess profiles should NOT have namespaces yet, after all.

Comment #15

ankitjain28may CreditAttribution: ankitjain28may as a volunteer and commented 2 April 2018 at 07:40

@TR I will remove the namespace in profiles and make a new patch with interdiff.

Comment #16

ankitjain28may CreditAttribution: ankitjain28may as a volunteer and at Google Summer of Code commented 25 April 2018 at 20:52

Status:

Needs work

» Needs review

File	Size
Add-namespaces-2798891-16.patch	70.09 KB
8.6.x: PHP 7 & MySQL 5.5 Patch Failed to Apply
interdiff.txt	1.25 KB

Added interdiff and patch, Please review.

Comment #17

TR CreditAttribution: TR commented 2 April 2018 at 17:13

Status:

Needs review

» Reviewed & tested by the community

Looks good. Patch in #16 addresses all the concerns I raised in #9. And the tests pass!

Comment #18

catch

he/him

English

CreditAttribution: catch at Third and Grove commented 4 April 2018 at 20:43

Title:

Add project namespaces to Drupal core

» Define project dependencies in core module .info files

Re-titling because this looks like a feature addition when it's really clean-up.

Comment #19

catch

he/him

English

CreditAttribution: catch at Third and Grove commented 6 April 2018 at 12:13

Status:

Reviewed & tested by the community

» Fixed

Committed d90ac78 and pushed to 8.6.x. Thanks!

Comment #20

6 April 2018 at 12:15

catch committed d90ac78 on 8.6.x

Issue #2798891 by ankitjain28may, trobey, TR: Define project...

Comment #21

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott at Thunder commented 6 April 2018 at 12:49

Here are some follow ups:

Scan .info.yml as part of PHPCS - I created #2959148: Enable PHPCS for info.yml files
Support project in install profiles - #2855026: Installation profiles do not support project:module format for dependencies
Enable the DrupalPractice.InfoFiles.NamespacedDependency rule so we can comprehensively address this. Depends on the other two. Also enabling now shows that this change missed lots of test modules.

Comment #22

TR CreditAttribution: TR commented 6 April 2018 at 21:26

this change missed lots of test modules

@alexpott: Specifically, what's missing? The only .info.yml files that were left out were those that were under core/profiles, and those weren't
"missed" they were deliberately excluded because of #2855026: Installation profiles do not support project:module format for dependencies. After that goes in the profiles can be cleaned up all at once, rather than doing some parts of them now and some later.

Comment #23

20 April 2018 at 21:29

Status:

Fixed

» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Comment #24

geerlingguy CreditAttribution: geerlingguy at Midwestern Mac, LLC commented 9 November 2018 at 16:51

Coming over from #3011936: Apply new format for dependencies... and this feels so... weird.

When I define a module dependency with composer, it's:

drupal/modulename

And now when I define a module dependency in my module's .info.yml file, it's:

modulename:modulename

But if I define a module dependency for one of the modules in the core/ directory, it's:

drupal:modulename

I am questioning my sanity; was there any CR about why this is the case, or what it's useful for? Especially as we start sticking this into Drupal's default coding standards review, I think it would be nice to know why this is done, and why are introducing another Drupalism here :/

Comment #25

trobey CreditAttribution: trobey as a volunteer commented 9 November 2018 at 17:24

@geerlingguy: You should be questioning your sanity or at least doing a bit more research. First there is a link to a CR posted right at the top in the Problem/Motivation section. That would be a good place to start. You should not be using

modulename:modulename

anywhere. It should be

project:module

everywhere and the project is optional. In many cases the name of the project is the same as the name of the module. We often refer to installing a "module" when we are installing a project. Projects can contain multiple modules and the lax terminology leads to confusion such as yours. For Project Dependency it makes a huge difference when these terms are used incorrectly. The last line of code is really just the above line of code applied to Drupal core. There is nothing special about this for Drupal core.

The last thing is ":" is often used for scope and this was implemented before Drupal started using Composer. It is not another Drupalism. PHP uses "::" but this has been criticized for not following a simpler one character for scope. I chose to use what is commonly used in many computer languages. Composer uses a forward slash which is more a symbol associated with directories than with scope. Some people may prefer one over the other but they are both acceptable and I do not think it is worthwhile starting an issue over them when we have a lot more important things to address.