Phasing out -p0 patches - May 31?
When we did the Git migration, we set up the testbots to continue to allow both -p0 (old style Drupal patches) and -p1 (the normal Git style patches), but started encouraging the community to switch. This allowed existing patches on issues to continue to test OK, and avoided irritating everybody enormously.
However, now we have a problem. How do we teach people to successfully apply patches if there can be more than one kind? I think we have to phase out -p0 patches sooner rather than later. This would mean:
- Disabling -p0 testing on the testbot, meaning that -p0 patches would fail to apply
- Announcing that -p1 must become the standard in the issue queue.
Would a May 31 phase-out date be OK with everybody? Are you seeing new -p0 patches in the queue?
Groups:
Login or register to post comments
I still see quite a lot of p0
I still see quite a lot of p0 patches in the queues. In fact, on issues where a patch is already in p0 form, I've found myself continuing to post updates to that patch in p0 format so as not to confuse people already on the issue.
It seems that announcing a phase out date, and then rigorously marking p0 patches as 'needs work' with appropriate links to the p1 docs is the only way to completely get rid of p0 style patches.
p1 is git diff default, so
p1 is git diff default, so that's what I use.
I go by Josh The Geek.
Drupal.org
GitHub
+1 to setting a date and
+1 to setting a date and encouraging people to standardize
I don't care so much when the date is but that sounds fine :-)
This needs to be done so
This needs to be done so let's go ahead and rip the band-aid off. +1
How?
It's called "script" and fundamentally works identically everywhere:
Elsewhere (untested):
#!/bin/bash
if [ $('grep -qe "^--- a/" $1') ]; then
patch -p0 < $1
else
patch -p1 < $1
fi
Windows:
@echo off
set CYGWIN=nontsec
set patchdir=%~dp1
set patchfile=%~nx1
cd /d "%patchdir%"
rem Determine amount of path prefixes to strip.
rem -p0 needed to cleanly apply own patches and patches from Drupal.org.
rem -p1 needed for git a/b patches.
grep -qe "^--- a/" "%patchfile%"
if not "%errorlevel%"=="0" (
patch -p0 < "%patchfile%"
) else (
patch -p1 < "%patchfile%"
)
The latter soon being available in http://drupal.org/sandbox/sun/1117610
Daniel F. Kudwien
unleashed mind
This is what the testbots are currently doing
This is what the testbots are currently doing.
The problem is that we can't train people to handle patches that might be in multiple formats. So to make the documentation clear, with no disclaimers like (and look inside the patch to see if it has a/ or b/, or use Sun's script) we need to go with one patch format.
Edit: Actually, this isn't what they're currently doing. They actually try p1 and if it fails try p0. It's a more authoritative result. Analyzing the patch had some real gotchas.
FWIW Filterdiff is an awesome
FWIW Filterdiff is an awesome tool for dealing with stripping and adding prefixes to patches.
I support this.
The Git migration went live at the end of February, after which time -p1 became the official documented standard. If a -p0 patch still exists as the "latest" patch in an issue post-May 31, that means (in most cases) that the solution hasn't been touched in more than 3 months. At that point, it probably could use a second set of eyes anyway, and a re-roll could act as a legitimate way to bump it back up into peoples' consciousness.
However, this won't actually catch and "code needs work" all -p0 patches, because only a small fraction of projects are testbot-enabled. So I don't think there's a way to get around documenting both, or at least adding a "Did this not work for you? Try the patch troubleshooting faq" or similar in the Git instructions tab.
Drush make and p1 compatibility
One thing worth noting, Drush Make (stable 2.2) does not yet support p1 patches. There is an issue for 3.x, and a working patch, but since Drupal.org uses drush make to build install profiles, it seems critical that this patch be back-ported to 2.x prior to May 31.
The drush make issue is
The drush make issue is important, but already most patches being used are -p1, so this change really is irrelevant. All we're really going to do is say "The testbot is no longer trying to test -p0 and you should use standard git patches all the time". Neither of these affects drush make, which is already broken.
Or do I misunderstand?
Drush make is not broken, our
Drush make is not broken, our .make file validation code for Install Profiles is broken right now: http://drupal.org/node/751242
Even if that issue were resolved prior to the p1 deadline it wouldn't matter since Drupal.org doesn't allow patches or third party downloads in it's make files anyway. That'll be added someday, but when it is, we'll update the whole mess to a newer drush and drush make version.
What's the rational for
What's the rational for standardizing on -p1 instead of -p0?
I really hate seeing all those 'a's and 'b's all the time.
Git uses -p1 by default
Since we're now using git, it seemed reasonable for people to be able to use "git apply" to apply patches and "git diff" or "git format-patch" to create them.
Fair enough!
Fair enough!
Update on testbot code deployment
I have the code nearly ready to deploy on the testbots and hope to do it next week. -p0 patches are failed in the queue and have a status like "this might be a -p0 patch..."
Why do we have to insist on -p1?
git applywill accept -p1 and most -p0 format patches. However, the testbot currently doesgit apply -p1, which makes Git reject all -p0 format patches.Why do we restrict the acceptable patch formats beyond what Git would do by default?