Bug 26299 - svn-[un]apply should be updated to work with git
Summary: svn-[un]apply should be updated to work with git
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 26300 (view as bug list)
Depends on:
Blocks: 26283
  Show dependency treegraph
 
Reported: 2009-06-10 15:41 PDT by Eric Seidel (no email)
Modified: 2009-06-25 04:00 PDT (History)
5 users (show)

See Also:


Attachments
Make svn-apply work with Git too (9.06 KB, patch)
2009-06-23 16:44 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Make svn-apply work with Git too (9.04 KB, patch)
2009-06-23 16:48 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Make svn-apply work with Git and return non-zero on patch failure (10.06 KB, patch)
2009-06-24 20:38 PDT, Eric Seidel (no email)
vestbo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2009-06-10 15:41:19 PDT
svn-apply should be updated to work with git

it's our magic patch function.  Git should get to share the magic! :)  That, and I'm writing scripts to use it, and I would like my scripts to work for git users too.
Comment 1 David Kilzer (:ddkilzer) 2009-06-10 16:05:13 PDT
- The svn-unapply script should also be updated.

- If svn-apply also works with git, it should probably be called something different.  Suggestions?  Maybe patch-apply?
Comment 2 Eric Seidel (no email) 2009-06-23 16:44:19 PDT
Created attachment 31756 [details]
Make svn-apply work with Git too


---
 2 files changed, 123 insertions(+), 29 deletions(-)
Comment 3 Eric Seidel (no email) 2009-06-23 16:48:14 PDT
Created attachment 31757 [details]
Make svn-apply work with Git too


---
 2 files changed, 123 insertions(+), 29 deletions(-)
Comment 4 Eric Seidel (no email) 2009-06-24 13:09:58 PDT
/me asks nicely for a review. :)
Comment 5 Eric Seidel (no email) 2009-06-24 20:38:09 PDT
Created attachment 31831 [details]
Make svn-apply work with Git and return non-zero on patch failure

---
 2 files changed, 144 insertions(+), 31 deletions(-)
Comment 6 Eric Seidel (no email) 2009-06-24 20:38:51 PDT
Comment on attachment 31757 [details]
Make svn-apply work with Git too

No luck getting this one reviewed, so I decided to solve bug 26300 too in hopes of getting them both in quicker! ;)
Comment 7 Eric Seidel (no email) 2009-06-24 20:39:16 PDT
*** Bug 26300 has been marked as a duplicate of this bug. ***
Comment 8 Tor Arne Vestbø 2009-06-24 23:57:09 PDT
Comment on attachment 31831 [details]
Make svn-apply work with Git and return non-zero on patch failure

Looks good to me
Comment 9 Eric Seidel (no email) 2009-06-25 00:45:45 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebKitTools/ChangeLog
	M	WebKitTools/Scripts/svn-apply
Committed r45153
http://trac.webkit.org/changeset/45153
Comment 10 David Kilzer (:ddkilzer) 2009-06-25 03:19:09 PDT
Comment on attachment 31831 [details]
Make svn-apply work with Git and return non-zero on patch failure

>+        I did not update svn-unapply, because it makes no sense in a Git world.
>+        You don't roll in and out patch files.  You make commits and deal with those.
>+        Git users can just git reset --hard to get the same functionality.

I think you missed the point of svn-unapply, although it still makes little sense for git users.  Some Subversion users may have more than one patch in their tree at the same time, e.g., working on two bugs at once (or testing two patches together), so being able to unapply one is useful in that case.

With git, it's usually easier to create a branch for each patch, and create a third branch for testing (with a cherry-picked commit/merged branch) if more than one patch needs to be tested together.

>+# These should be replaced by an scm class/module:
>+sub scmKnowsOfFile($);
>+sub scmCopy($$);
>+sub scmAdd($);
>+sub scmRemove($);

VCSUtils.pm is that module.

>+my $force = 0;
>+
>+my $optionParseSuccess = GetOptions(
>+    "merge!" => \$merge,
>+    "help!" => \$showHelp,
>+    "reviewer=s" => \$reviewer,
>+    "force!" => \$force
>+);

The $force switch doesn't actually do anything.  Did you upload the wrong patch?
Comment 11 David Kilzer (:ddkilzer) 2009-06-25 04:00:55 PDT
Comment on attachment 31831 [details]
Make svn-apply work with Git and return non-zero on patch failure

>         } else {
>             # Addition
>             rename($fullPath, "$fullPath.orig") if -e $fullPath;
>             applyPatch($patch, $fullPath);
>             unlink("$fullPath.orig") if -e "$fullPath.orig" && checksum($fullPath) eq checksum("$fullPath.orig");
>-            system "svn", "add", $fullPath;
>-            system "svn", "stat", "$fullPath.orig" if -e "$fullPath.orig";
>+            scmAdd($fullPath);
>+            # What is this for?
>+            system "svn", "stat", "$fullPath.orig" if $isSVN && -e "$fullPath.orig";
>         }

If the *.orig file is not removed, that shows the user that there is a *.orig file left over from applying the patch.