SCM-706 finer-grained handling of file rename status for gitexe provi…

…der, rename source is not added to the set of files for commit operation anymore
apache · Sep 20, 2015 · 65ff544 · 65ff544
1 parent e59eec4
commit 65ff544
Show file tree

Hide file tree

Showing 5 changed files with 103 additions and 71 deletions.
diff --git a/maven-scm-api/src/main/java/org/apache/maven/scm/ScmFileStatus.java b/maven-scm-api/src/main/java/org/apache/maven/scm/ScmFileStatus.java
@@ -59,10 +59,23 @@ public final class ScmFileStatus
 
     /**
      * The file has been renamed or moved in the working tree.
+     * Used by SCM that do not indicate if the renamed path is the old or new path.
      * @since 1.7
      */
     public static final ScmFileStatus RENAMED = new ScmFileStatus( "renamed" );
 
+    /**
+     * The file has been renamed or moved in the working tree. This is the source of rename operation.
+     * @since 1.9
+     */
+    public static final ScmFileStatus RENAMED_FROM = new ScmFileStatus( "renamed-from" );
+
+    /**
+     * The file has been renamed or moved in the working tree. This is the target of rename operation.
+     * @since 1.9
+     */
+    public static final ScmFileStatus RENAMED_TO = new ScmFileStatus( "renamed-to" );
+
     /**
      * The file has been copied in the working tree.
      * @since 1.7

diff --git a/...main/java/org/apache/maven/scm/provider/git/gitexe/command/checkin/GitCheckInCommand.java b/...main/java/org/apache/maven/scm/provider/git/gitexe/command/checkin/GitCheckInCommand.java
@@ -27,16 +27,17 @@
 import org.apache.maven.scm.ScmVersion;
 import org.apache.maven.scm.command.checkin.AbstractCheckInCommand;
 import org.apache.maven.scm.command.checkin.CheckInScmResult;
+import org.apache.maven.scm.command.status.StatusScmResult;
 import org.apache.maven.scm.log.ScmLogger;
 import org.apache.maven.scm.provider.ScmProviderRepository;
 import org.apache.maven.scm.provider.git.command.GitCommand;
-import org.apache.maven.scm.provider.git.repository.GitScmProviderRepository;
-import org.apache.maven.scm.provider.git.util.GitUtil;
 import org.apache.maven.scm.provider.git.gitexe.command.GitCommandLineUtils;
 import org.apache.maven.scm.provider.git.gitexe.command.add.GitAddCommand;
 import org.apache.maven.scm.provider.git.gitexe.command.branch.GitBranchCommand;
 import org.apache.maven.scm.provider.git.gitexe.command.status.GitStatusCommand;
 import org.apache.maven.scm.provider.git.gitexe.command.status.GitStatusConsumer;
+import org.apache.maven.scm.provider.git.repository.GitScmProviderRepository;
+import org.apache.maven.scm.provider.git.util.GitUtil;
 import org.codehaus.plexus.util.FileUtils;
 import org.codehaus.plexus.util.cli.CommandLineUtils;
 import org.codehaus.plexus.util.cli.Commandline;
@@ -81,12 +82,32 @@ protected CheckInScmResult executeCheckInCommand( ScmProviderRepository repo, Sc
 
         try
         {
-            if ( !fileSet.getFileList().isEmpty() )
+            List<File> addedOrModifiedFiles = new ArrayList<File>();
+            GitStatusCommand statusCmd = new GitStatusCommand();
+            statusCmd.setLogger( getLogger() );
+            StatusScmResult status = statusCmd.executeStatusCommand( repo, fileSet );
+            List<ScmFile> statusFiles = status.getChangedFiles();
+            for ( ScmFile file : statusFiles )
+            {
+                // we can not use 'git add' on deleted files, the 'git rm' already removes the working
+                // copy and notes the deletion in the index.  So we in effect filer the fileSet here
+                // for the added and modified files to add to the index.
+                if ( file.getStatus() != ScmFileStatus.DELETED && file.getStatus() != ScmFileStatus.UNKNOWN
+                     && file.getStatus() != ScmFileStatus.RENAMED_FROM )
+                {
+                    addedOrModifiedFiles.add( new File( file.getPath() ) );
+                }
+            }
+
+            ScmFileSet fileSetAddedOrModified = new ScmFileSet( fileSet.getBasedir(), addedOrModifiedFiles );
+
+            if ( !fileSetAddedOrModified.getFileList().isEmpty() )
             {
                 // if specific fileSet is given, we have to git-add them first
                 // otherwise we will use 'git-commit -a' later
 
-                Commandline clAdd = GitAddCommand.createCommandLine( fileSet.getBasedir(), fileSet.getFileList() );
+                Commandline clAdd = GitAddCommand.createCommandLine( fileSetAddedOrModified.getBasedir(),
+                                                                     fileSetAddedOrModified.getFileList() );
 
                 exitCode = GitCommandLineUtils.execute( clAdd, stdout, stderr, getLogger() );
 
@@ -95,7 +116,6 @@ protected CheckInScmResult executeCheckInCommand( ScmProviderRepository repo, Sc
                     return new CheckInScmResult( clAdd.toString(), "The git-add command failed.", stderr.getOutput(),
                                                  false );
                 }
-
             }
 
             // SCM-709: statusCommand uses repositoryRoot instead of workingDirectory, adjust it with

diff --git a/...c/main/java/org/apache/maven/scm/provider/git/gitexe/command/status/GitStatusCommand.java b/...c/main/java/org/apache/maven/scm/provider/git/gitexe/command/status/GitStatusCommand.java
@@ -19,8 +19,6 @@
  * under the License.
  */
 
