Refactored pre-commit hook to make it less invasive.

Hooks are now obtained via a convenient API like git commands, and
callers don't have to check for their existence.
The pre-commit hook has been updated accordingly.

Change-Id: I3383ffb10e2f3b588d7367b9139b606ec7f62758
Signed-off-by: Laurent Delaigue <laurent.delaigue@obeo.fr>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

5 files changed, 40 insertions, 178 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
index 875e12f57b..de09ad4f7c 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
@@ -660,8 +660,8 @@ public abstract class FS {
 	 *
 	 * @param repository
 	 *            The repository for which a hook should be run.
-	 * @param hook
-	 *            The hook to be executed.
+	 * @param hookName
+	 *            The name of the hook to be executed.
 	 * @param args
 	 *            Arguments to pass to this hook. Cannot be <code>null</code>,
 	 *            but can be an empty array.
@@ -669,11 +669,12 @@ public abstract class FS {
 	 * @throws JGitInternalException
 	 *             if we fail to run the hook somehow. Causes may include an
 	 *             interrupted process or I/O errors.
-	 * @since 3.7
+	 * @since 4.0
 	 */
-	public ProcessResult runIfPresent(Repository repository, final Hook hook,
+	public ProcessResult runHookIfPresent(Repository repository,
+			final String hookName,
 			String[] args) throws JGitInternalException {
-		return runIfPresent(repository, hook, args, System.out, System.err,
+		return runHookIfPresent(repository, hookName, args, System.out, System.err,
 				null);
 	}
 
@@ -683,8 +684,8 @@ public abstract class FS {
 	 *
 	 * @param repository
 	 *            The repository for which a hook should be run.
-	 * @param hook
-	 *            The hook to be executed.
+	 * @param hookName
+	 *            The name of the hook to be executed.
 	 * @param args
 	 *            Arguments to pass to this hook. Cannot be <code>null</code>,
 	 *            but can be an empty array.
@@ -703,9 +704,10 @@ public abstract class FS {
 	 * @throws JGitInternalException
 	 *             if we fail to run the hook somehow. Causes may include an
 	 *             interrupted process or I/O errors.
-	 * @since 3.7
+	 * @since 4.0
 	 */
-	public ProcessResult runIfPresent(Repository repository, final Hook hook,
+	public ProcessResult runHookIfPresent(Repository repository,
+			final String hookName,
 			String[] args, PrintStream outRedirect, PrintStream errRedirect,
 			String stdinArgs) throws JGitInternalException {
 		return new ProcessResult(Status.NOT_SUPPORTED);
@@ -713,13 +715,13 @@ public abstract class FS {
 
 	/**
 	 * See
-	 * {@link #runIfPresent(Repository, Hook, String[], PrintStream, PrintStream, String)}
+	 * {@link #runHookIfPresent(Repository, String, String[], PrintStream, PrintStream, String)}
 	 * . Should only be called by FS supporting shell scripts execution.
 	 *
 	 * @param repository
 	 *            The repository for which a hook should be run.
-	 * @param hook
-	 *            The hook to be executed.
+	 * @param hookName
+	 *            The name of the hook to be executed.
 	 * @param args
 	 *            Arguments to pass to this hook. Cannot be <code>null</code>,
 	 *            but can be an empty array.
@@ -738,13 +740,13 @@ public abstract class FS {
 	 * @throws JGitInternalException
 	 *             if we fail to run the hook somehow. Causes may include an
 	 *             interrupted process or I/O errors.
-	 * @since 3.7
+	 * @since 4.0
 	 */
-	protected ProcessResult internalRunIfPresent(Repository repository,
-			final Hook hook, String[] args, PrintStream outRedirect,
+	protected ProcessResult internalRunHookIfPresent(Repository repository,
+			final String hookName, String[] args, PrintStream outRedirect,
 			PrintStream errRedirect, String stdinArgs)
 			throws JGitInternalException {
-		final File hookFile = findHook(repository, hook);
+		final File hookFile = findHook(repository, hookName);
 		if (hookFile == null)
 			return new ProcessResult(Status.NOT_PRESENT);
 
@@ -764,11 +766,11 @@ public abstract class FS {
 		} catch (IOException e) {
 			throw new JGitInternalException(MessageFormat.format(
 					JGitText.get().exceptionCaughtDuringExecutionOfHook,
-					hook.getName()), e);
+					hookName), e);
 		} catch (InterruptedException e) {
 			throw new JGitInternalException(MessageFormat.format(
 					JGitText.get().exceptionHookExecutionInterrupted,
-					hook.getName()), e);
+							hookName), e);
 		}
 	}
 
@@ -778,15 +780,15 @@ public abstract class FS {
 	 *
 	 * @param repository
 	 *            The repository within which to find a hook.
-	 * @param hook
-	 *            The hook we're trying to find.
+	 * @param hookName
+	 *            The name of the hook we're trying to find.
 	 * @return The {@link File} containing this particular hook if it exists in
 	 *         the given repository, <code>null</code> otherwise.
-	 * @since 3.7
+	 * @since 4.0
 	 */
-	public File findHook(Repository repository, final Hook hook) {
+	public File findHook(Repository repository, final String hookName) {
 		final File hookFile = new File(new File(repository.getDirectory(),
-				Constants.HOOKS), hook.getName());
+				Constants.HOOKS), hookName);
 		return hookFile.isFile() ? hookFile : null;
 	}
 
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java
index ee29584239..a6984baa37 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java
@@ -126,13 +126,13 @@ public abstract class FS_POSIX extends FS {
 	}
 
 	/**
-	 * @since 3.7
+	 * @since 4.0
 	 */
 	@Override
-	public ProcessResult runIfPresent(Repository repository, Hook hook,
+	public ProcessResult runHookIfPresent(Repository repository, String hookName,
 			String[] args, PrintStream outRedirect, PrintStream errRedirect,
 			String stdinArgs) throws JGitInternalException {
-		return internalRunIfPresent(repository, hook, args, outRedirect,
+		return internalRunHookIfPresent(repository, hookName, args, outRedirect,
 				errRedirect, stdinArgs);
 	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32_Cygwin.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32_Cygwin.java
index d0abd33275..000bdcef5c 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32_Cygwin.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32_Cygwin.java
@@ -149,13 +149,13 @@ public class FS_Win32_Cygwin extends FS_Win32 {
 	}
 
 	/**
-	 * @since 3.7
+	 * @since 4.0
 	 */
 	@Override
-	public ProcessResult runIfPresent(Repository repository, Hook hook,
+	public ProcessResult runHookIfPresent(Repository repository, String hookName,
 			String[] args, PrintStream outRedirect, PrintStream errRedirect,
 			String stdinArgs) throws JGitInternalException {
-		return internalRunIfPresent(repository, hook, args, outRedirect,
+		return internalRunHookIfPresent(repository, hookName, args, outRedirect,
 				errRedirect, stdinArgs);
 	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/Hook.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/Hook.java
deleted file mode 100644
index c24c9a3d0a..0000000000
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/Hook.java
+++ /dev/null
@@ -1,149 +0,0 @@
-/*
- * Copyright (C) 2014 Obeo.
- * and other copyright owners as documented in the project's IP log.
- *
- * This program and the accompanying materials are made available
- * under the terms of the Eclipse Distribution License v1.0 which
- * accompanies this distribution, is reproduced below, and is
- * available at http://www.eclipse.org/org/documents/edl-v10.php
- *
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or
- * without modification, are permitted provided that the following
- * conditions are met:
- *
- * - Redistributions of source code must retain the above copyright
- *   notice, this list of conditions and the following disclaimer.
- *
- * - Redistributions in binary form must reproduce the above
- *   copyright notice, this list of conditions and the following
- *   disclaimer in the documentation and/or other materials provided
- *   with the distribution.
- *
- * - Neither the name of the Eclipse Foundation, Inc. nor the
- *   names of its contributors may be used to endorse or promote
- *   products derived from this software without specific prior
- *   written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
- * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
- * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
- * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
- * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
- * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
- * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
- * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
- * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
- * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-package org.eclipse.jgit.util;
-
-/**
- * An enum describing the different hooks a user can implement to customize his
- * repositories.
- *
- * @since 3.7
- */
-public enum Hook {
-	/**
-	 * Literal for the "pre-commit" git hook.
-	 * <p>
-	 * This hook is invoked by git commit, and can be bypassed with the
-	 * "no-verify" option. It takes no parameter, and is invoked before
-	 * obtaining the proposed commit log message and making a commit.
-	 * </p>
-	 * <p>
-	 * A non-zero exit code from the called hook means that the commit should be
-	 * aborted.
-	 * </p>
-	 */
-	PRE_COMMIT("pre-commit"), //$NON-NLS-1$
-
-	/**
-	 * Literal for the "prepare-commit-msg" git hook.
-	 * <p>
-	 * This hook is invoked by git commit right after preparing the default
-	 * message, and before any editing possibility is displayed to the user.
-	 * </p>
-	 * <p>
-	 * A non-zero exit code from the called hook means that the commit should be
-	 * aborted.
-	 * </p>
-	 */
-	PREPARE_COMMIT_MSG("prepare-commit-msg"), //$NON-NLS-1$
-
-	/**
-	 * Literal for the "commit-msg" git hook.
-	 * <p>
-	 * This hook is invoked by git commit, and can be bypassed with the
-	 * "no-verify" option. Its single parameter is the path to the file
-	 * containing the prepared commit message (typically
-	 * "&lt;gitdir>/COMMIT-EDITMSG").
-	 * </p>
-	 * <p>
-	 * A non-zero exit code from the called hook means that the commit should be
-	 * aborted.
-	 * </p>
-	 */
-	COMMIT_MSG("commit-msg"), //$NON-NLS-1$
-
-	/**
-	 * Literal for the "post-commit" git hook.
-	 * <p>
-	 * This hook is invoked by git commit. It takes no parameter and is invoked
-	 * after a commit has been made.
-	 * </p>
-	 * <p>
-	 * The exit code of this hook has no significance.
-	 * </p>
-	 */
-	POST_COMMIT("post-commit"), //$NON-NLS-1$
-
-	/**
-	 * Literal for the "post-rewrite" git hook.
-	 * <p>
-	 * This hook is invoked after commands that rewrite commits (currently, only
-	 * "git rebase" and "git commit --amend"). It a single argument denoting the
-	 * source of the call (one of <code>rebase</code> or <code>amend</code>). It
-	 * then accepts a list of rewritten commits through stdin, in the form
-	 * <code>&lt;old SHA-1> &lt;new SHA-1>LF</code>.
-	 * </p>
-	 * <p>
-	 * The exit code of this hook has no significance.
-	 * </p>
-	 */
-	POST_REWRITE("post-rewrite"), //$NON-NLS-1$
-
-	/**
-	 * Literal for the "pre-rebase" git hook.
-	 * <p>
-	 * </p>
-	 * This hook is invoked right before the rebase operation runs. It accepts
-	 * up to two parameters, the first being the upstream from which the branch
-	 * to rebase has been forked. If the tip of the series of commits to rebase
-	 * is HEAD, the other parameter is unset. Otherwise, that tip is passed as
-	 * the second parameter of the script.
-	 * <p>
-	 * A non-zero exit code from the called hook means that the rebase should be
-	 * aborted.
-	 * </p>
-	 */
-	PRE_REBASE("pre-rebase"); //$NON-NLS-1$
-
-	private final String name;
-
-	private Hook(String name) {
-		this.name = name;
-	}
-
-	/**
-	 * @return The name of this hook.
-	 */
-	public String getName() {
-		return name;
-	}
-}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/ProcessResult.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/ProcessResult.java
index f56bb1577e..77c9608c79 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/ProcessResult.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/ProcessResult.java
@@ -109,4 +109,13 @@ public class ProcessResult {
 	public Status getStatus() {
 		return status;
 	}
+
+	/**
+	 * @return <code>true</code> if the execution occurred and resulted in a
+	 *         return code different from 0, <code>false</code> otherwise.
+	 * @since 4.0
+	 */
+	public boolean isExecutedWithError() {
+		return getStatus() == ProcessResult.Status.OK && getExitCode() != 0;
+	}
 }