webform_patched being downloaded instead of webform for course module

Testbots work with release nids ... if we're loading the wrong project, its because we were passed the wrong nid.

Moving to project dependency for investigation.

We just need to update the project_dependency_excluded_dependencies variable adding webform_patched to it. https://drupal.org/comment/5518844#comment-5518844 has several examples of how to do this.

php -r "print json_encode(array('drupal', 'simpletest', 'leftandright', 'category', 'b2b_store_solution', 'onepagecv', 'testbot_exercise', 'opendeals', 'corporative_site', 'rulesmonkey', 'starterkit_touchpro', 'uberpaltw', 'contextual', 'translation', 'contact', 'file', 'webform_patched'));" | drush vset -y --format=json project_dependency_excluded_dependencies -

Unfortunately, now webform is not being picked up as a dependency at all.

@trobey, would you be willing to take a look at why this might be?

+ /var/www/drupal.org/tools/drush5/drush -v -r /var/www/drupal.org/htdocs pdsd course 7.x-1.x-dev
Initialized Drupal 7.26 root directory at /var/www/drupal.org/htdocs    [notice]
Initialized Drupal site default at sites/default                        [notice]
Array
(
    [1353266] => Array
        (
            [uri] => acl
            [version] => 7.x-1.0
            [tag] => 7.x-1.0
            [dependency_type] => 0
        )

    [1970468] => Array
        (
            [uri] => content_access
            [version] => 7.x-1.2-beta2
            [tag] => 7.x-1.2-beta2
            [dependency_type] => 0
        )

    [2194551] => Array
        (
            [uri] => ctools
            [version] => 7.x-1.4
            [tag] => 7.x-1.4
            [dependency_type] => 0
        )

    [2169589] => Array
        (
            [uri] => entity
            [version] => 7.x-1.3
            [tag] => 7.x-1.3
            [dependency_type] => 0
        )

    [2040551] => Array
        (
            [uri] => uuid
            [version] => 7.x-1.0-alpha5
            [tag] => 7.x-1.0-alpha5
            [dependency_type] => 0
        )

    [1965242] => Array
        (
            [uri] => views
            [version] => 7.x-3.7
            [tag] => 7.x-3.7
            [dependency_type] => 0
        )

    [2161705] => Array
        (
            [uri] => views_bulk_operations
            [version] => 7.x-3.2
            [tag] => 7.x-3.2
            [dependency_type] => 0
        )

    [2123171] => Array
        (
            [uri] => rules
            [version] => 7.x-2.6
            [tag] => 7.x-2.6
            [dependency_type] => 0
        )

    [2123723] => Array
        (
            [uri] => quiz
            [version] => 7.x-4.0-beta2
            [tag] => 7.x-4.0-beta2
            [dependency_type] => 0
        )

    [2158567] => Array
        (
            [uri] => ubercart
            [version] => 7.x-3.6
            [tag] => 7.x-3.6
            [dependency_type] => 0
        )

    [1972966] => Array
        (
            [uri] => certificate
            [version] => 7.x-2.0-beta4
            [tag] => 7.x-2.0-beta4
            [dependency_type] => 0
        )

    [1773836] => Array
        (
            [uri] => print
            [version] => 7.x-1.2
            [tag] => 7.x-1.2
            [dependency_type] => 0
        )

    [1110728] => Array
        (
            [uri] => signup
            [version] => 7.x-1.x-dev
            [tag] => 7.x-1.x
            [dependency_type] => 0
        )

)

rfay updated the project_dependency_excluded_dependencies variable. This variable removes dependencies from the dependencies list such as a dependency on a module in the Drupal core. This means no webform module will be selected. What would be needed is a variable that removes release nodes from consideration. But I do not think this is the best approach since it would require constant updating and then someone will have a dependency on the webform_patched project after it has been excluded from selection.

Most people conflate Drupal modules and Drupal projects. But they are very different. Drupal projects have unique names. Different Drupal modules with the same name can exist in multiple projects. The function drupal_parse_dependency() reads the module info file and passes the module dependencies to Project Dependency along with version requirements. Project Dependency also has information about the Drupal version (e.g. Drupal 7). This information is ambiguous. Consider the line from the info file:

