plexus-archiver / plexus-util Issue

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

plexus-archiver / plexus-util Issue

Karl Heinz Marbaise-3
Hi,

after diving more into this..

I found that if I upgrade plexus-utils to 3.1.0 in plexus-archiver the
tests in plexus-archiver stuck completely...as I already observed (which
I though was another cause)...

https://travis-ci.org/codehaus-plexus/plexus-archiver/jobs/321821294

So more diving into the details I realized that the following code in
plexus-utils causes the issue:

FileUtils.java:

     private static void doCopyFile( File source, File destination )
         throws IOException
     {
         // offload to operating system if supported
         if ( Java7Detector.isJava7() )
         {
             doCopyFileUsingNewIO( source, destination );
         }
         else
         {
             doCopyFileUsingLegacyIO( source, destination );
         }
     }

The real issue is located in the implementation of doCopyFileUsingNewIO
which uses:



     public static File copy( File source, File target )
         throws IOException
     {
         Path copy = Files.copy( source.toPath(), target.toPath(),
                    StandardCopyOption.REPLACE_EXISTING,
                    StandardCopyOption.COPY_ATTRIBUTES,
                    LinkOption.NOFOLLOW_LINKS );
         return copy.toFile();
     }

and If I correctly understand the whole thing is the real cause of that
based on the usage of StandardCopyOption.COPY_ATTRIBUTES which includes
copying of the last-modified of the file...which means not to change the
last-modified entry of the file and in result that is the reason of not
changing it and the stucking of the unit test in plexus-archiver...(see
travis build above)..


After I have changed that and only use

     public static File copy( File source, File target )
         throws IOException
     {
         Path copy = Files.copy( source.toPath(), target.toPath(),
                    StandardCopyOption.REPLACE_EXISTING,
                    LinkOption.NOFOLLOW_LINKS );
         return copy.toFile();
     }

it looks like working (see branch issue-fix):

What Do you think?

Kind regards
Karl Heinz Marbaise

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: plexus-archiver / plexus-util Issue

stephenconnolly
On Thu 11 Jan 2018 at 18:47, Plamen Totev <[hidden email]> wrote:

> Hi,
>
> On 12/28/2017 10:16 PM, Karl Heinz Marbaise wrote:
> > Hi,
> >
> > On 28/12/17 20:46, Stephen Connolly wrote:
> >> But shouldn’t we be copying the last modified time stamp always anyway
> >
> > I'm the same opinion...but that means in consequence that the test in
> > plexus-archiver is not correct...cause it's waiting for a change in the
> > last-modified time ...
> >
> > Hm...
> >
> > Kind regards
> > Karl Heinz Marbaise
> >
> >
>
> Plexus Archiver tests are using file copy operation to change the
> "last modified" timestamp of a file. This of course is not a perfect
> approach but I don't think there was better one pre-Java 7. But now as
> Java 7 is the minimum required version, there is
> Files#setLastModifiedTime that does better job. I've created a PR that
> changes the incompatible code[1]. It will allow the update of Plexus
> Utils to 3.1.0 as well.
>
> > After I have changed that and only use
> >
> >     public static File copy( File source, File target )
> >         throws IOException
> >     {
> >         Path copy = Files.copy( source.toPath(), target.toPath(),
> >                    StandardCopyOption.REPLACE_EXISTING,
> >                    LinkOption.NOFOLLOW_LINKS );
> >         return copy.toFile();
> >     }
> >
> > it looks like working (see branch issue-fix):
> >
> > What Do you think?
>
> The Plexus Archiver tests were not using `copy` for its intended
> purpose so the decision if the file attributes are copied should not
> be based on their usage. To me it makes more sense to copy the
> attributes, although copying the last modified timestamp may surprise
> some evelopers (like me for example) as `cp` on Linux updates the last
> modified timestamp of the copy.


Ahh but extraction from an archive should preserve the archive time stamp...

And dir-to-dir copy is really just archive-unarchive without the
intermediate archive file ;-)


>
> Regards,
> Plamen Totev
>
> [1]  https://github.com/codehaus-plexus/plexus-archiver/pull/77
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
> --
Sent from my phone