Improve MODULE_NAME calculation

I was making a comment, but decided to update the issue description with the changes that I suggest we do along the way and possible approach.

Thanks for the enhancement. Just a couple of thoughts:

• Not sure if MODULE_NAME is the appropriate name. It's more a PROJECT_NAME as it can be a module or theme or profile, or other things like recipes in the future.
• If a module has sub-modules, there is always more than one. This needs more thought on how to handle that. What's for sure that the requirements of jobs using that information will be different. If a job wants to enable those modules, i.e. the main module and all sub-modules, then a space separated list of all the (sub-)module names is required.
• One could argue that this method should just provide the name of the main module, but then there is that case where a Drupal module has no main module but a few sub-modules.

Agree.
* As MODULE_NAME is not documented and it's only used internally in one (soon to be two) job, we can totally change it to _PROJECT_NAME. As then _PROJECT_TYPE.
* Yeah. So far we haven't had a situation where we need to enable all the modules or a list of modules. I'd say let's wait until that need is created.
* Yup, that's what I said in the description about "the best guess". We can make the guess and show it in the job output. I think the best guess is usually the shortest name (ie: https://git.drupalcode.org/project/drupalorg - on the D7 branch)

We should still allow the option for a module maintainer to define _PROJECT_NAME and then only do the calculations if this is empty (default value). This way, we can easily fix edge cases where the above is not enough.

When working on #3397162: Tweak PHPStan config so paths are always correct and baseline is more usable → I had a question about using $CI_PROJECT_NAME or $MODULE_NAME which led me to this issue. The solution here does not impact that issue but I will work on it anyway, assuming we want to proceed as described above, by creating new variables _PROJECT_NAME and _PROJECT_TYPE, by searching for all *.info.yml

That'd be great! Thanks

Initial testing here https://git.drupalcode.org/project/scheduler/-/pipelines/352109
I set the 'upgrade status' core version back to D10 so that it ran properly to check against D11 compatibility

I also tested when the project has a shorter .info.yml by creating a dummy tiny.info.yml in the following test. This was correctly detected as the shortest. The 'upgrade status' job rightly objected to the project_type of "something-else"
https://git.drupalcode.org/project/scheduler/-/pipelines/352124

It took a few goes to get the find . -name "*.info.yml" correctly detecting the shortest name, because when trying to get just the filename via -execdir it was behaving differently in the pipelines compared to my terminal bash. So I removed that, and used | rev | cut -d/ -f1 | rev instead, to get the last part of the string using '/' as delimiter. If you know of a way to use cut to count backward from the end of the string then that would be even clearer.

Lots of debug still in, and D7 @todo, but this is ready for initial feedback.

Regarding the new variables, you suggested naming them with a leading underscore, so does this mean you consider they should be in the .variables.yml file? I was thinking they would be internal, and therefore in the hidden-varaibles.yml file, but still overridable if a project needs to do that. But it is not something that will change in an adhoc test run via the UI.

Tested on Decoupled Pages which has Nightwatch tests - the new --tag value--tag=$_PROJECT_NAME seems to work.
https://git.drupalcode.org/issue/decoupled_pages-3422594/-/jobs/3512897#...

The Upgrade Status job also appears to run as expected.

I've made the changes for Drupal 7. The only differences were:
(a) search for .info not .info.yml files
(b) do not create $_PROJECT_TYPE because there is no 'type' keyword, according to the documentation →

Tested and all works as expected
https://git.drupalcode.org/project/scheduler/-/pipelines/353307

In words, the logic for how I've coded it (in D10) is: "find all .info.yml files in the project top-level directory and all sub-directories, determine the shortest filename (ignoreing path) and use this as the project name. Read from this file to get project type"
The question I have is that there could be a scenario where say a test module has a shorter .info.yml file. I know we need some way to select when there are multiple files, but maybe we should only do this if there is not precisely one .info.yml in the top-level folder. If we find one .info.yml in the project top-level directory we use that file, and only if there are none (or more than one) we proceed to select the shortest of all files.

I'm happy with that logic. The shortest name assumes that some degree of naming conventions are followed, but they are not enforced. Your logic makes a lot of sense so I'd say go for it. Also, we can provide some documentation on the variable for the module name somewhere, so edge cases can easily overwrite it.

Regarding the name of the variables, I just added the underscore because the module name is one variable that is used by several jobs, but then again, we are calculating it (and improving that calculation here), so yeah, there is probably no need to offer them in the UI. So we can drop the underscore but we should add documentation about it somewhere just in case.

Thanks for confirming the plan. I've made the following changes:

• Remove leading underscore from both variable names
• Allow PROJECT_NAME to have a value set in .gitlab-ci.yml and therefore not do the processing for that variable (but still get PROJECT_TYPE from the file)
• If there is only one *.info.yml in the top folder, then use that to get the project name. Otherwise take all files and find the shortest

Not done the documentation yet.

The code changes look big, due to lots of debug. But without the debug the process is:

