PHPCS error in contributed module caused by core recipe README.txt

The PHPCS checks in core do not check .txt files. It runs on these extensions, <arg name="extensions" value="engine,inc,install,module,php,profile,test,theme,yml"/>. Where the module ipcountry is using a custom phpcs.xml that is checking .txt files.

I suggest that the custom phpcs.xml be modified to only check files in that module.

Where the module ipcountry is using a custom phpcs.xml that is checking .txt files.

Ah, thank you for noticing that.

But still, why is recipe.README.txt being moved into my modules codebase on GitLabCI if I don't have any recipes in the module and I am not testing recipes?

I think it still makes sense to patch recipe.README.txt so that it complies with coding standards, even if core doesn't about checking .txt files.

I don't know why phpcs for the module is running phpcs on core files, that sounds like a question for the gitlab templates project.

Drupal does not have coding standards for .txt files, so there isn't currently anything to comply with. The standard that does exist about the end of a file is for PHP files. The closest issue I know about is the Coding Standards one for markdown, #2952616: Adopt CommonMark spec for Markdown files → .

I don't know why phpcs for the module is running phpcs on core files, that sounds like a question for the gitlab templates project.

No, you misread. FILE: ...builds/project/ip2country/web/modules/custom/ip2country/recipes/README.txt is a file inside the ip2country project folder. Presumably it was copied there by the GitLabCI job, because it is not present in the module's codebase.

I believe there are two independent problems here:

1. Trivial core fix to delete the spurious newline

2. Figure it out whether the core composer.json or https://git.drupalcode.org/project/gitlab_templates/-/blob/main/scripts/... causes this

Yes, two independent issues, as you said. The MR addresses #1, but I don't know where to start with #2 because what's happening behind the scenes during GitLabCI deployment doesn't show up in the logs/artifacts.

I can narrow it down a bit because @quietone noted that my phpcs.xml.dist enables phpcs checking of txt files, while the core default does not check txt files.

Upon further examination, I see this exact same phpcs error on [modulename]/recipe/README.txt in every project I own that does phpcs checks on txt files. If you don't enable phpcs checks on txt files, you will never notice that recipe.README.txt file is getting copied into every project during GitLabCI testing.

I had tagged this issue with "Recipe initiative" because I though someone involved with the recipe project might recognize why this file is being copied into a contrib module folder.