dependencies[] = webform

While most people would say this is a dependency on the webform project it is really a dependency on the webform module. This could be interpreted to be either a dependency on the webform project or the webform_patched project. So which one should be chosen? Project Dependency starts with recommended releases, then suggested releases and finally any release. Within these it selects the highest major revision followed by the most recent release. About the same time as this issue was posted the webform project issued a new release. Apparently that release was not yet there so the webform_patched project was chosen. I would expect that if the dependency was run now that the webform project would be chosen.

Project Dependency is not the only one with this problem. The modules page also computes dependencies although it only has to consider the installed projects where Project Dependency considers all projects.

If Project Dependency is going to work in these types of situations then the only sure solution is to remove the ambiguity. This would require something like:

dependencies[] = webform:webform

or, another example

dependencies[] = ubercart:uc_order

In effect this would be using the project name to namespace the module name. Since project names are unique this would remove the ambiguity. drupal_parse_dependency() would need to be updated to return the project name but I expect this only would require a few lines of code. The project name could be optional although if it was required it would simplify Project Dependency. I do not think that any other code in Drupal core would need to be updated (at least immediately) although I would have to update Project Dependency to use the additional information.

With the ambiguity there is the possibility for a new project with a module with an identical name as a popular module to cause widespread testing problems. Even if I add a hack to avoid certain projects this would only allow the problem to be addressed after the fact.

I like the modulename (projectname:RELEASE) approach, as it could be done as a backwards compatible change ... i.e. no need to go adjust the thousands of info files which are already out in the wild. I would see the 'RELEASE' part of that being optional.

projectname:RELEASE would be a major change to drupal core (and affecting all contribs), so I would think that would be something that could not be accomplished until Drupal 9.

The problem here is that Drupal core has no idea at all of packages. Back in the day, one package typically had one module in it, and therefore we have the creeping ambiguity we have now.

It's not at all unreasonable for us to do what we've done in the past, and just blacklist packages that overlap namespace. Essentially, that's our only choice: We have to reject all projects that have duplicate modules in them, and accept only the most prominent one, and say "sorry".

@djdevin, sorry, the discussion about a new info file format was speculative. It wasn't meant to imply you should try it. Everything there is to be known about dependencies[] is in the handbook page on module info files, and we won't be changing it.

Since the project name is optional this change would not affect contrib. It simply adds additional information, when available, to the parsed component structure that is returned from drupal_parse_dependency().

Yes, Drupal core has no idea about packages and that is a problem. It means that Project Dependency has to go through a very convoluted process of starting from a module, then finding a release and only then is the project found. And since there is ambiguity this process is not going to work for every case.

Currently we have no way to blacklist packages. I will have to write that code. And an administrative interface. And it looks like rfay has volunteered to constantly update the blacklist. And then we will get the case where the blacklist does not work because someone wants to refer to a package that has been blacklisted.

This change is going to have to be made. It may not be today and it may not be tomorrow or even next year but we are going to have increasing problems until it is addressed. This is a major barrier to increasing projects (and the inevitable namespace conflicts) and the enhanced reliability from automated testing.

With the advent of semantic versioning it is no longer valid to wait until Drupal 9. In fact, drupal_parse_dependency() will have to be rewritten to accommodate semantic versioning and it might just be the right opportunity to also make this change. Note that this change is trivial compared to semantic versioning and will solve a structural issue regarding packages and modules that needs to be addressed.

I guess, spin off a core issue, but it's not the same as solving this generic problem. I agree that it's a necessary problem to solve.

I spun off a core issue #2205271: Project namespace for dependencies for a permanent fix. I think I can get a hack put together that will fix this particular case but it will make the already convoluted code even worse and make it harder to find anyone else that can understand it (and perhaps myself after a while).

A comment in the code:

 * Sadly, the component may appear in more than one product, since there's no
 * guaranteed uniqueness of a submodule name. So we're just preferring the
 * most recent release node (highest nid).

I have modified the code to first restrict the possible releases to those where the project name has the same name as the requested module name. If nothing is found then the possible releases is expanded to all releases with the requested module name. This will not always work but it makes it harder to create new projects that cause problems.

A patch is attached with this change.

