Problem/Motivation

Versioncontrol_project is a bunch of drupal.org-specific glue code between git and project. We need this to port Drupal.org to D7.

Level Of Effort

It is estimated to take 1 day to complete this task.

Comments

dww’s picture

Status: Active » Postponed

Agreed. And we need a 6.x-2.x branch to exist first. ;) Then a 6.x-2.0 release. Then we can really start thinking about this. But yeah, thanks for opening an issue for this and for curating the list at http://drupal.org/node/1006924

webchick’s picture

Issue tags: +git, +project, +drupal.org D7

Tagging for sprint.

lotyrin’s picture

Assigned: Unassigned » lotyrin
Status: Postponed » Active

Starting on this.

lotyrin’s picture

Status: Active » Needs review
StatusFileSize
new44.57 KB
new18.43 KB

And here's that. (Hah, and I said I was gonna go to bed.)

I haven't tested this, but I'm able to enable it (and drupalorg_git_gateway which depends on it) without error.

dww’s picture

Status: Needs review » Postponed

Thanks for getting this started! However, there's not much point in working on this until #834252: [meta] Port Project to Drupal 7 is done (or at least a lot further along) since many things are going to completely change in project and project_release...

halstead’s picture

@lotyrin thanks so much for this. It may not end up being compatible with project 7.x but it allowed me to continue porting and testing other modules. It also unblocked updating and testing Drupal.org-Git-Daemons.

halstead’s picture

@dww just said that this module is actually mostly compatible with the future project.module. A few things like hardcoded $type == 'project' need to be replaced with api calls that look up the correct entity types though.

dww’s picture

Status: Postponed » Active

Indeed project_release is still a big unknown. See #1551384: Port project_release to D7 -- it's unclear if we're going to field-ify or not. So, I'd hold off spending any time on versioncontrol_release at this point. Maybe you want to split that out into a separate issue that can be handled on a different timeframe.

But thanks to #1549854: Port project maintainers system all the project maintainer stuff is going to be the same in D7 as it is in D6. And I just skimmed through versioncontrol_project itself and it looks pretty portable at this point. See #1549538: Add a function to determine if a node or node type is a project instead of hardcoded checks on $node->type. Otherwise, I think this should be pretty straight-forward.

senpai’s picture

Title: Port Version Control Project to Drupal 7 » Port versioncontrol_project to D7
Version: 6.x-2.x-dev » 7.x-1.x-dev
Assigned: lotyrin » sdboyer
Priority: Major » Critical

Needed for the Drupal.org D7 Upgrade launch.

drumm’s picture

Specifically, I see

Notice: Undefined index: project in versioncontrol_release_project_release_form_alter_add() (line 394 of /var/www/dev/release-drupal_7.redesign.devdrupal.org/htdocs/sites/all/modules/versioncontrol_project/versioncontrol_release/versioncontrol_release.module).

on release node add/edit pages like http://release-drupal_7.redesign.devdrupal.org/node/add/project-release/...

drumm’s picture

For #10 specifically, you probably want project_release_get_release_project_nid(). I made this since the field isn't populated yet on node add.

sdboyer’s picture

Title: Port versioncontrol_project to D7 » [META] Port versioncontrol_project to D7

updating title to reflect meta status.

redhatmatt’s picture

Title: [META] Port versioncontrol_project to D7 » Port versioncontrol_project to D7

Estimated 12hr, spoke with Sam Boyer, a bit of unknowns in this issue.

dww’s picture

Title: Port versioncontrol_project to D7 » [META] Port versioncontrol_project to D7

- Restoring title from cross-post
- Is this including versioncontrol_project_release? If it includes both, is 12 hours realistic?
- What does it mean to give a meta issue an estimate? Is that for everything the meta issue touches and then we don't estimate the sub-tasks?

Thanks,
-Derek

senpai’s picture

Issue tags: +20hr

Bumping up an additional 8 hours to a total of 20hr because of versioncontrol_project_release is an unknown, and it deals heavily with formAPI.

Also, the actionable sub-tasks need to be estimated, not this meta issue, and the sub-tasks also need to be cross linked in this issue summary.

drumm’s picture

Status: Active » Needs review
StatusFileSize
new1.61 KB

This patch changes versioncontrol_release_ctools_plugin_directory() to versioncontrol_release_ctools_plugin_type() to keep up with the ctools plugin API and adds files to versioncontrol_release.info for the registry.

drumm’s picture

StatusFileSize
new10.62 KB

I pushed some code to drupalorg_versioncontrol to work with #16: http://drupalcode.org/project/drupalorg.git/commit/0526d15

Attached is another patch, apply #16 first, which allows the release node add page to load error-free and save the release without too many warnings. I think these two patches are good to commit, but more work needs to be done to line up the form structure, validation, and submission. Most notably, project_release has a Version field which is the full version string; project_release parses this and fills the component fields, like major and minor. I think this module can be simplified a bit.

Leaving assigned to sdboyer since this seems to work well enough for me to use it while working on #1800186: Update package-release-nodes.php to D7 in Drush.

drumm’s picture

I also filed #1878418: Queue release packaging at git code arrival for a new requirement.

dfletcher’s picture