core/composer.json contains the following:

    "extra": {
        "drupal-scaffold": {
            "file-mapping": {
                "[project-root]/.editorconfig": "assets/scaffold/files/editorconfig",
                "[project-root]/.gitattributes": "assets/scaffold/files/gitattributes",
                "[project-root]/recipes/README.txt": "assets/scaffold/files/recipes.README.txt",
                "[web-root]/.csslintrc": "assets/scaffold/files/csslintrc",
                "[web-root]/.eslintignore": "assets/scaffold/files/eslintignore",
                "[web-root]/.eslintrc.json": "assets/scaffold/files/eslintrc.json",
                "[web-root]/.ht.router.php": "assets/scaffold/files/ht.router.php",
...

As you can see, this is explicitly copying recipes.README.txt into the [project-root] directory, which is inside the contributed module when testing contributed modules. That it why it is present inside my module in GitLabCI testing, and that is why it is being tested when I run my module tests.

This line of code in core/composer.json was explicitly added by the issue i cited in the OP: 📌 Add support for recipes to drupal/recommended-project and drupal/legacy-project Fixed
Specifically, it was added in that issue by @thejimbirch in https://git.drupalcode.org/project/drupal/-/merge_requests/8113/diffs?co... (without the extra blank line at the end of the README) then the extra whitespace was explicitly added by @alexpott in https://git.drupalcode.org/project/drupal/-/merge_requests/8113/diffs?co...

This is only done in core 11.1+ and is not present in core 11.0, which explains why this error only just started showing up when core 11.1 was released and GitLabCI started to use that as the default version.

Why is this done? Why does recipes.README.txt need to be copied into the tested contributed module's directory structure?

As @ghost of drupal past said, this is two issues.

1. If you are going to copy a file into my project codebase during testing, then please fix the coding standards issues in that file. That's what the MR does. This is a simple one-character fix that has no functional effect on core but will stop test failures in contributed projects like mine. There was no reason to put in that blank line in the first place, so there should be no further justification needed to remove it.
2. Why is there a need to copy recipes.README.txt into my project codebase? It is not explained in the original issue, and as far as I can see it only causes problems. Should I re-open that other issue?

The objective is to have my tests stop failing because of this file that I don't want or need in my project codebase. The failures are due to actions taken by core in 11.1, and need to be fixed in core.

I added back the "Recipes initiative" tag because this change was made in an issue that also had this tag - this problematic change was clearly part of the recipes initiative.

This is RTBC for the newline quick fix.

Based on its contents the new README is meant to be scaffolded into a full Drupal site codebase, not into extension directories. The same must happen with .gitattribute and .editorconfig but to date I suppose those have been benign on CI. @tr: Whether we need a separate issue for solving that or if we should just continue here I'll leave to your discretion.

I'm happy to do it here, I just don't know *why* this is being put in [project-root] instead of [web-root]. Was it just a mistake? Was it deliberate? I don't know the reason, and the reason wasn't explained in the issue that added it, and I am reluctant to move that file because it might break something I'm not aware of.

That's why I'm asking for help here, and tagging it with "Recipes initiative" - to get the attention of someone who might know. I will also post a link to this issue in 📌 Add support for recipes to drupal/recommended-project and drupal/legacy-project Fixed to hopefully get the attention of someone who might know.

Recipes are applied to Drupal, they are not a part of Drupal with the exception of core’s recipes.

That is the reason the file is put in the project root and not the web root.

The question is, why is recipe/REAME.txt being inserted into my contributed module's code base during testing? Why is it needed? Why is it in core composer.json as an asset file that needs to be copied somewhere?

My module does not use recipes in any way.

That is not something my contributed module needs or wants, and is unexpected.

And as demonstrated here, it causes test failures when my contributed module is tested. Failures which do not show up in my local environment, because they are solely due to the test environment inserting unknown files into my code base.

Recipes are applied to Drupal, they are not a part of Drupal with the exception of core’s recipes.

That is the reason the file is put in the project root and not the web root.

They're not part of my contributed module either.
If that's the reason this file doesn't belong in [web-root], then it also doesn't belong in [project-root].
It doesn't belong there.

As far as I understand, the core recommended composer template is a recommendation on how to start a Drupal project using composer. Recipes are now a part of Drupal core.

While your issue here and the MR that fixes the errant extra line is in the recipes system, I feel like your comments are more about the testing system and why it uses the core recommended template that now includes recipes. That is beyond any of my knowledge, and I feel like it should have an additional issue in whatever issue queue is responsible for that.

Whether it is used for deployment or testing, why does recipe/README.txt have to be copied into a contributed module codebase?

Again, unexpected, unneeded, and unwanted - it doesn't add anything to the contributed project. And in this case actually breaks something. It doesn't belong there.

Testing only has value if it is done in the same conditions as deployment. So if drupal/core-recommended is what people use to build Drupal sites, then that is *exactly* what should be used in testing.

@tr: I think @thejimbirch agrees with you comments about CI but suggests you should open a separate issue for them, possibly in https://www.drupal.org/project/gitlab_templates → .

Committed and pushed d2e10cddc0b to 11.x and 0ae30d842e8 to 11.1.x. Thanks!

FWIW this happens because of the super odd way in which the contrib gitlab templates get your code in the correct place. See https://git.drupalcode.org/project/gitlab_templates/-/blob/main/scripts/... - this approach has always been fragile and we should change it. We should not be altering the code under-test in the way that we do. See also https://git.drupalcode.org/project/gitlab_templates/-/blob/main/scripts/... for how the project's composer.json is changed.

quietone → closed merge request !10651

I've created 🐛 Change how the templates link the project into Drupal Active to address the root cause.