You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2018/06/04 11:02:56 UTC

[GitHub] asfgit closed pull request #46: [WAGON-495] fix testWagonPutDirectoryWhenDirectoryAlreadyExists false positive

asfgit closed pull request #46: [WAGON-495] fix testWagonPutDirectoryWhenDirectoryAlreadyExists false positive
URL: https://github.com/apache/maven-wagon/pull/46
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/wagon-provider-test/src/main/java/org/apache/maven/wagon/WagonTestCase.java b/wagon-provider-test/src/main/java/org/apache/maven/wagon/WagonTestCase.java
index fb277c7f..e89f1e29 100644
--- a/wagon-provider-test/src/main/java/org/apache/maven/wagon/WagonTestCase.java
+++ b/wagon-provider-test/src/main/java/org/apache/maven/wagon/WagonTestCase.java
@@ -42,6 +42,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.security.NoSuchAlgorithmException;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
@@ -74,6 +75,10 @@ public int getSize()
         }
     }
 
+    protected static final String TEST_CONTENT = "test-resource.txt\n";
+
+    protected static final String TEST_CKSUM = cksum( TEST_CONTENT );
+
     protected static final String POM = "pom.xml";
 
     protected Repository localRepository;
@@ -233,16 +238,13 @@ protected void message( String message )
     public void testWagon()
         throws Exception
     {
-        if ( supportsGetIfNewer() )
-        {
-            setupRepositories();
+        setupRepositories();
 
-            setupWagonTestingFixtures();
+        setupWagonTestingFixtures();
 
-            fileRoundTripTesting();
+        fileRoundTripTesting();
 
-            tearDownWagonTestingFixtures();
-        }
+        tearDownWagonTestingFixtures();
     }
 
     public void testWagonGetIfNewerIsNewer()
