On 2024/11/25 21:00, Timofei Zhakov wrote:
> Hi Daniel!
>
> I am happy to see your feedback.
>
> On Sun, Nov 24, 2024 at 6:17 PM Daniel Shahaf <d...@daniel.shahaf.name> wrote:
>>
>> rin...@apache.org wrote on Thu, 26 Sep 2024 13:22 +00:00:
>>> Fix command-line tests on Linux by removing configuration of svneditor.sh
>>> script.
>>>
>>> See also 'Removing configuration of svneditor.sh script?' thread on dev@
>>> [1].
>>
>> The log message says "Fix", but neither the log message nor the thread
>> explains what wasn't working.
>
> This log message explains in context of the CMake build-system that wasn't
> supporting it previously. Probably, I didn't explain it enough in the
> log-message.
>
>> More importantly, using trunk@HEAD:
>>
> [...]
>>
>> Reverting r1920955 makes that test PASS.
>>
>> Please fix.
Ah, I have tested in tree build but haven't tested in out-of-tree build on the
reviewing.
> I've just checked the configuration you are talking about, and the problem has
> been reproduced. Thanks for noticing and reporting it!
>
> After some research, I found the source of it. The path to svneditor_script
> was previously being determined using process' current directory on Unix,
> while Windows reads it from something like the current script path (I am not
> exploring the details, just quoting an answer from stackoverflow).
>
> If we use the same approach as Windows on all platforms, we first will
> minimize
> the differences between platforms, and secondly resolve the issue you've just
> explained. I would like to suggest the following patch:
>
> [[[
> Index: subversion/tests/cmdline/svntest/main.py
> ===================================================================
> --- subversion/tests/cmdline/svntest/main.py (revision 1922052)
> +++ subversion/tests/cmdline/svntest/main.py (working copy)
> @@ -141,9 +141,7 @@
> svneditor_script = os.path.join(sys.path[0], 'svneditor.bat')
> else:
> # This script is in the build tree, not in the source tree.
> - svneditor_script = os.path.join(os.path.dirname(
> - os.path.dirname(os.path.abspath('.'))),
> - 'tests/cmdline/svneditor.sh')
> + svneditor_script = os.path.join(sys.path[0], 'svneditor.sh')
>
> # Username and password used by the working copies
> wc_author = 'jrandom'
> ]]]
>
> What do you think? Did it work for you?
The patch looks good to me (tested in tree and out-of-tree builds). However, I
think "# This script is in the build tree, ..." comment should be removed also.
--
Jun Omae <jun6...@gmail.com> (大前 潤)
