When browsing drupal's current list of add-on modules, the list is extremely lengthy, and hard to browse unless you know what you're looking for. I'd like to suggest that project nodes have dependencies. For example, for the cck module, there are a lot of sub-modules (email field, url field, etc), it would be good if we could get those out of the current list of modules, and on the cck project page as a list, because they do the user no good without the cck module.

Of course, there are some projects that are only useful as dependancies, and the other projects should be listed on the Modules page anyways.

Maybe projects could either be marked as a "library" for other projects, or a subproject of a project, instead of dependant... I'm just throwing some ideas down.

Files: 
CommentFileSizeAuthor
#152 projectinfolog.txt13.25 KBOwen Barton
#129 project.project_release_dependencies_102102_129.patch63.61 KBrfay
FAILED: [[SimpleTest]]: [MySQL] 328 pass(es), 112 fail(s), and 49 exception(es).
[ View ]
#123 project_info.CVS_to_git.patch16.06 KBrfay
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch project_info.CVS_to_git.patch.
[ View ]
#116 102102-project-info_26.patch51.71 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 349 pass(es), 117 fail(s), and 70 exception(es).
[ View ]
#111 102102-project-info.patch52.44 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_25.patch.
[ View ]
#107 102102-project-info.patch50.05 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] 307 pass(es), 127 fail(s), and 62 exception(es).
[ View ]
#104 102102-project-info.patch43.93 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.
[ View ]
#96 102102-project-info.patch43.86 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.
[ View ]
#94 102102-project-info.patch84.15 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_21.patch.
[ View ]
#87 project-HEAD.info.87.patch46.02 KBsun
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.
[ View ]
#79 project-HEAD.info.79.patch42.69 KBsun
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch project-HEAD.info.79.patch.
[ View ]
#78 project-HEAD.info.78.patch42.9 KBsun
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch project-HEAD.info.78.patch.
[ View ]
#77 project-HEAD.info.76.patch42.81 KBsun
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch project-HEAD.info.76.patch.
[ View ]
#75 project-HEAD.info.75.patch43.15 KBsun
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch project-HEAD.info.75.patch.
[ View ]
#73 project_info_bused_array_fix.patch892 byteshunmonk
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch project_info_bused_array_fix.patch.
[ View ]
#70 102102-project-info.patch42.94 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.
[ View ]
#69 102102-project-info.patch35.43 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.
[ View ]
#68 102102-project-info.patch32.2 KBboombatower
PASSED: [[SimpleTest]]: [MySQL] 202 pass(es).
[ View ]
#64 102102-project-info.patch25.29 KBboombatower
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#57 102102-project-info.patch25.18 KBboombatower
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#56 102102-project-info.patch23.28 KBboombatower
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#54 102102-pift.patch3.57 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-pift_4.patch.
[ View ]
#50 102102-pift.patch3.45 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-pift_3.patch.
[ View ]
#49 102102-pift.patch3.47 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-pift_2.patch.
[ View ]
#48 102102-project-info.patch23.17 KBboombatower
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#45 102102-project-info.patch22.94 KBboombatower
PASSED: [[SimpleTest]]: [MySQL] 202 pass(es).
[ View ]
#45 102102-pift.patch1.75 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-pift_1.patch.
[ View ]
#44 102102-project-info.patch22.87 KBboombatower
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#44 102102-pift.patch1.65 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-pift_0.patch.
[ View ]
#43 102102-project-info.patch22.7 KBboombatower
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#42 102102-project-info.patch38.55 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_10.patch.
[ View ]
#31 102102-pift.patch1.47 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-pift.patch.
[ View ]
#29 102102-project-info.patch38.55 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_9.patch.
[ View ]
#28 102102-project-info.patch46.51 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_8.patch.
[ View ]
#27 102102-project-info.patch36.74 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_7.patch.
[ View ]
#26 102102-project-info.patch39.47 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_6.patch.
[ View ]
#25 102102-project-info.patch35.56 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_5.patch.
[ View ]
#23 102102-project-info.patch29.77 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_4.patch.
[ View ]
#21 102102-project-info.patch24.86 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_3.patch.
[ View ]
#20 102102-project-info.patch23.83 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_2.patch.
[ View ]
#18 102102-project-info.patch9.39 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_1.patch.
[ View ]
#17 102102-project-info.patch6.25 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_0.patch.
[ View ]
#15 102102-project-info.patch5.24 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info.patch.
[ View ]
#10 project_info.tar_.gz4.6 KBnedjo
#10 project_info.diff1.58 KBnedjo
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch project_info.diff.
[ View ]

Comments

dww’s picture

FYI: you can already browse projects by category (which is now the default at http://drupal.org/project/Modules, in fact). when you do, there's a whole category for CCK and related modules: http://drupal.org/project/Modules/category/88. similarly, there are categories for Views (http://drupal.org/project/Modules/category/89) and Organic Groups (http://drupal.org/project/Modules/category/90) 3 of the biggest sets of modules that depend on each other in the way you describe...

also, many projects that currently depend on others are fairly good about documenting this and adding links in the project descriptions themselves.

however, it's true that a more formalized dependency system would be nice, if nothing else to standardize this info and visually format it consistantly. we've already got this in the .info files in 5.x core, in fact, so it makes some sense to do that to project nodes, too.

however, i'd be slightly worried about people not keeping these 2 sources of information in sync (and having the project.module parse the .info files and use them directly seems a little scary). also, in the future, you'll be able to specific version-specific dependencies (e.g. og-5.x-1.0 depends on views 5.x-1.0 or later, but og-5.x-1.2 requires views 5.x-1.1 (or something)). to continue that example, og 4.7.x-* does not depend on views at all, but 5.x-* does. it's not obvious if/how we can represent that via project-level dependencies as you're suggesting.

i'm not marking as "won't fix", i'm just sharing my ideas (and concerns), too. clearly, this needs more thought if anything's going to come of it.

thanks,
-derek

bradlis7’s picture

I did expect that there would be a few concerns (and many of your concerns were thoughts of mine as well). I think that parsing the .info file would be an option, and a lot less difficult than having the maintainers keep up with it seperately.

I think that having the ability to mark a module as a "library" module would be a good course of action though, because it doesn't make sense to list a module on the Modules download page if the module doesn't really do anything... IMO anyways. (Maybe there should be a way to specify a "library" or API module in the .info file). One thing to note, though, is that some modules provide both functionality, and a library for other modules to use... maybe once there is a way to specify library modules, developers should be encouraged to split off library functionality other functionality (or maybe there should be a "library" project type on drupal.org).

I realize that there are categories right now, but even still, there are a lot of projects to wade through. I'd like for things to be a little less cluttered, but I realize that it's going to add a level of complexity, so I'm not really sure if it's worth it or not.

bradlis7’s picture

Title:Project Dependancies on Other Projects» Project Dependancies, Subproject, and Library Support

I think all of these would be useful to have. Dependancies can be taken from the .info file.

moshe weitzman’s picture

well, one of the main reasons I like out .ini format is that the info can there can be used safely by PHP code like project module. i think project should reuse most of the info in the .ini, to be honest. some disagree, which is fine. dependencies are a good place to start. not sure what the UI should look like. maybe just state the dependencies on the project page for now.

dww’s picture

if we put code on d.o to parse .info files from drupal modules, it needs to go into project_d_o_hacks.module, not project.module, or else we undo a year's worth of effort on my part to try to make project.module useful for other sites.

the potential for supporting project dependencies and relationships in project.module itself is a reasonable (if low priority) feature reuqest. however, hard-coding .info file logic is out of the question. we need a separate module for that.

catch’s picture

Version:x.y.z» 5.x-1.x-dev

See also: http://groups.drupal.org/node/11998

This would be a useful thing to have.

nedjo’s picture

Title:Project Dependancies, Subproject, and Library Support» New module to parse project .info files and present dependency information
Assigned:Unassigned» nedjo

Since dependencies are at the module rather than the project level, I think what we need is actually a project_info module that parses and stores from .info files:

* the association of modules and their titles and descriptions with projects (projects being identified by nid), tracked by version, and
* the dependencies of modules on each other, again tracked by version

From this, we could present e.g. in blocks that appear on a project's page:

1. The projects that this project depends on.

2. The projects that depend on this project.

3. The modules included in this project (with their titles and descriptions).

I'll have a go at sketching this in as a new module.

I'm changing the issue title to reflect this specific aim. A "library" categorization of modules looks to me like a separate feature request (possibly just a new vocabulary or term).

nedjo’s picture

Versioning information is going to be tricky.

In the first place, dependencies vary between core compatibility versions. module_x's Drupal 5 version will have different dependencies than its Drupal 6 version. So we can't say module_x has an overall set of dependencies. Rather, we need to distinguish between dependencies by version.

But dependencies may also vary within a core compatibility version. E.g., a core 6.x version of a given module may exist in multiple releases. Each of these may have different dependencies.

And we know version information only on one side of this dependency. That is, we know that the 6.2 stable release of module_x is dependent on module_y, but we don't know what version of module_y is required (6.1, 6.2, etc.).

Possibly we need to track dependencies only by Drupal core compatibility--e.g., assign module_x a single set of dependencies for Drupal 6.

It's not going to be straightforward deciding how to display this information. But first things first--we need to capture it before displaying.

dww’s picture

@nedjo: It's great you're going to be working on this. If you haven't yet, please read http://groups.drupal.org/node/11998#comment-39642. Thanks!

nedjo’s picture

StatusFileSize
new1.58 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch project_info.diff.
[ View ]
new4.6 KB

Here's a first very rough draft, completely untested.

Some quick notes:

* Creates a new module, project_info, which is in an info directory in project.

* We capture information on modules and their dependencies by calling a processing function from the packaging script.

* Modules are stored in a new table, project_info, with their associated release node ID and the various data in the .info file (name, description, etc.). Looking again at this table, it may be too normalized--it may be better to store e.g. the core compatibility tid and the project node nid here too, to reduce the need to join on other tables.

* Modules themselves have a release associated with them, but for dependencies all we have is a module name. So we store dependencies as a relationship between a module ID (the ID field of the project_info table) and the dependency's module name.

A lot still to do. I'll pick this up later in the week. Comments meantime or suggestions for how I could test would be great.

drewish’s picture

Version:5.x-1.x-dev» 6.x-1.x-dev

subscribing, i guess we probably want to bump this forward to 6.x right?

Owen Barton’s picture

Subscribe

adrian’s picture

I've been working on this stuff recently, but from the perspective of an outside system.

http://groups.drupal.org/node/21295

Basically I am building a directory structure of easily mirror-able meta-information, that can be parsed on the client side.
This is specifically with the goal of writing something like apt-get for Drupal.

I'm already working on automating these dependencies in my code, but I'd very happily support this getting into the generated info files itself (I already use the values from the .info files where available)

Something to keep in mind, is that modules provided by a package might change between releases, so this shouldn't be tied to the project, but to the releases (I know it makes it more complex, but that's how it works)