@@ -341,7 +343,7 @@ protected void assertGetIfNewerTest( ProgressAnswer progressAnswer, boolean expe
 
             assertNotNull( "check checksum is not null", checksumObserver.getActualChecksum() );
 
-            assertEquals( "compare checksums", "6b144b7285ffd6b0bc8300da162120b9",
+            assertEquals( "compare checksums", TEST_CKSUM,
                           checksumObserver.getActualChecksum() );
 
             // Now compare the contents of the artifact that was placed in
@@ -923,7 +925,7 @@ protected TransferEvent createTransferEvent( Wagon wagon, Resource resource, int
     protected int putFile()
         throws Exception
     {
-        String content = "test-resource.txt\n";
+        String content = TEST_CONTENT;
         putFile( resource, "test-resource", content );
         return content.length();
     }
@@ -1049,7 +1051,7 @@ protected void fileRoundTripTesting()
 
         assertNotNull( "check checksum is not null", checksumObserver.getActualChecksum() );
 
-        assertEquals( "compare checksums", "6b144b7285ffd6b0bc8300da162120b9", checksumObserver.getActualChecksum() );
+        assertEquals( "compare checksums", TEST_CKSUM, checksumObserver.getActualChecksum() );
 
         checksumObserver = new ChecksumObserver();
 
@@ -1057,7 +1059,7 @@ protected void fileRoundTripTesting()
 
         assertNotNull( "check checksum is not null", checksumObserver.getActualChecksum() );
 
-        assertEquals( "compare checksums", "6b144b7285ffd6b0bc8300da162120b9", checksumObserver.getActualChecksum() );
+        assertEquals( "compare checksums", TEST_CKSUM, checksumObserver.getActualChecksum() );
 
         // Now compare the conents of the artifact that was placed in
         // the repository with the contents of the artifact that was
@@ -1087,4 +1089,22 @@ protected Repository createFileRepository( String url )
         return repository;
     }
 
+    protected static String cksum( String content )
+    {
+        String checkSum;
+        try
+        {
+            ChecksumObserver obs = new ChecksumObserver();
+            byte[] buf = content.getBytes( StandardCharsets.ISO_8859_1 );
+            obs.transferProgress( null, buf, buf.length );
+            obs.transferCompleted( null );
+            checkSum = obs.getActualChecksum();
+        }
+        catch ( Exception e )
+        {
+            checkSum = null;
+        }
+        return checkSum;
+    }
+
 }
diff --git a/wagon-providers/wagon-scm/src/main/java/org/apache/maven/wagon/providers/scm/ScmWagon.java b/wagon-providers/wagon-scm/src/main/java/org/apache/maven/wagon/providers/scm/ScmWagon.java
index 8fbbdeff..d6531eef 100644
--- a/wagon-providers/wagon-scm/src/main/java/org/apache/maven/wagon/providers/scm/ScmWagon.java
+++ b/wagon-providers/wagon-scm/src/main/java/org/apache/maven/wagon/providers/scm/ScmWagon.java
@@ -19,6 +19,15 @@
  * under the License.
  */
 
+import java.io.File;
+import java.io.IOException;
+import java.text.DecimalFormat;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Random;
+
+import org.apache.maven.scm.CommandParameter;
+import org.apache.maven.scm.CommandParameters;
 import org.apache.maven.scm.ScmBranch;
 import org.apache.maven.scm.ScmException;
 import org.apache.maven.scm.ScmFile;
@@ -30,6 +39,7 @@
 import org.apache.maven.scm.command.add.AddScmResult;
 import org.apache.maven.scm.command.checkout.CheckOutScmResult;
 import org.apache.maven.scm.command.list.ListScmResult;
+import org.apache.maven.scm.command.update.UpdateScmResult;
 import org.apache.maven.scm.manager.NoSuchScmProviderException;
 import org.apache.maven.scm.manager.ScmManager;
 import org.apache.maven.scm.provider.ScmProvider;
@@ -47,14 +57,6 @@
 import org.codehaus.plexus.util.FileUtils;
 import org.codehaus.plexus.util.StringUtils;
 
-import java.io.File;
-import java.io.IOException;
-import java.text.DecimalFormat;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Random;
-import java.util.Stack;
-
 /**
  * Wagon provider to get and put files from and to SCM systems, using Maven-SCM as underlying transport.
  * <p/>
@@ -93,6 +95,11 @@
      */
     private String scmVersionType;
 
+    /**
+     * Empty string or subdir ending with slash.
+     */
+    private String partCOSubdir = "";
+
     private File checkoutDirectory;
 
     /**
@@ -369,18 +376,19 @@ private void putInternal( File source, String targetName )
 
             ScmProvider scmProvider = getScmProvider( scmRepository.getProvider() );
 
-            String checkoutTargetName = source.isDirectory() ? targetName : getDirname( targetName );
-            String relPath = checkOut( scmProvider, scmRepository, checkoutTargetName, target );
+            boolean isDirectory = source.isDirectory();
+            String checkoutTargetName = isDirectory ? targetName : getDirname( targetName );
+            String relPath = ensureDirs( scmProvider, scmRepository, checkoutTargetName, target );
 
             File newCheckoutDirectory = new File( checkoutDirectory, relPath );
 
-            File scmFile = new File( newCheckoutDirectory, source.isDirectory() ? "" : getFilename( targetName ) );
+            File scmFile = new File( newCheckoutDirectory, isDirectory ? "" : FileUtils.removePath( targetName, '/' ) );
 
             boolean fileAlreadyInScm = scmFile.exists();
 
             if ( !scmFile.equals( source ) )
             {
-                if ( source.isDirectory() )
+                if ( isDirectory )
                 {
                     FileUtils.copyDirectoryStructure( source, scmFile );
                 }
@@ -393,7 +401,7 @@ private void putInternal( File source, String targetName )
             if ( !fileAlreadyInScm || scmFile.isDirectory() )
             {
                 int addedFiles = addFiles( scmProvider, scmRepository, newCheckoutDirectory,
-                                           source.isDirectory() ? "" : scmFile.getName() );
+                                           isDirectory ? "" : scmFile.getName() );
 
                 if ( !fileAlreadyInScm && addedFiles == 0 )
                 {
@@ -437,14 +445,16 @@ private void putInternal( File source, String targetName )
      * @param targetName
      * @return
      * @throws TransferFailedException
+     * @throws IOException
      */
-    private String checkOut( ScmProvider scmProvider, ScmRepository scmRepository, String targetName,
+    private String ensureDirs( ScmProvider scmProvider, ScmRepository scmRepository, String targetName,
                              Resource resource )
-        throws TransferFailedException
+        throws TransferFailedException, IOException
     {
-        checkoutDirectory = createCheckoutDirectory();
-
-        Stack<String> stack = new Stack<String>();
+        if ( checkoutDirectory == null )
+        {
+            checkoutDirectory = createCheckoutDirectory();
+        }
 
         String target = targetName;
 
@@ -453,81 +463,82 @@ private String checkOut( ScmProvider scmProvider, ScmRepository scmRepository, S
         // Check whether targetName, which is a relative path into the scm, exists.
         // If it doesn't, check the parent, etc.
 
-        try
+        for ( ;; )
         {
-            while ( target.length() > 0 && !scmProvider.list( scmRepository,
-                                                              new ScmFileSet( new File( "." ), new File( target ) ),
-                                                              false, makeScmVersion() ).isSuccess() )
+            try
+            {
+                ScmResult res = tryPartialCheckout( target );
+                if ( !res.isSuccess() )
+                {
+                    throw new ScmException( "command failed: " + res.getCommandOutput().trim() );
+                }
+                break;
+            }
+            catch ( ScmException e )
             {
-                stack.push( getFilename( target ) );
+                if ( partCOSubdir.length() == 0 )
+                {
+                    fireTransferError( resource, e, TransferEvent.REQUEST_GET );
+
+                    throw new TransferFailedException( "Error checking out: " + e.getMessage(), e );
+                }
                 target = getDirname( target );
             }
         }
-        catch ( ScmException e )
-        {
-            fireTransferError( resource, e, TransferEvent.REQUEST_GET );
 
-            throw new TransferFailedException( "Error listing repository: " + e.getMessage(), e );
-        }
+        // now create the subdirs in target, if it's a parent of targetName
 
-        // ok, we've established that target exists, or is empty.
-        // Check the resource out; if it doesn't exist, that means we're in the svn repo url root,
-        // and the configuration is incorrect. We will not try repo.getParent since most scm's don't
-        // implement that.
+        String res =
+            partCOSubdir.length() >= targetName.length() ? "" : targetName.substring( partCOSubdir.length() ) + '/';
 
+        ArrayList<File> createdDirs = new ArrayList<File>();
+        File deepDir = new File( checkoutDirectory, res );
+
+        boolean added = false;
         try
         {
-            String repoUrl = getRepository().getUrl();
-            if ( "svn".equals( scmProvider.getScmType() ) )
+            mkdirsThrow( deepDir, createdDirs );
+            if ( createdDirs.size() != 0 )
             {
-                // Subversion is the only SCM that adds path structure to represent tags and branches.
-                // The rest use scmVersion and scmVersionType.
-                repoUrl += "/" + target.replace( '\\', '/' );
-            }
-            scmRepository = getScmRepository( repoUrl );
-            CheckOutScmResult ret =
-                scmProvider.checkOut( scmRepository, new ScmFileSet( new File( checkoutDirectory, "" ) ),
-                                      makeScmVersion(), false );
+                File topNewDir = createdDirs.get( 0 );
+                String relTopNewDir =
+                    topNewDir.getPath().substring( checkoutDirectory.getPath().length() + 1 ).replace( '\\', '/' );
 
-            checkScmResult( ret );
+                addFiles( scmProvider, scmRepository, checkoutDirectory, relTopNewDir );
+                added = true;
+            }
         }
         catch ( ScmException e )
         {
-            fireTransferError( resource, e, TransferEvent.REQUEST_GET );
+            fireTransferError( resource, e, TransferEvent.REQUEST_PUT );
 
-            throw new TransferFailedException( "Error checking out: " + e.getMessage(), e );
+            throw new TransferFailedException( "Failed to add directory " + createdDirs.get( 0 ) + " to working copy",
+                                               e );
         }
-
-        // now create the subdirs in target, if it's a parent of targetName
-
-        String relPath = "";
-
-        while ( !stack.isEmpty() )
+        finally
         {
-            String p = stack.pop();
-            relPath += p + "/";
-
-            File newDir = new File( checkoutDirectory, relPath );
-            if ( !newDir.mkdirs() )
+            if ( !added && createdDirs.size() != 0 )
             {
-                throw new TransferFailedException(
-                    "Failed to create directory " + newDir.getAbsolutePath() + "; parent should exist: "
-                        + checkoutDirectory );
+                FileUtils.deleteDirectory( createdDirs.get( 0 ) );
             }
+        }
 
-            try
-            {
-                addFiles( scmProvider, scmRepository, checkoutDirectory, relPath );
-            }
-            catch ( ScmException e )
-            {
-                fireTransferError( resource, e, TransferEvent.REQUEST_PUT );
+        return res;
+    }
 
-                throw new TransferFailedException( "Failed to add directory " + newDir + " to working copy", e );
+    private static void mkdirsThrow( File f, List<File> createdDirs )
+        throws IOException
+    {
+        if ( !f.isDirectory() )
+        {
+            File parent = f.getParentFile();
+            mkdirsThrow( parent, createdDirs );
+            if ( !f.mkdir() )
+            {
+                throw new IOException( "Failed to create directory " + f.getAbsolutePath() );
             }
+            createdDirs.add( f );
         }
-
-        return relPath;
     }
 
     /**
@@ -551,7 +562,8 @@ private int addFiles( ScmProvider scmProvider, ScmRepository scmRepository, File
 
         if ( scmFilePath.length() != 0 )
         {
-            AddScmResult result = scmProvider.add( scmRepository, new ScmFileSet( basedir, new File( scmFilePath ) ) );
+            AddScmResult result =
+                scmProvider.add( scmRepository, new ScmFileSet( basedir, new File( scmFilePath ) ), mkBinaryFlag() );
 
             /*
              * TODO dirty fix to work around files with property svn:eol-style=native if a file has that property, first
@@ -560,7 +572,9 @@ private int addFiles( ScmProvider scmProvider, ScmRepository scmRepository, File
              */
             if ( !result.isSuccess() )
             {
-                result = scmProvider.add( scmRepository, new ScmFileSet( basedir, new File( scmFilePath ) ) );
+                result =
+                    scmProvider.add( scmRepository, new ScmFileSet( basedir, new File( scmFilePath ) ),
+                                     mkBinaryFlag() );
             }
 
             addedFiles = result.getAddedFiles().size();
@@ -584,6 +598,26 @@ private int addFiles( ScmProvider scmProvider, ScmRepository scmRepository, File
         return addedFiles;
     }
 
+    private CheckOutScmResult checkOut( ScmProvider scmProvider, ScmRepository scmRepository, ScmFileSet fileSet )
+        throws ScmException
+    {
+        ScmVersion ver = makeScmVersion();
+        CommandParameters parameters = mkBinaryFlag();
+        // TODO: AbstractScmProvider 6f7dd0c ignores checkOut() parameter "version"
+        parameters.setScmVersion( CommandParameter.SCM_VERSION, ver );
+        parameters.setString( CommandParameter.RECURSIVE, Boolean.toString( false ) );
+        parameters.setString( CommandParameter.SHALLOW, Boolean.toString( true ) );
+
+        return scmProvider.checkOut( scmRepository, fileSet, ver, parameters );
+    }
+
+    private CommandParameters mkBinaryFlag() throws ScmException
+    {
+        CommandParameters parameters = new CommandParameters();
+        parameters.setString( CommandParameter.BINARY, Boolean.toString( true ) );
+        return parameters;
+    }
+
     /**
      * @return true
      */
@@ -592,6 +626,12 @@ public boolean supportsDirectoryCopy()
         return true;
     }
 
+    private boolean supportsPartialCheckout( ScmProvider scmProvider )
+    {
+        String scmType = scmProvider.getScmType();
+        return ( "svn".equals( scmType ) || "cvs".equals( scmType ) );
+    }
+
     public void putDirectory( File sourceDirectory, String destinationDirectory )
         throws TransferFailedException, ResourceDoesNotExistException, AuthorizationException
     {
@@ -646,16 +686,19 @@ public void get( String resourceName, File destination )
 
         fireGetInitiated( resource, destination );
 
-        String url = getRepository().getUrl() + "/" + resourceName;
-
-        // remove the file
-        url = url.substring( 0, url.lastIndexOf( '/' ) );
+        fireGetStarted( resource, destination );
 
         try
         {
-            ScmRepository scmRepository = getScmRepository( url );
-
-            fireGetStarted( resource, destination );
+            String subdir = getDirname( resourceName );
+            ScmResult res = tryPartialCheckout( subdir );
+            if ( !res.isSuccess() && ( partCOSubdir.length() == 0 || res instanceof UpdateScmResult ) )
+            {
+                // inability to checkout SVN or CVS subdir is not fatal. We just assume it doesn't exist
+                // inability to update existing subdir or checkout root is fatal
+                throw new ScmException( "command failed: " + res.getCommandOutput().trim() );
+            }
+            resourceName = resourceName.substring( partCOSubdir.length() );
 
             // TODO: limitations:
             // - destination filename must match that in the repository - should allow the "-d" CVS equiv to be passed
@@ -666,24 +709,6 @@ public void get( String resourceName, File destination )
 
             File scmFile = new File( checkoutDirectory, resourceName );
 
-            File basedir = scmFile.getParentFile();
-
-            ScmProvider scmProvider = getScmProvider( scmRepository.getProvider() );
-
-            String reservedScmFile = scmProvider.getScmSpecificFilename();
-
-            if ( reservedScmFile != null && new File( basedir, reservedScmFile ).exists() )
-            {
-                scmProvider.update( scmRepository, new ScmFileSet( basedir ), makeScmVersion() );
-            }
-            else
-            {
-                // TODO: this should be checking out a full hierarchy (requires the -d equiv)
-                basedir.mkdirs();
-
-                scmProvider.checkOut( scmRepository, new ScmFileSet( basedir ), makeScmVersion() );
-            }
-
             if ( !scmFile.exists() )
             {
                 throw new ResourceDoesNotExistException( "Unable to find resource " + destination + " after checkout" );
@@ -712,6 +737,49 @@ public void get( String resourceName, File destination )
         fireGetCompleted( resource, destination );
     }
 
+    private ScmResult tryPartialCheckout( String subdir )
+        throws ScmException, IOException
+    {
+        String url = getRepository().getUrl();
+
+        String desiredPartCOSubdir = "";
+
+        ScmRepository scmRepository = getScmRepository( url );
+        ScmProvider scmProvider = getScmProvider( scmRepository.getProvider() );
+        if ( subdir.length() != 0 && supportsPartialCheckout( scmProvider ) )
+        {
+            url += ( url.endsWith( "/" ) ? "" : "/" ) + subdir;
+
+            desiredPartCOSubdir = subdir + "/";
+
+            scmRepository = getScmRepository( url );
+        }
+
+        if ( !desiredPartCOSubdir.equals( partCOSubdir ) )
+        {
+            FileUtils.deleteDirectory( checkoutDirectory );
+            partCOSubdir = desiredPartCOSubdir;
+        }
+
+        ScmResult res;
+        if ( checkoutDirExists( scmProvider ) )
+        {
+            res = scmProvider.update( scmRepository, new ScmFileSet( checkoutDirectory ), makeScmVersion() );
+        }
+        else
+        {
+            res = checkOut( scmProvider, scmRepository, new ScmFileSet( checkoutDirectory ) );
+        }
+        return res;
+    }
+
+    private boolean checkoutDirExists( ScmProvider scmProvider )
+    {
+        String reservedScmFile = scmProvider.getScmSpecificFilename();
+        File pathToCheck = reservedScmFile == null ? checkoutDirectory : new File( checkoutDirectory, reservedScmFile );
+        return pathToCheck.exists();
+    }
+
     /**
      * @return a List&lt;String&gt; with filenames/directories at the resourcepath.
      * @see org.apache.maven.wagon.AbstractWagon#getFileList(java.lang.String)
@@ -764,15 +832,8 @@ public boolean resourceExists( String resourceName )
         }
     }
 
-    private String getFilename( String filename )
-    {
-        String fname = StringUtils.replace( filename, "/", File.separator );
-        return FileUtils.filename( fname );
-    }
-
-    private String getDirname( String filename )
+    private String getDirname( String resourceName )
     {
-        String fname = StringUtils.replace( filename, "/", File.separator );
-        return FileUtils.dirname( fname );
+        return FileUtils.getPath( resourceName, '/' );
     }
 }
diff --git a/wagon-providers/wagon-scm/src/test/java/org/apache/maven/wagon/providers/scm/AbstractScmCvsWagonTest.java b/wagon-providers/wagon-scm/src/test/java/org/apache/maven/wagon/providers/scm/AbstractScmCvsWagonTest.java
index fc7b7f82..eac45626 100644
--- a/wagon-providers/wagon-scm/src/test/java/org/apache/maven/wagon/providers/scm/AbstractScmCvsWagonTest.java
+++ b/wagon-providers/wagon-scm/src/test/java/org/apache/maven/wagon/providers/scm/AbstractScmCvsWagonTest.java
@@ -41,6 +41,6 @@ protected String getTestRepositoryUrl()
     {
         String repository = getTestFile( "target/test-classes/test-repo-cvs" ).getAbsolutePath();
 
-        return "scm:cvs|local|" + repository + "|repository/newfolder";
+        return "scm:cvs|local|" + repository + "|repository";
     }
 }


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Re: [GitHub] asfgit closed pull request #46: [WAGON-495] fix testWagonPutDirectoryWhenDirectoryAlreadyExists false positive

Posted by Vishal Kumar Gupta <gr...@gmail.com>.
 asfgit closed pull request #46: [WAGON-495] fix
testWagonPutDirectoryWhenDirectoryAlreadyExists false positive
URL: https://github.com/apache/maven-wagon/pull/46




This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/wagon-provider-test/src/main/java/org/apache/maven/wagon/WagonTestCase.java
b/wagon-provider-test/src/main/java/org/apache/maven/
wagon/WagonTestCase.java
index fb277c7f..e89f1e29 100644
--- a/wagon-provider-test/src/main/java/org/apache/maven/
wagon/WagonTestCase.java
+++ b/wagon-provider-test/src/main/java/org/apache/maven/
wagon/WagonTestCase.java
@@ -42,6 +42,7 @@

 import java.io.File;
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.security.NoSuchAlgorithmException;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
@@ -74,6 +75,10 @@ public int getSize()
         }
     }

+    protected static final String TEST_CONTENT = "test-resource.txt\n";
+
+    protected static final String TEST_CKSUM = cksum( TEST_CONTENT );
+
     protected static final String POM = "pom.xml";

     protected Repository localRepository;
@@ -233,16 +238,13 @@ protected void message( String message )
     public void testWagon()
         throws Exception
     {
-        if ( supportsGetIfNewer() )
-        {
-            setupRepositories();
+        setupRepositories();

-            setupWagonTestingFixtures();
+        setupWagonTestingFixtures();

-            fileRoundTripTesting();
+        fileRoundTripTesting();

-            tearDownWagonTestingFixtures();
-        }
+        tearDownWagonTestingFixtures();
     }

     public void testWagonGetIfNewerIsNewer()
@@ -341,7 +343,7 @@ protected void assertGetIfNewerTest( ProgressAnswer
progressAnswer, boolean expe

             assertNotNull( "check checksum is not null",
checksumObserver.getActualChecksum()
);

-            assertEquals( "compare checksums", "
6b144b7285ffd6b0bc8300da162120b9",
+            assertEquals( "compare checksums", TEST_CKSUM,
                           checksumObserver.getActualChecksum() );

             // Now compare the contents of the artifact that was placed in
@@ -923,7 +925,7 @@ protected TransferEvent createTransferEvent( Wagon
wagon, Resource resource, int
     protected int putFile()
         throws Exception
     {
-        String content = "test-resource.txt\n";
+        String content = TEST_CONTENT;
         putFile( resource, "test-resource", content );
         return content.length();
     }
@@ -1049,7 +1051,7 @@ protected void fileRoundTripTesting()

         assertNotNull( "check checksum is not null",
checksumObserver.getActualChecksum()
);

-        assertEquals( "compare checksums", "6b144b7285ffd6b0bc8300da162120b9",
checksumObserver.getActualChecksum() );
+        assertEquals( "compare checksums", TEST_CKSUM,
checksumObserver.getActualChecksum()
);

         checksumObserver = new ChecksumObserver();

@@ -1057,7 +1059,7 @@ protected void fileRoundTripTesting()

         assertNotNull( "check checksum is not null",
checksumObserver.getActualChecksum()
);

-        assertEquals( "compare checksums", "6b144b7285ffd6b0bc8300da162120b9",
checksumObserver.getActualChecksum() );
+        assertEquals( "compare checksums", TEST_CKSUM,
checksumObserver.getActualChecksum()
);

         // Now compare the conents of the artifact that was placed in
         // the repository with the contents of the artifact that was
@@ -1087,4 +1089,22 @@ protected Repository createFileRepository( String
url )
         return repository;
     }

+    protected static String cksum( String content )
+    {
+        String checkSum;
+        try
+        {
+            ChecksumObserver obs = new ChecksumObserver();
+            byte[] buf = content.getBytes( StandardCharsets.ISO_8859_1 );
+            obs.transferProgress( null, buf, buf.length );
+            obs.transferCompleted( null );
+            checkSum = obs.getActualChecksum();
+        }
+        catch ( Exception e )
+        {
+            checkSum = null;
+        }
+        return checkSum;
+    }
+
 }
diff --git a/wagon-providers/wagon-scm/src/main/java/org/apache/
maven/wagon/providers/scm/ScmWagon.java b/wagon-providers/wagon-scm/
src/main/java/org/apache/maven/wagon/providers/scm/ScmWagon.java
index 8fbbdeff..d6531eef 100644
--- a/wagon-providers/wagon-scm/src/main/java/org/apache/
maven/wagon/providers/scm/ScmWagon.java
+++ b/wagon-providers/wagon-scm/src/main/java/org/apache/
maven/wagon/providers/scm/ScmWagon.java
@@ -19,6 +19,15 @@
  * under the License.
  */

+import java.io.File;
+import java.io.IOException;
+import java.text.DecimalFormat;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Random;
+
+import org.apache.maven.scm.CommandParameter;
+import org.apache.maven.scm.CommandParameters;
 import org.apache.maven.scm.ScmBranch;
 import org.apache.maven.scm.ScmException;
 import org.apache.maven.scm.ScmFile;
@@ -30,6 +39,7 @@
 import org.apache.maven.scm.command.add.AddScmResult;
 import org.apache.maven.scm.command.checkout.CheckOutScmResult;
 import org.apache.maven.scm.command.list.ListScmResult;
+import org.apache.maven.scm.command.update.UpdateScmResult;
 import org.apache.maven.scm.manager.NoSuchScmProviderException;
 import org.apache.maven.scm.manager.ScmManager;
 import org.apache.maven.scm.provider.ScmProvider;
@@ -47,14 +57,6 @@
 import org.codehaus.plexus.util.FileUtils;
 import org.codehaus.plexus.util.StringUtils;

-import java.io.File;
-import java.io.IOException;
-import java.text.DecimalFormat;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Random;
-import java.util.Stack;
-
 /**
  * Wagon provider to get and put files from and to SCM systems, using
Maven-SCM as underlying transport.
  * <p/>
@@ -93,6 +95,11 @@
      */
     private String scmVersionType;

+    /**
+     * Empty string or subdir ending with slash.
+     */
+    private String partCOSubdir = "";
+
     private File checkoutDirectory;

     /**
@@ -369,18 +376,19 @@ private void putInternal( File source, String
targetName )

             ScmProvider scmProvider = getScmProvider(
scmRepository.getProvider() );

-            String checkoutTargetName = source.isDirectory() ? targetName
: getDirname( targetName );
-            String relPath = checkOut( scmProvider, scmRepository,
checkoutTargetName, target );
+            boolean isDirectory = source.isDirectory();
+            String checkoutTargetName = isDirectory ? targetName :
getDirname( targetName );
+            String relPath = ensureDirs( scmProvider, scmRepository,
checkoutTargetName, target );

             File newCheckoutDirectory = new File( checkoutDirectory,
relPath );

-            File scmFile = new File( newCheckoutDirectory,
source.isDirectory() ? "" : getFilename( targetName ) );
+            File scmFile = new File( newCheckoutDirectory, isDirectory ?
"" : FileUtils.removePath( targetName, '/' ) );

             boolean fileAlreadyInScm = scmFile.exists();

             if ( !scmFile.equals( source ) )
             {
-                if ( source.isDirectory() )
+                if ( isDirectory )
                 {
                     FileUtils.copyDirectoryStructure( source, scmFile );
                 }
@@ -393,7 +401,7 @@ private void putInternal( File source, String
targetName )
             if ( !fileAlreadyInScm || scmFile.isDirectory() )
             {
                 int addedFiles = addFiles( scmProvider, scmRepository,
newCheckoutDirectory,
-                                           source.isDirectory() ? "" :
scmFile.getName() );
+                                           isDirectory ? "" :
scmFile.getName() );

                 if ( !fileAlreadyInScm && addedFiles == 0 )
                 {
@@ -437,14 +445,16 @@ private void putInternal( File source, String
targetName )
      * @param targetName
      * @return
      * @throws TransferFailedException
+     * @throws IOException
      */
-    private String checkOut( ScmProvider scmProvider, ScmRepository
scmRepository, String targetName,
+    private String ensureDirs( ScmProvider scmProvider, ScmRepository
scmRepository, String targetName,
                              Resource resource )
-        throws TransferFailedException
+        throws TransferFailedException, IOException
     {
-        checkoutDirectory = createCheckoutDirectory();
-
-        Stack<String> stack = new Stack<String>();
+        if ( checkoutDirectory == null )
+        {
+            checkoutDirectory = createCheckoutDirectory();
+        }

         String target = targetName;

@@ -453,81 +463,82 @@ private String checkOut( ScmProvider scmProvider,
ScmRepository scmRepository, S
         // Check whether targetName, which is a relative path into the
scm, exists.
         // If it doesn't, check the parent, etc.

-        try
+        for ( ;; )
         {
-            while ( target.length() > 0 && !scmProvider.list(
scmRepository,
-                                                              new
ScmFileSet( new File( "." ), new File( target ) ),
-                                                              false,
makeScmVersion() ).isSuccess() )
+            try
+            {
+                ScmResult res = tryPartialCheckout( target );
+                if ( !res.isSuccess() )
+                {
+                    throw new ScmException( "command failed: " +
res.getCommandOutput().trim() );
+                }
+                break;
+            }
+            catch ( ScmException e )
             {
-                stack.push( getFilename( target ) );
+                if ( partCOSubdir.length() == 0 )
+                {
+                    fireTransferError( resource, e,
TransferEvent.REQUEST_GET );
+
+                    throw new TransferFailedException( "Error checking
out: " + e.getMessage(), e );
+                }
                 target = getDirname( target );
             }
         }
-        catch ( ScmException e )
-        {
-            fireTransferError( resource, e, TransferEvent.REQUEST_GET );

-            throw new TransferFailedException( "Error listing repository:
" + e.getMessage(), e );
-        }
+        // now create the subdirs in target, if it's a parent of targetName

-        // ok, we've established that target exists, or is empty.
-        // Check the resource out; if it doesn't exist, that means we're
in the svn repo url root,
-        // and the configuration is incorrect. We will not try
repo.getParent since most scm's don't
-        // implement that.
+        String res =
+            partCOSubdir.length() >= targetName.length() ? "" :
targetName.substring( partCOSubdir.length() ) + '/';

+        ArrayList<File> createdDirs = new ArrayList<File>();
+        File deepDir = new File( checkoutDirectory, res );
+
+        boolean added = false;
         try
         {
-            String repoUrl = getRepository().getUrl();
-            if ( "svn".equals( scmProvider.getScmType() ) )
+            mkdirsThrow( deepDir, createdDirs );
+            if ( createdDirs.size() != 0 )
             {
-                // Subversion is the only SCM that adds path structure to
represent tags and branches.
-                // The rest use scmVersion and scmVersionType.
-                repoUrl += "/" + target.replace( '\\', '/' );
-            }
-            scmRepository = getScmRepository( repoUrl );
-            CheckOutScmResult ret =
-                scmProvider.checkOut( scmRepository, new ScmFileSet( new
File( checkoutDirectory, "" ) ),
-                                      makeScmVersion(), false );
+                File topNewDir = createdDirs.get( 0 );
+                String relTopNewDir =
+                    topNewDir.getPath().substring(
checkoutDirectory.getPath().length() + 1 ).replace( '\\', '/' );

-            checkScmResult( ret );
+                addFiles( scmProvider, scmRepository, checkoutDirectory,
relTopNewDir );
+                added = true;
+            }
         }
         catch ( ScmException e )
         {
-            fireTransferError( resource, e, TransferEvent.REQUEST_GET );
+            fireTransferError( resource, e, TransferEvent.REQUEST_PUT );

-            throw new TransferFailedException( "Error checking out: " +
e.getMessage(), e );
+            throw new TransferFailedException( "Failed to add directory "
+ createdDirs.get( 0 ) + " to working copy",
+                                               e );
         }
-
-        // now create the subdirs in target, if it's a parent of targetName
-
-        String relPath = "";
-
-        while ( !stack.isEmpty() )
+        finally
         {
-            String p = stack.pop();
-            relPath += p + "/";
-
-            File newDir = new File( checkoutDirectory, relPath );
-            if ( !newDir.mkdirs() )
+            if ( !added && createdDirs.size() != 0 )
             {
-                throw new TransferFailedException(
-                    "Failed to create directory " +
newDir.getAbsolutePath() + "; parent should exist: "
-                        + checkoutDirectory );
+                FileUtils.deleteDirectory( createdDirs.get( 0 ) );
             }
+        }

-            try
-            {
-                addFiles( scmProvider, scmRepository, checkoutDirectory,
relPath );
-            }
-            catch ( ScmException e )
-            {
-                fireTransferError( resource, e, TransferEvent.REQUEST_PUT
);
+        return res;
+    }

-                throw new TransferFailedException( "Failed to add
directory " + newDir + " to working copy", e );
+    private static void mkdirsThrow( File f, List<File> createdDirs )
+        throws IOException
+    {
+        if ( !f.isDirectory() )
+        {
+            File parent = f.getParentFile();
+            mkdirsThrow( parent, createdDirs );
+            if ( !f.mkdir() )
+            {
+                throw new IOException( "Failed to create directory " +
f.getAbsolutePath() );
             }
+            createdDirs.add( f );
         }
-
-        return relPath;
     }

     /**
@@ -551,7 +562,8 @@ private int addFiles( ScmProvider scmProvider,
ScmRepository scmRepository, File

         if ( scmFilePath.length() != 0 )
         {
-            AddScmResult result = scmProvider.add( scmRepository, new
ScmFileSet( basedir, new File( scmFilePath ) ) );
+            AddScmResult result =
+                scmProvider.add( scmRepository, new ScmFileSet( basedir,
new File( scmFilePath ) ), mkBinaryFlag() );

             /*
              * TODO dirty fix to work around files with property
svn:eol-style=native if a file has that property, first
@@ -560,7 +572,9 @@ private int addFiles( ScmProvider scmProvider,
ScmRepository scmRepository, File
              */
             if ( !result.isSuccess() )
             {
-                result = scmProvider.add( scmRepository, new ScmFileSet(
basedir, new File( scmFilePath ) ) );
+                result =
+                    scmProvider.add( scmRepository, new ScmFileSet(
basedir, new File( scmFilePath ) ),
+                                     mkBinaryFlag() );
             }

             addedFiles = result.getAddedFiles().size();
@@ -584,6 +598,26 @@ private int addFiles( ScmProvider scmProvider,
ScmRepository scmRepository, File
         return addedFiles;
     }

+    private CheckOutScmResult checkOut( ScmProvider scmProvider,
ScmRepository scmRepository, ScmFileSet fileSet )
+        throws ScmException
+    {
+        ScmVersion ver = makeScmVersion();
+        CommandParameters parameters = mkBinaryFlag();
+        // TODO: AbstractScmProvider 6f7dd0c ignores checkOut() parameter
"version"
+        parameters.setScmVersion( CommandParameter.SCM_VERSION, ver );
+        parameters.setString( CommandParameter.RECURSIVE,
Boolean.toString( false ) );
+        parameters.setString( CommandParameter.SHALLOW, Boolean.toString(
true ) );
+
+        return scmProvider.checkOut( scmRepository, fileSet, ver,
parameters );
+    }
+
+    private CommandParameters mkBinaryFlag() throws ScmException
+    {
+        CommandParameters parameters = new CommandParameters();
+        parameters.setString( CommandParameter.BINARY, Boolean.toString(
true ) );
+        return parameters;
+    }
+
     /**
      * @return true
      */
@@ -592,6 +626,12 @@ public boolean supportsDirectoryCopy()
         return true;
     }

+    private boolean supportsPartialCheckout( ScmProvider scmProvider )
+    {
+        String scmType = scmProvider.getScmType();
+        return ( "svn".equals( scmType ) || "cvs".equals( scmType ) );
+    }
+
     public void putDirectory( File sourceDirectory, String
destinationDirectory )
         throws TransferFailedException, ResourceDoesNotExistException,
AuthorizationException
     {
@@ -646,16 +686,19 @@ public void get( String resourceName, File
destination )

         fireGetInitiated( resource, destination );

-        String url = getRepository().getUrl() + "/" + resourceName;
-
-        // remove the file
-        url = url.substring( 0, url.lastIndexOf( '/' ) );
+        fireGetStarted( resource, destination );

         try
         {
-            ScmRepository scmRepository = getScmRepository( url );
-
-            fireGetStarted( resource, destination );
+            String subdir = getDirname( resourceName );
+            ScmResult res = tryPartialCheckout( subdir );
+            if ( !res.isSuccess() && ( partCOSubdir.length() == 0 || res
instanceof UpdateScmResult ) )
+            {
+                // inability to checkout SVN or CVS subdir is not fatal.
We just assume it doesn't exist
+                // inability to update existing subdir or checkout root is
fatal
+                throw new ScmException( "command failed: " +
res.getCommandOutput().trim() );
+            }
+            resourceName = resourceName.substring( partCOSubdir.length() );

             // TODO: limitations:
             // - destination filename must match that in the repository -
should allow the "-d" CVS equiv to be passed
@@ -666,24 +709,6 @@ public void get( String resourceName, File destination
)

             File scmFile = new File( checkoutDirectory, resourceName );

-            File basedir = scmFile.getParentFile();
-
-            ScmProvider scmProvider = getScmProvider(
scmRepository.getProvider() );
-
-            String reservedScmFile = scmProvider.getScmSpecificFilename();
-
-            if ( reservedScmFile != null && new File( basedir,
reservedScmFile ).exists() )
-            {
-                scmProvider.update( scmRepository, new ScmFileSet( basedir
), makeScmVersion() );
-            }
-            else
-            {
-                // TODO: this should be checking out a full hierarchy
(requires the -d equiv)
-                basedir.mkdirs();
-
-                scmProvider.checkOut( scmRepository, new ScmFileSet(
basedir ), makeScmVersion() );
-            }
-
             if ( !scmFile.exists() )
             {
                 throw new ResourceDoesNotExistException( "Unable to find
resource " + destination + " after checkout" );
@@ -712,6 +737,49 @@ public void get( String resourceName, File destination
)
         fireGetCompleted( resource, destination );
     }

+    private ScmResult tryPartialCheckout( String subdir )
+        throws ScmException, IOException
+    {
+        String url = getRepository().getUrl();
+
+        String desiredPartCOSubdir = "";
+
+        ScmRepository scmRepository = getScmRepository( url );
+        ScmProvider scmProvider = getScmProvider(
scmRepository.getProvider() );
+        if ( subdir.length() != 0 && supportsPartialCheckout( scmProvider
) )
+        {
+            url += ( url.endsWith( "/" ) ? "" : "/" ) + subdir;
+
+            desiredPartCOSubdir = subdir + "/";
+
+            scmRepository = getScmRepository( url );
+        }
+
+        if ( !desiredPartCOSubdir.equals( partCOSubdir ) )
+        {
+            FileUtils.deleteDirectory( checkoutDirectory );
+            partCOSubdir = desiredPartCOSubdir;
+        }
+
+        ScmResult res;
+        if ( checkoutDirExists( scmProvider ) )
+        {
+            res = scmProvider.update( scmRepository, new ScmFileSet(
checkoutDirectory ), makeScmVersion() );
+        }
+        else
+        {
+            res = checkOut( scmProvider, scmRepository, new ScmFileSet(
checkoutDirectory ) );
+        }
+        return res;
+    }
+
+    private boolean checkoutDirExists( ScmProvider scmProvider )
+    {
+        String reservedScmFile = scmProvider.getScmSpecificFilename();
+        File pathToCheck = reservedScmFile == null ? checkoutDirectory :
new File( checkoutDirectory, reservedScmFile );
+        return pathToCheck.exists();
+    }
+
     /**
      * @return a List&lt;String&gt; with filenames/directories at the
resourcepath.
      * @see org.apache.maven.wagon.AbstractWagon#getFileList(
java.lang.String)
@@ -764,15 +832,8 @@ public boolean resourceExists( String resourceName )
         }
     }

-    private String getFilename( String filename )
-    {
-        String fname = StringUtils.replace( filename, "/", File.separator
);
-        return FileUtils.filename( fname );
-    }
-
-    private String getDirname( String filename )
+    private String getDirname( String resourceName )
     {
-        String fname = StringUtils.replace( filename, "/", File.separator
);
-        return FileUtils.dirname( fname );
+        return FileUtils.getPath( resourceName, '/' );
     }
 }
diff --git a/wagon-providers/wagon-scm/src/test/java/org/apache/
maven/wagon/providers/scm/AbstractScmCvsWagonTest.java
b/wagon-providers/wagon-scm/src/test/java/org/apache/
maven/wagon/providers/scm/AbstractScmCvsWagonTest.java
index fc7b7f82..eac45626 100644
--- a/wagon-providers/wagon-scm/src/test/java/org/apache/
maven/wagon/providers/scm/AbstractScmCvsWagonTest.java
+++ b/wagon-providers/wagon-scm/src/test/java/org/apache/
maven/wagon/providers/scm/AbstractScmCvsWagonTest.java
@@ -41,6 +41,6 @@ protected String getTestRepositoryUrl()
     {
         String repository = getTestFile( "target/test-classes/test-repo-cvs"
).getAbsolutePath();

-        return "scm:cvs|local|" + repository + "|repository/newfolder";
+        return "scm:cvs|local|" + repository + "|repository";
     }
 }




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services



On Mon, Jun 4, 2018 at 4:32 PM, GitBox <gi...@apache.org> wrote:

> asfgit closed pull request #46: [WAGON-495] fix
> testWagonPutDirectoryWhenDirectoryAlreadyExists false positive
> URL: https://github.com/apache/maven-wagon/pull/46
>
>
>
>
> This is a PR merged from a forked repository.
> As GitHub hides the original diff on merge, it is displayed below for
> the sake of provenance:
>
> As this is a foreign pull request (from a fork), the diff is supplied
> below (as it won't show otherwise due to GitHub magic):
>
> diff --git a/wagon-provider-test/src/main/java/org/apache/maven/wagon/WagonTestCase.java
> b/wagon-provider-test/src/main/java/org/apache/maven/
> wagon/WagonTestCase.java
> index fb277c7f..e89f1e29 100644
> --- a/wagon-provider-test/src/main/java/org/apache/maven/
> wagon/WagonTestCase.java
> +++ b/wagon-provider-test/src/main/java/org/apache/maven/
> wagon/WagonTestCase.java
> @@ -42,6 +42,7 @@
>
>  import java.io.File;
>  import java.io.IOException;
> +import java.nio.charset.StandardCharsets;
>  import java.security.NoSuchAlgorithmException;
>  import java.text.SimpleDateFormat;
>  import java.util.ArrayList;
> @@ -74,6 +75,10 @@ public int getSize()
>          }
>      }
>
> +    protected static final String TEST_CONTENT = "test-resource.txt\n";
> +
> +    protected static final String TEST_CKSUM = cksum( TEST_CONTENT );
> +
>      protected static final String POM = "pom.xml";
>
>      protected Repository localRepository;
> @@ -233,16 +238,13 @@ protected void message( String message )
>      public void testWagon()
>          throws Exception
>      {
> -        if ( supportsGetIfNewer() )
> -        {
> -            setupRepositories();
> +        setupRepositories();
>
> -            setupWagonTestingFixtures();
> +        setupWagonTestingFixtures();
>
> -            fileRoundTripTesting();
> +        fileRoundTripTesting();
>
> -            tearDownWagonTestingFixtures();
> -        }
> +        tearDownWagonTestingFixtures();
>      }
>
>      public void testWagonGetIfNewerIsNewer()
> @@ -341,7 +343,7 @@ protected void assertGetIfNewerTest( ProgressAnswer
> progressAnswer, boolean expe
>
>              assertNotNull( "check checksum is not null", checksumObserver.getActualChecksum()
> );
>
> -            assertEquals( "compare checksums", "
> 6b144b7285ffd6b0bc8300da162120b9",
> +            assertEquals( "compare checksums", TEST_CKSUM,
>                            checksumObserver.getActualChecksum() );
>
>              // Now compare the contents of the artifact that was placed in
> @@ -923,7 +925,7 @@ protected TransferEvent createTransferEvent( Wagon
> wagon, Resource resource, int
>      protected int putFile()
>          throws Exception
>      {
> -        String content = "test-resource.txt\n";
> +        String content = TEST_CONTENT;
>          putFile( resource, "test-resource", content );
>          return content.length();
>      }
> @@ -1049,7 +1051,7 @@ protected void fileRoundTripTesting()
>
>          assertNotNull( "check checksum is not null", checksumObserver.getActualChecksum()
> );
>
> -        assertEquals( "compare checksums", "
> 6b144b7285ffd6b0bc8300da162120b9", checksumObserver.getActualChecksum() );
> +        assertEquals( "compare checksums", TEST_CKSUM, checksumObserver.getActualChecksum()
> );
>
>          checksumObserver = new ChecksumObserver();
>
> @@ -1057,7 +1059,7 @@ protected void fileRoundTripTesting()
>
>          assertNotNull( "check checksum is not null", checksumObserver.getActualChecksum()
> );
>
> -        assertEquals( "compare checksums", "
> 6b144b7285ffd6b0bc8300da162120b9", checksumObserver.getActualChecksum() );
> +        assertEquals( "compare checksums", TEST_CKSUM, checksumObserver.getActualChecksum()
> );
>
>          // Now compare the conents of the artifact that was placed in
>          // the repository with the contents of the artifact that was
> @@ -1087,4 +1089,22 @@ protected Repository createFileRepository( String
> url )
>          return repository;
>      }
>
> +    protected static String cksum( String content )
> +    {
> +        String checkSum;
> +        try
> +        {
> +            ChecksumObserver obs = new ChecksumObserver();
> +            byte[] buf = content.getBytes( StandardCharsets.ISO_8859_1 );
> +            obs.transferProgress( null, buf, buf.length );
> +            obs.transferCompleted( null );
> +            checkSum = obs.getActualChecksum();
> +        }
> +        catch ( Exception e )
> +        {
> +            checkSum = null;
> +        }
> +        return checkSum;
> +    }
> +
>  }
> diff --git a/wagon-providers/wagon-scm/src/main/java/org/apache/
> maven/wagon/providers/scm/ScmWagon.java b/wagon-providers/wagon-scm/
> src/main/java/org/apache/maven/wagon/providers/scm/ScmWagon.java
> index 8fbbdeff..d6531eef 100644
> --- a/wagon-providers/wagon-scm/src/main/java/org/apache/
> maven/wagon/providers/scm/ScmWagon.java
> +++ b/wagon-providers/wagon-scm/src/main/java/org/apache/
> maven/wagon/providers/scm/ScmWagon.java
> @@ -19,6 +19,15 @@
>   * under the License.
>   */
>
> +import java.io.File;
> +import java.io.IOException;
> +import java.text.DecimalFormat;
> +import java.util.ArrayList;
> +import java.util.List;
> +import java.util.Random;
> +
> +import org.apache.maven.scm.CommandParameter;
> +import org.apache.maven.scm.CommandParameters;
>  import org.apache.maven.scm.ScmBranch;
>  import org.apache.maven.scm.ScmException;
>  import org.apache.maven.scm.ScmFile;
> @@ -30,6 +39,7 @@
>  import org.apache.maven.scm.command.add.AddScmResult;
>  import org.apache.maven.scm.command.checkout.CheckOutScmResult;
>  import org.apache.maven.scm.command.list.ListScmResult;
> +import org.apache.maven.scm.command.update.UpdateScmResult;
>  import org.apache.maven.scm.manager.NoSuchScmProviderException;
>  import org.apache.maven.scm.manager.ScmManager;
>  import org.apache.maven.scm.provider.ScmProvider;
> @@ -47,14 +57,6 @@
>  import org.codehaus.plexus.util.FileUtils;
>  import org.codehaus.plexus.util.StringUtils;
>
> -import java.io.File;
> -import java.io.IOException;
> -import java.text.DecimalFormat;
> -import java.util.ArrayList;
> -import java.util.List;
> -import java.util.Random;
> -import java.util.Stack;
> -
>  /**
>   * Wagon provider to get and put files from and to SCM systems, using
> Maven-SCM as underlying transport.
>   * <p/>
> @@ -93,6 +95,11 @@
>       */
>      private String scmVersionType;
>
> +    /**
> +     * Empty string or subdir ending with slash.
> +     */
> +    private String partCOSubdir = "";
> +
>      private File checkoutDirectory;
>
>      /**
> @@ -369,18 +376,19 @@ private void putInternal( File source, String
> targetName )
>
>              ScmProvider scmProvider = getScmProvider(
> scmRepository.getProvider() );
>
> -            String checkoutTargetName = source.isDirectory() ? targetName
> : getDirname( targetName );
> -            String relPath = checkOut( scmProvider, scmRepository,
> checkoutTargetName, target );
> +            boolean isDirectory = source.isDirectory();
> +            String checkoutTargetName = isDirectory ? targetName :
> getDirname( targetName );
> +            String relPath = ensureDirs( scmProvider, scmRepository,
> checkoutTargetName, target );
>
>              File newCheckoutDirectory = new File( checkoutDirectory,
> relPath );
>
> -            File scmFile = new File( newCheckoutDirectory,
> source.isDirectory() ? "" : getFilename( targetName ) );
> +            File scmFile = new File( newCheckoutDirectory, isDirectory ?
> "" : FileUtils.removePath( targetName, '/' ) );
>
>              boolean fileAlreadyInScm = scmFile.exists();
>
>              if ( !scmFile.equals( source ) )
>              {
> -                if ( source.isDirectory() )
> +                if ( isDirectory )
>                  {
>                      FileUtils.copyDirectoryStructure( source, scmFile );
>                  }
> @@ -393,7 +401,7 @@ private void putInternal( File source, String
> targetName )
>              if ( !fileAlreadyInScm || scmFile.isDirectory() )
>              {
>                  int addedFiles = addFiles( scmProvider, scmRepository,
> newCheckoutDirectory,
> -                                           source.isDirectory() ? "" :
> scmFile.getName() );
> +                                           isDirectory ? "" :
> scmFile.getName() );
>
>                  if ( !fileAlreadyInScm && addedFiles == 0 )
>                  {
> @@ -437,14 +445,16 @@ private void putInternal( File source, String
> targetName )
>       * @param targetName
>       * @return
>       * @throws TransferFailedException
> +     * @throws IOException
>       */
> -    private String checkOut( ScmProvider scmProvider, ScmRepository
> scmRepository, String targetName,
> +    private String ensureDirs( ScmProvider scmProvider, ScmRepository
> scmRepository, String targetName,
>                               Resource resource )
> -        throws TransferFailedException
> +        throws TransferFailedException, IOException
>      {
> -        checkoutDirectory = createCheckoutDirectory();
> -
> -        Stack<String> stack = new Stack<String>();
> +        if ( checkoutDirectory == null )
> +        {
> +            checkoutDirectory = createCheckoutDirectory();
> +        }
>
>          String target = targetName;
>
> @@ -453,81 +463,82 @@ private String checkOut( ScmProvider scmProvider,
> ScmRepository scmRepository, S
>          // Check whether targetName, which is a relative path into the
> scm, exists.
>          // If it doesn't, check the parent, etc.
>
> -        try
> +        for ( ;; )
>          {
> -            while ( target.length() > 0 && !scmProvider.list(
> scmRepository,
> -                                                              new
> ScmFileSet( new File( "." ), new File( target ) ),
> -                                                              false,
> makeScmVersion() ).isSuccess() )
> +            try
> +            {
> +                ScmResult res = tryPartialCheckout( target );
> +                if ( !res.isSuccess() )
> +                {
> +                    throw new ScmException( "command failed: " +
> res.getCommandOutput().trim() );
> +                }
> +                break;
> +            }
> +            catch ( ScmException e )
>              {
> -                stack.push( getFilename( target ) );
> +                if ( partCOSubdir.length() == 0 )
> +                {
> +                    fireTransferError( resource, e,
> TransferEvent.REQUEST_GET );
> +
> +                    throw new TransferFailedException( "Error checking
> out: " + e.getMessage(), e );
> +                }
>                  target = getDirname( target );
>              }
>          }
> -        catch ( ScmException e )
> -        {
> -            fireTransferError( resource, e, TransferEvent.REQUEST_GET );
>
> -            throw new TransferFailedException( "Error listing repository:
> " + e.getMessage(), e );
> -        }
> +        // now create the subdirs in target, if it's a parent of
> targetName
>
> -        // ok, we've established that target exists, or is empty.
> -        // Check the resource out; if it doesn't exist, that means we're
> in the svn repo url root,
> -        // and the configuration is incorrect. We will not try
> repo.getParent since most scm's don't
> -        // implement that.
> +        String res =
> +            partCOSubdir.length() >= targetName.length() ? "" :
> targetName.substring( partCOSubdir.length() ) + '/';
>
> +        ArrayList<File> createdDirs = new ArrayList<File>();
> +        File deepDir = new File( checkoutDirectory, res );
> +
> +        boolean added = false;
>          try
>          {
> -            String repoUrl = getRepository().getUrl();
> -            if ( "svn".equals( scmProvider.getScmType() ) )
> +            mkdirsThrow( deepDir, createdDirs );
> +            if ( createdDirs.size() != 0 )
>              {
> -                // Subversion is the only SCM that adds path structure to
> represent tags and branches.
> -                // The rest use scmVersion and scmVersionType.
> -                repoUrl += "/" + target.replace( '\\', '/' );
> -            }
> -            scmRepository = getScmRepository( repoUrl );
> -            CheckOutScmResult ret =
> -                scmProvider.checkOut( scmRepository, new ScmFileSet( new
> File( checkoutDirectory, "" ) ),
> -                                      makeScmVersion(), false );
> +                File topNewDir = createdDirs.get( 0 );
> +                String relTopNewDir =
> +                    topNewDir.getPath().substring(
> checkoutDirectory.getPath().length() + 1 ).replace( '\\', '/' );
>
> -            checkScmResult( ret );
> +                addFiles( scmProvider, scmRepository, checkoutDirectory,
> relTopNewDir );
> +                added = true;
> +            }
>          }
>          catch ( ScmException e )
>          {
> -            fireTransferError( resource, e, TransferEvent.REQUEST_GET );
> +            fireTransferError( resource, e, TransferEvent.REQUEST_PUT );
>
> -            throw new TransferFailedException( "Error checking out: " +
> e.getMessage(), e );
> +            throw new TransferFailedException( "Failed to add directory "
> + createdDirs.get( 0 ) + " to working copy",
> +                                               e );
>          }
> -
> -        // now create the subdirs in target, if it's a parent of
> targetName
> -
> -        String relPath = "";
> -
> -        while ( !stack.isEmpty() )
> +        finally
>          {
> -            String p = stack.pop();
> -            relPath += p + "/";
> -
> -            File newDir = new File( checkoutDirectory, relPath );
> -            if ( !newDir.mkdirs() )
> +            if ( !added && createdDirs.size() != 0 )
>              {
> -                throw new TransferFailedException(
> -                    "Failed to create directory " +
> newDir.getAbsolutePath() + "; parent should exist: "
> -                        + checkoutDirectory );
> +                FileUtils.deleteDirectory( createdDirs.get( 0 ) );
>              }
> +        }
>
> -            try
> -            {
> -                addFiles( scmProvider, scmRepository, checkoutDirectory,
> relPath );
> -            }
> -            catch ( ScmException e )
> -            {
> -                fireTransferError( resource, e, TransferEvent.REQUEST_PUT
> );
> +        return res;
> +    }
>
> -                throw new TransferFailedException( "Failed to add
> directory " + newDir + " to working copy", e );
> +    private static void mkdirsThrow( File f, List<File> createdDirs )
> +        throws IOException
> +    {
> +        if ( !f.isDirectory() )
> +        {
> +            File parent = f.getParentFile();
> +            mkdirsThrow( parent, createdDirs );
> +            if ( !f.mkdir() )
> +            {
> +                throw new IOException( "Failed to create directory " +
> f.getAbsolutePath() );
>              }
> +            createdDirs.add( f );
>          }
> -
> -        return relPath;
>      }
>
>      /**
> @@ -551,7 +562,8 @@ private int addFiles( ScmProvider scmProvider,
> ScmRepository scmRepository, File
>
>          if ( scmFilePath.length() != 0 )
>          {
> -            AddScmResult result = scmProvider.add( scmRepository, new
> ScmFileSet( basedir, new File( scmFilePath ) ) );
> +            AddScmResult result =
> +                scmProvider.add( scmRepository, new ScmFileSet( basedir,
> new File( scmFilePath ) ), mkBinaryFlag() );
>
>              /*
>               * TODO dirty fix to work around files with property
> svn:eol-style=native if a file has that property, first
> @@ -560,7 +572,9 @@ private int addFiles( ScmProvider scmProvider,
> ScmRepository scmRepository, File
>               */
>              if ( !result.isSuccess() )
>              {
> -                result = scmProvider.add( scmRepository, new ScmFileSet(
> basedir, new File( scmFilePath ) ) );
> +                result =
> +                    scmProvider.add( scmRepository, new ScmFileSet(
> basedir, new File( scmFilePath ) ),
> +                                     mkBinaryFlag() );
>              }
>
>              addedFiles = result.getAddedFiles().size();
> @@ -584,6 +598,26 @@ private int addFiles( ScmProvider scmProvider,
> ScmRepository scmRepository, File
>          return addedFiles;
>      }
>
> +    private CheckOutScmResult checkOut( ScmProvider scmProvider,
> ScmRepository scmRepository, ScmFileSet fileSet )
> +        throws ScmException
> +    {
> +        ScmVersion ver = makeScmVersion();
> +        CommandParameters parameters = mkBinaryFlag();
> +        // TODO: AbstractScmProvider 6f7dd0c ignores checkOut() parameter
> "version"
> +        parameters.setScmVersion( CommandParameter.SCM_VERSION, ver );
> +        parameters.setString( CommandParameter.RECURSIVE,
> Boolean.toString( false ) );
> +        parameters.setString( CommandParameter.SHALLOW, Boolean.toString(
> true ) );
> +
> +        return scmProvider.checkOut( scmRepository, fileSet, ver,
> parameters );
> +    }
> +
> +    private CommandParameters mkBinaryFlag() throws ScmException
> +    {
> +        CommandParameters parameters = new CommandParameters();
> +        parameters.setString( CommandParameter.BINARY, Boolean.toString(
> true ) );
> +        return parameters;
> +    }
> +
>      /**
>       * @return true
>       */
> @@ -592,6 +626,12 @@ public boolean supportsDirectoryCopy()
>          return true;
>      }
>
> +    private boolean supportsPartialCheckout( ScmProvider scmProvider )
> +    {
> +        String scmType = scmProvider.getScmType();
> +        return ( "svn".equals( scmType ) || "cvs".equals( scmType ) );
> +    }
> +
>      public void putDirectory( File sourceDirectory, String
> destinationDirectory )
>          throws TransferFailedException, ResourceDoesNotExistException,
> AuthorizationException
>      {
> @@ -646,16 +686,19 @@ public void get( String resourceName, File
> destination )
>
>          fireGetInitiated( resource, destination );
>
> -        String url = getRepository().getUrl() + "/" + resourceName;
> -
> -        // remove the file
> -        url = url.substring( 0, url.lastIndexOf( '/' ) );
> +        fireGetStarted( resource, destination );
>
>          try
>          {
> -            ScmRepository scmRepository = getScmRepository( url );
> -
> -            fireGetStarted( resource, destination );
> +            String subdir = getDirname( resourceName );
> +            ScmResult res = tryPartialCheckout( subdir );
> +            if ( !res.isSuccess() && ( partCOSubdir.length() == 0 || res
> instanceof UpdateScmResult ) )
> +            {
> +                // inability to checkout SVN or CVS subdir is not fatal.
> We just assume it doesn't exist
> +                // inability to update existing subdir or checkout root
> is fatal
> +                throw new ScmException( "command failed: " +
> res.getCommandOutput().trim() );
> +            }
> +            resourceName = resourceName.substring( partCOSubdir.length()
> );
>
>              // TODO: limitations:
>              // - destination filename must match that in the repository -
> should allow the "-d" CVS equiv to be passed
> @@ -666,24 +709,6 @@ public void get( String resourceName, File
> destination )
>
>              File scmFile = new File( checkoutDirectory, resourceName );
>
> -            File basedir = scmFile.getParentFile();
> -
> -            ScmProvider scmProvider = getScmProvider(
> scmRepository.getProvider() );
> -
> -            String reservedScmFile = scmProvider.
> getScmSpecificFilename();
> -
> -            if ( reservedScmFile != null && new File( basedir,
> reservedScmFile ).exists() )
> -            {
> -                scmProvider.update( scmRepository, new ScmFileSet(
> basedir ), makeScmVersion() );
> -            }
> -            else
> -            {
> -                // TODO: this should be checking out a full hierarchy
> (requires the -d equiv)
> -                basedir.mkdirs();
> -
> -                scmProvider.checkOut( scmRepository, new ScmFileSet(
> basedir ), makeScmVersion() );
> -            }
> -
>              if ( !scmFile.exists() )
>              {
>                  throw new ResourceDoesNotExistException( "Unable to find
> resource " + destination + " after checkout" );
> @@ -712,6 +737,49 @@ public void get( String resourceName, File
> destination )
>          fireGetCompleted( resource, destination );
>      }
>
> +    private ScmResult tryPartialCheckout( String subdir )
> +        throws ScmException, IOException
> +    {
> +        String url = getRepository().getUrl();
> +
> +        String desiredPartCOSubdir = "";
> +
> +        ScmRepository scmRepository = getScmRepository( url );
> +        ScmProvider scmProvider = getScmProvider(
> scmRepository.getProvider() );
> +        if ( subdir.length() != 0 && supportsPartialCheckout( scmProvider
> ) )
> +        {
> +            url += ( url.endsWith( "/" ) ? "" : "/" ) + subdir;
> +
> +            desiredPartCOSubdir = subdir + "/";
> +
> +            scmRepository = getScmRepository( url );
> +        }
> +
> +        if ( !desiredPartCOSubdir.equals( partCOSubdir ) )
> +        {
> +            FileUtils.deleteDirectory( checkoutDirectory );
> +            partCOSubdir = desiredPartCOSubdir;
> +        }
> +
> +        ScmResult res;
> +        if ( checkoutDirExists( scmProvider ) )
> +        {
> +            res = scmProvider.update( scmRepository, new ScmFileSet(
> checkoutDirectory ), makeScmVersion() );
> +        }
> +        else
> +        {
> +            res = checkOut( scmProvider, scmRepository, new ScmFileSet(
> checkoutDirectory ) );
> +        }
> +        return res;
> +    }
> +
> +    private boolean checkoutDirExists( ScmProvider scmProvider )
> +    {
> +        String reservedScmFile = scmProvider.getScmSpecificFilename();
> +        File pathToCheck = reservedScmFile == null ? checkoutDirectory :
> new File( checkoutDirectory, reservedScmFile );
> +        return pathToCheck.exists();
> +    }
> +
>      /**
>       * @return a List&lt;String&gt; with filenames/directories at the
> resourcepath.
>       * @see org.apache.maven.wagon.AbstractWagon#getFileList(
> java.lang.String)
> @@ -764,15 +832,8 @@ public boolean resourceExists( String resourceName )
>          }
>      }
>
> -    private String getFilename( String filename )
> -    {
> -        String fname = StringUtils.replace( filename, "/", File.separator
> );
> -        return FileUtils.filename( fname );
> -    }
> -
> -    private String getDirname( String filename )
> +    private String getDirname( String resourceName )
>      {
> -        String fname = StringUtils.replace( filename, "/", File.separator
> );
> -        return FileUtils.dirname( fname );
> +        return FileUtils.getPath( resourceName, '/' );
>      }
>  }
> diff --git a/wagon-providers/wagon-scm/src/test/java/org/apache/
> maven/wagon/providers/scm/AbstractScmCvsWagonTest.java
> b/wagon-providers/wagon-scm/src/test/java/org/apache/
> maven/wagon/providers/scm/AbstractScmCvsWagonTest.java
> index fc7b7f82..eac45626 100644
> --- a/wagon-providers/wagon-scm/src/test/java/org/apache/
> maven/wagon/providers/scm/AbstractScmCvsWagonTest.java
> +++ b/wagon-providers/wagon-scm/src/test/java/org/apache/
> maven/wagon/providers/scm/AbstractScmCvsWagonTest.java
> @@ -41,6 +41,6 @@ protected String getTestRepositoryUrl()
>      {
>          String repository = getTestFile( "target/test-classes/test-repo-cvs"
> ).getAbsolutePath();
>
> -        return "scm:cvs|local|" + repository + "|repository/newfolder";
> +        return "scm:cvs|local|" + repository + "|repository";
>      }
>  }
>
>
>
>
> ----------------------------------------------------------------
> This is an automated message from the Apache Git Service.
> To respond to the message, please log on GitHub and use the
> URL above to go to the specific comment.
>
> For queries about this service, please contact Infrastructure at:
> users@infra.apache.org
>
>
> With regards,
> Apache Git Services
>