I still don't quite understand why we can't use the same technique used earlier with project_dependency_excluded_dependencies , where the project is simply blacklisted and not added to the database. Is that technique now MIA?

There will eventually be a case that will require a blacklist and it was the first thing that I considered in fixing this issue. A drawback of the blacklist is that it requires someone after the fact to add a module to the blacklist. A blacklist is a reactive strategy and places an undue burden on people maintaining the testbots.

The traditional style for a project is to have a module with the same name as the project. The webform project follows this style. In this case there is a proactive strategy that a project name, which is unique, should also confer a presumptive preference to the identical module name which may not be unique. This is similar to the strategy of prefixing function names with the module name to prevent function name collisions. Neither is guaranteed to work and name spaces (PSR-0) is being introduced for a rigorous approach to preventing name space collisions for functions. Similarly we will eventually need name spacing using the project name to uniquely specify the module.

Interesting approach. It does fail a couple places I'm familiar with. The https://drupal.org/project/google_analytics modules has traditionally had its primary module as googleanalytics. But who would fork that.

I think it's a worthy heuristic. Since the blacklist approach was already there, I was just biased toward it. And it did solve the earlier problems.

Let's go with the heuristic and see how we do.

I usually wait a couple of weeks to give people a chance to review but in this case it seems pointless. It is unlikely anyone else will be able to figure out the code or be able to test it. I have tested it with the release in this issue and it selects the webform project. The changes are mostly rearranging code with just a few additional lines so the risk of a problem is small. So I just committed the change to 7.x-1.x-dev. This will not solve all possible issues but an issue has been filed with Drupal core for that.

Thanks - Let me know when you want to deploy. If you want to deploy the dev, that's OK for this module.

Go ahead and deploy the dev.

Deployed. I imagine there's followon work?

I rebuilt dependencies for webform + course. YAY that webform gets loaded. I see lots of complaints about `Undefined variable: parse_component project_dependency.drupal.inc:103 [notice]` though.

