Re: JavaHL jar file containing class files for unit tests

Den tors 6 aug. 2026 kl 16:18 skrev Branko Čibej <[email protected]>:

> On 2. 8. 2026 15:07, Daniel Sahlberg wrote:
>
> Den ons 29 juli 2026 kl 12:52 skrev Branko Čibej <[email protected]>:
>
>> On 29. 7. 2026 10:58, Daniel Sahlberg wrote:
>>
>>
>>
>> Den ons 29 juli 2026 kl 04:32 skrev Jun Omae <[email protected]>:
>>
>>> On 2026/07/28 23:58, Daniel Sahlberg wrote:
>>> > Den mån 27 juli 2026 kl 00:09 skrev Jun Omae <[email protected]
>>> <mailto:[email protected]>>:
>>> >
>>> >     > Is there a problem if we include the test classes?
>>> >
>>> >     No direct issues have been found so far. However, static
>>> initializers within test classes are executed simply by loading the class.
>>> >
>>> >     [[[
>>> >     $ grep -r 'static *{' subversion/bindings/javahl
>>> >
>>>  subversion/bindings/javahl/tests/org/apache/subversion/javahl/UtilTests.java:
>>>    static {
>>> >
>>>  subversion/bindings/javahl/tests/org/apache/subversion/javahl/UtilTests.java:
>>>    static {
>>> >     ]]]
>>> >
>>> >
>>> > Thank you!
>>> >
>>> > I think I got it working on 1.14.x, successfully created a JAR file
>>> without the tests.
>>> >
>>> > It still seems to fail on trunk. Can you do the same steps above on
>>> trunk?
>>> >
>>> > Cheers,
>>> > Daniel
>>> >
>>>
>>> Hm, I get the same failures on trunk.
>>> It seems that the changes of the behavior is introduced in r1933892.
>>> At least, ./configure should stop and/or warn it if the junit is NOT
>>> optional when javahl enabled.
>>>
>>> [[[
>>> $ /bin/sh autogen.sh
>>> $ ./configure --prefix=/dev/shm/svn/trunk --without-apxs --without-swig
>>> --without-swig-{perl,python,ruby} --enable-javahl
>>> --with-jdk=/usr/lib/jvm/java-11-openjdk-amd64 --without-junit
>>> PYTHON=/usr/bin/python3
>>> $ make -j$(nproc) all
>>> $ make javahl
>>>
>>> $ find subversion/bindings/javahl -name '*.class' | wc -l
>>> 277
>>> $ find subversion/bindings/javahl -name '*Test*.class' | wc -l
>>> 0    #==> tests/**/*.java are compiled yet.
>>>
>>> $ make install
>>> $ make install-javahl    # Why this task builds tests/**/*.java?
>>> ...
>>> /dev/shm/subversion-trunk/subversion/bindings/javahl/tests/org/tigris/subversion/javahl/BasicTests.java:1708:
>>> error: cannot find symbol
>>>         assertEquals("wrong revision from commit",
>>>         ^
>>>   symbol:   method assertEquals(String,long,int)
>>>   location: class BasicTests
>>> /dev/shm/subversion-trunk/subversion/bindings/javahl/tests/org/tigris/subversion/javahl/BasicTests.java:1756:
>>> error: cannot find symbol
>>>         assertEquals("wrong revision from update",
>>>         ^
>>>   symbol:   method assertEquals(String,long,int)
>>>   location: class BasicTests
>>> Note: Some input files use or override a deprecated API.
>>> Note: Recompile with -Xlint:deprecation for details.
>>> 100 errors
>>> ]]]
>>>
>>
>> Thanks for confirming! It seems like 1.15.x is also affected by this
>> (I'll bring it up on the release thread).
>>
>> To add three more data points to the discussion, I checked Fedora 43, 44
>> and FreeBSD 14. The tests are present in Fedora's svn-javahl.jar but not in
>> FreeBSD's svn-javahl.jar. Obviously different distributors use different
>> options when building. That is unfortunate since we are distributing
>> different ABIs.
>>
>> I think it would be cleaner to have the tests in svn-javahl-tests.jar if
>> built. Maybe we want to install this alongside svn-javahl.jar. That way
>> Fedora and Ubuntu can keep distributing the tests and FreeBSD not, while
>> everyone's svn-javahl.jar is the same.
>>
>> Does that make sense?
>>
>>
>>
>> The following makes sense:
>>
>> 1. There is no regression in autotools. It is what it is. Note that
>> putting test classes in a separate jar has wider consequences than just the
>> ABI. For example, JavaHL tests would fail in autotools builds. We'd have to
>> change the way the classpath is constructed in `make check-*-javahl`.
>>
>> 2. No change is needed in 1.15.x. Autotools are consistent with earlier
>> releases and CMake doesn't build JavaHL. IFF we want to change this in
>> CMake for 1.15, then we have further questions to consider:
>>
>> 2.1. CMake and Autotools/vcxproj should produce identical results given
>> identical options. In this case specifically I mean that the CMake build
>> should create the same lib/dll names and the same JavaHL jar (singular) as
>> the vcxproj generator.
>>
>>
>> 3. Regardless of the answer to 2: If we decide to not include test
>> classes in svn-javhl.jar, then we have to consider how to mitigate the ABI
>> change.
>>
>> Personally I'd prefer changing the build requirements to changing the ABI
>> in svn-javahl.jar. That means that --enable-javahl implies that JUnit is
>> mandatory.
>>
>> -- Brane
>>
>>
> Is the patch below an acceptable way of requiring --with-junit?
>
> Cheers,
> Daniel
>
>
>  [[[
> Index: configure.ac
> ===================================================================
> --- configure.ac        (revision 1936330)
> +++ configure.ac        (working copy)
> @@ -1748,6 +1748,7 @@
>
>  dnl Possibly compile JavaHL
>  do_javahl_build=no
> +has_junit=no
>
>
> This initialisation is not needed.
>
>  AC_ARG_ENABLE(javahl,
>    AS_HELP_STRING([--enable-javahl],
>               [Enable compilation of Java high-level bindings (requires
> C++)]),
> @@ -1755,6 +1756,21 @@
>          do_javahl_build="yes"
>      fi
>    ])
> +AC_ARG_WITH(junit,
> +AS_HELP_STRING([--with-junit=PATH],
> +               [Specify a path to the junit JAR file.]),
> +[
> +    if test "$withval" != "no"; then
> +      if test -n "$JAVA_CLASSPATH"; then
> +        JAVA_CLASSPATH="$withval:$JAVA_CLASSPATH"
> +      else
> +        JAVA_CLASSPATH="$withval"
> +      fi
> +      JAVAHL_TESTS_TARGET="javahl-tests"
> +      JAVAHL_COMPAT_TESTS_TARGET="javahl-compat-tests"
> +      has_junit="yes"
>
> Should be;
>
>   have_junit="yes"
>
>
> +    fi
> +])
>
>  JAVAHL_OBJDIR=""
>  INSTALL_EXTRA_JAVAHL_LIB=""
> @@ -1763,6 +1779,12 @@
>  JAVAHL_COMPAT_TESTS_TARGET=""
>  LT_CXX_LIBADD=""
>  if test "$do_javahl_build" = "yes"; then
> +  dnl Since r1933892 junit is required to build the JavaHL bindings
> +  if test "$has_junit" = "no"; then
>
>
> And here:
>
>   if test "$have_junit" != "yes"; then
>
>
> The idea is to set the have_junit variable exactly once. If it's not set,
> "$have_junit" will be an empty string.
>
> +    AC_MSG_ERROR([--with-junit must be specified if building the Java
> high-level
> +                  bindings.])
> +  fi
> +
>    dnl Check for suitable JDK
>    if test "$JDK_SUITABLE" = "no"; then
>      AC_MSG_ERROR([Cannot compile JavaHL without a suitable JDK.
> @@ -1802,21 +1824,6 @@
>  AC_SUBST(JAVAHL_OBJDIR)
>  AC_SUBST(FIX_JAVAHL_LIB)
>  AC_SUBST(LT_CXX_LIBADD)
> -
> -AC_ARG_WITH(junit,
> -AS_HELP_STRING([--with-junit=PATH],
> -               [Specify a path to the junit JAR file.]),
> -[
> -    if test "$withval" != "no"; then
> -      if test -n "$JAVA_CLASSPATH"; then
> -        JAVA_CLASSPATH="$withval:$JAVA_CLASSPATH"
> -      else
> -        JAVA_CLASSPATH="$withval"
> -      fi
> -      JAVAHL_TESTS_TARGET="javahl-tests"
> -      JAVAHL_COMPAT_TESTS_TARGET="javahl-compat-tests"
> -    fi
> -])
>  AC_SUBST(JAVA_CLASSPATH)
>  AC_SUBST(JAVAHL_TESTS_TARGET)
>  AC_SUBST(JAVAHL_COMPAT_TESTS_TARGET)
> ]]]
>
>
>
 Out of curiosity, why do $do_javahl_build and others follow another
pattern?

Thanks,
Daniel