On 2026/05/14 20:51, Timofei Zhakov wrote: > On Tue, May 12, 2026 at 11:16 AM Jun Omae <[email protected]> wrote: >> >> On 2026/05/11 3:50, Timofei Zhakov wrote: >>> On Fri, May 8, 2026 at 4:30 AM Jun Omae <[email protected]> wrote: >>>> >>>> On 2026/05/07 20:59, Jun Omae wrote: >>>>> On 2026/05/05 0:09, [email protected] wrote: >>>>>> Author: rinrab >>>>>> Date: Mon May 4 15:09:48 2026 >>>>>> New Revision: 1933812 >>>>>> >>>>>> Log: >>>>>> cmake: Import targets.cmake before processing dependencies; This would >>>>>> allow >>>>>> potentially refering to those targets in dependency search. Although >>>>>> cmake >>>>>> allows to link a non-existing target, the target that it is linked to >>>>>> has to be >>>>>> declared before. >>>>>> >>>>>> * CMakeLists.txt >>>>>> (target_exports, ra-libs, fs-libs, the-include): Move them up. >>>>>> >>>>>> Modified: >>>>>> subversion/trunk/CMakeLists.txt >>>>> Encountered another issue with SVN_ENABLE_NLS=ON on macOS and Windows.... >>>>> >>>>> [[[ >>>>> FAILED: [code=1] libsvn_delta-1.0.16.0.dylib >>>>> : && /usr/bin/cc -dynamiclib -Wl,-headerpad_max_install_names >>>>> -current_version 0.16.0 -o libsvn_delta-1.0.16.0.dylib ... >>>>> Undefined symbols for architecture x86_64: >>>>> "_libintl_dgettext", referenced from: >>>>> _write_handler in svndiff.c.o >>>>> _write_handler in svndiff.c.o >>>>> _write_handler in svndiff.c.o >>>>> _close_handler in svndiff.c.o >>>>> _svn_txdelta_read_svndiff_window in svndiff.c.o >>>>> _read_window_header in svndiff.c.o >>>>> _read_window_header in svndiff.c.o >>>>> _read_window_header in svndiff.c.o >>>>> _read_window_header in svndiff.c.o >>>>> _read_window_header in svndiff.c.o >>>>> _read_window_header in svndiff.c.o >>>>> ... >>>>> ld: symbol(s) not found for architecture x86_64 >>>>> clang: error: linker command failed with exit code 1 (use -v to see >>>>> invocation) >>>>> ]]] >>>>> >>>>> [[[ >>>>> V:\usr\src\subversion\subversion.git\Release\cmake\svn_private_config.h(126,11): >>>>> error C1083: Cannot open include file: 'libintl.h': >>>>> No such file or directory >>>>> [V:\usr\src\subversion\subversion.git\Release\cmake\libsvn_fs_util.vcxproj] >>>>> (compiling source file '../../subversion/libsvn_fs_util/fs-util.c') >>>>> ]]] >>>> Proposed patch, cmake-nls-r1933902.patch.txt, attached. >>>> >>>> * Find gettext and intl packages before include(../targets.cmake). >>>> * Use include_directories("${Intl_INCLUDE_DIRS}") rather than >>>> set_target_properties(Intl::Intl PROPERTIES >>>> INTERFACE_INCLUDE_DIRECTORIES "${Intl_INCLUDE_DIRS}" ...) >>>> because svn_private_config.h globally includes <libintl.h>. >>>> >>>> I confirmed that the builds successfully on Ubuntu, macOS and Windows. >>> >>> Your fix looks fine. Again, I'm really really sorry for the need for >>> those follow-ups. >> >> Thanks for the reviewing. Committed in r1934074. >> >>> Now as I think, it might be better to revert this change. I wanted to >>> move this include to the top because I need the targets available to >>> tune a tiny thing in one of the dependencies but I don't know I might >>> have simply done it with this specific one so I don't break the trunk >>> once again. Thoughts? >> >> I think the reverting is not needed. Current builds successfully. Many >> changes already exist after this change. I don't think It is simple to >> revert this change. > > Okay. > >> The builds are prone to break on macOS and Windows that it needs *.h and >> *.lib files in non-standard locations. It might to be good to enable >> SVN_ENABLE_NLS, SVN_ENABLE_APACHE_MODULES, ... in auto-builds. > > I'm also +1 to have those options enabled on our CI. > >> E.g. >> [[[ >> diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml >> index 64634c2be..285680697 100644 >> --- a/.github/workflows/cmake.yml >> +++ b/.github/workflows/cmake.yml >> @@ -154,14 +154,15 @@ jobs: >> libexpat1-dev >> zlib1g-dev >> libsqlite3-dev >> + apache2-dev >> ninja-build >> ${{ matrix.extra_packages }} >> >> - name: Install dependencies (Mac OS, homebrew) >> if: runner.os == 'macOS' >> run: | >> - $packages = @('apr', 'apr-util', 'expat', 'sqlite', 'apache-serf') >> - brew install ninja $packages >> + $packages = @('apr', 'apr-util', 'expat', 'sqlite', >> 'apache-serf', 'gettext') >> + brew install ninja apache2 $packages >> $PKG_CONFIG_PATH = ` >> $packages | ` >> % { "$(& brew --prefix $_)/lib/pkgconfig" } | ` >> @@ -184,6 +185,8 @@ jobs: >> -DBUILD_SHARED_LIBS=${{ matrix.build_shared }} >> -DSVN_ENABLE_TESTS=ON >> -DSVN_ENABLE_RA_SERF=ON >> + -DSVN_ENABLE_NLS=ON >> + -DSVN_ENABLE_APACHE_MODULES=${{ runner.os != 'Windows' && 'ON' || >> 'OFF' }} >> -DCMAKE_INSTALL_PREFIX=${{ github.workspace }}/installdir >> -DVCPKG_TARGET_TRIPLET=${{ matrix.vcpkg_triplet }} >> -DSVN_TEST_CONFIGURE_FOR_PARALLEL=ON >> ]]] Thanks for the feedback. Pushed the changes in r1934221.
-- Jun Omae <[email protected]> (大前 潤)
