You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by mi...@apache.org on 2022/07/22 08:54:43 UTC

[maven-scm] branch master updated: [SCM-807] JGit provider check-in fails unless the Maven project is in the working copy root

This is an automated email from the ASF dual-hosted git repository.

michaelo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-scm.git


The following commit(s) were added to refs/heads/master by this push:
     new 21e43b23b [SCM-807] JGit provider check-in fails unless the Maven project is in the working copy root
21e43b23b is described below

commit 21e43b23b3f824620761725d0d15903df3794cc5
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Thu Jul 7 15:42:05 2022 +0200

    [SCM-807] JGit provider check-in fails unless the Maven project is in the working copy root
    
    Fix JGit provider to always provide relative paths to ScmFileSet.
    
    This closes #152
---
 .../org/apache/maven/scm/manager/ScmManager.java   | 18 ++++----
 .../local/command/checkin/LocalCheckInCommand.java | 13 +++---
 .../scm/provider/git/jgit/command/JGitUtils.java   | 54 +++++++++++++++++-----
 .../jgit/command/checkin/JGitCheckInCommand.java   |  2 +-
 maven-scm-providers/pom.xml                        |  2 +-
 .../tck/command/checkin/CheckInCommandTckTest.java | 54 ++++++++++++++++++++++
 6 files changed, 116 insertions(+), 27 deletions(-)

diff --git a/maven-scm-api/src/main/java/org/apache/maven/scm/manager/ScmManager.java b/maven-scm-api/src/main/java/org/apache/maven/scm/manager/ScmManager.java
index 518e44235..cbbdb9bc3 100644
--- a/maven-scm-api/src/main/java/org/apache/maven/scm/manager/ScmManager.java
+++ b/maven-scm-api/src/main/java/org/apache/maven/scm/manager/ScmManager.java
@@ -126,7 +126,8 @@ public interface ScmManager
      *
      * @param repository the source control system
      * @param fileSet    the files to be added
-     * @return an {@link org.apache.maven.scm.command.add.AddScmResult} that contains the files that have been added
+     * @return an {@link AddScmResult} that contains the file paths (relative to {@code fileSet.getBasedir()}) that
+     * have been added
      * @throws ScmException if any
      *
      */
@@ -139,7 +140,8 @@ public interface ScmManager
      * @param repository the source control system
      * @param fileSet    the files to be added
      * @param message    a string that is a comment on the new added file
-     * @return an {@link AddScmResult} that contains the files that have been added
+     * @return an {@link AddScmResult} that contains the file paths (relative to {@code fileSet.getBasedir()}) that
+     * have been added
      * @throws ScmException if any
      */
     AddScmResult add( ScmRepository repository, ScmFileSet fileSet, String message )
@@ -266,13 +268,13 @@ public interface ScmManager
      * Save the changes you have done into the repository. This will create a new version of the file or directory in
      * the repository.
      * <p>
-     * When the fileSet has no entries, the fileSet.getBaseDir() is recursively committed. When the fileSet has entries,
-     * the commit is non-recursive and only the elements in the fileSet are committed.
+     * When the fileSet has no entries, the {@code fileSet.getBasedir()} is recursively committed. When the fileSet
+     * has entries, the commit is non-recursive and only the elements in the fileSet are committed.
      *
      * @param repository the source control system
      * @param fileSet    the files to check in (sometimes called commit)
      * @param message    a string that is a comment on the changes that where done
-     * @return TODO
+     * @return result object encapsulating all file paths (relative to {@code fileSet.getBasedir()})
      * @throws ScmException if any
      */
     CheckInScmResult checkIn( ScmRepository repository, ScmFileSet fileSet, String message )
@@ -282,14 +284,14 @@ public interface ScmManager
      * Save the changes you have done into the repository. This will create a new version of the file or directory in
      * the repository.
      * <p>
