Support Drupal 8-style *.info.yml files

So I think this will require

1) Code (hopefully "mature" code somehow copied from D8) that can parse dependencies and determine dependency requirements. If D8 is organized on this line in a way that can work, and if it's even possible to get code like this into a D6 or D7 environment, then all will be good.
2) A switch that says "Use the other dependency code if this is D8"

If the first is not possible (dependent on symfony?) then somebody will have to write an alternate dependency requirement parser, and that will be awful and super fragile.

#2 sounds like a good proposal to me.

The drupal_parse_dependency function has been removed in Drupal 8. I will need to use Symfony to get the string and then pass it in like Drupal 7.

Attached is a patch. It requires libYAML and the PECL YAML PHP library. The drupal.org server may need those installed since they are generally not included in the default PHP implementation. It does not depend on Symfony\Yaml. Drupal 7 does not have Symfony included so that approach seems too complicated for reading a simple YAML file.

I have done some simple testing but this could use additional testing. Note that this only fixes new releases; old releases are not fixed because the .info.yml files are parsed when constructing a new release.

Attached is an updated patch that fixes some of these issues. I removed project_dependency_parse_infoyml_file(). The problem with the tense you mentioned is gone because that code has been deleted. I also changed the code to use a loop over the extensions.

I can add the $release['tag'] to the function arguments for project_dependency_info_parse(). I generally try to avoid changes to function signatures but in this case I think it is okay since it is unlikely that anyone else is calling this function. But at present I cannot dump $release['tag'] and I cannot test this change and it will significantly increase the odds that the patch will break drupal.org. Once my drupal.org infrastructure development site is reactivated #1517040: Review Drupal.org development site for trobey then I should be able to test this and I probably can make this change.

Drupal 7 does not require the PHP Yaml library. It also does not require Symfony. So there is no difference in that this feature will add a requirement above those for Drupal 7. How long will it take to install Symfony and how many changes will that require and add to maintenance? Adding the PHP Yaml library is quick and only has to be done for one server. There is a possibility that the two parsers will produce different results but this is a very simple file and only a few things are actually needed so I doubt this is a concern. Note that when this code is ported to Drupal 8 it is a simple matter to change the call to the PHP Yaml library and this would be easier than removing the local install of Symfony from Project Dependency.

Just for awareness ... I did some testing with the PECL YAML library a few months ago, and there were definitely issues with it parsing a bit differently:

#2031597: Weird YAML data
#1920902-53: Add a Drupal Yaml wrapper so we can default to PECL Yaml component if it is available

Here is an updated patch using the Symfony Yaml parser code.

@trobey please let us know when you have a testing environment on this, and let me know if there's anything I can do to speed that along.

This patch uses the $release['tag'] to determine whether to expect .info or .info.yml files. It looks for .info.yml files for D8 and above so that we do not run into problems when D9 appears.

I do not see how we can use Libraries for the Yaml classes. These files are not available anywhere. They are modified versions of those from Symfony so that they can be loaded by Project Dependency. The original Symfony files will not work. Reading the license it allows for modifications and the license does not need to be included unless substantial parts of Symfony are used. Maintenance is not going to be as simple as using the PHP Yaml parser but that is the price of using the Symfony Yaml parser. It is likely that updated Symfony code will not need to be used since the parsing that we need is relatively simple.

I have done enough testing on this that I am reasonably confident it will work for parsing .info.yml files. This will not fix past releases since the files are already parsed. I cannot test the entire release and testing process so it is possible there are other problems.

We absolutely can't bring in Symfony components without using Libraries module or some technique like it. Remember this has to coexist in a D7 Drupal.org environment. This is not about licensing (although someone will raise that issue) but about namespacing. And we should be using Symfony components without changes if at all possible.

I still maintain that the module that brings in symfony components should be a completely separate module. It will be useful all over D7. It might be an extension of Libraries or it might be something *like* Libraries. @Xano, are you willing to put something like that together?

I would also *really* like to use the code that D8 uses to detect dependencies (for D8 dependency chasing) instead of writing our own.

The simpletest.test and simpletest.module files in the simpletest module (in core) has code for PSR-0 within it. Does anyone know if PSR-0 code is in Drupal 7? This might have been added recently.

Attached is an update that uses the Drupal Symfony module. I spent many hours trying to get Symfony installed using the Libraries API module and X Autoload but with no success but perhaps I was just missing something. The Drupal Symfony module installs Symfony inside that module. The Class Loader module by Sun installs the class loader component of Symfony inside that module. If we really want Symfony in libraries then it probably would be better to file an issue on the Drupal Symfony project than to duplicate that effort.

PSR-0 for test classes is discussed in #1693398: Allow PSR-0 test classes to be used in D7 (followups). This also discusses Class Loader and X Autoload. X Autoload is supposed to support libraries but I do not think Class Loader does.

Putting back to needs review, but hope you can try this out in the most realistic environment possible.

After some more testing I am backing out the change to use the release tag to determine the way to parse the file and reverting to using the file extension. The problem is that the projects with early Drupal 8 releases used .info files. It is also possible ports from Drupal 7 to Drupal 8 will not have the .info files converted yet. I would question a release where the .info file has not been converted since it will not work but it is possible and it is not a good idea to be throwing errors like

[trobey@devwww]$ drush pdpp ip2country
Processing dependencies for ip2country
WD php: Symfony\Component\Yaml\Exception\ParseException: Unable to [error]
parse in
"\/var\/tmp\/project_dependency_sourcecode_directory\/ip2country\/tests\/ip2country_test.info"
at line 1 (near "name = ip2country hook tests"). in
Symfony\Component\Yaml\Parser->parse() (line 243 of
/var/www/dev/pd-drupal_7.redesign.devdrupal.org/htdocs/sites/all/modules/symfony/vendor/symfony/symfony/src/Symfony/Component/Yaml/Parser.php).
Cannot modify header information - headers already sent by (output [warning]
started at /usr/local/sbin/drush/includes/output.inc:37)
bootstrap.inc:1216
Symfony\Component\Yaml\Exception\ParseException: Unable to parse in "\/var\/tmp\/project_dependency_sourcecode_directory\/ip2country\/tests\/ip2country_test.info" at line 1 (near "name = ip2country hook tests"). in Symfony\Component\Yaml\Parser->parse() (line 243 of /var/www/dev/pd-drupal_7.redesign.devdrupal.org/htdocs/sites/all/modules/symfony/vendor/symfony/symfony/src/Symfony/Component/Yaml/Parser.php).
Drush command terminated abnormally due to an unrecoverable error. [error]

