New release nodes loose version string and project association

I've created a similar test release for project module: http://d6.drupal.org/node/367004 I did not notice any problems, so headed over to your example. The problem was not immediately obvious, so I needed to make a shot to show where is the problem.

So the issue is that the core files table does not have the proper file names. Apart from that, the file seems to be created properly. So when the files table is fed with the files, the version is not properly added to the name. Here is the corresponding files table entry for the project_issue release.

object(stdClass)[106]
public 'fid' => string '3028104' (length=7)
public 'uid' => string '46549' (length=5)
public 'filename' => string 'project_issue-.tar.gz' (length=21)
public 'filepath' => string 'files/projects/project_issue-.tar.gz' (length=36)
public 'filemime' => string 'application/octet-stream' (length=24)
public 'filesize' => string '72194' (length=5)
public 'status' => string '1' (length=1)
public 'timestamp' => string '1234943366' (length=10)

Tried to track this down with the assumption that it would be a concatenation / variable rename issue in the packaging script. However realized that it already gets the data empty: "'SELECT * FROM project_release_nodes WHERE nid = 366998":

object(stdClass)[106]
  public 'nid' => string '366998' (length=6)
  public 'pid' => string '75232' (length=5)
  public 'version' => string '' (length=0)
  public 'tag' => string 'DRUPAL-6--1-0-ALPHA1' (length=20)
  public 'rebuild' => string '0' (length=1)
  public 'version_major' => string '1' (length=1)
  public 'version_minor' => null
  public 'version_patch' => string '0' (length=1)
  public 'version_extra' => string 'alpha1' (length=6)

Looks like the version_* fields are right, but the version field itself is empty.

This record is saved via project_release_db_save() which expects the version to be in $node->project_release['version']. This is what it's phpdoc says:

 *   ... Even
 *   though this is NOT a fully loaded $node object, the release-related
 *   values are in the $node->project_release array due to manual #tree and
 *   #parents hacking in project_release_form().

The version element is set up with this code:

  _project_release_form_add_text_element($form['rel_id']['version'], t('Version string'), $release->project_release['version'], $is_edit, $admin, TRUE, 20, 255);

  // The version string belongs in $node->project_release[], so we set
  // #parents to ensure the form value is placed there during validate,
  // preview, and submit.
  $form['rel_id']['version']['#parents'] = array('project_release', 'version');

However, when the HTML is generated, this field is neither under rel_id[] nor project_release[]:

<div class="form-item" id="edit-version-wrapper">
 <label for="edit-version">Version string: <span class="form-required" title="This field is required.">*</span></label>
 <input type="text" maxlength="40" name="version" id="edit-version" size="30" value="4.6.x-1.x-dev" readonly="1" style="width:auto" class="form-text required" />
</div>

I'd say this makes it end up disconnected to the project_release[] data. Hope this helps fixing the issue.

Gosh, Chad just pointed out that the commit above was not actually pushed into production. Heh. At least I got a tour around some obscure parts of the the release system. We are trying to push the change out and retest.

Ok, deployment of the change got the version field properly in the release table: 'SELECT * FROM project_release_nodes WHERE nid = 367006'

object(stdClass)[94]
public 'nid' => string '367006' (length=6)
public 'pid' => string '75232' (length=5)
public 'version' => string '4.6.x-1.x-dev' (length=13)
public 'tag' => string 'DRUPAL-4-6' (length=10)
public 'rebuild' => string '1' (length=1)
public 'version_major' => string '1' (length=1)
public 'version_minor' => null
public 'version_patch' => null
public 'version_extra' => string 'dev' (length=3)

This is a test release for an older project-issue version. This is better then before the push of the updated version, but still does not get the version to the actual file name: SELECT * FROM project_release_files WHERE nid = 367006:

object(stdClass)[94]
public 'fid' => string '3028104' (length=7)
public 'nid' => string '366998' (length=6)
public 'filehash' => string '562fd14f7db17478a856a36d9b259c5a' (length=32)

And the suffix is missing form the filename still: SELECT * FROM files WHERE fid = 3028104

object(stdClass)[94]
  public 'fid' => string '3028104' (length=7)
  public 'uid' => string '46549' (length=5)
  public 'filename' => string 'project_issue-.tar.gz' (length=21)
  public 'filepath' => string 'files/projects/project_issue-.tar.gz' (length=36)
  public 'filemime' => string 'application/octet-stream' (length=24)
  public 'filesize' => string '72194' (length=5)
  public 'status' => string '1' (length=1)
  public 'timestamp' => string '1234943366' (length=10)

Digging deeper, found that cvs.module tries to remove this file upload option so that the packager script can fill in the file info later:

  // Regardless of if we're on page #1 or #2, we want to remove the
  // file selector, since if we're doing this via a CVS tag, the file
  // will be filled in later by the packaging script.
  // TODO (feature): it'd be nice if this was optional, so that some
  // sites might want to still allow file attachments for releases...
  unset($form['project_release_files']);

However, there is still a file entry ending up in the database, so this is not effective. As far as I see, only project_release_node_submit() adds a file entry, and only if it sees a $new_file, so I suspect that something is going wrong with it still getting that $new_file.

That is only set however if either isset($form_state['project_release']['new_file']) or there was a temp file for this release. The new_file key is only set if there was a file uploaded with the key 'project_release_files', which the cvs module removal should prevent. The temp file code also looks like only acting if there was a file uploaded already.

Anyway, looks like despite the intention of cvs module, a file entry is added right away, and thus contains wrong data. Still needs fixing.