-     * When the fileSet has no entries, the fileSet.getBaseDir() is recursively committed. When the fileSet has entries,
-     * the commit is non-recursive and only the elements in the fileSet are committed.
+     * When the fileSet has no entries, the {@code fileSet.getBasedir()} is recursively committed. When the fileSet
+     * has entries, the commit is non-recursive and only the elements in the fileSet are committed.
      *
      * @param repository the source control system
      * @param fileSet    the files to check in (sometimes called commit)
      * @param revision   branch/tag/revision
      * @param message    a string that is a comment on the changes that where done
-     * @return TODO
+     * @return result object encapsulating all file paths (relative to {@code fileSet.getBasedir()})
      * @throws ScmException if any
      */
     CheckInScmResult checkIn( ScmRepository repository, ScmFileSet fileSet, ScmVersion revision, String message )
diff --git a/maven-scm-providers/maven-scm-provider-local/src/main/java/org/apache/maven/scm/provider/local/command/checkin/LocalCheckInCommand.java b/maven-scm-providers/maven-scm-provider-local/src/main/java/org/apache/maven/scm/provider/local/command/checkin/LocalCheckInCommand.java
index 563b56659..d88f15d87 100644
--- a/maven-scm-providers/maven-scm-provider-local/src/main/java/org/apache/maven/scm/provider/local/command/checkin/LocalCheckInCommand.java
+++ b/maven-scm-providers/maven-scm-provider-local/src/main/java/org/apache/maven/scm/provider/local/command/checkin/LocalCheckInCommand.java
@@ -34,6 +34,7 @@ import org.apache.maven.scm.command.checkin.CheckInScmResult;
 import org.apache.maven.scm.provider.ScmProviderRepository;
 import org.apache.maven.scm.provider.local.command.LocalCommand;
 import org.apache.maven.scm.provider.local.repository.LocalScmProviderRepository;
+import org.apache.maven.scm.util.FilenameUtils;
 import org.codehaus.plexus.util.FileUtils;
 import org.codehaus.plexus.util.StringUtils;
 
@@ -63,12 +64,12 @@ public class LocalCheckInCommand
 
         File source = new File( root, module );
 
-        File baseDestination = fileSet.getBasedir();
+        File basedir = fileSet.getBasedir();
 
-        if ( !baseDestination.exists() )
+        if ( !basedir.exists() )
         {
             throw new ScmException(
-                "The working directory doesn't exist (" + baseDestination.getAbsolutePath() + ")." );
+                "The working directory doesn't exist (" + basedir.getAbsolutePath() + ")." );
         }
 
         if ( !root.exists() )
@@ -92,14 +93,14 @@ public class LocalCheckInCommand
 
             if ( files.isEmpty() )
             {
-                files = FileUtils.getFiles( baseDestination, "**", null, false );
+                files = FileUtils.getFiles( basedir, "**", null, false );
             }
 
             for ( File file : files )
             {
-                String path = file.getPath().replace( '\\', '/' );
+                String path = FilenameUtils.normalizeFilename( file.getPath() );
                 File repoFile = new File( repoRoot, path );
-                file = new File( baseDestination, path );
+                file = new File( basedir, path );
 
                 ScmFileStatus status;
 
diff --git a/maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/JGitUtils.java b/maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/JGitUtils.java
index 81dc26aa9..00216a0e9 100644
--- a/maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/JGitUtils.java
+++ b/maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/JGitUtils.java
@@ -73,7 +73,6 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Date;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -238,6 +237,26 @@ public class JGitUtils
      */
     public static List<ScmFile> getFilesInCommit( Repository repository, RevCommit commit )
         throws MissingObjectException, IncorrectObjectTypeException, CorruptObjectException, IOException
+    {
+        return getFilesInCommit( repository, commit, null );
+    }
+
+    /**
+     * get a list of all files in the given commit
+     *
+     * @param repository the repo
+     * @param commit     the commit to get the files from
+     * @param baseDir    the directory to which the returned files should be relative.
+     *                   May be {@code null} in case they should be relative to the working directory root.
+     * @return a list of files included in the commit
+     *
+     * @throws MissingObjectException
+     * @throws IncorrectObjectTypeException
+     * @throws CorruptObjectException
+     * @throws IOException
+     */
+    public static List<ScmFile> getFilesInCommit( Repository repository, RevCommit commit, File baseDir )
+        throws MissingObjectException, IncorrectObjectTypeException, CorruptObjectException, IOException
     {
         List<ScmFile> list = new ArrayList<>();
         if ( JGitUtils.hasCommits( repository ) )
@@ -254,7 +273,16 @@ public class JGitUtils
                 List<DiffEntry> diffs = df.scan( parent.getTree(), commit.getTree() );
                 for ( DiffEntry diff : diffs )
                 {
-                    list.add( new ScmFile( diff.getNewPath(), ScmFileStatus.CHECKED_IN ) );
+                    final String path;
+                    if ( baseDir != null )
+                    {
+                        path = relativize ( baseDir.toURI(), new File( repository.getWorkTree(), diff.getNewPath() ) );
+                    }
+                    else
+                    {
+                        path = diff.getNewPath();
+                    }
+                    list.add( new ScmFile( path, ScmFileStatus.CHECKED_IN ) );
                 }
             }
         }