when it is not necessary. Even if there are later project releases for Drupal 8, any project that had an early Drupal 8 release is going to throw these errors. The bottom line is that Drupal 8 releases can have .info files. Updated patch is attached.

The other problem I have run into is that many of the projects with Drupal 8 releases do not have dependencies. It is hard to port a project until the dependencies have been ported and there just are not that many projects ported. So it took some time to find an example. Here it is:

[trobey@devwww]$ drush pdpp geofield
Processing dependencies for geofield
Processed geofield: 7.x-2.0-beta1
Processed geofield: 7.x-1.2
Processed geofield: 8.x-1.x-dev
Processed geofield: 7.x-2.0-alpha2
Processed geofield: 7.x-2.0-alpha1
Processed geofield: 7.x-1.1
Processed geofield: 7.x-1.0
Processed geofield: 7.x-1.0-rc1
Processed geofield: 7.x-2.x-dev
Processed geofield: 7.x-1.0-beta2
Processed geofield: 7.x-1.0-beta1
Processed geofield: 7.x-1.0-alpha5
Processed geofield: 7.x-1.0-alpha4
Processed geofield: 7.x-1.0-alpha3
Processed geofield: 7.x-1.0-alpha2
Processed geofield: 7.x-1.0-alpha1
Processed geofield: 7.x-1.x-dev

[trobey@devwww]$ drush pdsd geofield 8.x-1.x-dev
Array
(
[2001872] => Array
(
[uri] => geophp
[version] => 8.x-1.x-dev
[tag] => 8.x-1.x
[dependency_type] => 0
)

)

I checked and this release does use a .info.yml file.

Note that this requires the Symfony module. I get an error using drush dl symfony although I do not believe this is a problem. But I switched to using wget and untarring the project. Then following the instructions on the project page:

cd sites/all/modules/symfony
curl -s http://getcomposer.org/installer | php
php composer.phar install

The development site required adding an option to the php command to get it to work.

Also, I would assume this is only going to fix things going forward. If the .info.yml file was not parsed when the release was made then the information is missing from the database tables. If a project makes a new release this should work but if it is dependent on a project that has dependencies that was released before this fix the dependencies probably are not going to be in the database tables. As I mentioned above, there are few Drupal 8 releases with dependencies at this point. Running drush pdap probably would fix this. It might be a good idea to test drush pdap before trying to use it to fix past releases.

@trobey asked in IRC:

What should I do when a Drupal 8 release has a .info file instead of a .info.yml file?
A lot of the first Drupal 8 releases do this (because the files had not yet been converted to Yaml?).

I think the answer is: We have to do whatever Drupal 8 does. Which at this point (I think) is to accept and use either one, right? If that's likely to change, then I don't think we should make any changes here until it's stable.

What's the plan for this, @Xano?

Some more information about info files in the Yaml format. The change was committed by Dries on March 6, 2013.

https://drupal.org/node/1793074#comment-7144950

Here are some projects with earlier Drupal 8 releases:

Bad Judgement 8.x-1.0 released on March 1, 2011 has a .info file.

Bad Judgement 8.x-2.0 released on March 8, 2011 has a .info file.

HTML Mail 8.x-2.26 released on April 18, 2011 has a .info file.

Mail MIME 8.x-2.6 released on April 18, 2011 has a .info file.

Mail System 8.x-1.18 released April 18, 2011 has a .info file.

Emogrifier 8.x-1.12 released April 18, 2011 has a .info file.

Include 8.x-1.6 released April 18, 2011 has a .info file.

Echo 8.x-1.5 released April 18, 2011 has a .info file.

Filter transliteration 8.x-1.1 released April 18, 2011 has a .info file.

Mail System 8.x-2.0 released April 21, 2011 has a .info file.

Mail System 8.x-2.1 released April 21, 2011 has a .info file.

HTML Mail 8.x-2.28 released April 21, 2011 has a .info file.

Mail System 8.x-2.2 released April 21, 2011 has a .info file.

Mail MIME 8.x-2.7 released April 21, 2011 has a .info file.

Mail MIME 8.x-2.8 released April 21, 2011 has a .info file.

HTML Mail 8.x-2.29 released April 22, 2011 has a .info file.

Mail System 8.x-2.3 released April 23, 2011 has a .info file.

HTML Mail 8.x-2.32 released April 23, 2011 has a .info file.

…

All of these project releases are legitimate Drupal 8 releases that followed the requirements for Drupal 8 at the time of the release. Is it okay to change the requirements for a release after the fact? We have plenty of project releases for Drupal 8 that predate this change. There are about 145 project releases before March 6, 2013 other than development releases that probably all have .info files.

The drush pdpp command processes all the releases for the specified project. If I assume that all the Drupal 8 releases have a .info.yml and parse them an error is thrown and the command stops. Even if a project has released a newer Drupal 8 release with a .info.yml file drush pdpp will still fail. Is it acceptable for project dependency to break under these circumstances? Is someone going to go back and fix or delete all these releases? If even one project fails then drush pdap will no longer work because it tries to process all projects. Is it okay to break drush pdap?

Another option is to not parse anything if a Drupal 8 release has an .info file. I have not tried this to see if it will work. At best I would assume it would return no dependencies. Is this what we want?

It is no problem to read a .info file for a Drupal 8 release. This will return the proper dependencies for the release. The release will not work for a Drupal 8 core after March 6, 2013 because Drupal 8 core will require a .info.yml file. It very well may work for a Drupal 8 core before March 6, 2013.

