You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "ASF GitHub Bot (JIRA)" <ji...@apache.org> on 2018/06/04 11:03:00 UTC

[jira] [Commented] (WAGON-497) ScmWagon#put() strips parent dirs from the target path if they already exist in SCM

    [ https://issues.apache.org/jira/browse/WAGON-497?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16500066#comment-16500066 ] 

ASF GitHub Bot commented on WAGON-497:
--------------------------------------

asfgit closed pull request #40: [WAGON-497] fix ScmWagon.put() strips existing parent directories
URL: https://github.com/apache/maven-wagon/pull/40
 
 
   

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..4c604c73 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
@@ -475,6 +475,7 @@ private String checkOut( ScmProvider scmProvider, ScmRepository scmRepository, S
         // and the configuration is incorrect. We will not try repo.getParent since most scm's don't
         // implement that.
 
+        target = target.replace( '\\', '/' );
         try
         {
             String repoUrl = getRepository().getUrl();
@@ -482,7 +483,11 @@ private String checkOut( ScmProvider scmProvider, ScmRepository scmRepository, S
             {
                 // Subversion is the only SCM that adds path structure to represent tags and branches.
                 // The rest use scmVersion and scmVersionType.
-                repoUrl += "/" + target.replace( '\\', '/' );
+                if ( target.length() > 0 )
+                {
+                    repoUrl += "/" + target;
+                    target = "";
+                }
             }
             scmRepository = getScmRepository( repoUrl );
             CheckOutScmResult ret =
@@ -500,7 +505,7 @@ private String checkOut( ScmProvider scmProvider, ScmRepository scmRepository, S
 
         // now create the subdirs in target, if it's a parent of targetName
 
-        String relPath = "";
+        String relPath = target.concat( target.length() > 0 ? "/" : "" );
 
         while ( !stack.isEmpty() )
         {
@@ -508,7 +513,8 @@ private String checkOut( ScmProvider scmProvider, ScmRepository scmRepository, S
             relPath += p + "/";
 
             File newDir = new File( checkoutDirectory, relPath );
-            if ( !newDir.mkdirs() )
+            newDir.mkdir();
+            if ( !newDir.isDirectory() )
             {
                 throw new TransferFailedException(
                     "Failed to create directory " + newDir.getAbsolutePath() + "; parent should exist: "
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


> ScmWagon#put() strips parent dirs from the target path if they already exist in SCM
> -----------------------------------------------------------------------------------
>
>                 Key: WAGON-497
>                 URL: https://issues.apache.org/jira/browse/WAGON-497
>             Project: Maven Wagon
>          Issue Type: Bug
>          Components: wagon-scm
>    Affects Versions: 3.0.0, 3.1.0
>            Reporter: Ilya Basin
>            Assignee: Michael Osipov
>            Priority: Major
>             Fix For: 3.1.0
>
>
> Here're the symptoms of the bug:
>  ScmCvsExeWagonTest.testWagonGetFileList() wants to checkin multiple files into "/file-list/".
>  When the test repo is fresh, only the first file is checked in correctly. All other files are checked into the repo root.
> Reason (and why it works fine with SVN):
> In ScmWagon.checkOut() targetName is split to existing parth an non-existing part.
> in case of svn:
>  - checks out repoUrl + existingPart
>  - mkdirs missingPart
>  - return missingPart
> the caller copies the new file into coDir/ + missingPart/
> on second put() all dirs exist
>  - checks out repoUrl + allPart
>  - return empty string
> the caller copies the new file into coDir/
>  in case of cvs (on second put()):
>  - checks out repoUrl WITHOUT allPart
>  - returns empty string
>  the caller copies the new file into coDir/
> My current goal is to re-enable ScmCvsExeWagonTest. There are numerous bugs in wagon-scm and fixing just one is not enough to run even a single test case.
> However, the proposed pull request will let you reach the point in [WagonTestCase#testWagonGetFileList()|https://github.com/apache/maven-wagon/blob/be94400731575f54ff537ec90359460f42a561cc/wagon-provider-test/src/main/java/org/apache/maven/wagon/WagonTestCase.java#L741] when all test files are checked in correctly.
> UPD: The test case "testWagon" which could reveal that is disabled by mistake: it runs only if supportsGetIfNewer(), but we don't call getIfNewer() there.
> UPD2: the cvs rls command is not widely supported, but ScmWagon cannot checkout when list() unsupported: it throws "Failed to create directory ", although the directory already exists.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)