@@ -299,7 +327,7 @@ public class JGitUtils
     public static List<ScmFile> addAllFiles( Git git, ScmFileSet fileSet )
         throws GitAPIException, NoFilepatternException
     {
-        URI baseUri = fileSet.getBasedir().toURI();
+        URI workingCopyRootUri = git.getRepository().getWorkTree().toURI();
         AddCommand add = git.add();
         for ( File file : fileSet.getFileList() )
         {
@@ -310,9 +338,8 @@ public class JGitUtils
 
             if ( file.exists() )
             {
-                String path = relativize( baseUri, file );
+                String path = relativize( workingCopyRootUri, file );
                 add.addFilepattern( path );
-                add.addFilepattern( file.getAbsolutePath() );
             }
         }
         add.call();
@@ -330,15 +357,20 @@ public class JGitUtils
         // rewrite all detected files to now have status 'checked_in'
         for ( String entry : allInIndex )
         {
-            ScmFile scmfile = new ScmFile( entry, ScmFileStatus.ADDED );
-
             // if a specific fileSet is given, we have to check if the file is
             // really tracked
-            for ( Iterator<File> itfl = fileSet.getFileList().iterator(); itfl.hasNext(); )
+            for ( File file : fileSet.getFileList() )
             {
-                String path = FilenameUtils.normalizeFilename( relativize( baseUri, itfl.next() ) );
-                if ( path.equals( FilenameUtils.normalizeFilename( scmfile.getPath() ) ) )
+                if ( !file.isAbsolute() )
+                {
+                    file = new File( fileSet.getBasedir(), file.getPath() );
+                }
+                String path = FilenameUtils.normalizeFilename( relativize( workingCopyRootUri, file ) );
+                if ( path.equals( FilenameUtils.normalizeFilename( entry ) ) )
                 {
+                    // returned ScmFiles should be relative to given fileset's basedir
+                    ScmFile scmfile = new ScmFile( relativize( fileSet.getBasedir().toURI(), file ),
+                            ScmFileStatus.ADDED );
                     addedFiles.add( scmfile );
                 }
             }
@@ -495,4 +527,4 @@ public class JGitUtils
         }
         return result;
     }
-}
\ No newline at end of file
+}
diff --git a/maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/checkin/JGitCheckInCommand.java b/maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/checkin/JGitCheckInCommand.java
index f2f0cffad..8e0d91c2f 100644
--- a/maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/checkin/JGitCheckInCommand.java
+++ b/maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/checkin/JGitCheckInCommand.java
@@ -127,7 +127,7 @@ public class JGitCheckInCommand
                 RevCommit commitRev = command.call();
 
                 logger.info( "commit done: " + commitRev.getShortMessage() );