Same case for dependencies. New dependencies might be added or removed.

boombatower’s picture

Assigned:nedjo» boombatower

I'm going to be working on this since I need it for project_issue_file_test (PIFT).

boombatower’s picture

Status:Active» Needs review
StatusFileSize
new5.24 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info.patch.
[ View ]

Initial patch to confirm direction.

boombatower’s picture

Title:New module to parse project .info files and present dependency information» Parse project .info files: present module list and dependency information
boombatower’s picture

StatusFileSize
new6.25 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_0.patch.
[ View ]

This should store the list of modules.

boombatower’s picture

StatusFileSize
new9.39 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_1.patch.
[ View ]

This should store dependencies to the highest dev release in the same drupal core API version...later we can add support for the advanced style .info files supported by D7.

I'll add some sort of views support for this.

boombatower’s picture

Module list view done, got to head out...I'll get it and dependency view done before posted.

boombatower’s picture

StatusFileSize
new23.83 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_2.patch.
[ View ]

With views!

boombatower’s picture

StatusFileSize
new24.86 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_3.patch.
[ View ]

Added API.

Now I will attempt to integrate this with pifr.

boombatower’s picture

PIFT integration was simple:

Index: pift.cron.inc
===================================================================
RCS file: /cvs/drupal-contrib/contributions/modules/project_issue_file_test/pift.cron.inc,v
retrieving revision 1.31
diff -u -r1.31 pift.cron.inc
--- pift.cron.inc 23 Oct 2009 22:32:12 -0000 1.31
+++ pift.cron.inc 26 Oct 2009 19:04:05 -0000
@@ -278,8 +278,8 @@
     else {
       // Load the Drupal core API release (branch) compatible with this branch.
       $api_release = node_load(pift_core_api_release($api_tid));
-      $item['dependency'] = $api_release;
-      $item['plugin_argument']['modules'] = array(); // TODO Include the list of modules.
+      $item['dependency'] = implode(',', array_merge(array($api_release), project_info_dependency_list_get($branch->nid)));
+      $item['plugin_argument']['modules'] = project_info_module_list_get($branch->nid);

       // If the core branch has not already been added then add it.
       if (!isset($branches[$api_release])) {

I'll work on deploying on d6.d.o and testing the .info file parsing on release build, and I suppose I need to figure out some sort of script to run on all projects as we talked about.

boombatower’s picture

StatusFileSize
new29.77 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_4.patch.
[ View ]

The view will need to be tailored to a particular release for a project since the current argument is just pid, but that is minor since we don't need to display them...we really need the parsing for pift.

This patch is update to a new schema that keeps track of the specific "module" that is depended on in addition to the release.

boombatower’s picture

After quick chat with dww:

Plan: copy package-release-nodes.php as parse-legacy-info.php and hack it to run in tmp and not touch db.

Preferably come up with hack to temporarily store dependencies and parse in second round without having to re-checkout everything.

boombatower’s picture

StatusFileSize
new35.56 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_5.patch.
[ View ]

More up-to-date, still needs work to finish #24.

boombatower’s picture

StatusFileSize
new39.47 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_6.patch.
[ View ]

Seems to work well, I'll wait for d6.d.o to come back up for final testing.

boombatower’s picture

StatusFileSize
new36.74 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_7.patch.
[ View ]

Cleaned version.

boombatower’s picture

StatusFileSize
new46.51 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_8.patch.
[ View ]

Through testing on d6.d.o fixed this so far, plus requires #424372: :: in .info files causes Fatal error.

boombatower’s picture

StatusFileSize
new38.55 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_9.patch.
[ View ]

Seems to be working well.

boombatower’s picture

Latest patch against PIFT.

Index: pift.cron.inc
===================================================================
RCS file: /cvs/drupal-contrib/contributions/modules/project_issue_file_test/pift.cron.inc,v
retrieving revision 1.37
diff -u -r1.37 pift.cron.inc
--- pift.cron.inc 3 Nov 2009 23:42:28 -0000 1.37
+++ pift.cron.inc 12 Nov 2009 03:41:36 -0000
@@ -278,8 +278,8 @@
     else {
       // Load the Drupal core API release (branch) compatible with this branch.
       $api_release = node_load(pift_core_api_release($api_tid));
-      $item['dependency'] = $api_release->nid;
-      $item['plugin_argument']['modules'] = array(); // TODO Include the list of modules.
+      $item['dependency'] = implode(',', array_merge(array($api_release->nid), project_info_dependency_list_get($branch->nid)));
+      $item['plugin_argument']['modules'] = project_info_module_list_get($branch->nid);

       // If the core branch has not already been added then add it.
       if (!isset($branches[$api_release])) {
Index: pift.info
===================================================================
RCS file: /cvs/drupal-contrib/contributions/modules/project_issue_file_test/pift.info,v
retrieving revision 1.5
diff -u -r1.5 pift.info
--- pift.info 14 Apr 2009 02:29:50 -0000 1.5
+++ pift.info 12 Nov 2009 03:41:36 -0000
@@ -10,6 +10,7 @@
files[] = pift.project.inc
files[] = pift.test.inc
dependencies[] = project
+dependencies[] = project_info
dependencies[] = project_issue
dependencies[] = project_release
dependencies[] = cvs

boombatower’s picture

StatusFileSize
new1.47 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-pift.patch.
[ View ]

Ass patch file.

boombatower’s picture

This is ready to go, works great. @see http://d6.drupal.org/node/350371

nedjo’s picture

Wow! It's great to see this longstanding need close to completed. Thanks the great work.

I browsed through d6.drupal.org and found what look to be a couple of remaining issues.

1. Some releases show the release's module itself as a dependency. E.g. Views http://d6.drupal.org/node/621428. This is probably happening because the release includes modules that depend on each other, e.g., views_ui on views.

2. Some dependencies are missing. E.g. CCK fieldgroup tabs, http://d6.drupal.org/node/553732. Correctly shows tabs as a dependency but misses fieldgroup. At first glance it looks like we're only getting dependencies shown where the module name matches the project shortname.

How hard would it be to capture and display the human readable module names, and also use the project node titles? Doing so would make the blocks more user friendly.

It might also be useful to capture and offer the .info description to offer a sense of what each included module does. Of course this could be done in a follow up patch.

In many cases a project will include multiple modules, some of them less needed or even fairly obscure. In these cases, listing the combined dependencies of all modules in a release could give the mistaken impression that all these other modules must be downloaded and installed before the release can be used.

Should we consider listing modules and dependencies in a single block? Maybe a horizontal one in the content area? It might look sort of like the admin/build/modules page table:

Includes the following modules

Name Description Dependencies
PIFR Server Manage queue of tests and list of clients. Project issue file review: PIFR; Views: Views; Tabs: Tabs
boombatower’s picture

Personally, I would rather hold of on the UI stuff. If we want to just hide the blocks that is fine. I need this functionality to do contrib testing (which we really want in place a soon as possible so ppl write tests as they port their modules).

I think your ideas make some sense, but I would rather get the architecture in place before tweaking the UI.

nedjo’s picture

Agreed that the UI can wait. But it's worth thinking about now to make sure the data structure is correct.

Currently the patch captures two types of data: modules (in 'project_info_module') and dependencies (in 'project_info_dependency'). The latter tracks the relationship between releases and the modules that are required by them.

This won't give us enough information to associate dependencies with a particular module. To do so, we would need 'project_info_dependency' instead to track a dependency relationship between two modules by project_info_module.module_id.

boombatower’s picture

For the specifically, I am not sure what that gets us. I mean we can display it, but I am not sure it is all that useful.

nedjo’s picture

Status:Needs review» Needs work
<?php
+function project_info_package_info_process_dependencies($rid, array $dependencies) {
+  foreach (
$dependencies as $dependency) {
+    if (
$nid = project_get_nid_from_uri($dependency)) {
?>

This looks to be the source of the problem I noted above. We're only recognizing dependencies that have a module name that matches a project shortname, so dependencies will be saved only for modules that match their project's name.

Another more minor issue is that, at the time we're processing a given release, we don't yet have full data for other releases, so dependencies we determine may not be up to date.

Beyond this, though, dependency is fundamentally a relationship between two modules. Starting with my original patch (sorry!) [edit: on second glance, no, I was indeed tracking by module], in the proposed schema we're capturing not this original relationship but the more general relationship between a release and the modules that it requires. This release-module data may be useful in some cases, but not all, and in particular I'm realizing now that it doesn't give us the level of information we need to provide the most useful information on dependencies to site admins evaluating a release or a project--e.g., the table I suggested in #33.

Rather than tossing out the module-to-module dependency data and storing just the generalized project-to-release info, we should capture and store the actual dependency relationship--from which we can easily determine the release-module dependency any time we want.

Proposed schema:

<?php
/**
 * Implementation of hook_schema().
 */
function project_info_schema() {
 
$schema = array();

 
$schema['project_info_module'] = array(
   
'description' => 'The modules contained by a project release.',
   
'fields' => array(
     
'module_id' => array(
       
'description' => 'The unique ID of a module.',
       
'type' => 'serial',
       
'unsigned' => TRUE,
       
'not null' => TRUE,
      ),
     
'name' => array(
       
'description' => 'The machine readable name of a module.',
       
'type' => 'varchar',
       
'length' => 255,
       
'not null' => TRUE,
       
'default' => '',
      ),
     
'title' => array(
       
'description' => "The human readable name of a module as contained in the module's .info file.",
       
'type' => 'varchar',
       
'length' => 255,
       
'not null' => TRUE,
       
'default' => '',
      ),
     
'description' => array(
       
'description' => "A description of a module as contained in the module's .info file.",
       
'type' => 'text',
       
'size' => 'medium'
     
),
     
'rid' => array(
       
'description' => 'The {node}.nid of the project_release node that includes a module.',
       
'type' => 'int',
       
'unsigned' => TRUE,
       
'not null' => TRUE,
       
'default' => 0,
      ),
    ),
   
'primary key' => array('module_id'),
   
'indexes' => array(
     
'rid' => array('rid'),
     
'name' => array('name'),
    ),
  );

 
$schema['project_info_dependency'] = array(
   
'description' => 'The dependencies of a module.',
   
'fields' => array(
     
'module_id' => array(
       
'description' => 'ID of a module.',
       
'type' => 'int',
       
'not null' => TRUE,
       
'default' => 0,
      ),
     
'dependency_id' => array(
       
'description' => 'ID of a module that the module is dependent upon.',
       
'type' => 'int',
       
'not null' => TRUE,
       
'default' => 0,
      ),
    ),
   
'primary key' => array('module_id', 'dependency_id'),
  );

}
?>

We could determine the releases a given release depends on with a query something like the following:

<?php
$result
= db_query('SELECT DISTINCT(pim.rid) FROM project_info_module pim INNER JOIN project_info_dependency pid ON pim.module_id = pid.dependency_id INNER JOIN project_info_module pim2 ON pid.module_id = pim2.module_id AND pim2.rid = 3');
?>

(though we'd need an extra join or two plus where clauses to get the recommended stable release for the correct core version).

[edit: fixed error in schema--module name is not unique. And corrected reference to schema in #10.]

nedjo’s picture

Rough, untested function for saving dependency data per the schema in #37:

<?php
function project_info_package_info_process_dependencies($module_id, $rid, array $dependencies) {
  static
$api_vid;
  static
$api_tids = array();

  if (
$empty($api_vid)) {
   
$api_vid = _project_release_get_api_vid();
  }

  if (!isset(
$api_tids[$rid])) {
   
// Determine the release's core API version.
   
$api_tids[$rid] = db_result(db_query('SELECT td.tid FROM {term_node} tn INNER JOIN {term_data} td ON tn.tid = td.tid WHERE td.vid = %d AND tn.nid = %d', $api_vid, $rid));
  }

  foreach (
$dependencies as $dependency) {
    if (
$dependency_id = db_result(db_query("SELECT pim.module_id FROM project_info_module pim INNER JOIN {term_node} tn ON pim.rid = tn.nid INNER JOIN {term_data} td ON tn.tid = td.tid WHERE td.vid = %d AND tn.tid = %d AND pim.module_name = '%s'", $api_vid, $api_tids[$rid], $module_id, $dependency)) {
     
$info = array(
       
'module_id' => $module_id,
       
'dependency_id' => $dependency_id,
      );
     
drupal_write_record('project_info_dependency', $info);
    }
  }
}
?>
nedjo’s picture

To return to a key issue I noted in #8: modules indicate their dependencies on other modules, but we don't know the specific versions required.

So we have to either (a) make an inference or (b) add a way for modules to specify their dependencies' versions.

If we're making an inference, probably our best guess is that what's required is the current recommended stable release for the given core api version. But of course, this won't always be the case. E.g. http://drupal.org/project/apachesolr_views requires Views 3 (6.x-3).

The alternative is to introduce a way for modules to indicate the version they require. For e.g. a reliable testing system, I'm pretty sure we need this.

The most straightforward way would be an API change:

dependencies[] = 'views-6.x-3'

but it's obviously far too late for that for d6, and probably for d7 too.

But we could add an optional and supplemental:

version_dependencies[] = 'views-6.x-3'

and parse this along with the other data. That looks like the only way we'd get reliable dependency information.

boombatower’s picture

That's the thing...the code was intended as placeholder code until we implement the Drupal 7.x style .info files which allow versions to be specified. Then having the relationship to a release makes more sense, not to mention the entire point I am writing this feature, testing. Testing needs the releases as it need something to checkout that it can test against.

For 6.x I thing we should just make an inference and we can choose to add an interface or workaround if we want. The focus is on getting Drupal 7.x contrib testing working. Thus I would like to design this with that in mind instead of adding things to 6.x.

You are correct in #37 that it only loads dependencies where they match the module shortname. That was an oversight when writing the code, but the design works fine. All I need to do is a run a query to determine which project contains he module first and then load that project (as I thought about, just failed to code).

As for the display part I am not sure what we gain, and such. I think we can store more module information and such, but the key is we want this functionality NOW. So apparent from holding up the patch I would really like to move this forward, and deploy this weekend, if possible.

I guess what I am hitting at. There are ton of things to make this 100%, currently with the one issue fixed this should be a 90-95% solution. That gets us a start, then people can play with contrib testing (hopefully encourage them to write tests as they port) and we can work out the bugs in contrib testing while we worry about how to make this 100%. I mean there are a lot of things to consider, and it may be possible to integrate with the Drupal 7 code that already does this, but I'd like to spend more time discussing and figuring that out.

I'll take a look at the code samples and fix/merge stuff. I think your schema makes sense and provides us with more detail. I'm just concerned that we will continue and delay this too long.

boombatower’s picture

The code in #38 appears to be too simple for our needs. The query to determine which module_id the module depends on does nothing to pick a release other then that the release is in the same drupal core compatibility. Which means we are doing them with no respect to order (version) or any kind nor stable/dev.

I am working on changing the code to point to modules, but the code from #38 won't work. For now I will choose the dev release for the latest stable in the core compatibility tid. That will suite 90%+ of modules and we can use D7 .info format later to gain 100%.

boombatower’s picture

Status:Needs work» Needs review
StatusFileSize
new38.55 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_10.patch.
[ View ]

Views integration is not complete, but I believe I have workable code using module => module relationships. I need to run it on d6.drupal.org to see how it does, but its looking good.

I am not sure I about views integrations. I pulled that out of the patch so we can commit the structure part and then decide on displaying information.

This will also require a slight teak to pift patch.

boombatower’s picture

StatusFileSize
new22.7 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

I should actually role patch. :)

boombatower’s picture

StatusFileSize
new1.65 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-pift_0.patch.
[ View ]
new22.87 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Updated both patches.

boombatower’s picture

StatusFileSize
new1.75 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-pift_1.patch.
[ View ]
new22.94 KB
PASSED: [[SimpleTest]]: [MySQL] 202 pass(es).
[ View ]

Updated after some testing. Appears to work, I'll be running whole thing once more to double check.

boombatower’s picture

Latest patch looks for on project_info side. Ran entire thing on d6.d.o last night and today I check the results with:

<?php
print_r
(project_info_module_list_get(350371));
print_r(project_info_dependency_list_get(350371));
?>
Array
(
    [pifr_server] => Array
        (
            [module_id] => 16968
            [rid] => 350371
            [name] => pifr_server
            [title] => PIFR Server
            [description] => Manage queue of tests and list of clients.
        )

    [pifr_client] => Array
        (
            [module_id] => 16970
            [rid] => 350371
            [name] => pifr_client
            [title] => PIFR Client
            [description] => Request tests from server and performs actual testing.
        )

    [pifr_assertion] => Array
        (
            [module_id] => 16972
            [rid] => 350371
            [name] => pifr_assertion
            [title] => Assertion
            [description] => Provide generalize PIFR review assertion implementation.
        )

    [pifr_simpletest] => Array
        (
            [module_id] => 16974
            [rid] => 350371
            [name] => pifr_simpletest
            [title] => SimpleTest
            [description] => Provide PIFR review implementation for SimpleTest.
        )

    [pifr] => Array
        (
            [module_id] => 16976
            [rid] => 350371
            [name] => pifr
            [title] => PIFR
            [description] => Profiles base functionality for PIFR Client and PIFR Server.
        )

)
Array
(
    [0] => Array
        (
            [module_id] => 170
            [rid] => 95897
            [name] => views
            [title] => Views
            [description] => Create customized lists and queries from your database.
        )

    [1] => Array
        (
            [module_id] => 3380
            [rid] => 239151
            [name] => tabs
            [title] => Tabs
            [description] => A helper module for creating tabbed pages.
        )

    [2] => Array
        (
            [module_id] => 11172
            [rid] => 313695
            [name] => pifr
            [title] => Project Issue File Review
            [description] => Automatically review patches attached to issues.
        )

    [3] => Array
        (
            [module_id] => 35304
            [rid] => 518454
            [name] => chart
            [title] => Chart API
            [description] => Google chart API integration.
        )

)

nedjo’s picture

Great work.

Yes, this looks like the best approach to take at this time. Eventually we'll probably want to point to the recommended release rather than a dev one, but as an interim step, to meet the testing needs, dev makes sense.

+++ info/project_info.package.inc 1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,220 @@
+      if ($release_node->project_release['rebuild']) {
+        // Release represents a dev branch, store it.
+        $best_release = $release;
+      }
+      elseif ($best_release) {
+        // Release represents a stable branch, since a dev branch has already
+        // been found then stop and use the dev branch as the best branch.
+        break;
+      }

Is this just the same as:

<?php
     
if ($release_node->project_release['rebuild']) {
       
// Release represents a dev branch, store it.
       
$best_release = $release;
        break;
      }
?>

?

+++ info/project_info.package.inc 1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,220 @@
+  foreach ($dependencies as $dependency) {
+    // Cycle through the releases made by the project until a dev branch is
+    // found of the latest stable series that matches the core API. The
+    // releases are in descending order from the largest release version to
+    // the smallest release version.
+    $releases = project_info_package_releases_get($dependency, $api_tid);

Because some D7 .info dependency data will have versions specified, we need to strip that out as an interim step until it's supported. We can just use whatever's before the first space.

When adding dependency version support, some of the needed code is in core's module.inc.

boombatower’s picture

StatusFileSize
new23.17 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Shouldn't be the same.

for instance: views 3.x-dev, views 2.x-dev, views 2.7...thus it will stop on views 2.x-dev, not views 3.x-dev.

I added

<?php
// Remove any version identifiers from depenencies since those are not
// processed yet.
foreach ($info[$module]['dependencies'] as &$dependency) {
  list(
$dependency) = explode(' ', $dependency, 2);
}
?>

for other bit.

boombatower’s picture

StatusFileSize
new3.47 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-pift_2.patch.
[ View ]

Updated pift patch after some testing.

boombatower’s picture

StatusFileSize
new3.45 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-pift_3.patch.
[ View ]

This seems to work as well as intended.

ñull’s picture

+

Pasqualle’s picture

question: does this work with sub-modules also?

example: for media_brightcove module dependencies[] = emvideo, but emvideo is a submodule in emfield module
so I guess from the dependencies array the project_info.module will not be able to find the emfield project node.

boombatower’s picture

Yes, it does.

boombatower’s picture

StatusFileSize
new3.57 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-pift_4.patch.
[ View ]

Re-rolled.

boombatower’s picture

Committed to pift.

boombatower’s picture

StatusFileSize
new23.28 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Added condition:

<?php
   
if (!empty($info[$module]['dependencies']) && is_array($info[$module]['dependencies'])) {
     
// Remove any version identifiers from depenencies since those are not
      // processed yet.
     
foreach ($info[$module]['dependencies'] as &$dependency) {
        list(
$dependency) = explode(' ', $dependency, 2);
      }
    }
?>
boombatower’s picture

StatusFileSize
new25.18 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Re-rolled patch.

dww’s picture

Status:Needs review» Needs work

I only had a chance to very quickly skim this. Unfortunately, boombatower didn't pursue the design we discussed in IRC months ago. :( It's okay for project* itself to have a notion of dependencies, but that needs to be separate from all this incredibly drupal-specific stuff about "modules" and info files. The architecture here blends them all together into a big tangle. As I said from the beginning, the basic architecture needs to be something like:

project - handles projects

project_release - handles releases

project_release_dependency - handles dependency info about what releases depend on each other. This could potentially just live inside project_release itself, depending on how big/complicated/costly it ends up being.

drupalorg_release_dependency - stores the really specific stuff in here that only d.o cares about -- "modules", .info files, etc, and uses that data to populate the project_release_dependency info.

As this module currently stands, it would need to be committed to the drupalorg project, and we couldn't build anything useful on top of it "upstream" in project*. That'd really be a shame. I wish boombatower had implemented this as I thought we had agreed from the beginning -- designing it with an eye towards what's generally true about projects and releases, and what's specific about drupal projects and drupal releases. I haven't thought through the whole problem enough to design everything down to the exact schema it needs -- that's what I thought Jimmy was doing all this time. ;)

That said, I'm sure he's bummed that he's been waiting weeks for this, only to have it bumped back to CNW. So, maybe the best we can hope for in the short term is deploying all this d.o-specific hackery now (as part of the drupalorg project, *not* project itself), with a commitment to clean it up later. Sadly, in my experience maintaining project*, that rarely happens unless I do it myself, and my plate is already overflowing (hence, why it took me so long to actually look at this patch). So, I'm a bit at a loss of what to do now...

Given that the basic architecture isn't sound, I haven't reviewed closely, done a security audit, etc. If this goes into the drupalorg project as-is, it's going to need someone else on the infra team to adopt and maintain it. Otherwise, it needs to be majorly refactored before I can consider it for project* and maintaining it myself.

Thanks,
-Derek

nedjo’s picture

To answer dww's questions, we need to determine what about this patch is potentially generalizable to project releases as a whole.

What this patch does is:

* Allows releases to have components
* Allows a dependency relationship between component: a component of one release can depend on another component

It does not directly allow dependency relationships between releases, because we as yet have no firm way of determining that relationship (certainly not in D6, not fully in D7 either).

So the generalizable part looks to be that it introduces components of a release with dependency relationships.

What would need to change in the patch? Something along the lines of:

* Change the schema to refer to components rather than modules and info files.
* Perhaps, make release component data fetchers pluggable, so that a particular release system can declare callbacks to return information on available releases and their dependency information?

dww’s picture

@nedjo: Right, something like that -- call them release "components" instead of "modules", and provide a cleaner separation of how to populate component dependencies than directly parsing .info files. Instead of making component data fetchers "pluggable" per se, even if there was an API to manipulate dependency data, it could be up to each system to decide when/where/how to call these API functions as appropriate. On d.o, it'd still be done via our packaging script. Other sites would probably do the same, but they already need to customize the packaging script, so they can also parse their own component (or release) dependencies and record those.

Or, we punt and call this d.o-specific code.

Or, we put this in a new project_drupal glue project, since it's not technically drupal.org specific, it's just specific to sites using project* for drupal code. That might be a more appropriate home for #647454: Add verify/convert UI and release node verification for profile .make files, too (currently, that's slated to live in the drupalorg project).

Thanks for helping this move forward!
-Derek

boombatower’s picture

@dww So if you read back through the code it was originally implemented as dependencies between releases. As always I find the tone a bit annoying, but w/e. It was then changed for some reason (can't remember buts I'm sure its in the comments) to be based on modules. As nedjo pointed out there isn't necessarily anything drupal specific to that...and could easily be changed to component for a more generic feel.

Since it took so long to get affirmative feedback I still vote we deploy with this and deal with w/e we want to change later. Contrib testing has been postponed for months as it is waiting on the months of time before 2.x was deployed...this just needs to happen.

dww’s picture

@boombatower: I'm sure this is frustrating for you, and for that, I'm sorry. However, I wear so many hats and have so many responsibilities in this community that I can't be everywhere all the time. Plus, the vast majority of those hats don't pay the bills, so I also have to juggle paid gigs to support myself, too.

The bottom line is that I maintain a huge set of code here, with not much help. Not only does it have to be stable and secure enough to handle 1/2 million+ users on d.o, for years I've been moving it towards being useful for other sites. This hasn't been easy, but it's slowly been working and we're finally now starting to get real contributions from outside users, which is a huge win. I can't blindly throw in code just because it supposedly works for d.o. I explained all of this months ago when you were first asking me for help and direction. For whatever reason, you don't seem to have listened to or followed my advice, and now you're upset that when I finally carved out some time to look at this, I'm pointing out that the design isn't what I had requested, and therefore, I can't just commit this upstream. I then gave you two options: A) rewrite this to use the design I agreed to months ago or B) find someone to review/audit this as a d.o-specific hack in the drupalorg project.

boombatower’s picture

I understand why you won't commit, and as I also wrote it was originally designed as you and I discussed. It was changed because it was decided the extra detail would be helpful down the road. I however do not see why it is completely unusable because it has the concept of a module. Many projects have sub-sections/extensions of their project, call them what you will. As such it makes sense that you could support it...choose a name we'll use it and everybody is happy. If a project doesn't want sub-sections they simply have 1 section for a release that is the same name as the release or w/e, but the extra detail is supported when needed.

I am happy to revise this, but I just don't appreciate the "my way or the high way attitude" without much additional consideration. Obviously for the deployment this will be used, but I am interested in fixing this up so everyone is happy to commit it to project itself.

Don't get me wrong, I understand being extremely busy as I am in a similar situation to you. Being virtually the only person to cleanup SimpleTest in core, the automated testing system (all 3 components), work responsibilities, and the variety of other modules I maintain I understand your position. Now that you have taken a look and understanding all that has happened leading up to the code you reviewed, I would just like a bit more "productive" ideas to go forward.

If you are just completely against having the extra detail (even though it can be ignored when not needed) then that's your call, but I feel it is a waste of code. To reiterate I am happy to refactor this in order to be make it suitable to project, but rewriting to make it unusable to for d.o is not something I have time or interest for nor something that seems necessary (as explained above).

boombatower’s picture

StatusFileSize
new25.29 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Re-rolled to escape command line arguments in batch script per killies review.

dww’s picture

To clarify: I never said or implied a "my way or the high way attitude". I just said the code needs to be general or it needs to live in the d.o-specific hacks project. And if it's a d.o-specific hack, that's fine, but it's harder for me to make time to review/audit/maintain the code, since there's less symbiosis involved. That's all I've been trying to say...

This week I have very little time for Drupal work again, so I can't give too much detailed constructive feedback. I've tried to give the feedback I could on the little time I've had. If it was going to go into project itself, it'd need (at least):

A) to not use terminology like "modules", but instead something like "components".

B) to not hard-code that the way to populate component dependency data is to parse .info files. i.e. we need an API like I proposed in #60, and d.o-specific code to use this API when parsing .info files during packaging, etc.

Generally, as I said originally, you need to think about this problem more broadly than asking "what's the minimum that d.o needs to work?" and instead ask questions like "if this is a tool for project + release management, what parts are generally true and what parts are specific to the fact that we want this for d.o and drupal code?". I'm not saying you have to do a bunch of work for an imaginary non-d.o use-case. I'm saying you need to design it in such a way that the d.o-specific and drupal-specific parts are isolated from the general parts. Really, if you had done this from the beginning, it wouldn't have taken any more work. And, if you can't remember or explain why it's doing module/component specific dependencies, that's not a sign that you have a strong enough grasp of the problem and that the code isn't well enough documented. That's your responsibility as the author, not my responsibility as the reviewer. If there's really no reason to complicate this with component-level dependencies, other than we think we might need that some day, we're over-engineering this and making it more complicated (and also less generally useful) than we need to. I'd hope that you can provide a better reason than "It was changed because it was decided the extra detail would be helpful down the road." Please don't use the passive voice. It wasn't just changed because "it was decided". Someone decided. Was that you? Was that someone else? Who decided? Why did they decide? We need to be clear about all of this before the fate of this issue can truly resolved.

Thanks for sticking with it,
-Derek

boombatower’s picture

What needs to be done:

  • Change 'module' terminology to 'component'.
  • Decide if we want to keep the component to component dependencies around, or revert to release to release dependencies.
  • Design 6.x api for specifying 7.x style dependency restrictions. (#698932: "test_dependencies" for dependencies / integration tests)
  • Implement 7.x style dependency algorithm (assuming we copy-past/include d7 core code and tweak).
  • Figure out why dependency releases' components are sometimes included in the component list for the primary release.
  • Figure out why specific releases dependencies are not parsed for all components. (example #695278: Multiple problems with contrib testing XML sitemap 7.x-2.x/HEAD)
  • Design and implement API for performing actual parsing of information, vs storage and retrieval API.

The parsing API was an oversight which needs to be fixed. The component to component depdencies (after reading above) was added since people will download a project, say ubercart, and only wish to enable several modules. It is more useful (plan is to display dependency information as part of redesign) to show dependencies of the 'components' they are interested in rather then for the entire release, since they do not need of all them.

Although that is a d.o specific usecase for the same reason I see components being useful for all projects so I see the dependencies between them. Something we need to decide before moving forward, if we want to make that a separate d.o specific bit. My thought would be to include it in project since I see it as useful in a general sense, but I'll leave the final decision to dww.

boombatower’s picture

Here is the API provided to access the data.

  • project_info_component_get($component_id)
  • project_info_component_list_get($rid)
  • project_info_component_dependencies_get($component_id)
  • project_info_dependency_list_get($rid)
  • project_info_dependency_list_get_components(array $components, array $dependencies = array())

The basic API seems sound. Please look at the patch for documentation if you want more details.

The parsing step may require highly customized parsing code. Since it is also called from the build script which to my knowledge is drupal specific and is intended to be replaced for use by other projects the entire parsing step can be handled however other projects choose. The only logical API then is a storage API to be used after data is parsed.

The module already provides such an API.

  • project_info_package_clear($rid)
  • project_info_package_list_store($rid, array $info)
  • project_info_package_info_process_dependencies($rid, array $component, array $dependencies)

The last function will either require a way for projects to map the list of dependencies to release IDs, or be modified to expect a list of RIDs (I'm leaning towards latter).

My attack plan then, is to pull out the Drupal specific parsing functions and add them to the package-release-nodes.php. They can then call the storage functions from there.

After getting all that working I want to add in the new dependencies d7 style stuff. Then write tests and confirm on live data.

boombatower’s picture

Status:Needs work» Needs review
StatusFileSize
new32.2 KB
PASSED: [[SimpleTest]]: [MySQL] 202 pass(es).
[ View ]

Early version of cleanup, but basic API moving around is complete and D7 style dependencies are supported. I have not written a test for this, nor tested manually. Anyone interested can take a look and make sure the direction is agreeable.

boombatower’s picture

StatusFileSize
new35.43 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.
[ View ]

Provides a test for project_info storage and retrieval API. Not sure if/how to test the release script since 1) it is drupal specific (where do you want test), 2) it doesn't have any existing test...so I would have to test the script itself before I can even start testing project_info integration.

batch.inc API:
project_info_batch_process_release(array $release)
project_info_batch_process_dependencies($rid, array $component, array $dependencies)

It includes the Drupal specific file, by default, just like the release script does. All the Drupal specific stuff is in there.

boombatower’s picture

StatusFileSize
new42.94 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.
[ View ]

hrm...trying to simulate a release tag in test, but I get: http://imagebin.ca/view/rsv8Ooh.html.

boombatower’s picture

Status:Needs review» Needs work

After a several hour talk with dww and hunmonk at Drupalcon we determined that the design of the patch is exactly what we were all looking for. We agreed on moving a few things around and adding a few new API functions, but the design of the patch will be kept the same. The following are the action items to be completed.

  • Move everything form project_info module (added in patch) to project release module.
  • Ensure that all Drupal specific parsing is in an .inc file (already is), need to determine where we want .inc.
  • Tweak column names in {project_release_dependency} from component_id and dependency_id to:
    • source_id
    • target_id
    • dependency_type (used to be "required", recommended or required)
  • Change rid to release_nid in {project_release_component}.
  • Prefer component_load to component_get (and load_all)
  • Remove static cache and join table instead.
  • Use graph.inc to build tree of dependencies and just determine list.
marvil07’s picture

subscribing

hunmonk’s picture

StatusFileSize
new892 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch project_info_bused_array_fix.patch.
[ View ]

please add the attached patch to the ongoing work -- it's necessary to bulletproof against incorrect definitions of the dependencies array. see #870934-6: Packaging of mailarchive module is breaking the packaging script

develCuy’s picture

Subscribing.

sun’s picture

Status:Needs work» Needs review
StatusFileSize
new43.15 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch project-HEAD.info.75.patch.
[ View ]

Bullet 1.

Status:Needs review» Needs work

The last submitted patch, project-HEAD.info.75.patch, failed testing.

sun’s picture

Status:Needs work» Needs review
StatusFileSize
new42.81 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch project-HEAD.info.76.patch.
[ View ]

Bullet 3.

sun’s picture

StatusFileSize
new42.9 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch project-HEAD.info.78.patch.
[ View ]

Bullet 4.

sun’s picture

StatusFileSize
new42.69 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch project-HEAD.info.79.patch.
[ View ]

Bullet 5.

sun’s picture

Hope this helps to move this along. The goals of the following bullets are beyond my horizon:

# Ensure that all Drupal specific parsing is in an .inc file (already is), need to determine where we want .inc.
# Remove static cache and join table instead.
# Use graph.inc to build tree of dependencies and just determine list.

btw, no idea why testbot is unable to apply the patch. It's against Project HEAD.

Status:Needs review» Needs work

The last submitted patch, project-HEAD.info.79.patch, failed testing.

Owen Barton’s picture

Status:Needs work» Needs review

#79: project-HEAD.info.79.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, project-HEAD.info.79.patch, failed testing.

rfay’s picture

subscribe

boombatower’s picture

I get same result bot gets (click on "Review log" on qa.drupal.org):

Hunk #1 succeeded at 316 with fuzz 2 (offset 5 lines).                                                                                                                               
Hunk #2 succeeded at 355 with fuzz 2 (offset 5 lines).                                                                                                                               
Hunk #3 succeeded at 384 (offset 5 lines).                                                                                                                                           
Hunk #4 succeeded at 440 with fuzz 1 (offset 5 lines).                                                                                                                               
patching file release/project_release.batch.inc                                                                                                                                      
patching file release/project_release.drupal.inc                                                                                                                                     
patching file release/project_release.info                                                                                                                                           
patching file release/project_release.install                                                                                                                                        
Hunk #1 FAILED at 344.                                                                                                                                                               
Hunk #2 succeeded at 784 with fuzz 1 (offset 96 lines).
1 out of 2 hunks FAILED -- saving rejects to file release/project_release.install.rej
patching file release/project_release.module
Hunk #3 succeeded at 2194 with fuzz 1 (offset 52 lines).
patching file release/project_release.package.inc
patching file release/project_release.test

Just logging the fact that I am hoping to finish this up in the next few days.

boombatower’s picture

@sun: Any chance you still have that patched code to generate a new patch from? Debating if I should start from scratch or try to merge your patch(es) and mine.

sun’s picture

Status:Needs work» Needs review
StatusFileSize
new46.02 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.
[ View ]

Luckily, yes.

boombatower’s picture

Status:Needs review» Needs work

Guess, I'll just start with #70 again. Created github mirror of project module and committed patch in #70 http://github.com/boombatower/project.

boombatower’s picture

Great news, ok http://github.com/boombatower/project/commit/ef6854badda2355b87b2cbae421... now I'll see if I can finish off the remaining bullets.

- Ensure that all Drupal specific parsing is in an .inc file (already is), need to determine where we want .inc.
- Remove static cache and join table instead.
- Use graph.inc to build tree of dependencies and just determine list.

boombatower’s picture

Status:Needs work» Needs review

We may need to do a bit more inventory on the function names as I see project_release_get_components(), and project_release_component_load(), etc on get vs load...I'll double check notes. (EDIT: seems like those match what is in project_release.module so possibly ok)

- Current include file is project_release.drupal.inc did we want package-info.php or something like package-release-nodes.php (seems like the word drupal should be in both).
- Fixed "Remove static cache and join table instead."
- Working on graph.inc, but I don't think it gives us any advantages over current code. In order for it to work I have to load up the "edges" for all the dependencies of a component at which point I already know the dependencies of that component. As used with Drupal core all modules are entered into the graph and then the paths are determined. If we were to do likewise I would need to update cache is some manor the evaluated lists of dependencies and then update them all whenever any change is made or selectively if we want to write that algorithm. Either way those both sound like overkill for our current needs and as such I am proposing we don't bother with graph.inc.

I will ping dww to have a look at these points and review the code @ http://github.com/boombatower/project.

dww’s picture

Fantastic to see progress again here. Sadly, with the d.o redesign launch just around the corner, there's no chance I can look at this until mid next week at the earliest. Feel free to keep plowing ahead as you see fit, and we can sync up next week. Cool?

Thanks!
-Derek

Berdir’s picture

Subscribing, I'm relying on the test_dependencies thing for privatemsg/token tests in #946812: Proper entity loading, uri callback and more tokens. I'm relying on the fact that the testbot currently silently ignores test cases that require modules which are not available.

Owen Barton’s picture

I am not sure what would be involved in testing this (is it sufficient to just generate dummy data?), but if someone can give some ideas I am happy to take a look, if it would help nudge this along.

This is somewhat of a dependency for improvements in the pm code in Drush that I would like to get in for Drush 4.x (basically, so you can drush pm-install a module, and it will download the project and dependencies and enable it all in one go - right now we have no sane way to figure out needed projects from the module dependencies). Once we have the relationships available centrally, I am planning on working on a simple cron script that can run on a d.o. server to dump all the project data into an easy to query format (perhaps sqllite or something - the xml feed doesn't work too well, because the parse time gets really big to chomp through all projects every day) and set up some way that users can rsync the file locally to allow fast queries, as well as autocompletion and other fun stuff.

boombatower’s picture

StatusFileSize
new84.15 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_21.patch.
[ View ]

I rolled a patch from my github repositories to make this easier to review for some.

git diff --no-prefix 102102 master

Status:Needs review» Needs work

The last submitted patch, 102102-project-info.patch, failed testing.

boombatower’s picture

Status:Needs work» Needs review
StatusFileSize
new43.86 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.
[ View ]

I had my master branch messed up, this patch should be correct.

Status:Needs review» Needs work

The last submitted patch, 102102-project-info.patch, failed testing.

sun’s picture

Status:Needs work» Needs review
+++ release/project_release.batch.inc
@@ -0,0 +1,203 @@
+define('PROJECT_RELEASE_INFO_BATCH_DIRECTORY', '/tmp/project-info');

A variable would be more suitable.

+++ release/project_release.batch.inc
@@ -0,0 +1,203 @@
+  $form['op'] = array('#type' => 'submit', '#value' => t('Submit'));

'op' should be 'submit'.

+++ release/project_release.batch.inc
@@ -0,0 +1,203 @@
+  $i = 0;
...
+    // Only process 20 releases in a single batch operation.
+    if ($i++ == 20) {

This effectively processes 22, not 20 - because it's zero-based, and because $i++ increases $i only after execution. Should be:

++$i < 20

+++ release/project_release.drupal.inc
@@ -0,0 +1,380 @@
+  $files = file_scan_directory(PROJECT_RELEASE_INFO_BATCH_DIRECTORY . '/' . $directory, '\.info$');

Merely filtering by file extension is not sufficient - see #944198: Functions that call drupal_system_listing() act on potentially invalid system items

+++ release/project_release.drupal.inc
@@ -0,0 +1,380 @@
+function project_release_info_batch_process_dependencies($rid, array $component, array $dependencies) {
+  package_release_info_process_dependencies($rid, $component, $dependencies);
+}

Move into .batch.inc?

Powered by Dreditor.

dww’s picture

Status:Needs review» Needs work

Thanks for the review, sun! Sounds like this needs work, not review, then...

boombatower’s picture

The constant is fine...it follows the format of package-release-nodes.php and is only run to catch up. Changed other items.

What do you mean by #944198: Functions that call drupal_system_listing() act on potentially invalid system items?

I have no idea where the function came from and it seems to have absolutely no purpose (possibly during re-merging and tweaking of patches??).

Everything else has been committed...I'll re-roll once my question is resolved.

sun’s picture

On #944198: Perhaps this should be tackled in a separate issue, but: Currently, the testing framework breaks when a project repo happens to contain an arbitrary .info file. When scanning for .info files, the regular expression has to make sure that the .info file's basename is a valid basename for system items in order to build hook names and function callbacks.

For example, if there happens to be a example.foo.info, then this is not a valid .info file and must not be considered as a module/theme/profile anywhere. Right now, Libraries API's HEAD branch tests are permanently failing, because the testing framework picked up an example .info file in the repo at some point. Debugging revealed that in addition to the testing framework, Drupal core also picks up invalid .info files (#944198: Functions that call drupal_system_listing() act on potentially invalid system items)

Perhaps a separate issue, but I thought since we're touching/adding this functionality anyway here it might be good to take it into account already.

boombatower’s picture

Haven't looked through all the code in the issue, but can we either use the code for D7 or a modified regular express?

sun’s picture

Yeah, we don't have to wait for that core patch. It essentially uses:

/**
+ * Regular expression to match PHP function names.
+ *
+ * @see http://php.net/manual/en/language.functions.php
+ */
+define('DRUPAL_PHP_FUNCTION_PATTERN', '[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*');
...
-  $files = drupal_system_listing('/\.info$/', 'modules', 'name', 0);
+  $files = drupal_system_listing('/^' . DRUPAL_PHP_FUNCTION_PATTERN . '\.info$/', 'modules', 'name', 0);
boombatower’s picture

Status:Needs work» Needs review
StatusFileSize
new43.93 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.
[ View ]
diff --git a/release/project_release.drupal.inc b/release/project_release.drupal.inc
index bf5c395..12e7254 100644
--- a/release/project_release.drupal.inc
+++ b/release/project_release.drupal.inc
@@ -8,6 +8,13 @@
  * @author Jimmy Berry ("boombatower", http://drupal.org/user/214218)
  */

+/**
+ * Regular expression to match PHP function names. (From Drupal 7)
+ *
+ * @see http://php.net/manual/en/language.functions.php
+ */
+define('DRUPAL_PHP_FUNCTION_PATTERN', '[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*');
+
module_load_include('package.inc', 'project_release');

/**
@@ -353,7 +360,7 @@ function project_release_info_batch_process_release(array $release) {

   // Scan checkout for .info files and create a list in the same format that
   // project release uses, so that standard API functions can be used.
-  $files = file_scan_directory(PROJECT_RELEASE_INFO_BATCH_DIRECTORY . '/' . $directory, '\.info$');
+  $files = file_scan_directory(PROJECT_RELEASE_INFO_BATCH_DIRECTORY . '/' . $directory, '/^' . DRUPAL_PHP_FUNCTION_PATTERN . '\.info$/');
   $info_files = array();
   foreach ($files as $file) {
     $info_files[] = $file->filename;

Status:Needs review» Needs work

The last submitted patch, 102102-project-info.patch, failed testing.

boombatower’s picture

I suppose that is right, the tests are most likely out of date with the more recent testing efforts of project and what not. The testbot can't find the testcase 'PIFTTestCase' which makes sense since I was uisng the pift test case for lack of a proper project one which we know have.

boombatower’s picture

Status:Needs work» Needs review
StatusFileSize
new50.05 KB
FAILED: [[SimpleTest]]: [MySQL] 307 pass(es), 127 fail(s), and 62 exception(es).
[ View ]

On all the tests I get

Trying to get property of non-object Notice project_release.views.inc 291 project_release_views_data()

I rewrote tests and fixed up things that were not completed from above. Tests now pass, but have @todo's for following items.

- dependency_type has yet to be implemented...not sure how that slipped by, should be simple enough.
- also need to take a look at future proofing how it pull vcs information

Status:Needs review» Needs work

The last submitted patch, 102102-project-info.patch, failed testing.

boombatower’s picture

Do not worry about testbot results as this patch is needed for them to work properly anyway. :) The good thing is they don't fatal error like they used to, but if you run them locally with the appropriate dependencies all should be fine.

boombatower’s picture

Also need an update path for previous version of patch deployed on d.o. Current drupal.org scheme is as follows.

CREATE TABLE `project_info_module` (
  `module_id` int(10) unsigned NOT NULL AUTO_INCREMENT,
  `rid` int(10) unsigned NOT NULL,
  `name` varchar(255) NOT NULL DEFAULT '',
  `title` varchar(255) NOT NULL,
  `description` mediumtext,
  PRIMARY KEY (`module_id`),
  KEY `rid` (`rid`),
  KEY `name` (`name`)
) ENGINE=InnoDB AUTO_INCREMENT=270615 DEFAULT CHARSET=utf8

CREATE TABLE `project_info_dependency` (
  `module_id` int(10) unsigned NOT NULL,
  `dependency_id` int(10) unsigned NOT NULL,
  PRIMARY KEY (`module_id`,`dependency_id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8

boombatower’s picture

StatusFileSize
new52.44 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 102102-project-info_25.patch.
[ View ]

Implemented dependency_type. I would like to get drupal test working, but whenever I modify test to enable cvs (cvslog) module the test user is unable to create projects or project releases. Other then that things seem to be working.

As for the upgrade path, we should probably just re-run the batch process to re-generate all the data since a) some/much of it is incorrect, b) it did not take into account dependency_type.

I'll see about making a patch for pift to update it for this patch.

kiam’s picture

   /**
    * Setup basic tasks for all project issue tests.
    */
-  function setUp() {
+  public function setUp() {

By default, PHP methods are public; the two lines are perfectly equivalent.

Dave Reid’s picture

+++ project.testundefined
@@ -5,13 +5,18 @@ class ProjectWebTestCase extends DrupalWebTestCase {
+    if (version_compare(PHP_VERSION, '5.3.0') >= 0) {
+      call_user_func_array('parent::setUp', $modules);
+    }
+    else {
+      call_user_func_array(array($this, 'parent::setUp'), $modules);

This is no longer necessary with the latest SimpleTest backport. Just do parent::setUp($modules);

Powered by Dreditor.

DamienMcKenna’s picture

Having this would also remove the need to patch almost every single module to work around limitations in Drush Make: #1048056: Add drush make file

sirkitree’s picture

Subscribe. Wish I had more to contribute here, but I don't. :(

Berdir’s picture

StatusFileSize
new51.71 KB
FAILED: [[SimpleTest]]: [MySQL] 349 pass(es), 117 fail(s), and 70 exception(es).
[ View ]

Trying to push this forward, here is a first patch that should apply again.

The only relevant change apart from getting it to apply again so far:
- Simplified the setUp() stuff as suggested by Dave Reid, this however requires 6.x-2.x-dev

TODO:
- package-release-nodes.php has been refactored completely, we need to figure out where the dependency processing should happen now.
- There is a failing test assertion at the end of the new tests, which seems to have failed already in the old patch according to the @TODO
- Port the partly implemented cvs/release creation part to vcs. I've played around with that but didn't came very far yet
- There are also many imho unrelated test fails and exceptions triggered by the new tests both in project.module and views.module. They need to be dealt with.

boombatower’s picture

Created http://drupal.org/sandbox/boombatower/1108570 and gave rfay, Berdir and myself commit access. We should start working in there.

boombatower’s picture

Push project master branch and committed patch.

rfay’s picture

@boombatower, are you going to do the code over there, but the discussion over here?

boombatower’s picture

@rfay: That was what I was thinking. Once done post a patch over here. We can obviously create issues or what have you in the sandbox project, but should be straight porting based on the TODOs in #116 against previous patch in #111.

bfroehle’s picture

subscribe

rfay’s picture

I'm taking a first orientation look at this today.

1) We have to make the dependency generation work.
2) We have to make project_release cause it to happen
3) We have to make PIFT use it
4) PIFR stuff (like downloading contrib dependencies) follows after

Edit: 1.5) Make a drush script to run the generation as a whole or pieces of it

Notes from boombatower in IRC:

rfay: yes, when you get to 3 and 4 we should probably talk again...I had some better ideas how how to use it then previous that should eleviate some of the issues
<boombatower> rfay: so the whole idea though is you need to have a batch script (or drush script) that generates a list of all modules contained within all projects
<boombatower> then second cycle is to resolve dependencies
<boombatower> that builds the backlog of data
<boombatower> then on standard release build from that point on it simple redoes it's list of modules and rechecks dependencies
<boombatower> the script shoudl be run once for deployment
<boombatower> so the inital script fills the two tables in order the first table then the second
and after that only parse for individual releases
rfay’s picture

StatusFileSize
new16.06 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch project_info.CVS_to_git.patch.
[ View ]

I got some work on this done today. It has quite a ways to go, of course.

* Changed the CVS stuff to git (still needs love and error checking)
* Fixed some naming issues
* Added two drush commands that allow us to debug this (and perhaps implement it) from the command line

My commits are on the rfay/initial_work topic branch

The interdiff is attached.

clemens.tolboom’s picture

hunziker’s picture

subscribe

plach’s picture

rfay’s picture

Made some more progress this weekend. I'm to the point of testing recursive dependency building.

I *think* I'm going to get away with just having one build phase, where we harvest all the dependencies from the info files. That's enough information for recursive dependency generation.

Then I think we should make dependencies be release nodes (like views-6.x-1.3) rather than components (like views_ui). So we use components to develop the list, but then it's release nodes that are returned.

I'll hope to have a patch before long (and stuff is checked into the sandbox).

rfay’s picture

Just an update: I got it into shape that I like better, and hope to be able to ask for a preliminary review this week. All work is checked into the sandbox.

rfay’s picture

Status:Needs work» Needs review
StatusFileSize
new63.61 KB
FAILED: [[SimpleTest]]: [MySQL] 328 pass(es), 112 fail(s), and 49 exception(es).
[ View ]

OK, here is a first shot at this. I hope you'll go easy on me (or help with the deficits) so that we can get this deployed.

No attention has yet been paid to tests, but the basic dependency harvesting/storing and dependency querying seem to work OK.

Below is a README about the changes here.

Note that this is the current contents of the rfay/initial_work branch of the project info sandbox.

A fully populated git repo is available to you (updated 2011-07-22) for testing. If there's anything else I can do to help with trying this out I'll be delighted to help.

Thanks to all who have worked on this previously (especially @boombatower) and to those who will chip in now.

Project Release Dependency Generation Information

Terms

  • Component: A module in a package. Many packages contain more than one module.
  • Release: A row in the project_release_nodes table that signifies a release package.

Processing technique

Before we can do recursive dependency checking, we have to have a complete
database of related components.

  1. Set the directory where git repositories should be stored. This is a semi-permanent directory, because it saves us the trouble of checking things out anytime we're processing dependencies. For example:
    drush vset PROJECT_RELEASE_DIRECTORY /home/rfay/tmp/git_repos
    or set it at admin/project/project-release-settings
  2. Pre-seed a set of git repositories. Although the system can check out each project (and will do so) it's one more level of pain, so it's better to use the techniques in http://drupal.org/node/1057386 to pre-build all the repositories.
  3. Create all first-level dependencies. This walks the info files for each release node and finds its explicit dependencies and creates rows in the component and dependency tables. Uses drush command drush prpa or the related drush commands described below.
  4. Create all recursive dependencies. Here we have to have #2 already in place, and we check each project for external dependencies, and where they exist, create new dependency rows for the top-level component.

Key functions

  • project_release_get_external_component_dependencies($release_nid) searches for dependency information at the for a given release.
  • project_release_process_release($shortname, $version) is called to process a new release node.
  • project_release_get_external_release_dependencies($depending_release_nid, $depending_components = array()) returns an array of release dependency information, given a release nid and optional component names. This will probably be the most common function to be called.

How we choose dependent packages

  • Get all release packages that contain a given component with a given major version tid.
  • The first one will be a dev release, if there is a dev release.
  • If there is a dev release and there are stable releases, take the first stable, otherwise use the dev.

Drush commands

  • drush project-release-process-project (prpp): Process and store all dependency information for a project or projects, as in drush prpp views ctools commerce
  • drush project-release-process-version (prpv): Process and store all dependency information for a single release node, as in drush prpv views 7.x-3.0-beta1
  • drush project-release-process-all (prpa): Process and store all dependency information for all projects. This one takes several hours to run, but provides completion information as it runs. drush prpa
  • drush project-release-show-dependencies (prsd): Given a project and version, shows the external packages (releases) that depend on this project and version. drush prsd views 7.x-3.0-beta1.

Database Tables and Modifications

There are two tables introduced with this patch.

  • project_release_component: One row for every component of every release.
  • project_release_dependency: One row for every component which is a dependency of another.

(Note that these obsolete the project_info_* tables that are currently deployed on drupal.org.)

In addition, a timestamp is added to the project_release_nodes table so that all dependencies older than a given date may be rebuilt.

Changes from earlier approaches

  • There is now only one round of dependency generation (where each project's direct dependencies are calculated). Everything else can be determined from that.
  • There is (almost) no web UI for this at this time. Since project dependencies need only be calculated on a project node creation, there's really no need. Initial work (and fixups, or paranoid debugging) is available with the drush commands.

Open Issues

  • The info file feature 'test_dependencies' is not currently being respected. We can either test this now, or go with what we have at this point and add that later. It is a very attractive feature.

TODO

  • Tests for the new features.
  • project_release_get_external_component_dependencies() may need to be cached.

Status:Needs review» Needs work

The last submitted patch, project.project_release_dependencies_102102_129.patch, failed testing.

boombatower’s picture

Let's hold off on test_dependencies since this really just needs to get some updated code committed and deployed to fix the current situation. If I remember correctly there was to be a type field of some sort on the dependency table that would specify recommended, or required or somesuch and for testing we would grab all, but for d.o data purposes it was more correct to have both. Realistically, respecting recommends[] might make sense so probably need to discuss further. Should be a relatively small patch on top of this once the dust settles.

Great to see the conversion to drush commands and looks like you've done an excellent job rfay. I am not sure if we should focus on getting tests passing, or just ensuring it seems to work on d.o dataset and get things rolling from there. I think smaller progress improvements will be much easier as we've already seen this patch get out of sync several times and I don't want to risk that happening again.

Again I really appreciate you picking this up rfay.

boombatower’s picture

+++ b/release/README.release_dependencies.md
@@ -0,0 +1,63 @@
+1. Set the directory where git repositories should be stored. This is a semi-permanent directory, because it saves us the trouble of checking things out anytime we're processing dependencies. For example: ¶

Trailing space.

+++ b/release/package-release-nodes.php
@@ -226,6 +228,10 @@ function package_releases($type, $project_id = 0) {
+    // Process the module .info files.
+    // @todo: Fix me. This requires us to *gather* the required $info_files.
+    //project_release_info_process_all($release_node->nid, $info_files, $project_short_name, $version);
+

Seems important? Or is that just left over?

+++ b/release/project_release.install
@@ -392,6 +398,95 @@ function project_release_schema() {
+      'core_api' => array(
+        'type' => 'varchar',
+        'length' => 8,
+        'description' => t('The core api version of this component, as in "6.x" or "7.x".'),
+        'not null' => TRUE,
+      ),

Not sure we want 'core_api' as dww really wanted to make sure this didn't end up being Drupal specific, which that really is. Keeping the Drupal specific portions in the drupal.inc file.

+++ b/release/project_release.install
@@ -392,6 +398,95 @@ function project_release_schema() {
+      'dependency_type' => array(
+        'description' => 'Dependency type, PROJECT_RELEASE_DEPENDENCY_*.',
+        'type' => 'int',
+        'unsigned' => TRUE,
+        'not null' => TRUE,
+      ),

Just noting again that looks like everything is in place for being able to use the recommends stuff.

+++ b/release/project_release.module
@@ -10,6 +10,12 @@ define('PROJECT_RELEASE_UPDATE_STATUS_CURRENT', 0);
+/*
+ * Constants for the possible values of {project_release_dependency}.dependency_type.
+ */
+define('PROJECT_RELEASE_DEPENDENCY_REQUIRED', 0);
+define('PROJECT_RELEASE_DEPENDENCY_RECOMMENDED', 1);
+

So we can pick up recommends[] = view_EXTRA and store in recommended and have testing always pull both types (all). Which I feel like was there before, so we should probably check.

+++ b/release/project_release.module
@@ -29,6 +35,15 @@ function project_release_init() {
+ * Implementation of hook_perm().
+ */
+function project_release_perm() {
+  return array(
+    'parse project_release_info batch',
+  );
+}

The permission is never used and was related to batch API processing which has been replaced by drush commands.

Powered by Dreditor.

dww’s picture

@rfay: Thanks for taking a stab at this painful issue. ;)

Did I miss a place where all this was being discussed? I don't see any issues over at http://drupal.org/project/issues/1108570

Although I'm extremely encouraged to see progress here again, I'm totally baffled by your readme in #129. Why does this code care about Git at all? Why have you taken us a huge leap back towards making this only usable by drupal.org again? I hate to say this, but please (re)read comments #58 through #62. :( Those were dark times in this issue's history, but they're just as relevant again today as they were nearly 1.5 years ago.

I thought that between those comments, and the big debate at DCSF with boombatower (more or less recorded in this issue as comment #71, although I have some disagreements about how that was formulated), we had real clarity on what needed to happen to make this viable code I could commit into project_release and maintain. Now, it's like we're starting all over again with a ton of drupal.org-specific code that needs to be refactored.

I'm sure everyone now hates me. And you all think I'm a mean obstructionist who just wants to find a reason to stall this issue. The truth couldn't be further from that. I'm just trying to do my "job" as the maintainer of Project*, a drupal.org infra team member, and so on. And I'm deeply confused why I have to repeat myself over and over in here. It's probably due to various failings in our collaboration tools -- we have no good way to keep a summary of "important stuff you must know if you want to work on this issue" in a single obvious place that everyone will read...

Anyway, I'm extremely sorry to have to say all this. I wish I could afford to just swoop in and fix all this code, but I simply can't.

@rfay: Pretty please stay with it, read my previous concerns, and address them. If you do, I promise to be helpful and kind, and to get this committed and deployed ASAP. ;)

@boombatower: Please don't hate me, and please keep helping get rfay on track based on your (hard-won) understanding of the problem and my concerns about previous implementations.

Thanks!
-Derek

p.s. Or punt, call this a drupal.org-only system, move all the code into drupalorg_*, and volunteer to maintain it and port it to future versions of core for the indefinite future. Your choice. Again, #58 to #62 explain the pros and cons of that approach.

rfay’s picture

@dww, I scarcely understand the entire debate (although I did make an attempt to grok it before getting to work), but what I did was change CVS checkouts to git. And it's all in a Drupal-specific file, which I *thought* was what the debate was about, to have a pluggable way to deal with this. So there's no real conceptual difference from what was already going on in this issue. (Unless I'm missing something big, which is certainly possible). But my work here is not a departure in architecture from what was in this issue last November, for example. Feel free to correct me.

Thanks for your openness to doing this a different way (drupalorg), also. I guess we have that as a possibility.

dww’s picture

@rfay: I certainly didn't carefully review the entire patch. However, on quick glance I saw lots of things that troubled me. References to "info" and "core_api" and other such things directly in project_release code.

Stepping back, my concerns about this issue can be summarized thusly:

  1. We can't have drupal or drupal.org-specific code living in project_release -- it's a general tool for other sites to use, too.
  2. The thing that's generally true of releases managed by project_release is that they might have dependencies on other releases hosted on the same site.
  3. How to figure out the dependencies between releases is site-specific, and needs to be pluggable.
  4. For every line of code, variable name, function name, and/or concept, you should ask this question about it: "Is this specific to releases of Drupal code living on drupal.org, or is it generally true of releases?" Based on your answer to that question, you know if it should live in project_release or in a drupalorg-specific plugin. The architecture of the system should ideally make it easy for things to live in either place as needed.

I hope that helps...

Also, I suggest you read #1024942: Add ctools plugin system for site-specific logic for packaging release nodes -- that's highly relevant, and the way we should be handling the actual drupalorg-specific plugins. Namely, project_release's dependency checking stuff should invoke a CTools plugin at the appropriate moment to get site-specific logic, and our implementation of the plugin should live in drupalorg_project. So, the README in project_release about this stuff should say: "step 1: checkout the example plugin from drupalorg_project, copy it to your site-specific module's directory, and customize it for your needs". ;)

Thanks again for sticking with this!

Cheers,
-Derek

rfay’s picture

@dww, it kind of looks to me like you hadn't looked at the patches from SF until now, which is too bad. All I really did was update the concepts that were already there. Some of it is new code, but it's all the same ideas.

I think the right thing to do is to create a new project for this, so it doesn't have to fill so many constraints. The problems boombatower and I are trying to solve are first the testing infrastructure (which is ever-so-hobbled by this stuff being broken both before and after the project git migration changes.) There are lots of other reasons d.o needs to understand dependencies. But I suspect we may not be able to get this done if we remain in the project* umbrella; the constraints imposed by that are artificial in the context of what we're trying to accomplish.

@boombatower, What do you think about working toward promoting this sandbox to something like drupalorg_project_dependencies and renaming stuff?

@dww, your input, oversight, and perspective are much appreciated at all levels. I just think we probably have to decouple this (as you suggested) from the project infrastructure in order to solve the very immediate problems that are causing trouble for the entire developer community. Unfortunately, the entire old partly-working system was completely abandoned (accidentally) during the project commits for the Git migration, and so every new release node created ends up with a full core test suite being run for every test, meaning long wait times and overloaded testbots (#1126112: Full tests of Drupal being done for some contrib patches). You indicated in IRC that you thought this wasn't worthy of effort on the project side, so we have focused on solving it (here) in the long-term way. In addition, all contrib projects that have external dependencies are untestable and have always been, a huge problem that needs to be solved. We just have to get this done.

dww’s picture

@rfay: I've said repeatedly I haven't looked closely at all the patches, that's not news. ;) I thought that by making my concerns (painfully) clear over the course of this issue, that the people working on it would be moving the code in the "right" direction without my intimate involvement at every step. I'm happy you're trying to update the previous efforts to bring them into line with the Git world we're now in, but if the prior patches were still not separating wheat from chaff, and you're building on a house of chaff, we still need to find the wheat...

Anyway, I agree this has to get done. I don't think I'm being unreasonable here with "artificial" constraints. See the Drupal.org development guidelines. Honestly, if y'all just wrote this as I requested/spec'ed all along, we'd a) be done by now and b) everyone would be happy. But, since I appear to have completely failed to explain how this should be written in a way that y'all could work with it, we're in this crappy situation here. I'll take as much responsibility for that as anyone.

So, I can certainly see why "punt to building the whole thing as a d.o-specific system" is attractive. Then, you can just do whatever you want without having to appease the ugly dww monster. However, the downsides of that are real and should be considered carefully before going forward:

Anything beyond the test bot that cares about project and release dependencies will also have to be drupalorg-specific code. That sucks. It just means more and more parts of this are going to have to live in code no one wants to maintain, increasing the (already huge) cost of upgrading d.o to future versions of core. If most of the plumbing (and data structures) to represent release dependencies were done cleanly in project*, we could build more goodness on top of it directly in project* so that other people might care about it and help us maintain/expand/upgrade it. We want blocks on project nodes that tell you about dependencies (in both directions) and it'd be great to do that generically in Project* instead of further bloating drupalorg*.

So, in the interest of "let's just get it done for the test bot", you're going to lock us into a path that means *anything* that knows about release/project dependencies will have to be site-specific code that only the d.o infra team will maintain.

But, I can't force you to do it my way. So, if you think that price is worth not having to conform to my "artificial" constraints on how the system is designed, a) knock yourselves out and b) I'm deeply sorry we've all sunk so much time into trying to do it "right".

Cheers,
-Derek

rfay’s picture

@dww, I do apologize for the unfortunate choice of the word "arbitrary". The constraints are not arbitrary in any way, we're just talking about different design goals. You have long-term design goals for project that are sadly not met with this approach, and we have design goals for testing and d.o that have a hard time getting done if the project goals come first.

rfay’s picture

I haven't succeeded in getting a big tarball of preseeded repositories up there for download, but that doesn't mean you can't test this.

The full set of repos is 4-5GB, and it will automatically be created if you run drush prap. It just takes longer that way.

But if you just wanted to test, say, commerce + views + ctools you could do

drush prpp views ctools addressfield commerce rules entity

and all releases of all those projects would be processed. It's not a bad test. You could then use drush prsd commerce 7.x-1.x-dev to find the dependendencies of that version, etc.

rfay’s picture

I did finally get a full set of repositories prepared that you can use as a starter directory, http://randyfay.com/sites/default/files/gitrepos.tgz. It's 1.6GB; If you want to run a full set of dependencies, it will be useful in speeding things up. If you just want to do basic testing, the approach in #139 will probably do.

dww’s picture

I dare to even ask this, ;) but is this whole "parse the entire Git history for all of contrib" thing just a 1-time operation to catch up for historical releases? Once that's done, we never have to do it again, right? All we need to do when packaging a new release is record what components it includes and directly depends on, no? Just curious what you're talking about, based only on comments here not a thorough review of the code.

Thanks,
-Derek

rfay’s picture

Hi Derek -

Parsing everything takes about 6 hours on my laptop and is a one-time operation. After that we only need to keep up with new release nodes. There are drush commands provided for fixing up specific releases and projects.

dww’s picture

Perfect, that's what I thought. Just wanted to double-check...

Thanks!
-Derek

kylebrowning’s picture

subscribing.

Owen Barton’s picture

Tried to test this out - I got a d.o sandbox set up using http://drupal.org/project/drupalorg_testing, and switched project to the sandbox branch. Followed the (very nice!) instructions above, but unfortunately it doesn't seem like drupalorg_testing has a core API taxonomy (has this been added recently?) and so bails in project_release_process_release() because project_release_info_core_api($node) is returning FALSE. Do I need a (more) complete d.o. database dump/environment to test this out (along the lines of [#1018070] or [#1018084]) or is there something I am missing here?

rfay’s picture

I should have mentioned that. Sorry, yes, you need to set up a full clone either locally which works nicely or on the excellent dev staging sites. If there's anything I can do to help, I'll be happy to. Thanks for taking this for a spin.

You *might* be able to get started by just creating a copied core api taxonomy, but it doesn't make sense to go down that way to me. Probably there's just another bugaboo around the corner.

rfay’s picture

OK, after thinking about this for a couple of weeks, I think we should move this out of project module.

Although dww's concerns are completely valid, I and many others just want to see features like contrib testing with dependencies and many other Drupal.org project dependency features work on Drupal.org. It would be great if we could easily accomplish that in a generic way for project module's other goals. However, I don't have the time or energy to accomplish project module's other objectives, so would like to focus on what needs to happen for drupal.org. I have no objection at all if 3281d would like to take this and turn it into something that works for their broader objectives, but I don't think we need to hold up the progress of drupal.org for that reason.

So I would say let's
* Make a module out of this. drupalorg_project_dependencies?
* Promote it to a full project.
* Get it deployed.

Thanks to all of you who have worked on this over the years. Let's get it at least *somewhere*.

rfay’s picture

Re: boombatower, #132: I fixed the whitespace and removed the perm and thanks for the reminder about the recommends stuff. This is still in the rfay/initial_work branch.

boombatower’s picture

Since there are limited resources to get this done and that current solution solves the immediate need I think it does make sense although definitely not the best result. I fear that this will never get fixed up for project module itself if we do this. Not really sure what my opinion on this is, definitely not what I would like, but it might be a good idea for now. I think we are very close to what is properly abstract for project module, but ...

dww’s picture

Sorry, I've had an intense week and didn't circle back to reply sooner...

I believe I've already made it clear why I'm not in favor doing all of this as a 1-off d.o hack. That has nothing to do with "3281d's broader objectives" and has everything to do with trying to make d.o sustainable and maintainable.

I also agree with boombatower: I think we're already very close to something that could live directly in project module. My passionate replies in here have had more to do with my frustration at apparently not being able to communicate than with the magnitude of the remaining concerns with the architecture. Although it seems my Friday is going to be totally shot by some unfortunate personal stuff that's erupting, and this weekend is San Francisco Carnival so I have a bunch of gigs and responsibilities, I'd be happy to find 30-60 minutes in the next week to go through this patch with rfay and chart out a path to the finish line if he's interested. Again, I don't think it's necessary, since I think rfay and boombatower are totally capable of designing something that's not a 1-off hack given everything I've already written, but if investing that time will help get this codebase into better shape for d.o deployment, it's worth it.

That said, if you really want to just go off with the sandbox and not bother, I won't take it personally.

wildkatana’s picture

I've been silently following this thread for a couple of months now, as it directly relates to my Google Summer of Code 2011 project: http://groups.drupal.org/node/145159 - Module and Theme Browser in Drupal Core. One of the things that I want the browser to be able to do is display module (and possibly theme) dependencies, and so something like this would be an important step so that drupal.org can expose that kind of data.

For my use case, this would work either as a part of Project itself or a part of Drupal.org specific code, but it seems like it would benefit more people if this were made a part of the Project module, since the API I am building will allow for externally hosted repositories of Projects (not just on drupal.org). Although not all people will be using the Project module to manage repositories of their own, there will be some who do, and this would benefit them.

However, I do need something like this sooner than later, since I want to include this in the Summer of Code project. I apologize that I don't have anything more to offer other than encouragement and a big +1 for this feature.

Thanks to everyone involved for your hard work on keeping this moving!

Sincerely,
Leighton Whiting

Owen Barton’s picture

StatusFileSize
new13.25 KB

A sandbox site http://projectinfo-drupal.redesign.devdrupal.org on staging.drupal.org has been set up, and I have set up this patch on the site. It all seems to work nicely (first try even!) - I have attached a log so you can see what I have done. If any other sandbox.d.o users would like to use this instance for working on this issue feel free. I am hoping to dig into this in more depth over the next couple of weeks, going over the code (I will keep an eye out for splitting of d.o specific code) and also looking at how to get project data easily available to drush.

rfay’s picture

Project:Project» Project Dependency
Version:6.x-1.x-dev»
Component:Projects» Code
Assigned:boombatower» Unassigned

I have turned this into a new and separate module, project_dependency. Moving this issue over there.

It seems that the rfay/refactor_to_fork_project branch is actually working already; the drush commands have changed to match the new project. The README.md has (the same) information about how to deploy it.

Setting up a test environment is just slightly different, as we need a *regular* project module as well as this module, project_dependency.

rfay’s picture

Created #1200486: Invoke hook_project_release_create_package() when a package is being created so that we'll know when a release node gets created.

Niklas Fiekas’s picture

Subscribe.

rfay’s picture

Version:» 6.x-1.x-dev
Status:Needs work» Needs review

OK - I promoted the project - it's now http://drupal.org/project/project_dependency. The work is now on the 6.x-1.x branch. And (edit) #1200486: Invoke hook_project_release_create_package() when a package is being created is ready to go in the Project module queue - maybe one of you can look and RTBC it.

Time to get this put to bed.

Again, the project URL has changed: http://drupal.org/project/project_dependency and of course the git URL has changed: git clone git://git.drupal.org/project/project_dependency.git --branch 6.x-1.x

bfroehle’s picture

@rfay: I assume you mean #1200486: Invoke hook_project_release_create_package() when a package is being created is ready to go in the Project module queue.

rfay’s picture

#1200486: Invoke hook_project_release_create_package() when a package is being created was committed.

@jthorson and I will try to get a sandbox set up to demonstrate this and then ask for its deployment.

I guess if there's not enough room on stagingvm or scratchvm, we could put it on a VM somewhere else.

Owen Barton’s picture

@rfay: There is already a sandbox set up for this - see #152 (not sure if there is enough room or not, but you could check).

rfay’s picture

@Owen, I thought I remembered that. May we use it? Sounds like a yes...

rfay’s picture

Status:Needs review» Fixed

You are all so quiet.

I'm requesting deployment in #1241704: Deploy Project Dependency, Project and PIFT on d.o

Marking this issue as fixed.

Status:Fixed» Closed (fixed)

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