-import java.net.URI;
-
 import org.apache.maven.scm.ScmException;
 import org.apache.maven.scm.ScmFileSet;
 import org.apache.maven.scm.command.status.AbstractStatusCommand;
@@ -32,6 +30,8 @@
 import org.codehaus.plexus.util.cli.CommandLineUtils;
 import org.codehaus.plexus.util.cli.Commandline;
 
+import java.net.URI;
+
 /**
  * @author <a href="mailto:brett@apache.org">Brett Porter</a>
  *
@@ -41,7 +41,7 @@ public class GitStatusCommand
     implements GitCommand
 {
     /** {@inheritDoc} */
-    protected StatusScmResult executeStatusCommand( ScmProviderRepository repo, ScmFileSet fileSet )
+    public StatusScmResult executeStatusCommand( ScmProviderRepository repo, ScmFileSet fileSet )
         throws ScmException
     {
         Commandline clRevparse = createRevparseShowToplevelCommand( fileSet );

diff --git a/.../main/java/org/apache/maven/scm/provider/git/gitexe/command/status/GitStatusConsumer.java b/.../main/java/org/apache/maven/scm/provider/git/gitexe/command/status/GitStatusConsumer.java
@@ -19,19 +19,19 @@
  * under the License.
  */
 
+import org.apache.commons.lang.StringUtils;
+import org.apache.maven.scm.ScmFile;
+import org.apache.maven.scm.ScmFileStatus;
+import org.apache.maven.scm.log.ScmLogger;
+import org.codehaus.plexus.util.cli.StreamConsumer;
+
 import java.io.File;
 import java.net.URI;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-import org.apache.commons.lang.StringUtils;
-import org.apache.maven.scm.ScmFile;
-import org.apache.maven.scm.ScmFileStatus;
-import org.apache.maven.scm.log.ScmLogger;
-import org.codehaus.plexus.util.cli.StreamConsumer;
-
 /**
  * @author <a href="mailto:struberg@yahoo.de">Mark Struberg</a>
  */
@@ -123,33 +123,36 @@ public void consumeLine( String line )
             return;
         }
 
-        ScmFileStatus status = null;
+        ScmFileStatus statusFrom = null;
+        ScmFileStatus statusTo = null;
+
+        String fileFrom = null;
+        String fileTo = null;
 
-        List<String> files = new ArrayList<String>();
-
         Matcher matcher;
         if ( ( matcher = ADDED_PATTERN.matcher( line ) ).find() )
         {
-            status = ScmFileStatus.ADDED;
-            files.add( resolvePath( matcher.group( 1 ), relativeRepositoryPath ) );
+            statusTo = ScmFileStatus.ADDED;
+            fileTo = resolvePath( matcher.group( 1 ), relativeRepositoryPath );
         }
         else if ( ( matcher = MODIFIED_PATTERN.matcher( line ) ).find() )
         {
-            status = ScmFileStatus.MODIFIED;
-            files.add( resolvePath( matcher.group( 1 ), relativeRepositoryPath ) );
+            statusTo = ScmFileStatus.MODIFIED;
+            fileTo = resolvePath( matcher.group( 1 ), relativeRepositoryPath );
         }
         else if ( ( matcher = DELETED_PATTERN.matcher( line ) ).find() )
         {
-            status = ScmFileStatus.DELETED;
-            files.add( resolvePath( matcher.group( 1 ), relativeRepositoryPath ) );
+            statusTo = ScmFileStatus.DELETED;
+            fileTo = resolvePath( matcher.group( 1 ), relativeRepositoryPath );
         }
         else if ( ( matcher = RENAMED_PATTERN.matcher( line ) ).find() )
         {
-            status = ScmFileStatus.RENAMED;
-            files.add( resolvePath( matcher.group( 1 ), relativeRepositoryPath ) );
-            files.add( resolvePath( matcher.group( 2 ), relativeRepositoryPath ) );
+            statusFrom = ScmFileStatus.RENAMED_FROM;
+            fileFrom = resolvePath( matcher.group( 1 ), relativeRepositoryPath );
+            statusTo = ScmFileStatus.RENAMED_TO;
+            fileTo = resolvePath( matcher.group( 2 ), relativeRepositoryPath );
             logger.debug( "RENAMED status for line '" + line + "' files added '" + matcher.group( 1 ) + "' '"
-                              + matcher.group( 2 ) );
+                          + matcher.group( 2 ) );
         }
         else
         {
@@ -158,54 +161,50 @@ else if ( ( matcher = RENAMED_PATTERN.matcher( line ) ).find() )
         }
 
         // If the file isn't a file; don't add it.
