Re: Release Ant 1.10.13?
Do you know if there is an issue with the "allow" class approach if multiple projects adopt that technique? E.g. if Netbeans or Groovy also have an allow class, will that cause a split package violation or since it isn't really referenced except for those early JDKs, that we should be okay? I will eventually try this out myself if searching doesn't help, but just wondering if someone has already checked this. On Sun, Dec 11, 2022 at 11:38 PM Jaikiran Pai wrote: > > Hello Stefan, > > On 18/11/22 2:40 pm, Stefan Bodewig wrote: > > On 2022-11-16, Jaikiran Pai wrote: > > > >> Users can still use the current released Ant version against these > >> recent Java versions, but they additionally have to set a system > >> property while launching Ant to allow setting the security > >> manager. Ant's mainline code has changes where it does the necessary > >> work (to the extent possible) to set this property internally without > >> forcing the users to do that. So releasing this version of Ant should > >> help projects building against these recent versions. > > I guess you've seen Earl Hood's response, I haven't looked into it > > myself, yet. > > Yes, that response helped me decide which way to go. Before that, I was > still inclined to try and get this working by doing necessary changes to > the launcher scripts - but that was going nowhere and it introduced the > unnecessary dual launch of Java (once to identify the version and then > the real launch). ewh's response gave me the confidence that using > "allow" as a class (on Java versions where it isn't recognized as a > predefined value) would be the better of the 2 approaches. > > I found some time this weekend to get this implemented in Ant and have > now committed > https://github.com/apache/ant/commit/82c70f3202d5aec4d99fa3b6314ba4a6c338cd94. > Tests against JDK 8, 11, 17, 19 and latest 20 EA all came back fine > against Linux and Windows. > > -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
Re: Release Ant 1.10.13?
Hello Stefan, On 26/11/22 11:33 pm, Stefan Bodewig wrote: On 2022-11-19, Stefan Bodewig wrote: while running all tests locally I realized https://github.com/apache/ant/pull/194 introduces a bug when tars have an encoding set. You can see this with UntarTest failing. A multibyte encoding like UTF16 may contain NUL bytes inside of the "normal" part of the name. I'll have to think about a way to solve this, but we should not release Ant with this regression. https://github.com/apache/ant/commit/e04fbe7ffa4f549bdc00bdfce688fb587120eedd fixes tthe problem, at least for our test. It makes parsing tar archives a bit slower, but that likely won't matter much for single-byte encodings (tar isn't meant to be used with multi-byte encodings). Also I don't think reading tars is what a normal build does for the majority of its time. Anyway, I'd appreciate a review of the code I've written there. What I wanted to do is to ask the encoding how it would represent a NUL and look search for that when finding the string terminator - as opposed to looking for a single NUL byte. Our testcase used UTF-16 BE with a BOM, so encoding "\0" ends up with two bytes BOM + 0x00 0x00 - while I really only wanted to 0x00 0x00. Next attempt was to encode an empty string to see whether there is a common prefix, but an empty string is encoded as 0 bytes, no BOM. :-) So finally I compared "X" to "X\0" and stripped what seems to be "X". I'm pretty sure this breaks for certain encodings, but not worse than it has worked before. And then I sprinkled some memoization on top. TBM I could also live with reverting the whole commit, saying "don't use multi-byte encodings in tars" and be done. The original test we had worked by accident, if the old test had used UTF16-LE instead and a 49 character file name it would have failed as we'd try to decode a byte array with an odd number of bytes. Finding A solution was a nice puzzle, but that doesn't mean we have to use it. I had a look at that commit and the PR discussion where the initial fix was made. I don't have enough knowledge of this area to provide relevant inputs, but from what you have explained here and in the PR and looking at the code it looks fine to me. You (and the original contributor) note that there are still issues that this change won't solve, but I think that is OK for now. I say that because the current state/fix addresses an actual issue that was reported[1]. I've verified that the current code in master with your changes does indeed allow extracting that .tar.gz fine (unlike Ant 1.10.12). Plus our testsuite continues to pass. So that's a good thing. [1] https://github.com/ibmruntimes/Semeru-Runtimes/issues/15 -Jaikiran - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
Re: Release Ant 1.10.13?
Hello Stefan, On 18/11/22 2:40 pm, Stefan Bodewig wrote: On 2022-11-16, Jaikiran Pai wrote: Users can still use the current released Ant version against these recent Java versions, but they additionally have to set a system property while launching Ant to allow setting the security manager. Ant's mainline code has changes where it does the necessary work (to the extent possible) to set this property internally without forcing the users to do that. So releasing this version of Ant should help projects building against these recent versions. I guess you've seen Earl Hood's response, I haven't looked into it myself, yet. Yes, that response helped me decide which way to go. Before that, I was still inclined to try and get this working by doing necessary changes to the launcher scripts - but that was going nowhere and it introduced the unnecessary dual launch of Java (once to identify the version and then the real launch). ewh's response gave me the confidence that using "allow" as a class (on Java versions where it isn't recognized as a predefined value) would be the better of the 2 approaches. I found some time this weekend to get this implemented in Ant and have now committed https://github.com/apache/ant/commit/82c70f3202d5aec4d99fa3b6314ba4a6c338cd94. Tests against JDK 8, 11, 17, 19 and latest 20 EA all came back fine against Linux and Windows. -Jaikiran - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
Re: [ant] branch master updated: add java.security.manager=allow while launching against Java 19
Hello ewh, Thank you for these experiments and reporting the results. This certainly helped me decide which way to go about it. When I had initially tried using "allow" as a Java class (as done in NetBeans), I was unsure if that would be the right way to go. It didn't feel clean and I thought maybe updating the launcher scripts would be easier and cleaner. Having experimented with the scripts and having seen how complex it got (and how it required different ways for different OS) and now having heard the results of your experiment, I believe this is the better approach to take. So I've reverted all the changes to our launcher scripts that I had done to first detect Java runtime version and then conditionally set the java.security.manager property and instead introduced this "allow" class and unconditionally set -Djava.security.manager=allow in the launcher scripts. Jan, thank you for pointing me to the "allow" class used in NetBeans. I copied over that code and have done some minor changes to it. The entire commit of this change is available here https://github.com/apache/ant/commit/82c70f3202d5aec4d99fa3b6314ba4a6c338cd94 My tests over the weekend with these changes have shown that it works fine across the various JDK versions I tested (8, 11, 17, 19 and latest 20 EA). So this looks good. My plan is to release Ant in this coming week, now that this work is done. I will wait a day or two for others in the team to catch up with this change and see if there are any additional suggestions or issues they notice. -Jaikiran On 18/11/22 5:07 am, Earl Hood wrote: Figured give an update wrt our project: The method used by Netbeans project as cited by Jan appears to work. I have not done full testing wrt to Ant as it appears the use of the SecurityManager in Ant is limited in scope to invoking another Java class in the same JVM, which we do not seem to do (nornally enable forking). Regardless, since Ant is included with our product, I implemented the Netbeans approach so we can set java.security.manager=allow unconditionally regardless of JRE version. Since I wanted to avoid creating a custom version of ant, for the one case we invoke the 'ant' command and not org.apache.tools.ant.launch.Launcher directly, I set the LOCALCLASSPATH env to point to a jar containing allow.class, and set ANT_OPTS=-Djava.security.manager=allow For the embedded scenarios, I updated our invocation scripts to set the sysprop when JVM is invoked and ensure allow.class is in classpath. For Ant itself, I think if the "allow" class is included in ant-launcher.jar, the command scripts can be updated to always set the system property, avoiding the need to invoke java twice: first time to get version and second time to actually do the job. --ewh On Tue, Feb 8, 2022, 12:35 AM Jan Lahoda wrote: FWIW, NetBeans added a SecurityManager called "allow", that uninstalls itself as soon as possible: https://github.com/apache/netbeans/blob/master/platform/o.n.bootstrap/src/allow.java Then -Djava.security.manager=allow works on the platforms supported by NetBeans - before JDK 12, "allow" is installed and quickly uninstalled, but setting another SecurityManager is allowed. Jan - To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
