On Mon, 8 May 2023 20:05:50 GMT, Chen Liang <li...@openjdk.org> wrote:
>> This patch replaces `FIXPATH` with `FixPath` on individual path argumenets. >> The root cause might be that JMH requires passing VM args to benchmarks in >> quotes, which might have triggered incorrect detections in `FIXPATH` (as in >> comparison, `$1_MICRO_JAVA_OPTIONS += >> --add-opens=java.base/java.io=ALL-UNNAMED` right above this patch seems to >> work fine) >> Also removed a useless and wrong Java flag to the java running javac, >> originally intended to enable running `make test TEST="loom.obsolete"` >> benchmarks. >> >> Since I only have a windows cygwin environment, I am not quite sure if this >> works elsewhere. > > Chen Liang has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains six additional commits since > the last revision: > > - Comment about the fix logic > We have a mix and match of spaces and tabs, might need to fix it later > - Is this the right way instead? > - space fails linux > - Expose fixpath import and let jmh run use it > - Merge branch 'master' into jmh-windows > - 8305669: RuntimeException when running benchmarks through make on > Windows/WSL Thanks for the conclusions. A quick peek into the benchmarks show there are currently three types of exports/opens: 1. java.io opens, present since the beginning of the benchmark suite 2. classfile and asm opens for the Class-File API benchmarks 3. jdk.internal.vm for a few loom benchmarks, added incorrectly to javac's vm args upon loom integration The latter two cases are easy to take care of; loom one is used by some of those deprecated benchmarks. I don't know where the first java.io opens is required and might need others help to investigate that. In addition, I think users can still pass in `--add-exports` and `--add-opens` with `MICRO_JAVA_OPTIONS` from test arguments, which may still get FixPath'd and break. If that is the case, this patch would still have some value. ------------- PR Comment: https://git.openjdk.org/jdk/pull/13550#issuecomment-1540723084