if [[ $PROJECT_NAME == "" ]]; then
TOP_LEVEL_INFO_FILES=($(ls *.info.yml || true));
NUMBER_OF_FILES=${#TOP_LEVEL_INFO_FILES[@]};
if [[ $NUMBER_OF_FILES == 1 ]]; then
PROJECT_NAME=$(echo $TOP_LEVEL_INFO_FILES | cut -d'.' -f1);
else
ALL_INFO_FILES=$(find . -name "*.info.yml" -not -path "./vendor/*" -not -path "./$_WEB_ROOT/*" | rev | cut -d/ -f1 | rev);
SHORTEST=$(echo "$ALL_INFO_FILES" | awk '{print length($0)"."$0 }' | sort -g -k1,1 | head -1);
PROJECT_NAME=$(echo $SHORTEST | cut -d'.' -f2);
fi
fi
# If we have a project name, then read the corresponding .info.yml to get the project type.
if [[ $PROJECT_NAME != "" ]]; then
find . -name "$PROJECT_NAME.info.yml"
INFO_YML=$(find . -name "$PROJECT_NAME.info.yml" | head -1)
PROJECT_TYPE=$(sed -n -e 's/^.*type:[[:space:]]*//p' $INFO_YML)
else
# The above did not produce any value so fall back to using $CI_PROJECT_NAME
PROJECT_NAME=${CI_PROJECT_NAME%-*}
fi;

Putting that here, to help with the review.

Thanks for the breakdown and the cleaned-up version. It helps understanding the logic. It looks good, much more solid than what we had.
Once we have the documentation changes (and clean up non-needed debug) I'd say it's ready.

I will make the same change for the D7 version, to keep them consistent, and run some test cases.

There is scope for compression, removing variables, such as $TOP_LEVEL_INFO_FILES and $NUMBER_OF_FILES and coding it like:

      if [[ $(ls *.info.yml | wc -l) == 1 ]]; then
        export PROJECT_NAME=$(ls *.info.yml | cut -d'.' -f1);

etc.
But this is much less readable and would need more explanation in the comments. So I hope you are OK with longer version, as in #12.

I'd prioritize readability here too, so all good.

Tested on D11
Two top-level .info.yml files and a third dummy one in lower folder, even shorter (with nonsense name and no type, so this fails as expected)
https://git.drupalcode.org/project/scheduler/-/jobs/3571384

Only one top-level .info.yml, the correct one
https://git.drupalcode.org/project/scheduler/-/pipelines/357839

No top-level .info.yml files, dummy file 'src/dig.yml' is the shortest - caters for blank "type"
https://git.drupalcode.org/project/scheduler/-/pipelines/357873

Remove 'src/dig.yml' - gets proper "type"
https://git.drupalcode.org/project/scheduler/-/pipelines/357925

I will do a test or two on D7 but the processing is identical except we have .info instead of .info.yml and there is no PROJECT_TYPE. That variable ends with a blank value. Should it be pre-set to default of 'module' in case the type is missing from the .info.yml in D11?

Thanks!

Here is a D11 test, there are two top-level .info.yml files so PROJECT_NAME is set in the .gitlab-ci.yml file to indicate which is the correct one. All runs as expected.
https://git.drupalcode.org/project/scheduler/-/pipelines/359024

Here is a basic test on D7 - the .info file exists
https://git.drupalcode.org/project/scheduler/-/pipelines/359047

Remove the .info file - now finds a test file, showing that the process works
https://git.drupalcode.org/project/scheduler/-/pipelines/359055

Add second top-level .info and restore the original so there are two. Shortest is selected.
https://git.drupalcode.org/project/scheduler/-/pipelines/359058

Can you trigger the downstream pipelines before I remove the debug, please?

Downstream pipelines: https://git.drupalcode.org/issue/gitlab_templates-3463740/-/pipelines/35...

Code wise things are looking really good and all the testing that you've done so far seems good too. I'll wait for the clean up and your green light to do a final review before setting it to RTBC.

I've removed the debug, and adjusted comments, and also aligned a few details between the two versions (main and main-d7). I will run a final test, becuase I noticed that it was not just debug that had to be removed, but some duplication of code.

I have also added a paragraph to the end of the Customizations doc page. That seemed the best place to put this, but if you want it moved let me know.

Ready for final review. The pipeline failed becauase Drupal 7.103 has been released.

I've gone through the code and it looks good to me. I think the location within the documentation is fine too. RTBC.

Rebased with 📌 Bump D7 version Active . Re-running downstream: https://git.drupalcode.org/project/gitlab_templates/-/pipelines/360017

Re-tested with two files, using override variable. All OK
https://git.drupalcode.org/project/scheduler/-/pipelines/360001

Without override - finds shortest name
https://git.drupalcode.org/project/scheduler/-/pipelines/360012

Only one file - uses that one. All OK
https://git.drupalcode.org/project/scheduler/-/pipelines/360027

Thanks for the additional testing and all the test cases you did overall, it's amazing! I'm merging this.

Thanks for the additional testing and all the test cases you did overall, it's amazing!

You're welcome. It was good to get this fixed. Yes it needed several test cases to prove all the various scenarios.

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