How should I proceed?

If D8 current requires yml files and ignores info files, then we can just ignore info files. We're not even out of alpha; releases created in alpha are not going to last or probably work anyway.

Well, I still have to deal with modules that are not properly ported. I do not think it is acceptable to have the drush scripts fail due to an error being thrown. So I rewrote the patch to handle errors more gracefully. The first thing is I changed from grabbing all the .info and .info.yml files to checking the version and for 7.x and earlier I grab the .info files and, otherwise, I grab the .info.yml files. This avoids applying the wrong parser on a file and throwing an error. Then I changed the code for the parsers to use the original parser for version 7.x and earlier and to use the Symfony Yaml parser for other versions (8.x and later). This still means that some 8.x releases with the wrong info file format will not have any information read and I need to try this to see if something breaks.

Here are the results:

[trobey@devwww]$ drush pdpp geofield
Processing dependencies for geofield
Processed geofield: 7.x-2.0-beta1
Processed geofield: 7.x-1.2
Processed geofield: 8.x-1.x-dev
Processed geofield: 7.x-2.0-alpha2
Processed geofield: 7.x-2.0-alpha1
Processed geofield: 7.x-1.1
Processed geofield: 7.x-1.0
Processed geofield: 7.x-1.0-rc1
Processed geofield: 7.x-2.x-dev
Processed geofield: 7.x-1.0-beta2
Processed geofield: 7.x-1.0-beta1
Processed geofield: 7.x-1.0-alpha5
Processed geofield: 7.x-1.0-alpha4
Processed geofield: 7.x-1.0-alpha3
Processed geofield: 7.x-1.0-alpha2
Processed geofield: 7.x-1.0-alpha1
Processed geofield: 7.x-1.x-dev

Geofield 8.x-1.x-dev has .info.yml files:

[trobey@devwww]$ drush pdsd geofield 8.x-1.x-dev
Array
(
[2001872] => Array
(
[uri] => geophp
[version] => 8.x-1.x-dev
[tag] => 8.x-1.x
[dependency_type] => 0
)

)

Geofield 7.x-1.2 still works:

[trobey@devwww htdocs]$ drush pdsd geofield 7.x-1.2
Array
(
[1831906] => Array
(
[uri] => geophp
[version] => 7.x-1.7
[tag] => 7.x-1.7
[dependency_type] => 0
)

)

Mail System still uses a .info file in the 8.x-2.34 release and other 8.x releases.

[trobey@devwww]$ drush pdpp mailsystem
Processing dependencies for mailsystem
Processed mailsystem: 7.x-3.0-alpha1
Processed mailsystem: 8.x-2.34
Processed mailsystem: 7.x-2.34
Processed mailsystem: 6.x-2.34
Processed mailsystem: 8.x-2.33
Processed mailsystem: 7.x-2.33
Processed mailsystem: 6.x-2.33
Processed mailsystem: 6.x-2.32
Processed mailsystem: 8.x-2.31
Processed mailsystem: 7.x-2.31
Processed mailsystem: 6.x-2.31
Processed mailsystem: 8.x-2.30
Processed mailsystem: 7.x-2.30
Processed mailsystem: 6.x-2.30
Processed mailsystem: 8.x-2.29
Processed mailsystem: 7.x-2.29
Processed mailsystem: 6.x-2.29
Processed mailsystem: 8.x-2.28
Processed mailsystem: 7.x-2.28
Processed mailsystem: 6.x-2.28
Processed mailsystem: 8.x-2.27
Processed mailsystem: 7.x-2.27
Processed mailsystem: 6.x-2.27
Processed mailsystem: 8.x-2.26
Processed mailsystem: 7.x-2.26
Processed mailsystem: 6.x-2.26
Processed mailsystem: 8.x-2.25
Processed mailsystem: 7.x-2.25
Processed mailsystem: 6.x-2.25
Processed mailsystem: 8.x-2.24
Processed mailsystem: 7.x-2.24
Processed mailsystem: 6.x-2.24
Processed mailsystem: 8.x-2.23
Processed mailsystem: 7.x-2.23
Processed mailsystem: 6.x-2.23
Processed mailsystem: 8.x-2.22
Processed mailsystem: 7.x-2.22
Processed mailsystem: 6.x-2.22
Processed mailsystem: 8.x-2.21
Processed mailsystem: 7.x-2.21
Processed mailsystem: 6.x-2.21
Processed mailsystem: 8.x-2.20
Processed mailsystem: 7.x-2.20
Processed mailsystem: 6.x-2.20
Processed mailsystem: 8.x-2.19
Processed mailsystem: 7.x-2.19
Processed mailsystem: 6.x-2.19
Processed mailsystem: 8.x-2.18
Processed mailsystem: 7.x-2.18
Processed mailsystem: 6.x-2.18
Processed mailsystem: 8.x-2.17
Processed mailsystem: 7.x-2.17
Processed mailsystem: 6.x-2.17
Processed mailsystem: 8.x-2.16
Processed mailsystem: 7.x-2.16
Processed mailsystem: 6.x-2.16
Processed mailsystem: 6.x-2.15
Processed mailsystem: 6.x-2.14
Processed mailsystem: 6.x-2.13
Processed mailsystem: 8.x-2.12
Processed mailsystem: 7.x-2.12
Processed mailsystem: 6.x-2.12
Processed mailsystem: 6.x-2.11
Processed mailsystem: 8.x-2.10
Processed mailsystem: 7.x-2.10
Processed mailsystem: 6.x-2.10
Processed mailsystem: 6.x-2.9
Processed mailsystem: 6.x-2.8
Processed mailsystem: 8.x-2.7
Processed mailsystem: 7.x-2.7
Processed mailsystem: 6.x-2.7
Processed mailsystem: 8.x-2.6
Processed mailsystem: 7.x-2.6
Processed mailsystem: 6.x-2.6
Processed mailsystem: 8.x-2.5
Processed mailsystem: 7.x-2.5
Processed mailsystem: 6.x-2.5
Processed mailsystem: 6.x-2.4
Processed mailsystem: 8.x-2.3
Processed mailsystem: 7.x-2.3
Processed mailsystem: 6.x-2.3
Processed mailsystem: 8.x-2.2
Processed mailsystem: 7.x-2.2
Processed mailsystem: 6.x-2.2
Processed mailsystem: 7.x-2.x-dev
Processed mailsystem: 6.x-2.x-dev
Processed mailsystem: 8.x-2.1
Processed mailsystem: 7.x-2.1
Processed mailsystem: 6.x-2.1
Processed mailsystem: 8.x-2.x-dev
Processed mailsystem: 8.x-2.0
Processed mailsystem: 8.x-1.x-dev
Processed mailsystem: 8.x-1.18
Processed mailsystem: 7.x-1.18
Processed mailsystem: 6.x-1.18
Processed mailsystem: 7.x-1.17
Processed mailsystem: 6.x-1.17
Processed mailsystem: 7.x-1.16
Processed mailsystem: 6.x-1.16
Processed mailsystem: 7.x-1.15
Processed mailsystem: 6.x-1.15
Processed mailsystem: 7.x-1.14
Processed mailsystem: 6.x-1.14
Processed mailsystem: 7.x-1.13
Processed mailsystem: 6.x-1.13
Processed mailsystem: 7.x-1.12
Processed mailsystem: 6.x-1.12
Processed mailsystem: 7.x-1.x-dev
Processed mailsystem: 6.x-1.x-dev
Processed mailsystem: 6.x-1.11
Processed mailsystem: 7.x-1.10
Processed mailsystem: 6.x-1.10
Processed mailsystem: 6.x-1.9
Processed mailsystem: 7.x-1.8
Processed mailsystem: 6.x-1.8
Processed mailsystem: 6.x-1.7
Processed mailsystem: 7.x-1.6
Processed mailsystem: 7.x-1.5
Processed mailsystem: 7.x-1.4
Processed mailsystem: 7.x-1.3
Processed mailsystem: 7.x-1.2
Processed mailsystem: 7.x-1.1

