We've decided to lose the -p0 off of patch, and make patches in a fairly normal Git way.

So the testbots will use patch -p1 to apply patches instead of patch -p0. Thus, patches created with any of these formats will work:

git diff <other_branch_or_commit>
git format-patch <other_branch_or_commit> --stdout

And many other formats; it's easy enough to make diff or cvs create a -p1 format as well.

If patches are created with git format-patch, maintainers can use git am to just do the commits for them right out of the patch.

CommentFileSizeAuthor
#9 pifr.allow_p0_or_p1_patches.patch12.32 KBrfay
#7 pifr.patch_format_1059990_06.patch1.08 KBrfay
#3 pifr.update_patches_to_p1.patch5 KBrfay
#1 pifr.patch_format_1059990_01.patch789 bytesrfay

Comments

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Active » Needs review
StatusFileSize
new pifr.patch_format_1059990_01.patch789 bytes

This small change should do the job.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Needs review » Fixed

Committed: http://drupal.org/cvs?commit=499042

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Fixed » Needs review
StatusFileSize
new pifr.update_patches_to_p1.patch5 KB

Oops - forgot that the various patches that are downloaded from qa have to be fixed for this. Here they are.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Needs review » Fixed

Committed to 6.x-3.x: http://drupal.org/cvs?commit=499052

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Fixed » Needs work

Not fixed. At least with syntax.patch, it doesn't catch the syntax error. The results we get are:

[08:14:31] Applied [syntax.patch] to:
 > a/install.php
 > b/install.php
[08:14:31] Invoking operation [syntax]...
[08:14:31] Checked [0 of 2] relevant file(s) for syntax.

And then it goes on to install (and fails miserably because we did introduce a syntax error).

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Needs work » Fixed
StatusFileSize
new pifr.patch_format_1059990_06.patch1.08 KB

It turned out that git.inc actually studies the internal format of the patch to see what files it has touched. So the regex that does that had to be adjusted. Hoping this will do it.

Committed: http://drupal.org/cvs?commit=499304

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Fixed » Needs work

berdir suggests that we could just try the patch with -p1 and if it failed, try it with -p0. It seems like an easy enough technique. Then both would be supported and all those patches wouldn't have to be rerolled.

This is an easier technique than I had envisioned and should be considered.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Needs work » Needs review
StatusFileSize
new pifr.allow_p0_or_p1_patches.patch12.32 KB

This patch attempts @berdir's approach. It turned out to be more complex than originally thought, but it is deployed on two testbots and seems to work.

Review appreciated from anybody who's willing to stick their head into this.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Needs review » Fixed

Committed: http://drupal.org/cvs?commit=502560

boombatower’s picture

boombatower commented

Would have like to been involved in this decision. Really don't understand it, among many of the changes I am seeing.

We talked about a select list of things and I come back to find loads of stuff...rather unnerving.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

@boombatower, the patch format was a decision of the git team, just implemented here. As far as I can tell, every other change was just finishing up git, adding instrumentation to solve real problems, and adding a UI to variables that didn't have one (so that it could be deployed successfully without shell access). There were still lots of things to be done to make git work.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Just noticed this: http://qa.drupal.org/pifr/test/130814

So it doesn't matter if it is -p0 or -p1, patches created with git format-patch* don't apply. And this is now the suggested way to create patches on d.o according to the git instructions tab...

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

@Berdir, actually your patch 0001-by-Berdir-Enforce-type-user-in-role.patch applies fine with the command the testbot is using. So something is definitely wrong... but it's not (AFAICT) git format-patch or patch -p1. I definitely agree about the conclusion you drew from the log. Nice catch.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

There are two issues here.

Surprisingly, patch v2.59 does not actually apply that patch correctly. After updating to patch v2.6, it was able to apply.

The second issue, though, is that #1079644-12: Recipient type not correctly implemented is supposed to be tested on 6.x-2.x (or at least that's what the issue is set to) and it does apply cleanly there. But instead it's being applied on 6.x-1.x (http://qa.drupal.org/pifr/test/130814)

And that seems to be because it was actually submitted as a 6.x-1.x patch (where it does not in fact apply). I have resubmitted it as a 6.x-2.x patch...

And Yippee! http://qa.drupal.org/pifr/test/130874

Berdir++

I fixed the pifr_setup script to update patch on install: https://github.com/scor/pifr_setup/commit/ea04b4d245c33bee34126b4fb5f970...

So leaving this issue as fixed.

Status: Fixed » Closed (fixed)

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