+ /var/www/drupal.org/tools/drush5/drush -v -r /var/www/drupal.org/htdocs pdsd course 7.x-1.x-dev
Initialized Drupal 7.26 root directory at /var/www/drupal.org/htdocs    [notice]
Initialized Drupal site default at sites/default                        [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:118   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:118   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:118   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:118   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:118   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:118   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:118   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:118   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:118   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:118   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:118   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:118   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Undefined variable: parse_component project_dependency.drupal.inc:103   [notice]
Array
(
    [1353266] => Array
        (
            [uri] => acl
            [version] => 7.x-1.0
            [tag] => 7.x-1.0
            [dependency_type] => 0
        )

    [1970468] => Array
        (
            [uri] => content_access
            [version] => 7.x-1.2-beta2
            [tag] => 7.x-1.2-beta2
            [dependency_type] => 0
        )

    [1960424] => Array
        (
            [uri] => ctools
            [version] => 7.x-1.3
            [tag] => 7.x-1.3
            [dependency_type] => 0
        )

    [2040551] => Array
        (
            [uri] => uuid
            [version] => 7.x-1.0-alpha5
            [tag] => 7.x-1.0-alpha5
            [dependency_type] => 0
        )

    [1965242] => Array
        (
            [uri] => views
            [version] => 7.x-3.7
            [tag] => 7.x-3.7
            [dependency_type] => 0
        )

    [1856704] => Array
        (
            [uri] => views_bulk_operations
            [version] => 7.x-3.1
            [tag] => 7.x-3.1
            [dependency_type] => 0
        )

    [2065197] => Array
        (
            [uri] => entity
            [version] => 7.x-1.2
            [tag] => 7.x-1.2
            [dependency_type] => 0
        )

    [2169589] => Array
        (
            [uri] => entity
            [version] => 7.x-1.3
            [tag] => 7.x-1.3
            [dependency_type] => 0
        )

    [2123171] => Array
        (
            [uri] => rules
            [version] => 7.x-2.6
            [tag] => 7.x-2.6
            [dependency_type] => 0
        )

    [2123723] => Array
        (
            [uri] => quiz
            [version] => 7.x-4.0-beta2
            [tag] => 7.x-4.0-beta2
            [dependency_type] => 0
        )

    [2158567] => Array
        (
            [uri] => ubercart
            [version] => 7.x-3.6
            [tag] => 7.x-3.6
            [dependency_type] => 0
        )

    [2095877] => Array
        (
            [uri] => webform
            [version] => 7.x-4.0-beta1
            [tag] => 7.x-4.0-beta1
            [dependency_type] => 0
        )

    [1972966] => Array
        (
            [uri] => certificate
            [version] => 7.x-2.0-beta4
            [tag] => 7.x-2.0-beta4
            [dependency_type] => 0
        )

    [1773836] => Array
        (
            [uri] => print
            [version] => 7.x-1.2
            [tag] => 7.x-1.2
            [dependency_type] => 0
        )

    [1110728] => Array
        (
            [uri] => signup
            [version] => 7.x-1.x-dev
            [tag] => 7.x-1.x
            [dependency_type] => 0
        )

)

It looks like there's a critical flaw here - Entity module is being listed twice - 7.x-1.2 and 7.x.1.3, causing a failure.

I see no versioning of entity module in course's info files. Nothing in rules module either.

Moving this to critical and reopening.

A typo and I was not using the new data structure used after the port of drupal.org to Drupal 7. I missed it because the data I was following was okay. The fixed are in the dev branch. Here is the output:

drush pdsd course 7.x-1.x-dev
Array
(
    [1353266] => Array
        (
            [uri] => acl
            [version] => 7.x-1.0
            [tag] => 7.x-1.0
            [dependency_type] => 0
        )

    [1970468] => Array
        (
            [uri] => content_access
            [version] => 7.x-1.2-beta2
            [tag] => 7.x-1.2-beta2
            [dependency_type] => 0
        )

    [2194551] => Array
        (
            [uri] => ctools
            [version] => 7.x-1.4
            [tag] => 7.x-1.4
            [dependency_type] => 0
        )

    [2169589] => Array
        (
            [uri] => entity
            [version] => 7.x-1.3
            [tag] => 7.x-1.3
            [dependency_type] => 0
        )

    [2040551] => Array
        (
            [uri] => uuid
            [version] => 7.x-1.0-alpha5
            [tag] => 7.x-1.0-alpha5
            [dependency_type] => 0
        )

    [1965242] => Array
        (
            [uri] => views
            [version] => 7.x-3.7
            [tag] => 7.x-3.7
            [dependency_type] => 0
        )

    [2161705] => Array
        (
            [uri] => views_bulk_operations
            [version] => 7.x-3.2
            [tag] => 7.x-3.2
            [dependency_type] => 0
        )

    [2123171] => Array
        (
            [uri] => rules
            [version] => 7.x-2.6
            [tag] => 7.x-2.6
            [dependency_type] => 0
        )

    [2123723] => Array
        (
            [uri] => quiz
            [version] => 7.x-4.0-beta2
            [tag] => 7.x-4.0-beta2
            [dependency_type] => 0
        )

    [2158567] => Array
        (
            [uri] => ubercart
            [version] => 7.x-3.6
            [tag] => 7.x-3.6
            [dependency_type] => 0
        )

    [2194183] => Array
        (
            [uri] => webform
            [version] => 7.x-3.20
            [tag] => 7.x-3.20
            [dependency_type] => 0
        )

    [1972966] => Array
        (
            [uri] => certificate
            [version] => 7.x-2.0-beta4
            [tag] => 7.x-2.0-beta4
            [dependency_type] => 0
        )

    [1773836] => Array
        (
            [uri] => print
            [version] => 7.x-1.2
            [tag] => 7.x-1.2
            [dependency_type] => 0
        )

    [1110728] => Array
        (
            [uri] => signup
            [version] => 7.x-1.x-dev
            [tag] => 7.x-1.x
            [dependency_type] => 0
        )

)

Deployed. Your care for this module and your expertise with it are ever so much appreciated by all of us, @trobey.

The dependencies look right to me. AND YAY, IT PASSED.

I do still see Undefined variable: parse_component project_dependency.drupal.inc:121 in the drush pdpp.

Array
(
    [1353266] => Array
        (
            [uri] => acl
            [version] => 7.x-1.0
            [tag] => 7.x-1.0
            [dependency_type] => 0
        )

    [1970468] => Array
        (
            [uri] => content_access
            [version] => 7.x-1.2-beta2
            [tag] => 7.x-1.2-beta2
            [dependency_type] => 0
        )

    [2194551] => Array
        (
            [uri] => ctools
            [version] => 7.x-1.4
            [tag] => 7.x-1.4
            [dependency_type] => 0
        )

    [2169589] => Array
        (
            [uri] => entity
            [version] => 7.x-1.3
            [tag] => 7.x-1.3
            [dependency_type] => 0
        )

    [2040551] => Array
        (
            [uri] => uuid
            [version] => 7.x-1.0-alpha5
            [tag] => 7.x-1.0-alpha5
            [dependency_type] => 0
        )

    [1965242] => Array
        (
            [uri] => views
            [version] => 7.x-3.7
            [tag] => 7.x-3.7
            [dependency_type] => 0
        )

    [2161705] => Array
        (
            [uri] => views_bulk_operations
            [version] => 7.x-3.2
            [tag] => 7.x-3.2
            [dependency_type] => 0
        )

    [2123171] => Array
        (
            [uri] => rules
            [version] => 7.x-2.6
            [tag] => 7.x-2.6
            [dependency_type] => 0
        )

    [2123723] => Array
        (
            [uri] => quiz
            [version] => 7.x-4.0-beta2
            [tag] => 7.x-4.0-beta2
            [dependency_type] => 0
        )

    [2158567] => Array
        (
            [uri] => ubercart
            [version] => 7.x-3.6
            [tag] => 7.x-3.6
            [dependency_type] => 0
        )

    [2194183] => Array
        (
            [uri] => webform
            [version] => 7.x-3.20
            [tag] => 7.x-3.20
            [dependency_type] => 0
        )

    [1972966] => Array
        (
            [uri] => certificate
            [version] => 7.x-2.0-beta4
            [tag] => 7.x-2.0-beta4
            [dependency_type] => 0
        )

    [1773836] => Array
        (
            [uri] => print
            [version] => 7.x-1.2
            [tag] => 7.x-1.2
            [dependency_type] => 0
        )

    [1110728] => Array
        (
            [uri] => signup
            [version] => 7.x-1.x-dev
            [tag] => 7.x-1.x
            [dependency_type] => 0
        )

)

Fix typo and committed to the dev branch.

Couldn't we also leverage package usage as part of the heuristic?

I had not thought about it but project usage would be a good heuristic. I am not quite sure how to retrieve project usage but then I have not looked. But in the end these are all heuristics. There will be cases where they do not work. And each heuristic will add to the complexity of the code and make it harder to maintain.

Heuristics might be okay if the answer is not known. But the people maintaining projects know which projects they need; they just do not have a way to express that information in the module info file. If they could then I could throw away all the heuristics and the code would be simpler and more robust and faster.

Consider the case where there are three projects, Project A, Project B and Project C. Project A has a module of the same name, Module A. Project C also consist on a single module C. Project B has two modules, Module 1 and Module 2. Module A has a dependency on Module 1 and Module C. The current version of the Project B is 1.3 so I select that as a dependency. Module C has a dependency on Module 2 but this also is a versioned dependency requiring release 1.2. I have to figure out that Module 1 and 2 are part of the same project and when I selected release 1.3 in the first step then I have to change that. Since the different releases might have different dependencies I may have to change the dependencies I have already found. First, modules do not have releases; projects have releases. But I just have a module name and a release and from these I guess the project using heuristics. So any data structure will likely have the release version attached to the module which is going to make it very difficult to discover different requirements among modules of the same project. If I am given the project in the info file then I can attach version requirements to the project instead of the module. Now I have a chance at tackling this problem without the code getting so complex that it is impossible to follow.

Fortunately most practical cases of dependencies are much simpler. But the number of complex cases is growing. I have looked at some of the more complicated cases in the issue queue and some will require substantial work. I have even spent considerable time trying to tackle these cases. The data structures become very complicated. I have come to the conclusion that given the current complexity of the code that I will probably break stuff and create short term problems that may outweigh any benefit in fixing the few cases that do not work. So I either need to simplify the code or come up with a great test suite in order to be able to address these complex cases.