-                checkedInFiles = JGitUtils.getFilesInCommit( git.getRepository(), commitRev );
+                checkedInFiles = JGitUtils.getFilesInCommit( git.getRepository(), commitRev, fileSet.getBasedir() );
                 if ( logger.isDebugEnabled() )
                 {
                     for ( ScmFile scmFile : checkedInFiles )
diff --git a/maven-scm-providers/pom.xml b/maven-scm-providers/pom.xml
index 0a51b6080..d19b1e7ca 100644
--- a/maven-scm-providers/pom.xml
+++ b/maven-scm-providers/pom.xml
@@ -37,9 +37,9 @@
   <modules>
     <module>maven-scm-provider-hg</module>
     <module>maven-scm-providers-git</module>
-    <module>maven-scm-provider-local</module>
     <module>maven-scm-providers-standard</module>
     <module>maven-scm-providers-svn</module>
+    <module>maven-scm-provider-local</module>
   </modules>
 
   <dependencies>
diff --git a/maven-scm-test/src/main/java/org/apache/maven/scm/tck/command/checkin/CheckInCommandTckTest.java b/maven-scm-test/src/main/java/org/apache/maven/scm/tck/command/checkin/CheckInCommandTckTest.java
index ac652c7d3..505386e19 100644
--- a/maven-scm-test/src/main/java/org/apache/maven/scm/tck/command/checkin/CheckInCommandTckTest.java
+++ b/maven-scm-test/src/main/java/org/apache/maven/scm/tck/command/checkin/CheckInCommandTckTest.java
@@ -26,8 +26,10 @@ import org.apache.maven.scm.ScmTckTestCase;
 import org.apache.maven.scm.command.add.AddScmResult;
 import org.apache.maven.scm.command.checkin.CheckInScmResult;
 import org.apache.maven.scm.command.checkout.CheckOutScmResult;
+import org.apache.maven.scm.provider.ScmProvider;
 import org.codehaus.plexus.util.FileUtils;
 import org.codehaus.plexus.util.IOUtil;
+import org.junit.Assume;
 import org.junit.Test;
 
 import java.io.File;
@@ -188,6 +190,58 @@ public abstract class CheckInCommandTckTest
         assertEquals( "check readme.txt contents", "/readme.txt", FileUtils.fileRead( readmeTxt ) );
     }
 
+    @Test
+    public void testCheckInCommandFilesetWithBasedirOtherThanWorkingCopyRoot()
+        throws Exception
+    {
+        ScmProvider scmProvider =  getScmManager().getProviderByUrl( getScmUrl() );
+
+        Assume.assumeFalse( "Local provider does not properly support basedir",
+            scmProvider.getScmType().equals( "local" ) );
+        // Make sure that the correct files was checked out
+        File fooJava = new File( getWorkingCopy(), "src/main/java/Foo.java" );
+
+        assertFalse( "check Foo.java doesn't yet exist", fooJava.canRead() );
+
+        // Change the files
+        createFooJava( fooJava );
+
+        AddScmResult addResult =
+            scmProvider.add( getScmRepository(), new ScmFileSet( new File( getWorkingCopy(), "src" ),
+                                                                 "main/java/Foo.java", null ) );
+
+        assertResultIsSuccess( addResult );
+
+        CheckInScmResult result =
+            getScmManager().checkIn( getScmRepository(), new ScmFileSet( new File( getWorkingCopy(), "src" ),
+                                     "**/Foo.java", null ),
+                                     "Commit message" );
+
+        assertResultIsSuccess( result );
+
+        List<ScmFile> files = result.getCheckedInFiles();
+
+        assertNotNull( files );
+
+        assertEquals( 1, files.size() );
+
+        ScmFile file1 = files.get( 0 );
+
+        assertEquals( ScmFileStatus.CHECKED_IN, file1.getStatus() );
+
+        assertPath( "main/java/Foo.java", file1.getPath() );
+
+        CheckOutScmResult checkoutResult =
+            getScmManager().checkOut( getScmRepository(), new ScmFileSet( getAssertionCopy() ) );
+
+        assertResultIsSuccess( checkoutResult );
+
+        fooJava = new File( getAssertionCopy(), "src/main/java/Foo.java" );
+
+        assertTrue( "check can read Foo.java", fooJava.canRead() );
+
+    }
+
     private void createFooJava( File fooJava )
         throws Exception
     {