[GitHub] ant pull request #37: [1.9.x branch] BZ-60644 Fix file corruption during cop...

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] ant pull request #37: [1.9.x branch] BZ-60644 Fix file corruption during cop...

bodewig
GitHub user jaikiran opened a pull request:

    https://github.com/apache/ant/pull/37

    [1.9.x branch] BZ-60644 Fix file corruption during copy task

    The commit here includes a fix for the issue reported in https://bz.apache.org/bugzilla/show_bug.cgi?id=60644. As described in that issue, the copy task corrupts the source file(s) that's being copied in cases where the target file points back to the source file through symlink.
   
    The commit here (intentionally) handles/fixes this issue in core/central location and not just in the Copy task so that all such copy operations (through the `ResourceUtils`) now have the safeguard to ensure that such copying doesn't cause the source file corruption and the copying itself is skipped if the source and destination are the same resource.
   
    This commit also includes a testcase to verify and fix the issue.
   
    This PR is against the 1.9.x branch. Once tested, approved and merged, I can make corresponding PR/commits to the master branch to fix it there too.
   
    P.S: Given that this change is in a central location (the `ResourceUtils`), for obvious reasons, this PR will have to run past the complete set of tests before being merged.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jaikiran/ant bz-60644

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/ant/pull/37.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #37
   
----

----


---

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

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant pull request #37: [1.9.x branch] BZ-60644 Fix file corruption during cop...

bodewig
Github user bodewig commented on a diff in the pull request:

    https://github.com/apache/ant/pull/37#discussion_r141578555
 
    --- Diff: src/main/org/apache/tools/ant/util/ResourceUtils.java ---
    @@ -666,6 +666,14 @@ private static void copyWithFilterSets(final Resource source, final Resource des
                                                final String outputEncoding,
                                                final Project project)
             throws IOException {
    +        if (source instanceof FileResource && dest instanceof FileResource) {
    --- End diff --
   
    I think it would be better to use `FileProvider` rather than `FileResource` here (and in the next blocks). Actually I'd probably move the `instanceof` checks into `areSame` (and replace them with `resource.as(FileProvider.class)` there. This allows for other resources that know they are working on files and removes at least one line of duplicate code per `if` block :-)


---

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

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #37: [1.9.x branch] BZ-60644 Fix file corruption during copy task

bodewig
In reply to this post by bodewig
Github user bodewig commented on the issue:

    https://github.com/apache/ant/pull/37
 
    Great change and I agree it should be fixed in the central location. Many thanks!


---

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

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant pull request #37: [1.9.x branch] BZ-60644 Fix file corruption during cop...

bodewig
In reply to this post by bodewig
Github user jaikiran commented on a diff in the pull request:

    https://github.com/apache/ant/pull/37#discussion_r141594830
 
    --- Diff: src/main/org/apache/tools/ant/util/ResourceUtils.java ---
    @@ -666,6 +666,14 @@ private static void copyWithFilterSets(final Resource source, final Resource des
                                                final String outputEncoding,
                                                final Project project)
             throws IOException {
    +        if (source instanceof FileResource && dest instanceof FileResource) {
    --- End diff --
   
    That makes sense. I have changed it to follow your suggestion and updated this PR. I am going to run this against both windows on linux, through a Jenkins job, before merging this.


---

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

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #37: [1.9.x branch] BZ-60644 Fix file corruption during copy task

bodewig
In reply to this post by bodewig
Github user jaikiran commented on the issue:

    https://github.com/apache/ant/pull/37
 
    Tests against various JDK versions against Windows and Linux went fine without any regressions. Will go ahead and merge this now.


---

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

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant pull request #37: [1.9.x branch] BZ-60644 Fix file corruption during cop...

bodewig
In reply to this post by bodewig
Github user asfgit closed the pull request at:

    https://github.com/apache/ant/pull/37


---

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