So the drush script still runs.

[trobey@devwww]$ drush pdsd mailsystem 8.x-2.34
Array
(
)

Mail System depends on the filter module which is in core so it should not show up. Since this patch does not parse .info files in 8.x releases there should be no dependencies found even if there are any dependencies specified in the .info files. So this looks okay.

@Xano: You have lost me. There are two functions for parsing code and these are already separate functions. One is drupal_parse_info_file() which is a drupal core function for parsing the old style info files. It is called once in Project Dependency. The other is Yaml::parse() which is the Symfony Yaml parser. It is called once in Project Dependency. There is no code in Project Dependency which parses either the old info file or the new info.yml file.

@Xano please get access to devwww.drupal.org (ssh access) and work with @trobey to use the various drush commands to verify correct behavior. If you give a decent analysis of that on several projects it will be good enough for RTBC. (drush pdpp + drush pdsd). You can set the project_dependency code directory to /tmp so that you'll have proper perms on it, if you don't have adequate perms.

While #2110201: devwww.drupal.org SSH access goes through, do let me know if you want any new dev sites rebuilt for fresh data/code or need a new one.

#2110645: test_dependencies broken in D8 has been marked as duplicate.

Please get this fix asap in. My tests are failing.

This is a minor update of @trobey's #26, with some of my druthers and bundling a fairly important fix where the pd stuff was ported before project* modules so the structure of the node wasn't defined yet.

If @trobey and @jthorson look at it and aren't afraid, we should try to get it in, as it looks to me like dependency generation is completely broken right now, not just for 8.x.

There are serious problems deploying this though, as it depends on symfony module, which requires a composer run. We'll check with @drumm on that immediately so we can figure out a path. It might be OK to check in a prebuilt symfony module with the composer dependencies already built.

I am sad that this doesn't actually use D8's InfoParser (\Drupal\Core\Extension\InfoParser), because the disconnect will certainly pain us. However, Getting *that* fully integrated here seems like way too much work.

So essentially: D8 contrib dependencies don't work now, so we have no risk on the D8 dependency area. The risk here is breaking D7 contrib dependencies, and I think *they* are already broken by the port fail. So we should finish this up and push it in.

The port problem area, which should probably be a separate issue but I'll leave here if nobody squawks, is:

@@ -368,10 +367,10 @@ function project_dependency_get_external_component_dependencies($release_nid, $d
   }
   // Get the release node and project node of the *depending* components.
   $release_node = node_load($release_nid);
-  $project_node = node_load($release_node->project_release['pid']);
+  $project_node = node_load($release_node->field_release_project['und'][0]['target_id']);

   // We don't need to study the dependencies of the drupal project.
