During install, {project_release_nodes} is creating records in {project_release_nodes} with bad data. E.g. for contrib projects that don't use version_minor in their version strings, the records are written with 0 values instead of NULL, which confuses project_release_table() once you start adding new releases.

CommentFileSizeAuthor
#3 drupalorg_testing_release_info.inc_.txt4.67 KBhunmonk
#1 do_testing_fix_release_node_saving.patch4.7 KBhunmonk
#1 drupalorg_testing_release_info.inc_.txt74.73 KBhunmonk

Comments

hunmonk’s picture

hunmonk commented
Status: Active » Needs review
StatusFileSize
new drupalorg_testing_release_info.inc_.txt74.73 KB
new do_testing_fix_release_node_saving.patch4.7 KB

we were trying to work around the fact that project_release_db_save() wasn't working correctly. this was because:

  1. some of the values in drupalorg_testing_release_info.inc were named incorrectly
  2. the CVS module was unsetting the file form field, so the file data wasn't getting put into $form_values

my solution for #2 is pretty hackish, but i'm not sure of a better way to do it, considering that we need to have CVS module enabled -- i welcome a better solution... :)

drupalorg_testing_release_info.inc needed to be rebuilt, so i've attached a patch for that as well...

dww’s picture

dww
we/he/they
commented
Status: Needs review » Needs work

A) This totally isn't going to work: function cvs_local_alter_project_release_form(&$form). The way that stuff works is a hack, I admit, but real test sites should be adding a symlink for cvs_local.inc that points to cvs_local.example.inc, and then you're going to get duplicate function definitions.

B) This hunk isn't going to do what you want:

-  $output .= '// $Id: drupalorg_testing_build_release_info.php,v 1.1 2008/10/12 15:45:23 aclight Exp $'."\n\n";
+  $output .= '// $Id: $'."\n\n";

If you don't want CVS to keep substituting that keyword, you have to do something like this:

  // Use a gratuitous string concatenation to fool CVS into leaving this keyword alone.
  $output .= '// $Id:' . '$'."\n\n";
hunmonk’s picture

hunmonk commented
Assigned: Unassigned » hunmonk
Status: Needs work » Needs review
StatusFileSize
new drupalorg_testing_release_info.inc_.txt4.67 KB

after further discussion w/ dww, we decided:

  1. my initial solution would cause problems for people installing over and over again in the same location for testing purposes.
  2. there is no sane way to restore the form data that CVS module is killing
  3. we'll just reinsert the file data in {project_release_nodes} in the case where file data exists
  4. going forward, we should be making our own CRUD API and getting away from using drupal_execute()

the patch still does fix the orginally reported problem -- as far as i can tell all data in {project_release_nodes} looks as it should now.

B) also makes sense, and i've included that in the patch as well.

drupalorg_testing_release_info.inc is unchanged, so just uploaded the patch to the profile again.

dww’s picture

dww
we/he/they
commented
Status: Needs review » Reviewed & tested by the community

Ok, after sorting out that all of these attachments are actually patch files and some of them are craftily named, if you apply http://drupal.org/files/issues/drupalorg_testing_release_info.inc_.txt from #3 and http://drupal.org/files/issues/drupalorg_testing_release_info.inc__0.txt from #1 (both of which are actually patches), then this seems to work. Ship it!

hunmonk’s picture

hunmonk commented
Status: Reviewed & tested by the community » Fixed

committed to HEAD.

Status: Fixed » Closed (fixed)

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