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. Please commit your svn-cmake-ctest-working-directory-v2.patch.txt. 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. -- Jun Omae <jun6...@gmail.com> (大前 潤)
