[DX] Provide an abstracted way to distinguish between core/contrib/custom projects

[klonos]

In #5791, we introduced logic in system_get_debug_info() that distinguishes projects between core/contrib/custom. There is a need for that in other places as well though:

• 3146: Need to filter the modules listing page by type of module

• 5186: Telemetry metrics: do not report theme names if they are custom (non-core/non-contrib)

• 6573: Telemetry metrics: do not report installation profile names if they are custom (non-core/non-contrib)

I'm sure that there will be other cases where this would be useful, so we should abstract it, and if possible include that information in the functions that return project data (like system_get_info(), system_list(), list_themes() etc.).

[klonos]

...I think that the best place to hook into would be:

• system_rebuild_module_data()/_system_rebuild_module_data()
• system_rebuild_theme_data()/_system_rebuild_theme_data()
• Not sure about layout templates 🤔 ...there's _update_build_layout_data(), but there aren't any respective system_rebuild_layout_data()/_system_rebuild_layout_data() functions (which there should be, for consistency - but that sounds like out of scope here, and something that should be handled in #2548 instead).

[klonos]

Initial PR up for testing/review/feedback: https://github.com/backdrop/backdrop/pull/4768

As a first step, I've only implemented changes for modules and themes. Before we can do the same for layout templates, there will need to be some work done to untangle things in #2548. The flexible layout templates are not "proper" projects - they are config, so they don't have .info files etc. ...so perhaps they should always be considered custom(?) ...perhaps these do not belong here in the first place, since these functions are more related to project data - we should be distinguishing these from other functions like system_list()/module_list()/list_themes().

[klonos]

...the source info introduced here is being added "on the fly". Should we be saving it in a new column in the {system} table as well? (see update_module_add_to_system()/system_get_files_database()/system_update_files_database())

[yorkshire-pudding]

I think this is a great idea @klonos . Thank you for the PR. Do you have any thoughts on how we should test it? I assume locally, by installing devel, we could test easily in the "Execute PHP". I know we already have code in core that looks for modules/contrib and modules/custom but I'm not sure that we document that anywhere in the user guide. I've tried searching but can't find mention of it, though I may have missed it. Perhaps we need to add documentation somewhere if we expect that? I'm not sure if someone would try another subfolder system (e.g. maybe someone thinks to group their modules by what they do).

I haven't looked in detail at the code, but it looks like there are a few PHPCS issues to sort.

[yorkshire-pudding]

@klonos - just watched the meeting from last week where this was discussed. I'm fairly sure the packaging script adds timestamp. I don't recall ever seeing mstamp. Here is a recently updated module:

You can also see that project is there. Which project .info file have you been using for reference?

One other use case I can see is where someone is using a contrib module, but are currently using the dev version either to test recently merged PRs to provide feedback before a release, or if the releases have fallen behind. If this is in a contrib subfolder then it will be picked up, but otherwise won't. I can't see a way around that unless we had a large array of all contrib projects.

[klonos]

@yorkshire-pudding thanks for taking the time to test things 🙏🏼

Do you have any thoughts on how we should test it?

As it is currently, the only application for this in the core UI would be the recently-added debug info page. So you can check if adding modules to the test site places them in the correct category in the debug report. Eventually, the plan is for this to also be used in other places, like the two telemetry-related issues I have linked in the issue summary, and #3146, which will make another use case for something that can be tested in the UI. Other places that come to mind where we are categorizing modules/projects in the UI is admin/reports/updates and admin/config/system/updates, but I have not added any of that in the current PR. Should I, or scope creep you think?

...by installing devel, we could test easily in the "Execute PHP".

Yes, for now, you can install the devel module, and then try calling a function like _system_rebuild_theme_data() and see if the projects returned in the array have the correct 'source' added. Something like this should work:

$modules = _system_rebuild_module_data();
$themes = _system_rebuild_theme_data();
dpm($modules, 'modules');
dpm($themes, 'themes');

I don't recall ever seeing mstamp. ... You can also see that project is there. Which project .info file have you been using for reference?

It's mtime actually, and I wasn't using any project for reference - I was actually going by what the existing code was doing. You see, that is the name of the key that is added to the project info arrays returned by functions _system_rebuild_module_data() and _system_rebuild_theme_data(). See:

• https://github.com/backdrop/backdrop/blob/1.x/core/modules/system/system.module#L3230
• https://github.com/backdrop/backdrop/blob/1.x/core/modules/system/system.module#L3230

I assumed that the timestamp gets added there, but that is actually local file modification timestamp - not the data in the timestamp entry in .info files. Thanks for pointing that out - I'll incorporate it in the logic for determining contrib vs non-contrib 👍🏼 ...although that data will be missing from non-packaged versions of contrib; so we can assume that something is likely contrib if timestamp exists in their .info, however we cannot assume that something is NOT contrib if that is missing from their .info (because this might be either a dev version or simply a module from the contrib repos in GitHub w/o any official release yet).

... but I'm not sure that we document that anywhere in the user guide. I've tried searching but can't find mention of it, though I may have missed it.

Yeah, I don't think that it is documented at the moment. It is something that is unofficially recommended as best practice, and it is supported (as in we have code/logic in core that accounts for it), it is however not mandated in any way.

I haven't looked in detail at the code, but it looks like there are a few PHPCS issues to sort.

Yeah, I'll work on fixing those 👍🏼

If this is in a contrib subfolder then it will be picked up, but otherwise won't. I can't see a way around that unless we had a large array of all contrib projects.

Indeed. I have no solution for that at the moment, but you have given me some ideas to try next 🔨 🔧

[yorkshire-pudding]

It's mtime actually, and I wasn't using any project for reference - I was actually going by what the existing code was doing. You see, that is the name of the key that is added to the project info arrays returned by functions _system_rebuild_module_data() and _system_rebuild_theme_data(). See:

• https://github.com/backdrop/backdrop/blob/1.x/core/modules/system/system.module#L3230
• https://github.com/backdrop/backdrop/blob/1.x/core/modules/system/system.module#L3230

@klonos - thanks for explaining that difference.

[klonos]

PR updated. The logic now considers something contrib if:

• it is not core (which means basically "not placed in the core directory")
• it has all 3 project, version, and timestamp specified in its .info file.

Ready for another round of review/testing.

[klonos]

...I have not addressed the case of dev versions of contrib in this iteration of the PR, but that would be tricky to do. Things I have considered:

• hooking into the update functionality (functions like _update_build_fetch_url()), to see if we have any project data for that project name in b.org
• making cURL calls to the GitHub API to see if the project name exists as a repo in https://github.com/backdrop-contrib

These seemed like overkill, and perhaps best left for a follow-up (or dealt with as part of the broader #664). The current implementation takes a best-efforts approach, which should be enough for our current needs, or at least as a first baby-step.

[yorkshire-pudding]

I agree that the approach is probably a good best efforts and anything else could be a follow up.

• making cURL calls to the GitHub API to see if the project name exists as a repo in https://github.com/backdrop-contrib

Regarding this, a cURL call to the GitHub API can cause problems if the rate limit is hit; without a token it is 60 per hour. However, simply checking (probably using cURL) if https://github.com/backdrop-contrib/$project exists seems to work well for bee and does not cost anything on the rate limit. If the namespace existed on backdrop-contrib it would be reasonable to assume it was contrib rather than custom.

I'm currently doing some improvements in bee and this is a simple function I'm adding:

function download_bee_check_url_exists($url) {
  // Initiate the curl request.
  $curl_handle = curl_init($url);

  // Compile the options for the curl request.
  // Set the user agent string.
  curl_setopt($curl_handle, CURLOPT_USERAGENT, BEE_USER_AGENT);
  // Allow curl to follow redirects.
  curl_setopt($curl_handle, CURLOPT_FOLLOWLOCATION, TRUE);
  // Specify that we don't need the body of the response.
  curl_setopt($curl_handle, CURLOPT_NOBODY, TRUE);

  // Execute the curl request.
  curl_exec($curl_handle);

  // Get the response code.
  $return_code = curl_getinfo($curl_handle, CURLINFO_RESPONSE_CODE);

  // Close the curl request.
  curl_close($curl_handle);

  // Return the result.
  return $return_code == 200;
}

You wouldn't need the follow redirects option; I just need that for the 'latest' release.

[klonos]

Thanks @yorkshire-pudding 🙏🏼 ...that method already being used with success elsewhere provides some confidence + a starting point instead of having to start from scratch (although I was actually thinking to use backdrop_http_request() in a very dump/simplistic way TBH).

I'll wait for some more feedback before I attempt to add anything further.