Hi Jaikiran, Looked through the changes and they look good and simple and robust. I like not having to use the new system property.
Thanks, ~Roger Whitcomb > On Jul 1, 2025, at 9:52 AM, Jaikiran Pai <jaiki...@apache.org> wrote: > > I had a detailed look at these failures. The majority of them were due to > the tearDown() in these tests no longer able to delete the test generated > read-only files. That would then cause subsequent tests to fail because they > weren't expecting a read-only file to be present. I fixed that issue through > this commit into Ant's repo > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_ant_commit_54350f7e967f965169a07f65ec925b621cef231d&d=DwICaQ&c=euGZstcaTDllvimEN8b7jXrwqOf-v5A_CdpgnVfiiMM&r=JYLg0shOavzSp2WxYFFvMuhqXdAQzkMRPf_4HQmFZZ4&m=xdy7XeVjARGFk7kSWJvmNeCKZ4CBakKqrfrJngbbrPVabAfalygFpUsjoUuye3AE&s=w5ycl9dq8asKYmj2d2Uu_KmARvM0RUiVBtplSg80TuU&e=. > It's a test-only fix and what it does is, on Windows, before doing the > tearDown(), it (re)sets the permissions on these test generated files to > "writable" and then does the delete. That way the tearDown() can delete the > input/output directories generated by the tests. > > Once that was committed almost all tests started passing expect for a few in > copy-test.xml and move-test.xml > https://urldefense.proofpoint.com/v2/url?u=https-3A__ci-2Dbuilds.apache.org_job_Ant_job_Ant-2520Master-2520Windows-2520-28latest-2520EA-2520JDK-29_77_-23showFailuresLink&d=DwICaQ&c=euGZstcaTDllvimEN8b7jXrwqOf-v5A_CdpgnVfiiMM&r=JYLg0shOavzSp2WxYFFvMuhqXdAQzkMRPf_4HQmFZZ4&m=xdy7XeVjARGFk7kSWJvmNeCKZ4CBakKqrfrJngbbrPVabAfalygFpUsjoUuye3AE&s=kAvaelp1weeemDqF-f2JpO-076ozsQqR5lzPO_zBUX0&e=. > Looking at those tests what they do is, they use the copy or move tasks with > the "force=true" property. These tests fail because even with "force=true", > the Copy/Move task can no longer delete (and then update) a pre-existing > read-only file on Windows. This is a functional issue, i.e. it means that the > copy/move tasks' force=true is no longer functional in this specific scenario > on Windows. In order to address this change in behaviour, I pushed a commit > to Ant's repo > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_ant_commit_fe3727ee48b5ff89f65de52dea91c1bc757e4705&d=DwICaQ&c=euGZstcaTDllvimEN8b7jXrwqOf-v5A_CdpgnVfiiMM&r=JYLg0shOavzSp2WxYFFvMuhqXdAQzkMRPf_4HQmFZZ4&m=xdy7XeVjARGFk7kSWJvmNeCKZ4CBakKqrfrJngbbrPVabAfalygFpUsjoUuye3AE&s=sadE-VFCvtmwu3-ew62_niRhhYCxa6nzg4-eCCXI2yc&e=. > Like I note, this is a change in the production source of Ant in > FileUtils.tryHardToDelete(). In this change, on Windows, the implementation > now sets the file to writable if it was read-only and File.delete() had > failed. It then retries the File.delete(). After this change, the remaining > test failures on Windows disappear (against Java 25 EA). > > The test results are now back to normal for both Linux and Windows against > Java 25 EA: > > https://urldefense.proofpoint.com/v2/url?u=https-3A__ci-2Dbuilds.apache.org_job_Ant_job_Ant-2520Master-2520Windows-2520-28latest-2520EA-2520JDK-29_79_&d=DwICaQ&c=euGZstcaTDllvimEN8b7jXrwqOf-v5A_CdpgnVfiiMM&r=JYLg0shOavzSp2WxYFFvMuhqXdAQzkMRPf_4HQmFZZ4&m=xdy7XeVjARGFk7kSWJvmNeCKZ4CBakKqrfrJngbbrPVabAfalygFpUsjoUuye3AE&s=XH-TpmwDQFSKvOGUkyGJttuzQ1e5ERqyG2u_U9w3tJQ&e= > > https://urldefense.proofpoint.com/v2/url?u=https-3A__ci-2Dbuilds.apache.org_job_Ant_job_Ant-2520Master-2520Linux-2520-28latest-2520EA-2520JDK-29_59_&d=DwICaQ&c=euGZstcaTDllvimEN8b7jXrwqOf-v5A_CdpgnVfiiMM&r=JYLg0shOavzSp2WxYFFvMuhqXdAQzkMRPf_4HQmFZZ4&m=xdy7XeVjARGFk7kSWJvmNeCKZ4CBakKqrfrJngbbrPVabAfalygFpUsjoUuye3AE&s=NXFeIZ6N7AWMELiTJ_lxvC_g5Hb2KAdXIEjIcV1uWhM&e= > > Please take a look at those 2 commits and I would like to hear if those look > OK or would need to be handled differently. The newly proposed JDK property > isn't used in these changes and we don't need to set it at least for the > current tests. > > -Jaikiran > >> On 27/06/25 7:55 pm, Jaikiran Pai wrote: >> Hello Stefan, >> >> On 26/06/25 5:16 pm, Stefan Bodewig wrote: >> > On 2025-06-26, Jaikiran Pai wrote: >> > >> >> However on Windows we are seeing a large number of test failures. I >> >> looked into those failures and those all relate to the "heads up" >> >> section in your mail about >> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__jdk.java.net_25_release-2Dnotes-23JDK-2D8355954&d=DwICaQ&c=euGZstcaTDllvimEN8b7jXrwqOf-v5A_CdpgnVfiiMM&r=JYLg0shOavzSp2WxYFFvMuhqXdAQzkMRPf_4HQmFZZ4&m=xdy7XeVjARGFk7kSWJvmNeCKZ4CBakKqrfrJngbbrPVabAfalygFpUsjoUuye3AE&s=PE7qNHljBCcpYaB61lr5MBpFRrfHf0gNXXwL0s4UwnA&e=. >> >> In the Ant >> >> testsuite there are several tests which attempt a >> >> java.io.File.delete() on a read-only file. Until Java 25, that call >> >> would delete the file. >> > >> >> As noted in the linked change, that's no longer the case by default in >> >> Java 25. I manually updated the Ant testsuite to set the newly >> >> introduced -Djdk.io.File.allowDeleteReadOnlyFiles system property to >> >> true and that got all those tests to pass on Windows too. >> > >> > Maybe we should change the tests, rather than set the system property? >> >> Yes, that's an option too. For reference, this is the run against Windows >> which shows these failures >> https://urldefense.proofpoint.com/v2/url?u=https-3A__ci-2Dbuilds.apache.org_job_Ant_job_Ant-2520Master-2520Windows-2520-28latest-2520EA-2520JDK-29_74_&d=DwICaQ&c=euGZstcaTDllvimEN8b7jXrwqOf-v5A_CdpgnVfiiMM&r=JYLg0shOavzSp2WxYFFvMuhqXdAQzkMRPf_4HQmFZZ4&m=xdy7XeVjARGFk7kSWJvmNeCKZ4CBakKqrfrJngbbrPVabAfalygFpUsjoUuye3AE&s=kkJYA-AcaUJAzNGxwI4jz2fknF6DWRO9uibHWmiN41c&e= >> >> That failure number is exaggerated by the fact that several of these >> failures are due to a side effect of few tests creating a read-only file >> then failing to delete it. Subsequent tests then use that same file (through >> the "touch" task) and check whether it's writable and notice that it isn't >> and that causes a lot of those tests to fail. >> >> > Is the file read-only on non-Windows platforms as well? >> >> In our tests, yes. >> >> > To me the description of the change doesn't say what the behavior is on >> > other >> > platforms. Is Windows the only one that now will not delete read-only >> > files - or is it the only one where read-only files would have been >> > removed prior to Java 25? >> >> On *nix platforms a File.delete() completes successfully (and returns true) >> for a read-only file, because the underlying *nix platform allows that. So >> it's only on Windows where the File.delete() will no longer succeed (and >> will returns false) against these read-only files. Previously, before >> https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.org_browse_JDK-2D8355954&d=DwICaQ&c=euGZstcaTDllvimEN8b7jXrwqOf-v5A_CdpgnVfiiMM&r=JYLg0shOavzSp2WxYFFvMuhqXdAQzkMRPf_4HQmFZZ4&m=xdy7XeVjARGFk7kSWJvmNeCKZ4CBakKqrfrJngbbrPVabAfalygFpUsjoUuye3AE&s=ZTfTD0u0hfTNO83uGZQIqwGBW0fFDP1Iv5eH8qYz-9A&e= >> got fixed recently, (only) on Windows the internal implementation of the >> JDK was removing the read-only attribute from the file before doing the >> delete, which is why the delete would succeed and we didn't notice this >> issue. >> >> > Also we could modify FileUtils.tryHardToDelete to remove the read-only >> > flag if we feel this is something that may hit Ant's users. After all it >> > has bitten our own testsuite. >> >> I haven't given it a deeper thought. I think once we take care of these >> antunit test failures in the testsuite, then it might give us a better >> picture if there are actual scenarios where some code intentionally wants to >> detail a read-only file. If there exists such a use case, then since the JDK >> is already providing a way to delete such files (through the use of this >> system property), then we would need to evaluate if we still want to add >> some code in FileUtils.tryHardToDelete or advice users to use that system >> property instead. >> >> In the next few days I will go over these tests to see how we can address >> these issues and if the usage of the new system property can be avoided. >> >> -Jaikiran >> >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org > For additional commands, e-mail: dev-h...@ant.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
