On Wed, Sep 25, 2024 at 11:58 AM Jun Omae <jun6...@gmail.com> wrote: > > On 2024/09/24 7:07, Timofei Zhakov wrote: > > On Mon, Sep 23, 2024 at 9:25 AM Jun Omae <jun6...@gmail.com> wrote: > >> > >> On 2024/09/22 22:55, Timofei Zhakov wrote: > >>> On Sun, Sep 22, 2024 at 2:02 AM Jun Omae <jun6...@gmail.com> wrote: > >>>> > >>>> On 2024/09/21 5:50, Timofei Zhakov wrote: > >>>>> On Fri, Sep 20, 2024 at 11:30 AM Jun Omae <jun6...@gmail.com> wrote: > >>>>>> > >>>>>> Hi, > >>>>>> > >>>>>> On 2024/09/16 21:41, rin...@apache.org wrote: > >>>>>>> Author: rinrab > >>>>>>> Date: Mon Sep 16 12:41:13 2024 > >>>>>>> New Revision: 1920717 > >>>>>>> > >>>>>>> URL: http://svn.apache.org/viewvc?rev=1920717&view=rev > >>>>>>> Log: > >>>>>>> Merge the `cmake` branch to trunk. > >>>>>> > >>>>>> I encounter the following error on all of cmdline.* tests with Visual > >>>>>> Studio > >>>>>> as cmake generator. > >>>>>> > >>>>>> [[[ > >>>>> [...] > >>>>>> ]]] > >>>>>> > >>>>>> The exception is raised when launching <build-dir>/svnadmin.exe > >>>>>> because the > >>>>>> file is actually located in <build-dir>/Release/svnadmin.exe. It is > >>>>>> not raised > >>>>>> when cmake with Ninja is used because <build-dir>/svnadmin.exe is > >>>>>> located. > >>>>>> > >>>>>> I think we could use $<TARGET_FILE_DIR:target> instead of > >>>>>> ${CMAKE_CURRENT_BINARY_DIR} as a workaround. > >>>>>> > >>>>>> [[[ > >>>>>> Index: CMakeLists.txt > >>>>>> =================================================================== > >>>>>> --- CMakeLists.txt (revision 1920797) > >>>>>> +++ CMakeLists.txt (working copy) > >>>>>> @@ -671,13 +671,13 @@ > >>>>>> "cmdline.${py_test_name}" > >>>>>> COMMAND > >>>>>> "${Python3_EXECUTABLE}" > >>>>>> "${CMAKE_CURRENT_SOURCE_DIR}/build/run_tests.py" > >>>>>> - --bin ${CMAKE_CURRENT_BINARY_DIR} > >>>>>> - --tools-bin ${CMAKE_CURRENT_BINARY_DIR} > >>>>>> + --bin $<TARGET_FILE_DIR:svnadmin> > >>>>>> + --tools-bin $<TARGET_FILE_DIR:entries-dump> > >>>>>> --verbose > >>>>>> --log-to-stdout > >>>>>> --set-log-level=WARNING > >>>>>> ${CMAKE_CURRENT_SOURCE_DIR} > >>>>>> - ${CMAKE_CURRENT_BINARY_DIR} > >>>>>> + $<TARGET_FILE_DIR:svnadmin> > >>>>>> ${py_test_path} > >>>>>> ) > >>>>>> endif() > >>>>>> ]]] > >>>>> > >>>>> Oh, I didn't notice this problem, because I am using a single config > >>>>> generator. I think the suggested code is great for resolving the > >>>>> issue, because I think there is no generator expression to get the > >>>>> target dir in the current config without a specific target. However, I > >>>>> think it's better to use svn's target dir since it's the main > >>>>> executable and maybe the same for tools (?). It actually should be the > >>>>> same for all targets, so it doesn't meter. > >>>>> > >>>>> Also, I think we should add some multi-config generator tests to the > >>>>> GitHub Actions workflow, to prevent similar problems in future. > >>>>> > >>>> > >>>> I use "svnadmin" for target directory because we use "svnadmin" to > >>>> create a > >>>> test repository before using "svn" to check out the repository. Well, > >>>> using > >>>> "svn" is okay to me. > >>> > >>> Okay. > >>> > >>>> Revised the proposed patch, in addition, added use of VS2022 to the > >>>> matrix: > >>>> cmake-test-build-dir.v2.patch.txt (attached). > >>> > >>> Would this be better to separate this patch by two different commits, > >>> with tests and GitHub actions? > >> > >> Yeah. Separated the commit to two commits: > >> > >> - > >> https://github.com/apache/subversion/commit/5dff28422740659ea9031d7287ce33785cd08a4b > >> - > >> https://github.com/apache/subversion/commit/c456dfd0311201baab3f3eaf94573ed8b1ba408a > > > > Great! > > > >>>> However, the same issue occurs on ra-test. I think the issue should be > >>>> occurred > >>>> also when Ninja is used with build directory being outside of source > >>>> tree (e.g. > >>>> %TMP%\svn-out). Because "../../svnserve/svnserve.exe" and "svnserve.exe" > >>>> are > >>>> searched. > >>>> > >>>> [[[ > >>>> 40: Test command: D:\a\subversion\subversion\out\Release\ra-test.exe > >>>> "--srcdir" "D:/a/subversion/subversion/subversion/tests/libsvn_ra" > >>>> 40: Working Directory: D:/a/subversion/subversion/out > >>>> 40: Test timeout computed to be: 10000000 > >>>> 40: PASS: ra-test 1: test svn_ra_get_location_segments > >>>> 40: PASS: ra-test 2: test ra_svn tunnel callback check > >>>> 40: svn_tests: E170013: Unable to connect to a repository at URL > >>>> 'svn+test://localhost/test-repo-tunnel' > >>>> 40: svn_tests: E200006: Could not find svnserve. Checked paths: > >>>> 40: D:\a\subversion\svnserve\svnserve.exe > >>>> 40: D:\a\subversion\subversion\out\svnserve.exe > >>>> 40: FAIL: ra-test 3: test ra_svn tunnel creation callbacks > >>>> 40: PASS: ra-test 4: lock multiple paths > >>>> 40: PASS: ra-test 5: test ra_get_dir2 > >>>> ... > >>>> ]]] > >>> > >>> I think changing the working directory of where the tests are running > >>> to the build directory in the current config. > >>> > >>> See 'svn-cmake-ctest-working-directory.patch.txt' in the attachments. > >> > >> Thanks for the patch. However, after the patch, tests with both ninja and > >> vs2022 failed. See > >> https://github.com/jun66j5/subversion/actions/runs/10987698238 > >> > >> I don't think `--srcdir` option should be changed. Reverting the changes > >> for the option, jobs except ubuntu successed. > >> > >> - > >> https://github.com/apache/subversion/compare/trunk...jun66j5:subversion:cmake-pytests-build-dir > >> - https://github.com/jun66j5/subversion/actions/runs/10988627279 > > > > Oups, I see. It is a copy-paste mistake, because this code has been > > duplicated from targets.cmake to target.cmake.ezt, and this variable > > got expanded. > > > > Attached a new version of the patch as > > 'svn-cmake-ctest-working-directory-v2.patch.txt' with reverted > > --srcdir change. Log message: > > > > [[[ > > cmake: Fix C-Tests failing in multi-config generators. > > > > * build/generator/templates/targets.cmake.ezt > > (tests): Use `$<TARGET_FILE_DIR:...>` generator expression instead of > > using > > `CMAKE_CURRENT_BINARY_DIR` variable for determining the path to a > > directory > > with built Subversion binaries > > ]]] > > > > Succeeded GitHub Actions (except tests, failing on Linux): > > https://github.com/rinrab/subversion/actions/runs/10993560311 > > > > Comparison of all these commits after doing some Git-magic: > > https://github.com/apache/subversion/compare/trunk...rinrab:subversion:cmake-test-build-dir > > > > Finally, I think we can start committing those changes into trunk. > > What do you think? > > > > Thanks! > > Agreed. Committed my proposed patches in r1920902 and r1920903.
Thanks! > Please commit your svn-cmake-ctest-working-directory-v2.patch.txt. Done in r1920906. > BTW, I noticed that another minor issue when using Ninja and make > generator. Running ctest, "cmdline/svn-test-work" directory is created > in top directory of source tree, not build directory. When using VS2022, > "cmdline/svn-test-work" directory is created in build directory. Also I can see directories like that after running tests, with repositories for testing: test-repo-get-deleted-rev-no-delete/ test-repo-lock/ test-repo-locsegs/ test-repo-tunnel/ test-repo-commit-locked-file-test/ test-repo-get-deleted-rev-errors/ -- Timofei Zhakov
