Bug 26299 – svn-[un]apply should be updated to work with git

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

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.

[David Kilzer (:ddkilzer)]

- 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?

[Eric Seidel (no email)]

Created attachment 31756 [details]
Make svn-apply work with Git too


---
 2 files changed, 123 insertions(+), 29 deletions(-)

[Eric Seidel (no email)]

Created attachment 31757 [details]
Make svn-apply work with Git too


---
 2 files changed, 123 insertions(+), 29 deletions(-)

[Eric Seidel (no email)]

/me asks nicely for a review. :)

[Eric Seidel (no email)]

Created attachment 31831 [details]
Make svn-apply work with Git and return non-zero on patch failure

---
 2 files changed, 144 insertions(+), 31 deletions(-)

[Eric Seidel (no email)]

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! ;)

[Eric Seidel (no email)]

*** Bug 26300 has been marked as a duplicate of this bug. ***

[Tor Arne Vestbø]

Comment on attachment 31831 [details]
Make svn-apply work with Git and return non-zero on patch failure

Looks good to me

[Eric Seidel (no email)]

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

[David Kilzer (:ddkilzer)]

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?

[David Kilzer (:ddkilzer)]

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.