Found several bugs involving the node API hooks in versioncontrol_project.module.

  • The functions versioncontrol_project_project_nodeapi_insert() and versioncontrol_project_project_nodeapi_delete() and incorrectly named, so the hooks are never fired.
  • The $node argument of those insert and delete hooks should not be a reference.
  • Both of those corrected insert/delete functions need logic to return early if the node type is not "project". The type-aware version of this hook can only be implemented in the module that owns projects nodes.
  • The versioncontrol_project_project_nodeapi_load() function is entirely superfluous, the correct implementation is already in versioncontrol_project_node_load().
dfletcher’s picture

Messed up the delete function name and fixed up the comments. Rerolled.

dww’s picture

Status: Needs review » Needs work

Thanks for pushing this forward. No time for a thorough review, but based on your comment and a quick skim of the patch, I saw something important. In D7, there's not just 1 node type called "project", so this isn't going to work:

+  if ($node->type != 'project') {

Instead, you want this:

  if (project_node_is_project($node)) {

Cheers,
-Derek

dfletcher’s picture

Status: Needs work » Needs review
StatusFileSize
new1.99 KB

Ok thank you Derek. Rerolled.

dfletcher’s picture

I started looking at versioncontrol_release and quickly ran into an issue where the code there is expecting a multistep form, which is how it works on the http://drupal.org/node/add/project-release

In my test setup this multistep form isn't happening by default. Does anyone know what module is doing this on the current drupal.org, and is that the same path forward for this form in 7?

I noticed that the project node links to node/add/project-release/[project nid] which might be used to circumvent the multistep when arg(3) is present. But that doesn't help if someone navigates to node/add/project-release without arg three.

I'm in #drupal-infrastructure if anyone wants to discuss this.

dfletcher’s picture

StatusFileSize
new20.74 KB

Ok this rolls up the previous patches into one big one.

It contains the following changes:

  • Drumm's patch from comments 16, 17 - fixes to versioncontrol_release
  • My patch, comments 19, 20, 22 - node API functions in versioncontrol_project
  • An attempt from myself to break out versioncontrol_release_nodeapi_OLD() into modern equivalents. Had to fix up versioncontrol_release_get_release_label() a bit to get this working, contained a query placeholder that didn't work and was calling db_fetch_array().
  • Another fix in versioncontrol_project, an error with the query in versioncontrol_project_user_view() that was breaking user pages. Replaced this query with a db_select() equivalent that joins the correct new tables: the data from project_projects is now in field_data_field_project_type.
marvil07’s picture

Assigned: sdboyer » Unassigned
Priority: Critical » Normal

While I was taking a look to the repository I saw that many commits by sdboyer and bdragon are already on the 7.x-1.x branch, so there is some code already ported.

Based on that, I guess it is ok to follow including incremental patches.

dfletcher: thanks for joining patches :-)

drumm: I have reviewed/done-manual-testing on versioncontrol_project module changes, and they look fine, but I did not really looked at versioncontrol_release changes in detail, if you (or other person willing to) can review that, I guess we can include the code. Again, many incremental commits in there, so let's follow it.

Some pending bits for the future (uncomplete list):

  • Make versioncontrol_project_enable() generic.
  • Convert views to follow changes in versioncontrol.
  • Fix versioncontrol_project_committers_page() (tablesort_sql()was removed from core in d7).

PS: Why is bot not testing this? (7.x-1.x testing is active)

drumm’s picture

Status: Needs review » Active

Yes, incremental changes are the way to go.

Pushed this and removed @todo db_select(). We don't care if queries are DBTNG or not, as long as they work.

drumm’s picture

marvil07’s picture

In the process of taking a look at #1878418: Queue release packaging at git code arrival I found a bug, so I just added the fix directly: hook_ctools_plugin_type() is needed, but it has a different propose than hook_ctools_plugin_directory(), so I re-added it.

drumm’s picture

I went ahead and made some commits to fix up versioncontrol_release. This module still needs all references to the {project_release_nodes} table replaced with using fields. {project_release_nodes}.tag is now field_release_vcs_label. There is also a reference to the old files table. And some queries that say they need review. All have code comments.

drumm’s picture

Priority: Normal » Critical
marvil07’s picture

I have added several commits to upstream fixing what is mentioned on comment 29.

f4e756a Issue #1007102: Use project instead project_project as node type.
9c6b382 Issue #1007102: Reimplement versioncontrol_release_versioncontrol_repository_pre_resync.
38c2910 Issue #1007102: Fix/simplify versioncontrol_release_project_release_form_alter_edit
b86fb72 Issue #1007102: Remove todo's comments from reviewed queries.
312a48b Issue #1007102: Rewrite one file query using an entity wrapper at versioncontrol_release_project_release_form_alter_edit.
drumm’s picture

Everything looks good except for

f4e756a Issue #1007102: Use project instead project_project as node type.

There are multiple project node types with arbitrary names. The query should be IN project_project_node_types().

marvil07’s picture

Thanks for the quick update, yep, fixed that problem:

d8dea59 Issue #1007102 by drumm, marvil07: Fix query on versioncontrol_project_enable.
32fcbaf Issue #1007102: Fix query results retrieve on versioncontrol_project_enable.
drumm’s picture

Assigned: Unassigned » marvil07
Status: Active » Fixed

Looks good. I can't find anything else to fix right now, so let's call this fixed. More specific issues should be opened if found in QA.

Status: Fixed » Closed (fixed)
Issue tags: -git, -project, -drupal.org D7, -20hr

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

Anonymous’s picture

Issue summary: View changes

Versioncontrol_project is a bunch of drupal.org-specific glue code between git and project. We need this to port Drupal.org to D7.