-  if ($project_node->project['uri'] == 'drupal') {
+  if ($project_node->field_project_machine_name['und'][0]['value'] == 'drupal') {
     return;
   }

BTW, I tested with
* geofield/geophp as trobey did above
* mailsystem as trobey did, same results
* payment/currency: currency is returned as the dependency
* google_analytics 8.x (fails because there is no token release for 8.x)
* commerce and related for 7.x-1.x

If anybody has other suggestions for testing they're easy to run.

Opened #2127399: Beta or full release for drupal.org? in symfony project requesting a better release.

My gut says to throw in the table -> field fix right now ... that much is trivial, and doesn't depend on input from the infra team, who are busy with bigger issues today.

Deploying the complete patch means introducing a new module to drupal.org, which at the very least probably needs to be approved by Neil.

I went through the code changes in #35 and they look okay to me. I just had a small comment on the port problem area. The code assumes that the node language is undefined. It would probably be more general to use the node language to retrieve the values:

$project_node = node_load($release_node->field_release_project[$release_node->language][0]['target_id']);
...
if ($project_node->field_project_machine_name[$project_node->language][0]['value'] == 'drupal') {

@jthorson I'm ok with doing the port problems separately.

@trobey AFAIK Drupal.org has no language-defined nodes (all und) and nothing about project module can be done without this field being populated. And the field's language might not be the same as the node, right? And it wouldn't make any sense to translate project names. I find this general usage troubling as well, but think our normal shortcuts might be ok. But using the node language wouldn't be any more right than what's happening here would it? Seems to me like if we were going to be more careful it would just check to see if the 'und' value existed and only access it if it did.

I moved the porting issue over to #2128435: Fix node structure problems introduced in D7 port

Here's the patch without the port problems from #2128435: Fix node structure problems introduced in D7 port. This now becomes dependent on that issue.

I guess I have to actually attach the patch.

Drumm approved this for deployment. @trobey could you give it another test in the current environment and confirm that you're OK with it?

I reviewed the patch and then applied the patch to the current dev release and tested it. It looks fine. There may be some projects that will need

drush pdpp

in order to update the project_dependency_component table.

Oh forgot. We still need symfony module for this. I'll make a separate request/merge for that and we'll move forward.

Committed in http://drupalcode.org/project/project_dependency.git/commitdiff/2bf1bb9

Symfony is deployed (but not yet enabled) on drupal.org

Deployment requires more code to be committed (see #20). Run composer install and commit the results.

And now symfony's dependencies are deployed... Next step is enable the module and check for smoke, then deploy this patch.

Deployed on drupal.org. Thanks so much for the great work @trobey. Please watch the dblog for issues.

Rebuilt dependencies for geofield and geophp and ...

+ /var/www/drupal.org/tools/drush5/drush -v -r /var/www/drupal.org/htdocs pdsd geofield 8.x-1.x-dev
Initialized Drupal 7.24-dev root directory at                           [notice]
/var/www/drupal.org/htdocs
Initialized Drupal site default at sites/default                        [notice]
Array
(
    [2001872] => Array
        (
            [uri] => geophp
            [version] => 8.x-1.x-dev
            [tag] => 8.x-1.x
            [dependency_type] => 0
        )

)

Command dispatch complete                                               [notice]

Yay @trobey !!!

@trobey would you mind doing a routine investgation of what's failing with ubercart 8.x-4.x? I saw these errors in the d.o log and they show up on pdsd as well (after rebuilding ubercart and views with pdpp)

I can't remember how PD decides what modules are part of core. But it seems like it's not making the right decision.

+ /var/www/drupal.org/tools/drush5/drush -v -r /var/www/drupal.org/htdocs pdsd ubercart 8.x-4.x-dev
Initialized Drupal 7.24-dev root directory at                           [notice]
/var/www/drupal.org/htdocs
Initialized Drupal site default at sites/default                        [notice]
WD project_dependency: Failed to find a release for component views     [notice]
as dependency of uc_order, release_nid=2141275
WD project_dependency: Failed to find a release for component views     [notice]
as dependency of uc_order, release_nid=2141275
WD project_dependency: Failed to find a release for component views     [notice]
as dependency of uc_catalog, release_nid=2141275
Array
(
    [1913238] => Array
        (
            [uri] => googleanalytics
            [version] => 8.x-1.x-dev
            [tag] => 8.x-1.x
            [dependency_type] => 0
        )

)

Setting to needs work as it seems like we have a miscalculation or something o what modules are core modules. We can move this to another issue if that's more relevant.

It does not look like I have ubercart 8.x-4.x on my dev system. I do not see any 8.x releases. Do I need to have my dev system rebuilt? I cannot think of any other way to test this.

Drupal core has several .theme and .profile files. I added them to the exceptions.

It might make more sense to just not look for .theme and .profile files for Drupal core releases. Updated patch attached.

Not sure how this worked before...

Can we make this work by just updating the pd_filename_type_exceptions variable?

It worked before because we were not relying on the project_dependency_components table to find the releases.

Yes, it is possible to just update the pd_filename_type_exceptions variable.

OK - If you can give me the tested drush command, I'll execute it on util and we'll be out of the woods right away. I believe the technique in https://drupal.org/comment/5078776#comment-5078776 is still the working technique.

Okay, I tested this on my dev site:

php -r "print json_encode(array('date.theme', 'bartik.theme', 'seven.theme', 'chameleon.theme', 'test_theme_phptemplate.theme', 'test_theme.theme', 'standard.profile', 'minimal.profile', 'testing.profile', 'default.profile'));" | drush vset --format=json project_dependency_filename_type_exceptions -

which outputs

project_dependency_filename_type_exceptions was set to [success]
array (
0 => 'date.theme',
1 => 'bartik.theme',
2 => 'seven.theme',
3 => 'chameleon.theme',
4 => 'test_theme_phptemplate.theme',
5 => 'test_theme.theme',
6 => 'standard.profile',
7 => 'minimal.profile',
8 => 'testing.profile',
9 => 'default.profile',
).

Then you will need to run

drush pdpp drupal

Check to make sure that you do not get a Skipped message for the 8.0-alpha* releases.

Then

drush -v pdsd ubercart 8.x-4.x-dev

should give you something like

Initialized Drupal 7.24-dev root directory at [notice]
/var/www/dev/pd-drupal.redesign.devdrupal.org/htdocs
Initialized Drupal site default at sites/default [notice]
Array
(
[1913238] => Array
(
[uri] => googleanalytics
[version] => 8.x-1.x-dev
[tag] => 8.x-1.x
[dependency_type] => 0
)

)

Command dispatch complete [notice]

Done - Ubercart now pdpp's and pdsd's without the errors. Now has no dependencies though :-) I thought it would still have google_analytics.

Thanks so much. Should we still pursue this patch, or is managing the variable a better way to go? Or maybe just patching the default in the variable?

I was thinking that we should just put in the second patch to have the check for .theme and .profile skipped for Drupal core. There are two reasons. The first is that Drupal core may add .theme and/or .profile files which would cause this to fail again. The second is adding so many exceptions may increase the chance that a theme or profile just might happen to choose to use an identical filename as one of the exceptions. That is also why I posted the second patch after checking that there is an easy way to skip the check for Drupal core releases.

My tests are all failing as they need to download php module. Php module is available. Can we get this deployed now, PLEASE?

GA does not depend on token or php. These are both absolutly optional. That is why the test_dependencies was introduced. Until now, this is for sure not working under D8 yet. On my local machine all optional modules are on disk and the tests are working well. It's only a problem on d.o testsystem.

How should I deploy this? I have no access to this machines and scripts and no clue how this works.

Send me the root ssh keys and servers and I will fix it. The permanent wining suxx. I do all my work for free and maintain several modules, too.

Hass,

Here's some free support for you.

Your module depends on 'token' and 'php'. The dependency rebuild doesn't download php, because it pukes when trying to first process the 'token' dependency.

Of course, there IS no 8.x compatible release for token ... not even -dev. So how do you propose we make your module work???

If you want to change the order of your dependencies in the info.yml file, and/or remove token from the list, we can rebuild the dependencies and you should get your PHP module.

I'm really not sure why this ticket is still open ... but will leave it to trobey to determine whether the work here is done or not.

Removed token in http://drupalcode.org/project/google_analytics.git/commit/e8a13da. Still failing https://qa.drupal.org/pifr/test/480418

I can confirm that project_dependency is no longer throwing an error in building the dependency tree ... but at the same time, it's still coming back with an empty array as a result.

I'm now speculating, but the function that reads the info.yml file is actually in core ... is the '#' symbol messing with parsing of the info file, perchance?

Project Dependency does not read the .info.yml file. Drupal core needs to have the dependencies for the module page. So we just use what Drupal core uses. Project Dependency really has no choice here since it has to produce the same dependencies as Drupal core. There is a twist here though. drupal.org uses Drupal 7 and it has to handle Drupal 8 projects (and Drupal 6 projects). The core maintainers always assume that Drupal 7 only has to understand Drupal 7 .info files. So Project Dependency has to backport the code from Drupal 8 core to Drupal 7 in order to understand Drupal 8 .info.yml files. Drupal 8 core uses Symfony. So Project Dependency uses the Symfony module to backport the Symfony stuff to Drupal 7. So Yaml::parse is used to parse the .info.yml file. If the problem is with parsing the .info.yml file then I will not be able to fix the problem since it is someone else's code. But tracing where the error occurs will take me a few hours and I cannot drop everything I am doing at the moment.

Normally a comment is no problem. I can try removing this completly. There is a parser for D8 info.yml. See http://cgit.drupalcode.org/project_dependency/tree/project_dependency.dr...

Still broken with token comment removed http://drupalcode.org/project/google_analytics.git/commit/fc2d124

GA does not depend on token or php. These are both absolutly optional. That is why the test_dependencies was introduced. Until now, this is for sure not working under D8 yet. On my local machine all optional modules are on disk and the tests are working well. It's only a problem on d.o testsystem.

How should I deploy this? I have no access to this machines and scripts and no clue how this works.

I have verified that the Yaml:parse is functioning properly and the dependencies are correct. I have also verified that test_dependencies is working. What php module are you trying to load?

https://www.drupal.org/project/php

Okay, I see the problem. The PHP module has been removed from core and made into its own project. So both Drupal 8 core and the PHP project have releases with a PHP module. Did you know that module names are not unique? Project Dependency can only guess at which one should be used. I have already filed an issue on Drupal core for them to fix this. You can find it at #2205271: Project namespace for dependencies.

So both Drupal 8 core and the PHP project have releases with a PHP module.

That sounds not correct. Drupal core D8 has no longer php module included. This means the name is not used twice. It is unique for D8 releases. Maybe project dependency has a problem as it is D7 and D7 has php module embedded, but D8 does not. So I do not understand the problem with this in mind.

Ah, but it is correct. Drupal core 8 no longer has the PHP module included but there are releases of Drupal core 8 that have the PHP module. So when Project Dependency looks up all the releases it finds Drupal core 8. I have verified this after many hours of debugging.

More to the point, module names are not unique. So the way that we specify dependencies in Drupal is not unique. Project names are unique. This problem is going to occur again and again until the way we specify dependencies is fixed. I have already added code several times to try to fix special cases and the code is getting so complicated that it is difficult to maintain. I spent a lot of hours chasing this problem again and others spent a lot of time responding to the problem with the testbots. This is a known problem. We do not need to be wasting the limited time of volunteers chasing it down again and again.

You mean there are older alpha versions of D8 core that have php module embedded? That is possible...

I think we all know that modules should use their namespace... I'm not aware of any other module that uses the same name like any other. I know the project is unique and the module name is not forced to be unique, but nobody is using other modules namespace as I know. This looks like a very rare exception here and it has a background we are aware of.

However this does not solve my problem... Can you hardcode an exception so core is no longer looked up for php filter module? I'd really like to see my tests green as it's wrong that they fail. There should appear other modules that have this requirement, too.

Although it is not common it does occur and I have had to hard code quite a few exceptions (see #2194683: webform_patched being downloaded instead of webform for course module). Also as djdevin noted in comment 2 of that thread it is possible for someone with malicious intent to craft a project with a commonly used module name that will crash the testbots.

The code in Project Dependency is getting rather complicated. It takes me a while to figure it out and I do not think anyone else understands it. There also were memory and speed issues associated with loading all the possible matches although I have these under control for the moment. There are genuinely rare cases where specific versions of a project are required by one project but not another that fail. I spent a lot of time trying to write code to solve such a problem and finally gave up not because I cannot write the code but because the result would be so complicated that there is no hope to maintain it.

Until someone cares enough to fix the core issue these test failures are going to happen. I can keep adding patches but it is fundamentally unsustainable. You can help by posting on the core issue and asking that it be fixed. I gave it a Major designation but as long as I am the only one complaining people are going to say it is a theoretical problem and why should we do anything to fix it.

The proposed fix is optional and just adds the project name to the structure. It would not take very long to code. It is backward compatible. It would probably be no more difficult than my adding another hack to Project Dependency.

I have coded changes to Project Dependency to handle the requested core change. This presumes that the fix will be very similar to the current patch #2205271: Project namespace for dependencies. The way that this code works is to look for the optional project prefix to the dependency requirement and if it is not there it guesses at the correct project. This still fails for Google Analytics.

$ drush pdpv google_analytics 8.x-2.x-dev
Processing dependencies for google_analytics: 8.x-2.x-dev
Array
(
    [google_analytics] => Array
        (
        )

)

Array
(
    [google_analytics] => Array
        (
            [0] => drupal:php
        )

)

Where the second array is the test dependencies. It can be seen that this code adds the drupal prefix to the dependency requirement and that is what is stored in the Project Dependency database table. If the core issue is accepted and the php:php can be put in the .info.yml file then Project Dependency should work properly. But until there is way for the author to specify their dependencies in a unique manner Project Dependency will sometimes fail because it cannot read minds.

It has been almost a year and still nothing. It is time to try something else since the Drupal core maintainers seem to be unwilling to support drupal.org. It seems clear that we need to have some measure of control over this if there is any hope of responding to users in anything approaching a reasonable time frame. I have found this issue has had a broad impact on project dependency since often a problem will appear to be caused by this issue and I cannot debug every one of them to ascertain whether it is a different issue. Additionally sitting on a patch for close to a year is asking for trouble when it finally gets committed since it will be hard to remember the code, things also change that can break the patch and I have to get my copy of drupal.org updated every time I work on this.

@joachim pointed out in comment 117 #2205271: Project namespace for dependencies that the project namespace only should be required for projects hosted on drupal.org. Thinking about that then perhaps it would be appropriate to separate the drupal.org requirements into a separate file. So I have established an additional project file. It is not my first choice but it appears there is no other path if we are to move forward and we cannot use the info file. The file is called .project for Drupal 7 (and earlier) and .project.yml for Drupal 8. If the file does not exist then the fallback is the current info file. Thus, if a project has trouble with dependencies they can create a project file to get things working again. If Drupal core finally decides to move forward then the changes to Project Dependency to handle the project namespace will already be in place. The drawbacks are having to specify information in two places and this will constrain the eventual type of resolution by Drupal core to the info file. I suppose others may eventually have the discussion about the long term resolution which I had hoped would occur but I think this path will not preclude how that long term resolution unfoldsvi .

The format of the project file is short and simple. In Drupal 8 this looks like:

name: 'Google Analytics'
project: 'google_analytics'
type: module
core: '8.x'
test_dependencies:
- php:php

Both dependencies and test_dependencies can be specified although ust the later is shown above. Dependencies are specified in a project:module format. Versions can also appear just as in the info files. The project namespace is required in the project file. It is optional in the info file (assuming Drupal core ever decides to proceed with the patch). Currently the packaging script on drupal.org supplies project and core but I have included it here because I ran into trouble trying to find the project when code is checked out using Git and the other ways where the packaging script has not supplied this information. But there is nothing that I have implemented that currently enforces this requirement. The only thing I am using is dependencies and test_dependencies.

While working on this I discovered the problem with the google_analytics project's dependency on the php project. The php module originally was part of Drupal core and was removed and placed in its own separate project. Thus the problem with the php project dependency appeared to be a result of guessing the wrong project (Drupal core). Tracking this down it happens there is a rare confluence of circumstances that is improbable but ends up being the problem. When drupal.org was ported from Drupal 6 to Drupal 7, project dependency was ported before some of the other projects. When these projects were later ported they changed some of the data structures which broke project dependency. When drupal.org on Drupal 7 went live this was discovered and we had to scramble to fix project dependency. There were a couple of weeks where project dependency did not store information about releases that occurred while we tried to quickly get things working again. For development branch releases these generally get updated and so the missing information is fixed. However in this case the php project happened to be spun off and its only commits to the development branch occurred during this period and has not been updated since nor has there been a formal release. Effectively the php project is not being maintained. It is rare for another project to use an unmaintained project as a dependency. The fix for this is to just kick off the process release for project dependency using

drush pdpv php 8.x-1.x-dev

The file cannot named ".project". This colide with .project files in Eclipse IDE.

Alexpott finally committed #2205271: Project namespace for dependencies. :-)))

I updated the patch. Payment has been updated so it is a good test case but it does not look right yet so a bit more debugging to make sure everything is working correctly.

1. +++ b/project_dependency.drupal.inc
   @@ -375,6 +386,33 @@ function project_dependency_info_batch_process_release(array $release) {
   +          $parsed_dependency = project_dependency_parse_dependency($dependency,
   +            $api_term->name);
   +          if (!isset($parsed_dependency['project'])) {
   

   We do not add line breaks here...

2. +++ b/project_dependency.drupal.inc
   @@ -375,6 +386,33 @@ function project_dependency_info_batch_process_release(array $release) {
   +              $project_name = project_dependency_field_value($project,
   +                'field_project_machine_name');
   +              $info[$component][$dependency_type][$key] = $project_name . ':' .
   +                $dependency;
   +            }
   

   Same

3. +++ b/project_dependency.drupal.inc
   @@ -520,10 +558,30 @@ function project_dependency_get_external_component_dependencies($release_node,
   +         $project_name = project_dependency_field_value($project,
   +           'field_project_machine_name');
   +         $external_dependencies[$key]->dependency = $project_name . ':' .
   +           $dependency->dependency;
   +       }
   

   Wrong line breaks

4. +++ b/project_dependency.drupal.inc
   @@ -602,3 +660,95 @@ function project_dependency_find_external_dependencies($release_nid, $component_
   +  $pid = project_dependency_field_value($release, 'field_release_project',
   

   line break issue

As I know the Payment issue is #2410151: _system_rebuild_module_data_ensure_required does not parse dependencies

Updated patch that fixes line breaks and issue with versions (3.x-dev versus 3.x). This should be ready. I am thinking about a beta release in the next day or so which will need to be deployed on drupal.org.

1. +++ b/project_dependency.drupal.inc
   @@ -109,16 +116,20 @@ function project_dependency_select_release($parsed_component, $api_tid) {
   +    $sql = 'SELECT DISTINCT release_nid FROM {project_dependency_component}
   +      WHERE name = :cname';
   +    $project_release_nids = db_query($sql, array(':cname' => $component))
   

   Double quotes should be used for SQL, no line breaks.

2. +++ b/project_dependency.drupal.inc
   @@ -375,6 +389,31 @@ function project_dependency_info_batch_process_release(array $release) {
   +            $project = project_dependency_guess_project($parsed_dependency,
   

   no line break

3. +++ b/project_dependency.drupal.inc
   @@ -375,6 +389,31 @@ function project_dependency_info_batch_process_release(array $release) {
   +              $info[$component][$dependency_type][$key] = $project_name . ':' .
   

   no line break

4. +++ b/project_dependency.drupal.inc
   @@ -520,10 +558,29 @@ function project_dependency_get_external_component_dependencies($release_node,
   +         $external_dependencies[$key]->dependency = $project_name . ':' .
   

   no line break

I try to keep most lines to 80 characters. This follows Drupal coding standards. Some people use editors that do not handle more than 80 characters very well. Git also does not handle more than 80 characters very well. For example if I do a diff on two commits I get

diff --git a/project_dependency.drupal.inc b/project_dependency.drupal.inc
index e02a6a4..4b35bf3 100644
--- a/project_dependency.drupal.inc
+++ b/project_dependency.drupal.inc
@@ -77,13 +77,10 @@ function project_dependency_info_parse(array $info_files, $t
 }
 ...

The lines are all truncated if they are over 80 characters and characters are added to the beginning of the line. If the changes are after the 80 characters they do not appear and this can result in mistakes if I am quickly diff'ing commits. What if the line was SQL and the truncated part contained '...; DELETE DATABASE drupal'?

There is also a readability issue with going over 80 characters. If my editor can expand then I may have something at the right side of the window and the next line is so far away that the eye has cannot see both at the same time which makes it harder to spot patterns. For example a variable may have one spelling in one place but a different spelling elsewhere. If these are not visible to the eye at the same time then there is a much higher probability of missing the error. So even if the editor can handle more than 80 characters the human eye has its own limitations.

I always try to use single quotes instead of double quotes whenever possible. Double quotes allow the use of PHP variables within the string. So in theory single quotes should execute marginally quicker than double quotes. It is also important to use single quotes to signal to the reader that this is a string. If they are tracing a variable through the code then this tells them they can safely ignore what is in the string. This is an example of improving readability through signaling intention. But it also has a security implication for SQL. If double quotes are used then someone can put in a variable instead of escaping it in a parameter list. Someone could concatenate single quoted strings and variables to avoid using a parameter list but this is a bit more obvious then a '$'.

I always split SQL that wraps more than a few lines. It is much more readable and avoids problems as noted above. If I was going to spend time making changes it would make more sense to convert these to dynamic queries.

The code in Project Dependency is a mess. It is what I call "stream of consciousness" coding where there is no effort to organize code so that it was easy to locate all the code that is acting on something. This is a major reason why there are very few people that understand this code and why it is hard to debug. This caused problems when drupal.org was ported from Drupal 6 to Drupal 7. We have tried to clean this up when we make changes but we try to limit the changes because it could quickly evolve into a complete rewrite.

We somehow "lost" the currency dependency, which was a dependency of payment.module today, it still worked this morning (CET), now the testbot no longer downloads that module.

Could this have anything to do with the core issue that was commit or something else that is going on here?

See #2410103: Update monitoring due to config object immutability and routing changes., see also the last branch test, that still had currency: https://qa.drupal.org/pifr/test/742098

Sorry for the interruption if that's a different issue.

@Berdir: The dependency used to just be currency but now it is currency:currency (3.x). Using just currency still should work. The patch for Drupal core for the project namespace has been committed. This issue has the corresponding patch to Project Dependency so the testbots can use the project namespace. This morning I found a bug in the patch where for dependencies on a development branch was failing to strip the '-dev'. The latest patch has this fix. My plan is to allow time for suggestions today and then, if everything is okay, commit the patch and create a beta release. Then tomorrow find someone to deploy the new release on drupal.org.

I do not like to be nitpicking about code style, but we have a rule that SQL should be double quoted always. I also have never seen that a function call with parameters will be broken in several lines in core. The current code is difficult to read.

We make line breaks at 80 chars for documentation and for arrays and such things. We also never make line breaks inside t(). Editors need to wrap the lines in these cases.

I also do like breaking SQL in several lines, but than it looks readable and extendable like

SELECT DISTINCT
  release_nid
FROM {project_dependency_component}
WHERE name = :cname

Every new columns make the commit very small than.

SELECT DISTINCT
  release_nid,
  release_foo,
  release_bar
FROM {project_dependency_component}
WHERE name = :cname

It's your code, just some suggestions...

Actually, it is not my code. I inherited it and while I usually check code with coder and coder tough love I am terrified about trying that for this code as there would have to be a lot of changes and I probably would end up breaking stuff. I always try to use single quotes when possible and neither coder nor coder tough love have flagged it. I have slowly tried to clean things up with help from others such as yourself. So I appreciate the suggestions.

Since this is now in the new beta release I am marking this issue as fixed (at least for now).

coder tough love is bad module. You should not use it. It implements non core standards. I already complained about this and later it was disabled on qa.d.o reviews for this reasons.

Coder is much better. Sometimes it has some false warnings, but in general it is fine. I use "minor" mostly to use the hardcore nitpicking review mode. Some things cannot found with coder... :-)

Thanks for the information on coder tough love. Yes, I use the minor setting on coder. Yes, some things cannot found with coder. But there are Drupal coding standards where there is a definite consensus and "coding standards" where consensus is not so much. The important thing is to try to all work towards the same style so that it is easier for people to pick up any code and be able to read it. Project Dependency is not there yet but we are trying to move in that direction.

I am tagging this as 'needs drupal.org deployment' per @drumm and they are working on getting it on drupal.org.

Deployed on Drupal.org and rebuild dependencies for php 8.x-1.x.