-        if ( !files.isEmpty() && status != null )
+        if ( workingDirectory != null )
         {
-            if ( workingDirectory != null )
+            if ( statusTo == ScmFileStatus.RENAMED_TO )
             {
-                if ( status == ScmFileStatus.RENAMED )
+                if ( isFile( fileFrom ) )
                 {
-                    String oldFilePath = files.get( 0 );
-                    String newFilePath = files.get( 1 );
-                    if ( isFile( oldFilePath ) )
-                    {
-                        logger.debug( "file '" + oldFilePath + "' is a file" );
-                        return;
-                    }
-                    else
-                    {
-                        logger.debug( "file '" + oldFilePath + "' not a file" );
-                    }
-                    if ( !isFile( newFilePath ) )
-                    {
-                        logger.debug( "file '" + newFilePath + "' not a file" );
-                        return;
-                    }
-                    else
-                    {
-                        logger.debug( "file '" + newFilePath + "' is a file" );
-                    }
+                    logger.debug( "file '" + fileFrom + "' is a file" );
+                    return;
                 }
-                else if ( status == ScmFileStatus.DELETED )
+                else
+                {
+                    logger.debug( "file '" + fileFrom + "' not a file" );
+                }
+                if ( !isFile( fileTo ) )
                 {
-                    if ( isFile( files.get( 0 ) ) )
-                    {
-                        return;
-                    }
+                    logger.debug( "file '" + fileTo + "' not a file" );
+                    return;
                 }
                 else
                 {
-                    if ( !isFile( files.get( 0 ) ) )
-                    {
-                        return;
-                    }
+                    logger.debug( "file '" + fileTo + "' is a file" );
                 }
             }
-
-            for ( String file : files )
+            else if ( statusTo == ScmFileStatus.DELETED )
             {
-                changedFiles.add( new ScmFile( file, status ) );
+                if ( isFile( fileTo ) )
+                {
+                    return;
+                }
             }
+            else
+            {
+                if ( !isFile( fileTo ) )
+                {
+                    return;
+                }
+            }
+        }
+
+        if ( statusFrom != null )
+        {
+            changedFiles.add( new ScmFile( fileFrom, statusFrom ) );
         }
+        changedFiles.add( new ScmFile( fileTo, statusTo ) );
     }
 
     private boolean isFile( String file )

diff --git a/...t/java/org/apache/maven/scm/provider/git/gitexe/command/status/GitStatusConsumerTest.java b/...t/java/org/apache/maven/scm/provider/git/gitexe/command/status/GitStatusConsumerTest.java
@@ -19,20 +19,20 @@
  * under the License.
  */
 
-import java.io.BufferedReader;
-import java.io.File;
-import java.io.FileReader;
-import java.io.IOException;
-import java.net.URI;
-import java.util.List;
-
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
 import org.apache.maven.scm.ScmFile;
 import org.apache.maven.scm.ScmFileStatus;
 import org.apache.maven.scm.log.DefaultLog;
 import org.codehaus.plexus.PlexusTestCase;
 
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.net.URI;
+import java.util.List;
+
 /**
  * @author <a href="mailto:struberg@yahoo.de">Mark Struberg</a>
  */
@@ -263,8 +263,8 @@ public void testConsumeRenamedFile()
 
         assertNotNull( changedFiles );
         assertEquals( 2, changedFiles.size() );
-        assertEquals( "OldCapfile", changedFiles.get(0).getPath() );
-        assertEquals( "NewCapFile", changedFiles.get(1).getPath() );
+        testScmFile( changedFiles.get( 0 ), "OldCapfile", ScmFileStatus.RENAMED_FROM );
+        testScmFile( changedFiles.get( 1 ), "NewCapFile", ScmFileStatus.RENAMED_TO );
         FileUtils.deleteDirectory( dir );
     }