Re: [PATCH] Specify editor guidelines from the EditorConfig file.

2024-05-21 Thread Daniel Sahlberg
Den mån 20 maj 2024 kl 18:53 skrev Timofey Zhakov :

> Hello,
>
> Thank you for rapid response,
>
> On Mon, May 20, 2024 at 6:16 PM Daniel Sahlberg
>  wrote:
> >
> > Hi
> >
> > I can find very little information about the guidelines keyword.
> >
> > Is this a widely supported option in .editorconfig or is it something
> specific to a certain IDE or even a certain plugin? In case of the latter I
> think it should be mentioned in the commit message.
> No, this keyword is not that much supported. I have a Visual Studio
> extension which provides support for this rule.
>

Thanks for the expanation!


>
> >
> > Is the max_line_length a better option?
> There are also some three common keywords for this rule: guidelines,
> max_line_length, and rulers. Can we add them all?
>

Why not?


> Do you agree with this if I mention details in the log message?
>

Sure! The reason I suggest to have it in the log message is if someone down
the road looks at the file and things "but guidelines isn't an editorconfig
option..?", then it helps to look at blame and see "ah, it was for Visual
Studio extention XYZ".


> With best regards,
> Timofei Zhakov
>


Re: [PATCH] Specify editor guidelines from the EditorConfig file.

2024-05-20 Thread Daniel Sahlberg
mån 20 maj 2024 kl. 17:50 skrev Timofey Zhakov :

> Hello everyone!
>
> There is an option in editorconfig for setting the maximum line
> length. I use it and I think it would be very useful to set it.
>
> Here is an article about this option and its support in different
> editors:
> https://github.com/editorconfig/editorconfig/wiki/Property-research:-maximum-line-length
> .
>
> [[[
> Specify editor guidelines from the EditorConfig file.
>
> Set the 'guidelines' property in the EditorConfig file
> to 79 symbols as described in the community guidelines.
>
> * .editorconfig:
>   (): Set the 'guidelines' property to 79 symbols.
> ]]]
>
> What do you think?


Hi

I can find very little information about the guidelines keyword.

Is this a widely supported option in .editorconfig or is it something
specific to a certain IDE or even a certain plugin? In case of the latter I
think it should be mentioned in the commit message.

Is the max_line_length a better option?

Kind regards
Daniel


Re: Remove py2 support from mailer.py?

2024-05-11 Thread Daniel Sahlberg
Hi,

I agree that we should not, at this point in time, let Python 2
compatibility hold us back if things can be done more efficient in Python
3. It has previously been agreed that we don't aggressively remove Python 2
support in 1.14 but /trunk is fine for me.

Maybe we should also start a discussion about actively removing Python 2
support in /trunk, ie for Subversion 1.15. I'm by no means an expert but at
least there is some code in the bindings where we can remove some
#if/#ifdef. This way we don't have to test for Python 2 (which we SHOULD do
if we officially keep the code in place).

Kind regards,
Daniel Sahlberg


Den fre 10 maj 2024 kl 10:20 skrev Greg Stein :

> Hey all,
>
> I am proposing to remove py2 support from mailer.py. It is an anchor on
> some of the coding options within the module.
>
> I would suggest installations requiring py2 for mailer.py "just don't
> upgrade".
>
> This tool is not part of our core distribution, and I would further note
> that py2 was EOL'd in January 2020. I do not believe that Apache Subversion
> should support versions of Python that no longer receive updates (security,
> or otherwise).
>
> No rush on this, and no need for +1 responses ... I'm interested in people
> saying "keep py2 support" and the rationale. I'll give it a week before
> declaring consensus for removal, if there is no counter-argument.
>
> Thanks!
> -g
>
> ps. to be clear: the ASF *does not* use mailer.py today (we use Andre
> Malo's svnmailer). We intend to switch to the py3 version of mailer.py in
> the near future. I am adding features that the ASF needs (eg. around "don't
> send over-large commit emails")
>
>


Re: Remove the IRC channels

2024-05-05 Thread Daniel Sahlberg
Den sön 5 maj 2024 kl 11:42 skrev Stefan Sperling :

> On Sun, May 05, 2024 at 11:20:28AM +0200, Daniel Sahlberg wrote:
> > What about this new topic for #svn?
> >
> > [[[
> > The Apache® Subversion® version control system (
> > https://subversion.apache.org/) | Read the book: http://www.svnbook.org/
> |
> > FAQ: https://subversion.apache.org/faq.html | This channel has limited
> use,
> > if you have questions please ask on the mailing list
> > https://subversion.apache.org/mailing-lists#users-ml
> > ]]]
> >
> > And for #svn-dev:
> >
> > [[[
> > Development of Apache® Subversion® (https://subversion.apache.org/) |
> This
> > channel has limited use, if you have questions please ask on the mailing
> > list https://subversion.apache.org/mailing-lists#dev-ml
> > ]]]
>
> Reads well. Fine with me!
>

Thank you, and also thanks to Nathan for reviewing the changes to the
website.

I have updated the topics as above and I've also merged the website changes
in r1917520.

I believe that is everything, but if you find something else, just let me
know.

Kind regards,
Daniel


Re: Remove the IRC channels

2024-05-05 Thread Daniel Sahlberg
sön 5 maj 2024 kl. 05:57 skrev Nathan Hartman :

> On Sat, May 4, 2024 at 4:15 PM Stefan Sperling  wrote:
>
>> On Sat, May 04, 2024 at 09:24:58PM +0200, Daniel Sahlberg wrote:
>> > Hi,
>> >
>> > I’m personally not an IRC user but I try to keep an eye on the IRC logs.
>> > For personal reasons I haven’t had time to do since the start of the
>> year
>> > but I spent some time tonight to browse the archives.
>> >
>> > Since January there were somewhere between 5 and 10 persons asking
>> > questions and NO ONE got a timely reply. On the other hand we have had a
>> > slow but steady stream of questions on users@ and they have received
>> > replies from different members of the community.
>> >
>> > I think the lack of replies on IRC reflects badly on the community. I’m
>> not
>> > able to put any energy into following the IRC channels and I cannot ask
>> > anyone else either. But I think the mailing lists could absorb these
>> > questions and give timely answers.
>> >
>> > For this reason, I suggest that we discontinue the IRC channels (remove
>> > them from libera.chat) or at least change the topic to indicate that
>> they
>> > are no longer “official” and to refer all questions to the mailing
>> lists.
>> > The website should of course be updated accordingly.
>> >
>> > Kind regards
>> >
>> > Daniel Sahlberg
>>
>> I am not sure whether outright removing (aka "dropping") our Libera
>> channels from ChanServ is a good idea. Anyone could re-register a
>> dropped channel and squat on the name and/or impersonate the project.
>>
>> Given this, I suppose the #svn users channel should be marked as
>> unmaintained in the topic. Redirecting questions to the mailing
>> list via the topic line should work well enough.
>>
>> The #svn-dev channel might still be useful for quick communication
>> during bursts of increased project activity. I would keep it around.
>>
>> Cheers,
>> Stefan
>
>
>
> I'd like to keep the IRC channels, at the very least so that someone won't
> squat on the name as already mentioned, but I agree that we should update
> the channel topics to direct user questions to the mailing list where they
> are much more likely to receive a timely response.
>
> I'd also recommend to update our text on the website to say that the IRC
> channels exist but the mailing lists are the more recommended way to
> communicate.
>
> Cheers,
> Nathan
>

Thanks both of you, valid points and valuable input!

What about this new topic for #svn?

[[[
The Apache® Subversion® version control system (
https://subversion.apache.org/) | Read the book: http://www.svnbook.org/ |
FAQ: https://subversion.apache.org/faq.html | This channel has limited use,
if you have questions please ask on the mailing list
https://subversion.apache.org/mailing-lists#users-ml
]]]

And for #svn-dev:

[[[
Development of Apache® Subversion® (https://subversion.apache.org/) | This
channel has limited use, if you have questions please ask on the mailing
list https://subversion.apache.org/mailing-lists#dev-ml
]]]

I've made a first draft of website updates to the staging site in r1917512.
(At the same time, I saw that the svnforum.org site no longer is a forum,
so I removed that reference in r1917511).

Kind regards,
Daniel


Remove the IRC channels

2024-05-04 Thread Daniel Sahlberg
Hi,

I’m personally not an IRC user but I try to keep an eye on the IRC logs.
For personal reasons I haven’t had time to do since the start of the year
but I spent some time tonight to browse the archives.

Since January there were somewhere between 5 and 10 persons asking
questions and NO ONE got a timely reply. On the other hand we have had a
slow but steady stream of questions on users@ and they have received
replies from different members of the community.

I think the lack of replies on IRC reflects badly on the community. I’m not
able to put any energy into following the IRC channels and I cannot ask
anyone else either. But I think the mailing lists could absorb these
questions and give timely answers.

For this reason, I suggest that we discontinue the IRC channels (remove
them from libera.chat) or at least change the topic to indicate that they
are no longer “official” and to refer all questions to the mailing lists.
The website should of course be updated accordingly.

Kind regards

Daniel Sahlberg


Re: Get the repository HEAD revision

2024-05-01 Thread Daniel Sahlberg
Thanks for the quick reply!

Den ons 1 maj 2024 kl 13:51 skrev Yasuhito FUTATSUKI <
futat...@yf.bsdclub.org>:

> Hello,
>
> On 2024/05/01 18:28, Daniel Sahlberg wrote:
> > Hi,
> >
> > This is based on a problem we have in the TortoiseSVN project and I'm not
> > proficient enough in the API to resolve it myself.
> >
> > We know of a path that existed at one point in time (basically a peg
> > revision). We would like to connect to the repository and figure out the
> > current HEAD revision (we don't care about the repository content at this
> > time).
> >
> > We use svn_client_open_ra_session2() for the connection and send in the
> > path. This may fail if the path has subsequently been removed. So we
> > instead tried to send in "/", but that failed for some users who don't
> have
> > read access to the repository root but only on certain subdirectories.
>
> const char *wri_abspath in svn_client_open_ra_session2() can be NULL,
> and const char *url can be pointed to non-exist node within the repository.
>
> Here is an example using Python bindings
> (svn_client_open_ra_session() acts as wri_abspath is NULL):
> [[[
> $ sudo -u mailman -g mailman svn ls svn+ssh://svn-agent-sysadmin@localhost
> /@
> svn: E170001: Authorization failed
> $ sudo -u mailman -g mailman python
> Python 2.7.5 (default, Nov 14 2023, 16:14:06)
> [GCC 4.8.5 20150623 (Red Hat 4.8.5-44)] on linux2
> Type "help", "copyright", "credits" or "license" for more information.
> >>> from svn import core,client,ra
> >>> ctx = client.create_context()
> >>> session =
> client.open_ra_session('svn+ssh://svn-agent-sysadmin@localhost/', ctx)
> >>> ra.get_latest_revnum(session)
> 2221
> >>> session2 = client.open_ra_session('
> https://svn.apache.org/repos/asf/non-existent/',ctx)
> >>> ra.get_latest_revnum(session2)
> 1917447
> >>> quit()
> ]]]
>
>
Thanks for the example, you were absolutely right of course and my initial
investigation was not correct. The svn_client_open_ra_session2() call
returned absolutely fine and the issue was later in the code where we tried
to do an svn_ra_stat(). It was a matter of bad design where one function
did more than it was supposed to do. My bad, sorry for the noise!

Kind regards,
Daniel


Get the repository HEAD revision

2024-05-01 Thread Daniel Sahlberg
Hi,

This is based on a problem we have in the TortoiseSVN project and I'm not
proficient enough in the API to resolve it myself.

We know of a path that existed at one point in time (basically a peg
revision). We would like to connect to the repository and figure out the
current HEAD revision (we don't care about the repository content at this
time).

We use svn_client_open_ra_session2() for the connection and send in the
path. This may fail if the path has subsequently been removed. So we
instead tried to send in "/", but that failed for some users who don't have
read access to the repository root but only on certain subdirectories.

I think what I'd want to do is to either connect to the repository without
any path at all, or to use a peg revision to say "the path was THIS at THAT
point in history". Is there a way to do this?

(And please excuse me if I'm using the wrong terminology above, I'm still
learning :-) ).

Kind regards,
Daniel


Re: [PATCH] Make svn_apply_autoprops.py Python 3-compatible

2024-05-01 Thread Daniel Sahlberg
Den ons 1 maj 2024 kl 06:06 skrev Khairul Azhar Kasmiran :

> [[[
> Make svn_apply_autoprops.py Python 3-compatible.
>
> * contrib/client-side/svn_apply_autoprops.py:
>   (get_autoprop_lines): Use raw strings for regexes, and `for line in
> fd` instead of `for line in fd.xreadlines()`.
>   (filter_walk): Pass directory names separately.
>   (main): Use `open()` instead of `file()`, and `os.walk()` instead of
> `os.path.walk()`.
> ]]]
>

Thank you!

Committed as r1917446.

Kind regards,
Daniel


>
> -- Khairul
>
> On Wed, May 1, 2024 at 12:04 PM Khairul Azhar Kasmiran
>  wrote:
> >
> > > Are there any systems where Python is only installed as "python" and
> not as "python3" or is this safe?
> >
> > If "python3" is missing, it should be installable via a package or can
> > be made available manually via a symbolic link, but I suppose this is
> > going to be troublesome for some so I've reverted the hashbang binary
> > back to "python" in the reattached patch.
> >
> > > I've tested and on Windows (Python 3.12) I get the following syntax
> warnings:
> > >
> > >
> C:\Devel\subversion_trunk\contrib\client-side\svn_apply_autoprops.py:71:
> SyntaxWarning: invalid escape sequence '\s'
> > >   re_start_autoprops = re.compile('^\s*\[auto-props\]\s*')
> > >
> C:\Devel\subversion_trunk\contrib\client-side\svn_apply_autoprops.py:72:
> SyntaxWarning: invalid escape sequence '\s'
> > >   re_end_autoprops = re.compile('^\s*\[\w+\]\s*')
> >
> > I was using Python 3.10 on both Windows and Linux and so was not
> > getting the warnings. I have since upgraded to Python 3.12 on Windows
> > and have made the necessary changes in the reattached patch. Testing
> > with Python 2.7 suggests that there are no problems with the raw
> > strings regarding backwards compatibility.
> >
> > [[[
> > Make svn_apply_autoprops.py Python 3-compatible.
> >
> > * contrib/client-side/svn_apply_autoprops.py:
> >   (get_autoprop_lines): Use raw strings for regexes, and `for line in
> > fd` instead of `for line in
> > fd.xreadlines()`.
> >   (filter_walk): Pass directory names separately.
> >   (main): Use `open()` instead of `file()`, and `os.walk()` instead of
> > `os.path.walk()`.
> > ]]]
> >
> > Best regards,
> > Khairul
> >
> > On Tue, Apr 30, 2024 at 11:31 PM Daniel Sahlberg
> >  wrote:
> > >
> > > Den mån 29 apr. 2024 kl 14:06 skrev Khairul Azhar Kasmiran <
> kaza...@gmail.com>:
> > >>
> > >> Hi everyone!
> > >>
> > >> As promised in [1], this patch makes svn_apply_autoprops.py Python
> > >> 3-compatible while keeping Python 2 compatibility. Afaik, the original
> > >> semantics are preserved 100% -- I think I did the conversion from
> > >> `os.path.walk()` to `os.walk()` correctly.
> > >>
> > >> [1] https://lists.apache.org/thread/j8cjrgxosz2cmcysymhy8pr4b5x9s96k
> > >>
> > >> [[[
> > >> Make svn_apply_autoprops.py Python 3-compatible.
> > >>
> > >> * contrib/client-side/svn_apply_autoprops.py: Set hashbang binary to
> `python3`.
> > >
> > >
> > > Are there any systems where Python is only installed as "python" and
> not as "python3" or is this safe?
> > >
> > >>
> > >>   (get_autoprop_lines): Use `for line in fd` instead of `for line in
> > >> fd.xreadlines()`.
> > >>   (filter_walk): Pass directory names separately.
> > >>   (main): Use `open()` instead of `file()`, and `os.walk()` instead of
> > >> `os.path.walk()`.
> > >> ]]]
> > >
> > >
> > > Thanks!
> > >
> > > I've tested and on Windows (Python 3.12) I get the following syntax
> warnings:
> > >
> > >
> C:\Devel\subversion_trunk\contrib\client-side\svn_apply_autoprops.py:71:
> SyntaxWarning: invalid escape sequence '\s'
> > >   re_start_autoprops = re.compile('^\s*\[auto-props\]\s*')
> > >
> C:\Devel\subversion_trunk\contrib\client-side\svn_apply_autoprops.py:72:
> SyntaxWarning: invalid escape sequence '\s'
> > >   re_end_autoprops = re.compile('^\s*\[\w+\]\s*')
> > >
> > > (I don't get those on Linux (Python 3.11)).
> > >
> > > It seems to help to use a raw string, but I'm not sure if this will
> kill backwards compatibility. ON the other hand if we set the hashbang to
> python3, we've explicitly killed any hope of backwards compatibility.
> > >
> > > Kind regards,
> > > Daniel
> > >
> > >
> > >
> > >>
> > >>
> > >> Best regards,
> > >> Khairul
>


Re: [PATCH] Make svn_apply_autoprops.py Python 3-compatible

2024-04-30 Thread Daniel Sahlberg
Den mån 29 apr. 2024 kl 14:06 skrev Khairul Azhar Kasmiran <
kaza...@gmail.com>:

> Hi everyone!
>
> As promised in [1], this patch makes svn_apply_autoprops.py Python
> 3-compatible while keeping Python 2 compatibility. Afaik, the original
> semantics are preserved 100% -- I think I did the conversion from
> `os.path.walk()` to `os.walk()` correctly.
>
> [1] https://lists.apache.org/thread/j8cjrgxosz2cmcysymhy8pr4b5x9s96k
>
> [[[
> Make svn_apply_autoprops.py Python 3-compatible.
>
> * contrib/client-side/svn_apply_autoprops.py: Set hashbang binary to
> `python3`.
>

Are there any systems where Python is only installed as "python" and not as
"python3" or is this safe?


>   (get_autoprop_lines): Use `for line in fd` instead of `for line in
> fd.xreadlines()`.
>   (filter_walk): Pass directory names separately.
>   (main): Use `open()` instead of `file()`, and `os.walk()` instead of
> `os.path.walk()`.
> ]]]
>

Thanks!

I've tested and on Windows (Python 3.12) I get the following syntax
warnings:

C:\Devel\subversion_trunk\contrib\client-side\svn_apply_autoprops.py:71:
SyntaxWarning: invalid escape sequence '\s'
  re_start_autoprops = re.compile('^\s*\[auto-props\]\s*')
C:\Devel\subversion_trunk\contrib\client-side\svn_apply_autoprops.py:72:
SyntaxWarning: invalid escape sequence '\s'
  re_end_autoprops = re.compile('^\s*\[\w+\]\s*')

(I don't get those on Linux (Python 3.11)).

It seems to help to use a raw string, but I'm not sure if this will kill
backwards compatibility. ON the other hand if we set the hashbang to
python3, we've explicitly killed any hope of backwards compatibility.

Kind regards,
Daniel




>
> Best regards,
> Khairul
>


Re: [PATCH] svn_apply_autoprops.py: Support @-containing filenames

2024-04-28 Thread Daniel Sahlberg
Den sön 28 apr. 2024 kl 16:07 skrev Khairul Azhar Kasmiran <
kaza...@gmail.com>:

> > Committed in r1917397.
>
> Hi Daniel, the indentation is somehow off [1]. My editor is showing
> that there are tabs there (in attached image).
>

Doh. Used the wrong tool for the job. Fixed in r1917405.

Thanks for noticing
/Daniel



>
> [1]
> https://github.com/apache/subversion/commit/014bf1137790e38c5d9a9ebcaa8eae37a952e720#diff-a8f344ecc1feeea3eae75bf0bdce5784cb1c945e8748015d7d1c4a42e61c8f9e
>
> -- Khairul
>
> On Sun, Apr 28, 2024 at 4:23 PM Daniel Sahlberg
>  wrote:
> >
> > Den sön 28 apr. 2024 kl 04:18 skrev Khairul Azhar Kasmiran <
> kaza...@gmail.com>:
> >>
> >> Hi everyone!
> >>
> >> Following on from [1], this patch prevents the error "a peg revision
> >> is not allowed here" when svn_apply_autoprops.py handles filenames
> >> containing `@`. Stack Overflow [2] gives the impression that these
> >> filenames are rare -- I have them because I have code that works with
> >> npm type packages like [3].
> >>
> >> The patch still appears routine to me, but maybe I've put the fix in
> >> the wrong place or something like that.
> >>
> >> [1] https://lists.apache.org/thread/k0o0ytr6h74wf92p4xyg8tvq5g7h4tj7
> >> [2]
> https://stackoverflow.com/questions/757435/how-to-escape-characters-in-subversion-managed-file-names
> >> [3] https://www.npmjs.com/package/@types/node
> >>
> >> [[[
> >> svn_apply_autoprops.py: Support @-containing filenames.
> >>
> >> * contrib/client-side/svn_apply_autoprops.py
> >>   (filter_walk): Append `@` to filenames containing `@`.
> >> ]]]
> >
> >
> > Thanks, I've tested and it seems to work fine. Committed in r1917397.
> >
> > Kind regards,
> > Daniel
> >
> >>
> >>
> >> Best regards,
> >> Khairul
>


Re: [PATCH] svn_apply_autoprops.py: Support @-containing filenames

2024-04-28 Thread Daniel Sahlberg
Den sön 28 apr. 2024 kl 04:18 skrev Khairul Azhar Kasmiran <
kaza...@gmail.com>:

> Hi everyone!
>
> Following on from [1], this patch prevents the error "a peg revision
> is not allowed here" when svn_apply_autoprops.py handles filenames
> containing `@`. Stack Overflow [2] gives the impression that these
> filenames are rare -- I have them because I have code that works with
> npm type packages like [3].
>
> The patch still appears routine to me, but maybe I've put the fix in
> the wrong place or something like that.
>
> [1] https://lists.apache.org/thread/k0o0ytr6h74wf92p4xyg8tvq5g7h4tj7
> [2]
> https://stackoverflow.com/questions/757435/how-to-escape-characters-in-subversion-managed-file-names
> [3] https://www.npmjs.com/package/@types/node
>
> [[[
> svn_apply_autoprops.py: Support @-containing filenames.
>
> * contrib/client-side/svn_apply_autoprops.py
>   (filter_walk): Append `@` to filenames containing `@`.
> ]]]


Thanks, I've tested and it seems to work fine. Committed in r1917397.

Kind regards,
Daniel


>
> Best regards,
> Khairul
>


Re: [PATCH] Make svn_apply_autoprops.py Windows-compatible

2024-04-27 Thread Daniel Sahlberg
>
>
> Brane has a valid comment that Subversion can store configuration also in
> the registry.
>

Sorry, it was Jun Omae who made this observation - thank you Jun!

Kind regards,
Daniel



However I don't think we should let perfect stand in the way of progress,
> this is in contrib so it is not officially supported by the Subversion
> project anyway. If someone wants to improve on it - feel free!
>
> Thank you for your contribution! I've tested and it seems to work fine for
> me.
>
> I've committed the patch in r1917382.
>
> Kind regards,
> Daniel
>
>
>>
>> On Thu, Apr 25, 2024 at 8:58 PM Khairul Azhar Kasmiran
>>  wrote:
>> >
>> > > Would you care to send that one as well?
>> >
>> > Yes, but there's another (probably routine) fix that I'd like to make
>> > in a separate unrelated patch and then I'll send that one in.
>> >
>> > -- Khairul
>> >
>> > On Thu, Apr 25, 2024 at 8:42 PM Daniel Sahlberg
>> >  wrote:
>> > >
>> > > Den tors 25 apr. 2024 kl 14:03 skrev Khairul Azhar Kasmiran <
>> kaza...@gmail.com>:
>> > >>
>> > >> > Any reason to keep it at this version instead of making the
>> necessary changes to support Python 3?
>> > >>
>> > >> I have already made the necessary changes for Python 3 locally (and I
>> > >> just found out today that the changes are compatible with both Python
>> > >> 2 and 3 afaik), but I'm adhering to "A patch submission should
>> contain
>> > >> one logical change; ...".
>> > >
>> > >
>> > > Great! Would you care to send that one as well?
>> > >
>> > > Kind regards,
>> > > Daniel
>> > >
>> > >>
>> > >> -- Khairul
>> > >>
>> > >> On Thu, Apr 25, 2024 at 7:51 PM Daniel Sahlberg
>> > >>  wrote:
>> > >> >
>> > >> > Den tors 25 apr. 2024 kl 12:30 skrev Khairul Azhar Kasmiran <
>> kaza...@gmail.com>:
>> > >> >>
>> > >> >> Oops sorry I should have used a raw string. Patch reattached.
>> > >> >>
>> > >> >> [[[
>> > >> >> Make svn_apply_autoprops.py Windows-compatible.
>> > >> >>
>> > >> >> * contrib/client-side/svn_apply_autoprops.py: Add default Windows
>> > >> >> Subversion configuration path.
>> > >> >> (process_autoprop_lines): Use `ON` instead of `*` for boolean
>> properties.
>> > >> >> (filter_walk): Replace `os.spawnvp()` with `subprocess.call()`.
>> > >> >> ]]]
>> > >> >>
>> > >> >> -- Khairul
>> > >> >>
>> > >> >> On Thu, Apr 25, 2024 at 6:18 PM Khairul Azhar Kasmiran
>> > >> >>  wrote:
>> > >> >> >
>> > >> >> > Thanks everyone for the comments!
>> > >> >> >
>> > >> >> > > * HKEY_CURRENT_USER\Software\Tigris.org\Subversion\Config
>> > >> >> >
>> > >> >> > I think reading from this registry key should be done in a
>> different
>> > >> >> > patch (probably not done by me) since it significantly
>> complicates
>> > >> >> > matters.
>> > >> >> >
>> > >> >> > > To make the script compatible with Windows needs to change
>> the reading configurations.
>> > >> >> >
>> > >> >> > I agree and in fact I've been using the `--config` option which
>> is
>> > >> >> > definitely not optimal (but only needs to be done once). I've
>> attached
>> > >> >> > an updated version of the patch that reads from
>> > >> >> > %APPDATA%\Subversion\config on Windows.
>> > >> >
>> > >> >
>> > >> > I think the new version is an improvement on the existing but I'd
>> like to try it out for myself before giving a formal +1. The script is in
>> contrib, so I don't think the fact that there are usecases where it DOESN'T
>> work should prevent it from being improved.
>> > >> >
>> > >> > I still have one question, in an earlier e-mail you wrote that it
>> was tested under Python 2.7. Any reason to keep it at this version instead
>> of mak

Re: [PATCH] Make svn_apply_autoprops.py Windows-compatible

2024-04-27 Thread Daniel Sahlberg
Den lör 27 apr. 2024 kl 03:18 skrev Khairul Azhar Kasmiran <
kaza...@gmail.com>:

> Can this patch be committed first before further changes are made?
>
> -- Khairul
>

Brane has a valid comment that Subversion can store configuration also in
the registry. However I don't think we should let perfect stand in the way
of progress, this is in contrib so it is not officially supported by the
Subversion project anyway. If someone wants to improve on it - feel free!

Thank you for your contribution! I've tested and it seems to work fine for
me.

I've committed the patch in r1917382.

Kind regards,
Daniel


>
> On Thu, Apr 25, 2024 at 8:58 PM Khairul Azhar Kasmiran
>  wrote:
> >
> > > Would you care to send that one as well?
> >
> > Yes, but there's another (probably routine) fix that I'd like to make
> > in a separate unrelated patch and then I'll send that one in.
> >
> > -- Khairul
> >
> > On Thu, Apr 25, 2024 at 8:42 PM Daniel Sahlberg
> >  wrote:
> > >
> > > Den tors 25 apr. 2024 kl 14:03 skrev Khairul Azhar Kasmiran <
> kaza...@gmail.com>:
> > >>
> > >> > Any reason to keep it at this version instead of making the
> necessary changes to support Python 3?
> > >>
> > >> I have already made the necessary changes for Python 3 locally (and I
> > >> just found out today that the changes are compatible with both Python
> > >> 2 and 3 afaik), but I'm adhering to "A patch submission should contain
> > >> one logical change; ...".
> > >
> > >
> > > Great! Would you care to send that one as well?
> > >
> > > Kind regards,
> > > Daniel
> > >
> > >>
> > >> -- Khairul
> > >>
> > >> On Thu, Apr 25, 2024 at 7:51 PM Daniel Sahlberg
> > >>  wrote:
> > >> >
> > >> > Den tors 25 apr. 2024 kl 12:30 skrev Khairul Azhar Kasmiran <
> kaza...@gmail.com>:
> > >> >>
> > >> >> Oops sorry I should have used a raw string. Patch reattached.
> > >> >>
> > >> >> [[[
> > >> >> Make svn_apply_autoprops.py Windows-compatible.
> > >> >>
> > >> >> * contrib/client-side/svn_apply_autoprops.py: Add default Windows
> > >> >> Subversion configuration path.
> > >> >> (process_autoprop_lines): Use `ON` instead of `*` for boolean
> properties.
> > >> >> (filter_walk): Replace `os.spawnvp()` with `subprocess.call()`.
> > >> >> ]]]
> > >> >>
> > >> >> -- Khairul
> > >> >>
> > >> >> On Thu, Apr 25, 2024 at 6:18 PM Khairul Azhar Kasmiran
> > >> >>  wrote:
> > >> >> >
> > >> >> > Thanks everyone for the comments!
> > >> >> >
> > >> >> > > * HKEY_CURRENT_USER\Software\Tigris.org\Subversion\Config
> > >> >> >
> > >> >> > I think reading from this registry key should be done in a
> different
> > >> >> > patch (probably not done by me) since it significantly
> complicates
> > >> >> > matters.
> > >> >> >
> > >> >> > > To make the script compatible with Windows needs to change the
> reading configurations.
> > >> >> >
> > >> >> > I agree and in fact I've been using the `--config` option which
> is
> > >> >> > definitely not optimal (but only needs to be done once). I've
> attached
> > >> >> > an updated version of the patch that reads from
> > >> >> > %APPDATA%\Subversion\config on Windows.
> > >> >
> > >> >
> > >> > I think the new version is an improvement on the existing but I'd
> like to try it out for myself before giving a formal +1. The script is in
> contrib, so I don't think the fact that there are usecases where it DOESN'T
> work should prevent it from being improved.
> > >> >
> > >> > I still have one question, in an earlier e-mail you wrote that it
> was tested under Python 2.7. Any reason to keep it at this version instead
> of making the necessary changes to support Python 3? Python 3 is available
> on Microsoft Store so it is almost part of the OS.
> > >> >
> > >> > (I think this change and Python3 compatibility should be two
> separate commits, but I'd like to raise the question).
> > >> >
> > >> 

Re: [PATCH] Make svn_apply_autoprops.py Windows-compatible

2024-04-25 Thread Daniel Sahlberg
Den tors 25 apr. 2024 kl 14:03 skrev Khairul Azhar Kasmiran <
kaza...@gmail.com>:

> > Any reason to keep it at this version instead of making the necessary
> changes to support Python 3?
>
> I have already made the necessary changes for Python 3 locally (and I
> just found out today that the changes are compatible with both Python
> 2 and 3 afaik), but I'm adhering to "A patch submission should contain
> one logical change; ...".
>

Great! Would you care to send that one as well?

Kind regards,
Daniel


> -- Khairul
>
> On Thu, Apr 25, 2024 at 7:51 PM Daniel Sahlberg
>  wrote:
> >
> > Den tors 25 apr. 2024 kl 12:30 skrev Khairul Azhar Kasmiran <
> kaza...@gmail.com>:
> >>
> >> Oops sorry I should have used a raw string. Patch reattached.
> >>
> >> [[[
> >> Make svn_apply_autoprops.py Windows-compatible.
> >>
> >> * contrib/client-side/svn_apply_autoprops.py: Add default Windows
> >> Subversion configuration path.
> >> (process_autoprop_lines): Use `ON` instead of `*` for boolean
> properties.
> >> (filter_walk): Replace `os.spawnvp()` with `subprocess.call()`.
> >> ]]]
> >>
> >> -- Khairul
> >>
> >> On Thu, Apr 25, 2024 at 6:18 PM Khairul Azhar Kasmiran
> >>  wrote:
> >> >
> >> > Thanks everyone for the comments!
> >> >
> >> > > * HKEY_CURRENT_USER\Software\Tigris.org\Subversion\Config
> >> >
> >> > I think reading from this registry key should be done in a different
> >> > patch (probably not done by me) since it significantly complicates
> >> > matters.
> >> >
> >> > > To make the script compatible with Windows needs to change the
> reading configurations.
> >> >
> >> > I agree and in fact I've been using the `--config` option which is
> >> > definitely not optimal (but only needs to be done once). I've attached
> >> > an updated version of the patch that reads from
> >> > %APPDATA%\Subversion\config on Windows.
> >
> >
> > I think the new version is an improvement on the existing but I'd like
> to try it out for myself before giving a formal +1. The script is in
> contrib, so I don't think the fact that there are usecases where it DOESN'T
> work should prevent it from being improved.
> >
> > I still have one question, in an earlier e-mail you wrote that it was
> tested under Python 2.7. Any reason to keep it at this version instead of
> making the necessary changes to support Python 3? Python 3 is available on
> Microsoft Store so it is almost part of the OS.
> >
> > (I think this change and Python3 compatibility should be two separate
> commits, but I'd like to raise the question).
> >
> > Kind regards,
> > Daniel Sahlberg
> >
> >
> >>
> >> >
> >> > [[[
> >> > Make svn_apply_autoprops.py Windows-compatible.
> >> >
> >> > * contrib/client-side/svn_apply_autoprops.py: Add default Windows
> >> > Subversion configuration path.
> >> > (process_autoprop_lines): Use `ON` instead of `*` for boolean
> properties.
> >> > (filter_walk): Replace `os.spawnvp()` with `subprocess.call()`.
> >> > ]]]
> >> >
> >> > -- Khairul
> >> >
> >> > On Thu, Apr 25, 2024 at 3:06 PM Branko Čibej 
> wrote:
> >> > >
> >> > > On 25. 04. 24 00:29, Jun Omae wrote:
> >> > >
> >> > > Hi,
> >> > >
> >> > > On Tue, Apr 23, 2024 at 8:42 PM Khairul Azhar Kasmiran
> >> > >  wrote:
> >> > >
> >> > > I've reattached the patch as a .txt file.
> >> > >
> >> > > On 2024/04/23 10:46:41 Khairul Azhar Kasmiran wrote:
> >> > >
> >> > > Hi everyone!
> >> > >
> >> > > This is a patch to make `contrib/client-side/svn_apply_autoprops.py`
> >> > > Windows-compatible -- I have just found out that `git svn` doesn't
> >> > > honor autoprops.
> >> > >
> >> > > In POSIX environment, Subversion configurations are loaded from
> >> > > ~/.subversion/config file.
> >> > >
> >> > >  33 # The default path to the Subversion configuration file.
> >> > >  34 SVN_CONFIG_FILENAME =
> os.path.expandvars('$HOME/.subversion/config')
> >> > >
> >> > > However, the following registry or file is used in Windows.
> >> > >
> >> > >  * HKEY_CURRENT_USER\Software\Tigris.org\Subversion\Config
> >> > >  * %USERPROFILE%\AppData\Roaming\Subversion\config
> >> > >
> >> > >
> >> > > This is actually %APPDATA%\Subversion\config, there's no guarantee
> that %APPDATA% points to the roaming profile.
> >> > >
> >> > >
> >> > > To make the script compatible with Windows needs to change the
> reading configurations.
> >> > >
> >> > >
> >> > > Yes.
> >> > >
> >> > > -- Brane
>


Re: [PATCH] Make svn_apply_autoprops.py Windows-compatible

2024-04-25 Thread Daniel Sahlberg
Den tors 25 apr. 2024 kl 12:30 skrev Khairul Azhar Kasmiran <
kaza...@gmail.com>:

> Oops sorry I should have used a raw string. Patch reattached.
>
> [[[
> Make svn_apply_autoprops.py Windows-compatible.
>
> * contrib/client-side/svn_apply_autoprops.py: Add default Windows
> Subversion configuration path.
> (process_autoprop_lines): Use `ON` instead of `*` for boolean properties.
> (filter_walk): Replace `os.spawnvp()` with `subprocess.call()`.
> ]]]
>
> -- Khairul
>
> On Thu, Apr 25, 2024 at 6:18 PM Khairul Azhar Kasmiran
>  wrote:
> >
> > Thanks everyone for the comments!
> >
> > > * HKEY_CURRENT_USER\Software\Tigris.org\Subversion\Config
> >
> > I think reading from this registry key should be done in a different
> > patch (probably not done by me) since it significantly complicates
> > matters.
> >
> > > To make the script compatible with Windows needs to change the reading
> configurations.
> >
> > I agree and in fact I've been using the `--config` option which is
> > definitely not optimal (but only needs to be done once). I've attached
> > an updated version of the patch that reads from
> > %APPDATA%\Subversion\config on Windows.
>

I think the new version is an improvement on the existing but I'd like to
try it out for myself before giving a formal +1. The script is in contrib,
so I don't think the fact that there are usecases where it DOESN'T work
should prevent it from being improved.

I still have one question, in an earlier e-mail you wrote that it was
tested under Python 2.7. Any reason to keep it at this version instead of
making the necessary changes to support Python 3? Python 3 is available on
Microsoft Store so it is almost part of the OS.

(I think this change and Python3 compatibility should be two separate
commits, but I'd like to raise the question).

Kind regards,
Daniel Sahlberg



> >
> > [[[
> > Make svn_apply_autoprops.py Windows-compatible.
> >
> > * contrib/client-side/svn_apply_autoprops.py: Add default Windows
> > Subversion configuration path.
> > (process_autoprop_lines): Use `ON` instead of `*` for boolean properties.
> > (filter_walk): Replace `os.spawnvp()` with `subprocess.call()`.
> > ]]]
> >
> > -- Khairul
> >
> > On Thu, Apr 25, 2024 at 3:06 PM Branko Čibej  wrote:
> > >
> > > On 25. 04. 24 00:29, Jun Omae wrote:
> > >
> > > Hi,
> > >
> > > On Tue, Apr 23, 2024 at 8:42 PM Khairul Azhar Kasmiran
> > >  wrote:
> > >
> > > I've reattached the patch as a .txt file.
> > >
> > > On 2024/04/23 10:46:41 Khairul Azhar Kasmiran wrote:
> > >
> > > Hi everyone!
> > >
> > > This is a patch to make `contrib/client-side/svn_apply_autoprops.py`
> > > Windows-compatible -- I have just found out that `git svn` doesn't
> > > honor autoprops.
> > >
> > > In POSIX environment, Subversion configurations are loaded from
> > > ~/.subversion/config file.
> > >
> > >  33 # The default path to the Subversion configuration file.
> > >  34 SVN_CONFIG_FILENAME =
> os.path.expandvars('$HOME/.subversion/config')
> > >
> > > However, the following registry or file is used in Windows.
> > >
> > >  * HKEY_CURRENT_USER\Software\Tigris.org\Subversion\Config
> > >  * %USERPROFILE%\AppData\Roaming\Subversion\config
> > >
> > >
> > > This is actually %APPDATA%\Subversion\config, there's no guarantee
> that %APPDATA% points to the roaming profile.
> > >
> > >
> > > To make the script compatible with Windows needs to change the reading
> configurations.
> > >
> > >
> > > Yes.
> > >
> > > -- Brane
>


Re: 1.14.4 release

2024-03-11 Thread Daniel Sahlberg
Den mån 11 mars 2024 kl 00:59 skrev Nathan Hartman :

> Hi all,
>
> Unfortunately the recently released 1.14.3 contains a new regression
> [1] that may crash the SWIG python bindings in repos.replay() under
> some conditions.
>
> It is already fixed on trunk in r1915316 and backported to the 1.14.x
> branch in r1915338.
>
> There was a discussion [2] about issuing a 1.14.4 release to fix it,
> as there is no known workaround for Python users other than patching
> some code or downgrading to 1.14.2.
>
> I volunteer to RM the 1.14.4 release. Of course, if anyone else would
> like to RM, feel free to speak up. I'll be happy to assist.
>
> My tentative plan is to try to reproduce the issue and verify the fix,
> and then proceed with the normal patch release process.
>
> There is at least a week before I can begin working on it, so there is
> time for the dev community to respond.
>
> Meanwhile, there are two proposed backports in STATUS that might merit
> some consideration:
> * r1890223 et al "Support building on Win64/ARM64"
> * r1914222 "Improve help message for svnmucc PUT"
>
> Each needs one additional vote to make it into the release.
>
> References:
>
> [1] 30 Jan 2024 mail to dev@ titled
> "[patch] publication for the fix of swig-py bug fixed in r1915316"
> archived at:
> https://lists.apache.org/thread/ds24b6w446horzvl4n8y6dqrfl3jb6s4
>
> [2] 1 Feb 2024 mail to dev@ titled
> "Re: svn commit: r1915317 - /subversion/branches/1.14.x/STATUS"
> archived at:
> https://lists.apache.org/thread/571hk0nykvrorx9wq0r0lyylpjj42qdw
>
> Thanks,
> Nathan
>

That sounds great Nathan, thank you for following up on this!

I'm travelling this week with limited amount of time but I will do my best
to make room next week.

Cheers,
Daniel


Re: Changing the permission checks in libsvn_subr/io.c

2024-02-20 Thread Daniel Sahlberg
Den mån 5 feb. 2024 kl 07:38 skrev Branko Čibej :

> On 04.02.2024 00:31, Evgeny Kotkov via dev wrote:
>
> Daniel Sahlberg   
> writes:
>
>
> Index: subversion/libsvn_subr/io.c
> ===
> --- subversion/libsvn_subr/io.c (revision 1915064)
> +++ subversion/libsvn_subr/io.c (working copy)
> @@ -2535,7 +2535,14 @@ svn_io__is_finfo_read_only(svn_boolean_t *read_onl
>apr_uid_t uid;
>apr_gid_t gid;
>
> -  *read_only = FALSE;
> +  *read_only = (access(file_info->fname, W_OK) != 0);
>
> There are a few problems, because this approach adds a I/O syscall (that
> checks access to a file by its name) into a fairly low-level function.
>
> Previously this function was analyzing the file information from the passed-in
> apr_finfo_t structure.  The new version, however, performs an I/O call, and
> that leads to a couple of problems:
>
> 1) Potential performance regression, not limited to just the revert.
>For example, all calls to open_pack_or_rev_file() in libsvn_fs_fs would
>now start to make additional access() calls, thus slowing down the
>repository operations.
>
> 2) This adds an opportunity for a desynchronization/race between the
>information in apr_finfo_t and the result of access(), because access()
>works by a filename.  For example, in a case where a file is recreated
>between the two calls, its result will correspond to a completely
>different file, compared to the one that produced the apr_finfo_t.
>
> Overall, I have a feeling that solving this specific edge-case might not be
> worth introducing these regressions (which look significant, at least to me).
>
>
> I agree. If it's worth solving, it should be done in a way that's specific
> to revert, not here.
>
> -- Brane
>

Thanks for the feedback and sorry for the late reply. I've been swamped
with other tasks lately and have not yet found the time to look at this but
it is for sure on my todo list.

Kind regards,
Daniel


Re: Assert-Anweisung schlug fehl (! svn_path_is_url(relative))

2024-02-06 Thread Daniel Sahlberg
Den tis 6 feb. 2024 kl 11:02 skrev Jürgen Loh :

> Hello,
>
> while using TourtoiseSVN 1.14.6, Build 29673 - 32-Bit under Windows 10
> 22H2 (Build 19045-3930) 32-Bit, I got this message:
>
> ---
> Subversion Exception!
> ---
> Subversion encountered a serious problem.
> Please take the time to report this on the Subversion mailing list
> with as much information as possible about what
> you were trying to do.
> But please first search the mailing list archives for the error message
> to avoid reporting the same problem repeatedly.
> You can find the mailing list archives at
> https://subversion.apache.org/mailing-lists.html
>
> Subversion reported the following
> (you can copy the content of this dialog
> to the clipboard using Ctrl-C):
>
> In Datei
>
>
>   
> »D:\Development\SVN\Releases\TortoiseSVN-1.14.6\ext\subversion\subversion\libsvn_subr\dirent_uri.c«,
>   Zeile 1634: Assert-Anweisung schlug fehl (! svn_path_is_url(relative))
> ---
> OK
> ---
>
> (I set TortoiseSVN to English but German language pack is also installed)
>
> What I did:
>
> On a SVN working copy on G:\ I did a "Show Log" on G:\web.  G:\ is
> actually a SUBST:
>
> [f:\dos\bat]subst
> G:\: => C:\User\Data
>
> Then I selected the last commit (HEAD), selected one of the changed
> files (a GIF image) and did a "Revert changes from this revision".  Then
> I selected "Revert" (Rückgängig) and got he message above and this log:
>
> ---
> Command: Merging revisions 1734-1733 of
> svn://t61/tupel.jloh.de/web/htdocs/nascom/journal/81/06/01.gif into
> G:\\web\htdocs\nascom\journal\81\06\01.gif, ignoring ancestry
> Error: In Datei
> Error:
> »D:\Development\SVN\Releases\TortoiseSVN-1.14.6\ext\subversion\subversion\libsvn_subr\dirent_uri.c«,
>
>
> Error:  Zeile 1634: Assert-Anweisung schlug fehl (!
> svn_path_is_url(relative))
> Completed!:
> ---
>
> Then I did the same on C:\User\Data and this time it worked!
>
> --
> Jürgen Loh
>
>
Can you reproduce the same issues on the command line client? Just to rule
out any issues in TortoiseSVN (but I don't think that is likely in this
case).

There are well known issues with SUBST. There is for example a long thread
on the APR mailing list (
https://lists.apache.org/thread/18x2jb81nf6zrjsnwf1k2wwooprkp0p5). However
TortoiseSVN 1.14.6 is using APR 1.7.4, so this particular bug should have
been fixed.

Can you, as a workaround, avoid using SUBST?

Kind regards,
Daniel Sahlberg


Re: svn commit: r1915317 - /subversion/branches/1.14.x/STATUS

2024-02-02 Thread Daniel Sahlberg
Den fre 2 feb. 2024 kl 08:20 skrev Jun Omae :

> On Thu, Feb 1, 2024 at 10:42 PM Daniel Sahlberg
>  wrote:
> >
> > Den fre 19 jan. 2024 kl 07:41 skrev :
> >>
> >> Author: jun66j5
> >> Date: Fri Jan 19 06:40:59 2024
> >> New Revision: 1915317
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1915317=rev
> >> Log:
> >> * STATUS: Nominate r1915316.
> >>
> >> Modified:
> >> subversion/branches/1.14.x/STATUS
> >>
> >> Modified: subversion/branches/1.14.x/STATUS
> >> URL:
> http://svn.apache.org/viewvc/subversion/branches/1.14.x/STATUS?rev=1915317=1915316=1915317=diff
> >>
> ==
> >> --- subversion/branches/1.14.x/STATUS (original)
> >> +++ subversion/branches/1.14.x/STATUS Fri Jan 19 06:40:59 2024
> >> @@ -45,6 +45,14 @@ Candidate changes:
> >> Votes:
> >>   +1: dsahlberg
> >>
> >> + * r1915316
> >> +   swig-py: Fix `none_dealloc` error caused by reference count issue in
> >> +   `apply_textdelta` invoked from `svn.repos.replay()`.
> >> +   Justification:
> >> + Fix Python interpreter crash which often occurs with large
> repositories.
> >> +   Votes:
> >> + +1: jun66j5
> >> +
> >>  Veto-blocked changes:
> >>  =
> >
> >
> > Is this serious enough that we should consider a 1.14.4 release? (Didn't
> have time to review the code yet but a quick glance positive).
> >
> > Kind regards,
> > Daniel
>
> In my opinion, yes. Python users have no workaround for the issue
> except downgrading to 1.14.2. I think it happens if Python is a
> long-running process like a daemon or with large repositories.
>
> At least, I'm considering to report it with the patch to distributors
> (Debian, Ubuntu, FreeBSD, etc.).
>
> --
> Jun Omae  (大前 潤)
>

Alright, since Nathan also thought about a release we should probably do
this.

It seems the above fix is already backported (r1915338).

There are two other fixes with two votes in the STATUS file:

 * r1914222
   Improve help message for svnmucc PUT.

This is only a help message change, but it is within the "core code" so I
think someone else has to approve it as well (but we can probably bend the
rules since it only affects translations).

 * r1912632
   Fix `invalid escape sequence` in Python scripts to prevent many
   `SyntaxWarning`s since Python 3.12.

This is the patch created by Jun last summer, review by futatuki and
committed by me. Jun, would you consider voting for this?

The two above should be fairly easy fixes and I think it would be good to
include.

Then we have the following group:

 * r1890223, r1890668, r1890673
   Support building on Win64/ARM64.

It is not approved and I'm not sure if we have someone who can look at it.

There is also the rewrite of mailer.py hook-script (in tools/hook-scripts).
I'm not sure if it is complete yet and I'm not sure if we want to release
it in a patch release. We've said before not to break Python2
compatibility, on the other hand the current script doesn't work at all
under Python3 and I'm leaning towards Python3 is the more important target
at this moment. @Greg Stein  can probably comment more on
the current status.

Kind regards,
Daniel


Re: Switching from SHA1 to a checksum type without known collisions in 1.15 working copy format

2024-02-01 Thread Daniel Sahlberg
Gentlemen,

It seems you have both had your say in what flaws there has been in the
process. Can we please leave this part of the discussion and continue on
the technical issues? I'd hate for this discussion to turn to pie-throwing
where someone in the end feel offended and leave the community. We are such
a small community and we can't afford to lose someone just because an
argument turns toxic (it has happened before so let's make sure it doesn't
happen again, please).

As for the technical side, can we break down the current status and the
desired future status to some points and then look at what options we have
for solutions?

Currently we use SHA1, which have known attacks. What are the risks?
- It has been argued that `svn st` will, especially with no-pristines, be
extra vulnerable to not detecting a modified file if someone can create a
collision with the checksum of the original file
- Someone also argued that a software could potentially be banned just
because it uses a checksum with a known attack, even if the checksum isn't
used in a security critical way.

What options do we have and how do they mitigate the above risks?
- Evgeny has already shown a possible solution with a salted hash (keeping
SHA-1).
- Can we switch to another hash function completely and does it offer any
benefits compared to the salted SHA-1?
- Should we even do both?

Any other points?

Any thoughts?

I would like to see this thread progress and I hope we can find consensus
on a way forward.

Kind regards,
Daniel Sahlberg


Den tors 18 jan. 2024 kl 14:36 skrev Evgeny Kotkov via dev <
dev@subversion.apache.org>:

> Daniel Shahaf  writes:
>
> > Procedurally, the long hiatus is counterproductive.
>
> This reminds me that the substantive discussion of your veto ended with my
> email from 8 Feb 2023 that had four direct questions to you and was left
> without an answer:
>
> ``
>   > That's not how design discussions work.  A design discussion doesn't go
>   > "state decision; state pros; implement"; it goes "state problem;
> discuss
>   > potential solutions, pros, cons; decide; implement" (cf. [4, 5, 6]).
>
>   Well, I think it may not be as simple as it seems to you.  Who decided
> that
>   we should follow the process you're describing?  Is there a thread with a
>   consensus on this topic?  Or do you insist on using this specific process
>   because it's the only process that seems obvious to you?  What
> alternatives
>   to it have been considered?
>
>   As far as I can tell, the process you're suggesting is effectively a
>   waterfall-like process, and there are quite a lot of concerns about its
>   effectiveness, because the decisions have to be made in the conditions of
>   a lack of information.
> ``
>
> It's been more than 11 months since that email, and those questions still
> don't have an answer.  So if we are to resume this discussion, let's do it
> from the proper point.
>
> > You guys are welcome to try to /convince/ me to change my opinion, or to
> > have the veto invalidated.  In either case, you will be more likely to
> > succeed should your arguments relate not only to the veto's implications
> > but also to its /sine qua non/ component: its rationale.
>
> Just in case, my personal opinion here is that the veto is invalid.
>
> Firstly, based on my understanding, the ASF rules prohibit casting a veto
> without an appropriate technical justification (see [1], which I personally
> agree with).  Secondly, it seems that the process you are imposing hasn't
> been
> accepted in this community.  As far as I know, this topic was tangentially
> discussed before (see [2], for example), and it looks like there hasn't
> been
> a consensus to change our current Commit-Then-Review process into some
> sort of Review-Then-Commit.
>
> (At the same time I won't even try to /convince/ you, sorry.)
>
> [1] https://www.apache.org/foundation/voting.html
> [2] https://lists.apache.org/thread/ow2x68g2k4lv2ycr81d14p8r8w2jj1xl
>
>
> Regards,
> Evgeny Kotkov
>


Re: svn commit: r1915317 - /subversion/branches/1.14.x/STATUS

2024-02-01 Thread Daniel Sahlberg
Den fre 19 jan. 2024 kl 07:41 skrev :

> Author: jun66j5
> Date: Fri Jan 19 06:40:59 2024
> New Revision: 1915317
>
> URL: http://svn.apache.org/viewvc?rev=1915317=rev
> Log:
> * STATUS: Nominate r1915316.
>
> Modified:
> subversion/branches/1.14.x/STATUS
>
> Modified: subversion/branches/1.14.x/STATUS
> URL:
> http://svn.apache.org/viewvc/subversion/branches/1.14.x/STATUS?rev=1915317=1915316=1915317=diff
>
> ==
> --- subversion/branches/1.14.x/STATUS (original)
> +++ subversion/branches/1.14.x/STATUS Fri Jan 19 06:40:59 2024
> @@ -45,6 +45,14 @@ Candidate changes:
> Votes:
>   +1: dsahlberg
>
> + * r1915316
> +   swig-py: Fix `none_dealloc` error caused by reference count issue in
> +   `apply_textdelta` invoked from `svn.repos.replay()`.
> +   Justification:
> + Fix Python interpreter crash which often occurs with large
> repositories.
> +   Votes:
> + +1: jun66j5
> +
>  Veto-blocked changes:
>  =
>

Is this serious enough that we should consider a 1.14.4 release? (Didn't
have time to review the code yet but a quick glance positive).

Kind regards,
Daniel


Re: svn commit: r1908547 - /subversion/trunk/subversion/svnserve/svnserve.c

2024-01-30 Thread Daniel Sahlberg
Den tis 30 jan. 2024 kl 01:39 skrev Jun Omae :

> On 2024/01/30 6:15, Daniel Sahlberg wrote:
> > Good catch! How about:
> >
> > [[[
> > Index: subversion/svnserve/svnserve.c
> > ===
> > --- subversion/svnserve/svnserve.c  (revision 1915424)
> > +++ subversion/svnserve/svnserve.c  (working copy)
> > @@ -574,7 +574,7 @@ accept_connection(connection_t **connection,
> >  || APR_STATUS_IS_ECONNABORTED(status)
> >  || APR_STATUS_IS_ECONNRESET(status));
> >
> > -  return status
> > +  return status && !sigtermint_seen
> > ? svn_error_wrap_apr(status, _("Can't accept client connection"))
> > : SVN_NO_ERROR;
> >  }
> > ]]]
>
> The `sigtermint_seen` variable is not defined if sigaction is unavailable.
> Instead, how about the following patch?
>
> [[[
> Index: subversion/svnserve/svnserve.c
> ===
> --- subversion/svnserve/svnserve.c  (revision 1915466)
> +++ subversion/svnserve/svnserve.c  (working copy)
> @@ -574,9 +574,14 @@ accept_connection(connection_t **connection,
>  || APR_STATUS_IS_ECONNABORTED(status)
>  || APR_STATUS_IS_ECONNRESET(status));
>
> -  return status
> -   ? svn_error_wrap_apr(status, _("Can't accept client connection"))
> -   : SVN_NO_ERROR;
> +  if (!status)
> +return SVN_NO_ERROR;
> +#if APR_HAVE_SIGACTION
> +  else if (sigtermint_seen)
> +return SVN_NO_ERROR;
> +#endif
> +  else
> +return svn_error_wrap_apr(status, _("Can't accept client
> connection"));
>  }
>
>  /* Add a reference to CONNECTION, i.e. keep it and it's pool valid unless
> ]]]
>

Oh, yes of course. That looks much better. Please commit!

Kind regards,
Daniel


Re: svn commit: r1915476 - /subversion/trunk/COMMITTERS

2024-01-30 Thread Daniel Sahlberg
Nice! Welcome!

Daniel

tis 30 jan. 2024 kl. 11:54 skrev :

> Author: vinc17
> Date: Tue Jan 30 10:54:12 2024
> New Revision: 1915476
>
> URL: http://svn.apache.org/viewvc?rev=1915476=rev
> Log:
> * COMMITTERS:
>   (vinc17) Add myself as a full committer.
>
> Modified:
> subversion/trunk/COMMITTERS
>
> Modified: subversion/trunk/COMMITTERS
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/COMMITTERS?rev=1915476=1915475=1915476=diff
>
> ==
> --- subversion/trunk/COMMITTERS [UTF-8] (original)
> +++ subversion/trunk/COMMITTERS [UTF-8] Tue Jan 30 10:54:12 2024
> @@ -65,6 +65,7 @@ Blanket commit access:
>futatuki   Yasuhito Futatsuki 
> jun66j5   Jun Omae 
>   dsahlberg   Daniel Sahlberg 
> +vinc17   Vincent Lefevre 
>
>  [[END ACTIVE FULL COMMITTERS.  LEAVE THIS LINE HERE; SCRIPTS LOOK FOR
> IT.]]
>
>
>
>


Re: Changing the permission checks in libsvn_subr/io.c

2024-01-29 Thread Daniel Sahlberg
Den mån 22 jan. 2024 kl 04:05 skrev Branko Čibej :

> On 13.01.2024 09:58, Daniel Sahlberg wrote:
>
> Since there wasn't any replies to this and I think the code was working
> fine in all my tests, I comitted as r1915214. Although I finally decided to
> solve the spurious revert messages in a different way, see a separate
> followup/committ e-mail.
>
>
> I think you just made the code more complicated for no good reason. The
> situation you described and fixed applies only to working copies that are
> shared by different users.
>
> You also have to ask yourself what happens when your user runs an "svn
> update" in the working copy where it's not the owner of the files and when
> your primary group doesn't have write access to the containing directory.
> In other words, I think you just fixed an edge case that has lots of other
> problems, not least that without using at least 'chgrp', the working copy
> will be either useless for you or quickly corrupted for others. I'm not
> even going to try to think about what happens to SQLite WAL files. Your
> exhaustive tests don't seem to cover these cases.
>
> In practice, such working copies tend to be used by prefixing all svn
> commands with 'sudo -u wc-owner' or starting with 'sudo -u wc-owner -s'.
> That's a solution that always works and doesn't require more code in svn
> that someone has to maintain and that only solves one very specific edge
> case.
>
> -- Brane
>
>
Are you referring to the changes in 1915214 or in 1915215?

r1915214 replaces a homegrown check for the current UID and GID, then
comparing with current file mask (total 23 loc) with calls to a library
function (total 4 loc). I presume there is a cost for some stat calls
within access(), so performance is probably a bit worse. (Maybe some of the
performance could be recovered in revert_wc_data, there is comment to use
svn_io_dirent2_t "[except] that we need the read only and execute bits
later", I presume for the now replaced checks). I would argue that the new
code is simpler and easier to maintain, but I'm of course open to other
opinions.

With r1915215, I can agree it makes the code more complicated. But it
solves another issue, the "spurious revert" messages reported here several
times (most recently by Vincent Lefevre). I'd be the first to admit that
the fix is still half-baked: The message is completely bonkers (and it
lacks a trailing \n), but at least the user is notified that something is
going on instead of getting the "reverted" message over and over again.

Kind regards,
Daniel


Re: svn commit: r1908547 - /subversion/trunk/subversion/svnserve/svnserve.c

2024-01-29 Thread Daniel Sahlberg
Den mån 29 jan. 2024 kl 13:50 skrev Jun Omae :

> On 2023/03/20 10:27, phi...@apache.org wrote:
> > Author: philip
> > Date: Mon Mar 20 01:27:41 2023
> > New Revision: 1908547
> >
> > URL: http://svn.apache.org/viewvc?rev=1908547=rev
> > Log:
> > Add SIGTERM/SIGINT handling to svnserve, this allows it to exit more
> > gracefully which allows tools like valgrind to monitor whether the
> > program exits cleanly and do things like leak detection.
> >
> > * subversion/svnserve/svnserve.c
> >   (sigtermint_handler): New handler.
> >   (accept_connection): Check for signal.
> >   (sub_main): Install handler, check for signal, add return.
>
> After r1908547, svnserve shows E04 error message even if sending
> SIGTERM/SIGINT to stop normally the process.
>
> [[[
> $ ~/svn/trunk-1915316/bin/svnserve -X &
> [1] 3088030
> $ kill %1
> $ svnserve: E04: Can't accept client connection: Interrupted system
> call
> ]]]
>
> Another example, the error messages are shown repeatedly while running
> check-swig-rb which is using svnserve.
>
> [[[
> $ make check-swig-rb
> ...
> Loaded suite .
> Started
> svnserve: E04: Can't accept client connection: Interrupted system call
> .svnserve: E04: Can't accept client connection: Interrupted system call
> .svnserve: E04: Can't accept client connection: Interrupted system call
> .svnserve: E04: Can't accept client connection: Interrupted system call
> .svnserve: E04: Can't accept client connection: Interrupted system call
> ]]]
>
> I think we should prevent the messages even if the system call is
> interrupted
> when stop request by SIGTERM/SIGINT.
>
> --
> Jun Omae  (大前 潤)
>
>
Good catch! How about:

[[[
Index: subversion/svnserve/svnserve.c
===
--- subversion/svnserve/svnserve.c  (revision 1915424)
+++ subversion/svnserve/svnserve.c  (working copy)
@@ -574,7 +574,7 @@ accept_connection(connection_t **connection,
 || APR_STATUS_IS_ECONNABORTED(status)
 || APR_STATUS_IS_ECONNRESET(status));

-  return status
+  return status && !sigtermint_seen
? svn_error_wrap_apr(status, _("Can't accept client connection"))
: SVN_NO_ERROR;
 }
]]]


Re: svn commit: r1915215 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_wc/revert.c svn/notify.c svnbench/notify.c

2024-01-29 Thread Daniel Sahlberg
Den mån 29 jan. 2024 kl 13:26 skrev Jun Omae :

> On 2024/01/13 18:16, dsahlb...@apache.org wrote:
> > Author: dsahlberg
> > Date: Sat Jan 13 09:16:26 2024
> > New Revision: 1915215
> >
> > URL: http://svn.apache.org/viewvc?rev=1915215=rev
> > Log:
> > Manage spurious Reverted message caused by non-W access to files
> > owned by another user. Part of Issue #4622.
> >
> > The revert notification comes from the code trying to add W permissions
> > but since there is already W (for another user) the code doesn't change
> > anything and the notification will come back next time as well.
> >
> > Changing to add a separate notification type "you don't have W access
> > and we can't do anything about it".
> >
> > The text should be tweaked further.
> >
> > Discussed on dev@:
> https://lists.apache.org/thread/p1ky889bxwy8okqly7h1lgckxfpldnxs
>
> After r1915215, make check and check-apache-javahl failed.
>
> [[[
> Summary of test results:
>   2558 tests PASSED
>   168 tests SKIPPED
>   81 tests XFAILED (17 WORK-IN-PROGRESS)
>   27 tests FAILED
> ]]]
>

That was a stupid mistake on my part. I obviously only checked part of the
testsuite. After r1915466, the testsuite passes again.



>
> [[[
> There was 1 error:
> 1)
> testDiff(org.apache.subversion.javahl.BasicTests)org.apache.subversion.javahl.ClientException:
> The operation was interrupted
> svn: Operation cancelled
> svn: Wrapped Java Exception
>
> at org.apache.subversion.javahl.SVNClient.revert(Native Method)
> at org.apache.subversion.javahl.SVNClient.revert(SVNClient.java:253)
> at
> org.apache.subversion.javahl.BasicTests.testDiff(BasicTests.java:3419)
> at
> java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
> at org.apache.subversion.javahl.RunTests.main(RunTests.java:119)
> Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 90 out of
> bounds for length 80
> ... 19 more
>
> FAILURES!!!
> Tests run: 148,  Failures: 0,  Errors: 1
>
> make: *** [Makefile:534: check-apache-javahl] Error 1
> ]]]
>

The java tests also pass for me now.

Kind regards,
Daniel


>
> --
> Jun Omae  (大前 潤)
>
>


Re: Switching from SHA1 to a checksum type without known collisions in 1.15 working copy format

2024-01-17 Thread Daniel Sahlberg
@Karl Fogel ,  @Evgeny Kotkov


Any chance for a comment on the questions in this thread?

I've also added my own comment below.

Kind regards,
Daniel



Den sön 14 jan. 2024 kl 00:56 skrev Nathan Hartman :

> On Fri, Jan 12, 2024 at 3:51 PM Johan Corveleyn  wrote:
>
>> On Fri, Jan 12, 2024 at 12:37 PM Daniel Shahaf 
>> wrote:
>> ...
>> > Procedurally, the long hiatus is counterproductive.  Neither kfogel nor
>> > I had the context in our heads, and the cache misses took their toll in
>> > tuits and in wallclock time.  Furthermore, I have less spare time for
>> > dev@ discussions than I did when I cast the veto (= a year ago next
>> > Saturday).  Going forward it might be preferable for threads not to
>> > hibernate.
>>
>> I agree, but obviously the hibernation is not some deliberate action
>> by anyone. It's just that most of us here have less spare time for
>> dev@ discussions (and for SVN development) than before. Especially for
>> such complex matters, and especially when people feel there are
>> walking into a minefield. There are only a few active devs left, and
>> tuits are running low ...
>>
>> ...
>> > That being the case, I have considered whether merging the feature
>> > branch outweighs letting dev@ take a not-only-/pro forma/ role in
>> > design discussions.  I am of the opinion that it does not, and
>> > therefore I reäfirrm the veto.
>>
>> It has become more clear to me (I was only following tangentially)
>> that your veto is focused on the development methodology and the lack
>> of design discussion. Is that a valid reason for a veto? We are low on
>> resources, someone still finds time to make some progress, no one
>> blocks it on technical grounds, and then someone vetoes it because we
>> don't have enough resources?
>>
>> That puts us pretty much in deadlock, because we are too low on
>> resources. Or maybe I misunderstand?
>>
>> To be clear: I appreciate your input, Daniel, and your insistence on a
>> more thorough design discussion. I assume it's coming from a genuine
>> concern that we formulate problems well, and think hard about possible
>> solutions (focusing on the precise problem we are trying to solve).
>> But at the end of the day, if that design discussion doesn't happen
>> (or not enough to your satisfaction anyway), is that grounds for a
>> veto? For me it's a tough call, because on the one hand you have a
>> point, but on the other hand ... you're blocking _some_ progress
>> because the process behind it is not perfect (which is hard to do with
>> the 3.25 tuits we have left).
>>
>> > P.S.  Could that BRANCH-README please state what's the problem the
>> branch
>> > means to solve, i.e., the goal / acceptance test?  "Make it possible to
>> > «svn add» SHA-1 collisions"?
>>
>> I agree that would be a good step.
>>
>> I too find it a bit unclear what problem we're actually trying to
>> solve, apart from a vague feeling that SHA-1 will become more and more
>> broken over time, and that this will cause fatal injury to SVN (in its
>> WC, protocol, dump format, or repository). And perhaps the fact that
>> security auditors are becoming more and more triggered by seeing SHA-1
>> (even if they don't understand the way it is used and its
>> ramifications). Making it possible to 'svn add' SHA-1 collisions is
>> not it, I think.
>>
>> --
>> Johan
>>
>
>
> Johan's reply sums up my thoughts pretty closely.
>
> I would very much like to *avoid* all of the following: deadlock, bad
> feelings, and members of this small community leaving because of deadlocks
> or bad feelings.
>
> I agree that (at the very least), BRANCH-README should define what problem
> the branch aims to solve, and perhaps that's really the main thing we need
> to discuss and resolve.
>
> Johan touched on one issue with SHA1: regardless how it is actually used
> in SVN and whether it is adequate for those purposes, there is customer
> perception. I can imagine, for example, the IT dept of some big
> $corporation could blacklist SHA1 because it is considered broken for
> cryptographic purposes. But they could blacklist it for everything. Even
> though it is safe and effective for our use cases, try explaining that to
> an admin who is struggling to meet such a blanket policy.
>
> I would like to add another reason to think about a post-SHA1 future: I'm
> writing on mobile so I can't easily grep for things now, but could our
> dependencies eventually remove the SHA1 implementation? (I just saw
> something about removal of DSA from some famous lib not too long ago. SHA1
> could be next?)
>
> When would SHA1 disappear? I don't know, but I consider it plausible to
> happen in about 5 years.
>
> If SHA1 is removed in the future, there will need to be a mad dash to
> replace it. Or we'll have to add a new dependency to use an alternate
> implementation. Or we'll have to implement our own SHA1 or copy some code
> into SVN. All of these seem bad to me.
>
> Switching to a different hash is also a bad idea, I think, because it is

Re: mbox archives

2024-01-13 Thread Daniel Sahlberg
Den fre 12 jan. 2024 kl 12:02 skrev Daniel Shahaf :

> I think it should be clear to members of our community how to access our
> Collective Memory (= the list archives) in the preferred form for
> reading, to borrow ALv2's definition of "Source".  I would expect that
> to be self-evident from ponymail's UI, and failing that, documented on
> our /mailing-lists.html page.
>

How about the changes in r1915225? (See
https://subversion-staging.apache.org/mailing-lists.html#downloading

Kind regards,
Daniel Sahlberg


Re: Switching from SHA1 to a checksum type without known collisions in 1.15 working copy format

2024-01-13 Thread Daniel Sahlberg
Den lör 13 jan. 2024 kl 00:50 skrev Johan Corveleyn :

> On Fri, Jan 12, 2024 at 12:37 PM Daniel Shahaf 
> wrote:
> ...
> > Procedurally, the long hiatus is counterproductive.  Neither kfogel nor
> > I had the context in our heads, and the cache misses took their toll in
> > tuits and in wallclock time.  Furthermore, I have less spare time for
> > dev@ discussions than I did when I cast the veto (= a year ago next
> > Saturday).  Going forward it might be preferable for threads not to
> > hibernate.
>
> I agree, but obviously the hibernation is not some deliberate action
> by anyone. It's just that most of us here have less spare time for
> dev@ discussions (and for SVN development) than before. Especially for
> such complex matters, and especially when people feel there are
> walking into a minefield. There are only a few active devs left, and
> tuits are running low ...
>

I agree with Johan on this. The long hiatus is unfortunate. But it won't
help to point fingers at this point.



>
> ...
> > That being the case, I have considered whether merging the feature
> > branch outweighs letting dev@ take a not-only-/pro forma/ role in
> > design discussions.  I am of the opinion that it does not, and
> > therefore I reäfirrm the veto.
>
> It has become more clear to me (I was only following tangentially)
> that your veto is focused on the development methodology and the lack
> of design discussion. Is that a valid reason for a veto? We are low on
> resources, someone still finds time to make some progress, no one
> blocks it on technical grounds, and then someone vetoes it because we
> don't have enough resources?
>
> That puts us pretty much in deadlock, because we are too low on
> resources. Or maybe I misunderstand?
>
> To be clear: I appreciate your input, Daniel, and your insistence on a
> more thorough design discussion. I assume it's coming from a genuine
> concern that we formulate problems well, and think hard about possible
> solutions (focusing on the precise problem we are trying to solve).
> But at the end of the day, if that design discussion doesn't happen
> (or not enough to your satisfaction anyway), is that grounds for a
> veto? For me it's a tough call, because on the one hand you have a
> point, but on the other hand ... you're blocking _some_ progress
> because the process behind it is not perfect (which is hard to do with
> the 3.25 tuits we have left).
>
> > P.S.  Could that BRANCH-README please state what's the problem the branch
> > means to solve, i.e., the goal / acceptance test?  "Make it possible to
> > «svn add» SHA-1 collisions"?
>
> I agree that would be a good step.
>
> I too find it a bit unclear what problem we're actually trying to
> solve, apart from a vague feeling that SHA-1 will become more and more
> broken over time, and that this will cause fatal injury to SVN (in its
> WC, protocol, dump format, or repository). And perhaps the fact that
> security auditors are becoming more and more triggered by seeing SHA-1
> (even if they don't understand the way it is used and its
> ramifications). Making it possible to 'svn add' SHA-1 collisions is
> not it, I think.
>

I also agree with this.

>From what I remember of the dicsussions earlier there were concerns that a
changed file might go undetected if someone change it to another file with
a collision with the original file. I think that might be a vaild point,
especially if we don't have the pristine files anymore.

I'd also like to understand why we need the multi-checksum format instead
of just plainly switching to XXX (insert favourite checksuming algorithm
here). Does it help us to have multiple types of checksums available? Would
we use BOTH as a resort (likelyhood of collision in SHA1 and in XXX at the
same time approaching zero)? Does it help backwards/forwards compatibility?

Kind regards,
Daniel Sahlberg


Re: Changing the permission checks in libsvn_subr/io.c

2024-01-13 Thread Daniel Sahlberg
Den ons 10 jan. 2024 kl 10:45 skrev Stefan Sperling :

> On Wed, Jan 10, 2024 at 09:44:51AM +0100, Johan Corveleyn wrote:
> > Interesting discussion. I agree it should at least be documented, and
> > perhaps be made a bit more clear from the output of 'revert' (but not
> > sure how far we can go without breaking compat). Changing the current
> > behavior is probably a more risky move, given the maturity of SVN and
> > backwards compatibility etc.
>
> Adding new notification types should not cause compatibility concerns.
> If adding new notifications breaks other software which reads command
> line client output than this other software has a bug: It should have
> been ignoring unknown lines of output in the first place.
>
> API users would likewise simply need to catch the new notification type
> in a switch-like statement and should also be ignoring unknown values.
>
> I would only see a compatibility problem if an existing notification
> about an important event no longer appears.
>
> In the past we have made significant changes to output from commands,
> such as when tree conflict detection was added in 1.6. That by itself
> has not resulted in any problems, as far as I know.
>

Thanks for the feedback (and sorry for mixing up the threads previously
saying there was no feedback here).

I've made a separate commit r1915215 adding code to catch the inconsistency
with the auth bits and report a separate notification.

Kind regards,
Daniel


Re: Changing the permission checks in libsvn_subr/io.c

2024-01-13 Thread Daniel Sahlberg
Den fre 5 jan. 2024 kl 08:45 skrev Daniel Sahlberg <
daniel.l.sahlb...@gmail.com>:

> Hi,
>
> When researching the spurious revert messages reported by Vincent Lefevre
> [1], I was looking at the code in svn_io__is_finfo_read_only() and
> svn_io_is_file_executable(). It gets the current UID and GID using the APR
> function apr_uid_current() and then compares to see if the owner of the
> file is the current user (then check for user write/execute permissions) or
> if the group owning the file is the current user's group (then check for
> group write/execute permissions) or otherwise check for world write/execute
> permissions.
>
> I think there is a failure mode when a user has write permissions to a
> file through a secondary group, something like this:
>
> [[[
> dsg@daniel-23:~/svn_trunk$ groups
> dsg adm dialout cdrom floppy sudo audio dip video plugdev netdev docker
> dsg@daniel-23:~/svn_trunk$ ls -l README
> -rw-rw-r-- 1 root sudo 2341 Sep 22  2022 README
> dsg@daniel-23:~/svn_trunk$ svn proplist -v README
> Properties on 'README':
>   svn:eol-style
> native
>   svn:keywords
> LastChangedDate
> dsg@daniel-23:~/svn_trunk$ svn revert README
> Reverted 'README'
> dsg@daniel-23:~/svn_trunk$ ls -l README
> -rw-rw-r-- 1 root sudo 2341 Sep 22  2022 README
> dsg@daniel-23:~/svn_trunk$ svn revert README
> Reverted 'README'
> dsg@daniel-23:~/svn_trunk$ ./subversion/svn/svn revert README
> dsg@daniel-23:~/svn_trunk$ ls -l README
> -rw-rw-r-- 1 root sudo 2341 Sep 22  2022 README
> dsg@daniel-23:~/svn_trunk$
> ]]]
>
> My user dsg has the primary group dsg and, among others, belong to the
> sudo group.
> The README file is owned by root:sudo and has 664 permission. I'm thus
> able to write to the file.
> svn revert will report Reverted, since the file README isn't owned by the
> user DSG or the group DSG and thus svn_io__is_finfo_read_only() will return
> that the file is readonly. Since the file doesn't have svn:needs-lock it
> should be RW and the Reverted message comes from Subversion trying to
> restore the W flag (which fails, thus a second svn revert call will report
> the same message).
> I initially thought about rewriting svn_io__is_finfo_read_only() to look
> also for the user's secondary groups but when asking on dev@apr.a.o if
> there is an APR way of listing a users secondary groups, Joe Orton
> suggested [2] to instead check with access(2).
>
> I've run the testsuite on Ubuntu 23.10 and OpenBSD 7.4. Initially I had an
> error on special_tests.py symlink_destination_change() which contains a
> dangling symlink. It seems the old code was checking for W on the actual
> symlink, while access() is checking on the target (which doesn't exists and
> thus ENOENT). To solve this I added the test for ENOENT in
> svn_io__is_finfo_read_only(). The same test is not needed on
> svn_io_is_file_executable() since it will not be called for a symlink (see
> revert_wc_data()). I'm not sure if this is the right way to solve this
> problem or if a change should be done in revert_wc_data() also for the
> read_only check.
>
> [[[
> Index: subversion/libsvn_subr/io.c
> ===
> --- subversion/libsvn_subr/io.c (revision 1915064)
> +++ subversion/libsvn_subr/io.c (working copy)
> @@ -2535,7 +2535,14 @@ svn_io__is_finfo_read_only(svn_boolean_t *read_onl
>apr_uid_t uid;
>apr_gid_t gid;
>
> -  *read_only = FALSE;
> +  *read_only = (access(file_info->fname, W_OK) != 0);
> +  /* svn_io__is_finfo_read_only can be called with a dangling
> +   * symlink. access() will check the permission on the missing
> +   * target and return -1 and errno = ENOENT. Check for ENOENT
> +   * and pretend the file is writeable, otherwise we will get
> +   * spurious Reverted messages on the symlink.
> +   */
> +  if (*read_only && errno == ENOENT) *read_only = FALSE;
>
>apr_err = apr_uid_current(, , pool);
>
> @@ -2542,16 +2549,6 @@ svn_io__is_finfo_read_only(svn_boolean_t *read_onl
>if (apr_err)
>  return svn_error_wrap_apr(apr_err, _("Error getting UID of process"));
>
> -  /* Check write bit for current user. */
> -  if (apr_uid_compare(uid, file_info->user) == APR_SUCCESS)
> -*read_only = !(file_info->protection & APR_UWRITE);
> -
> -  else if (apr_gid_compare(gid, file_info->group) == APR_SUCCESS)
> -*read_only = !(file_info->protection & APR_GWRITE);
> -
> -  else
> -*read_only = !(file_info->protection & APR_WWRITE);
> -
>  #else  /* WIN32 || __OS2__ || !APR_HAS_USER */
>*read_only = (file_info->protection & APR_FREADONLY);
>  #endif
> @@ -2565,27 +2562,7 @@ svn_io__is_finfo_exec

Re: mbox archives

2024-01-08 Thread Daniel Sahlberg
Den mån 8 jan. 2024 kl 10:08 skrev Daniel Shahaf :

> How is an interested community member supposed to get this list's archives
> in mbox format?
>
> Those on svn.haxx.se can be obtained from there, but what about the
> others?  gmane is down, lists.a.o has a download feature that seems to
> require either downloading one month at a time manually or using browser
> debug tools, and the other online archives we link to don't have obvious
> links to download mbox (or Maildir or whatever else) archives.
>
> Reconstructing the old mod_mbox links, such as <
> https://mail-archives.apache.org/mod_mbox/subversion-dev/202312.mbox>,
> actually works, but how's a user supposed to know to do that?
>
> Cheers,
>
> Daniel
> (who's in the data gathering phase for /^k.{5}/g's recent query ;-))
>

Hi,

Does this work?

$ curl "
https://lists.apache.org/api/mbox.lua?list=dev=subversion.apache.org=2022-12;
-o dev_subversion_apache_org_2022-12.mbox

It seems to download an mbox file but I have not tried to verify its
contents. (And yes, I constructed that from data in the browser's debug
tools).

You might get better replies at us...@infra.apache.org or
us...@ponymail.apache.org.

Kind regards,
Daniel Sahlberg


Re: Changing the permission checks in libsvn_subr/io.c

2024-01-05 Thread Daniel Sahlberg
Den fre 5 jan. 2024 kl 10:51 skrev Johan Corveleyn :

> On Fri, Jan 5, 2024 at 8:46 AM Daniel Sahlberg
>  wrote:
> ...
> > Since the file doesn't have svn:needs-lock it should be RW [and the
> Reverted message comes from Subversion trying to restore the W flag ...]
>
> Should it? Intuitively I'd say: since the file doesn't have
> svn:needs-lock Subversion shouldn't be looking at R or RW. Why should
> we make a file RW? Can't the user make a file readonly just locally,
> and expect Subversion not to care?
>
> Or is "making a file readonly" a committable local change? Will it
> show up on 'svn st' and can it be committed as some change that can be
> transferred to another working copy?
>
> I understand that svn:needs-lock adds extra handling of the readonly
> status of files, but without that property?
>

All good questions, and I probably agree with you: if svn:needs-lock isn't
set then Subversion could just ignore the R/RW status. But any change here
would change previous behaviour so it would need a solid consensus.

If the check is removed for files that doesn't svn:needs-lock, then we
might have to add code to restore RW status if svn:needs-lock is removed.

Making a file readonly is currently not a committable change, didn't check
'svn st' but it will be reverted by 'svn revert' and it will not be
transferred to another WC. It can only be committed indirectly via
svn:needs-lock.

Any discussion regarding svn:needs-lock probably also have to consider
svn:executable, since it is handled similarly (except on WIN32 and OS2,
where the concept of executable doesn't exists).

I havn't completely made up my mind, but I think I favour keeping the
current behaviour: R/RW status in indicated by the svn:needs-lock property
and you shouldn't change R/RW manually within a WC.

/Daniel


Changing the permission checks in libsvn_subr/io.c

2024-01-04 Thread Daniel Sahlberg
se if (apr_gid_compare(gid, file_info->group) == APR_SUCCESS)
-*executable = (file_info->protection & APR_GEXECUTE);
-
-  else
-*executable = (file_info->protection & APR_WEXECUTE);
-
+  svn_io_is_file_executable(executable, file_info->fname, pool);
 #else  /* WIN32 || __OS2__ || !APR_HAS_USER */
   *executable = FALSE;
 #endif
@@ -2599,12 +2576,7 @@ svn_io_is_file_executable(svn_boolean_t *executabl
   apr_pool_t *pool)
 {
 #if defined(APR_HAS_USER) && !defined(WIN32) &&!defined(__OS2__)
-  apr_finfo_t file_info;
-
-  SVN_ERR(svn_io_stat(_info, path, APR_FINFO_PROT | APR_FINFO_OWNER,
-  pool));
-  SVN_ERR(svn_io__is_finfo_executable(executable, _info, pool));
-
+  *executable = (access(path, X_OK) == 0);
 #else  /* WIN32 || __OS2__ || !APR_HAS_USER */
   *executable = FALSE;
 #endif
]]]

At the moment I've kept the apr_uid_current() call in
svn_io__is_finfo_read_only(). My original plan was to extend
svn_io__is_finfo_read_only to report if a file is readonly but owned by
someone else (see [3]) but thinking about it again I think we should rather
handle this in io_set_perms() - at the moment I'm running out of time but I
will come back to this before final commit.

Since this isn't my area of expertise, I'd appreciate if someone could
review it before I commit.

Kind regards,
Daniel Sahlberg



[1] https://lists.apache.org/thread/p1ky889bxwy8okqly7h1lgckxfpldnxs
[2] https://lists.apache.org/thread/3pr76fo7gzbqq397d9xvyd66gq5bwlmw
[3] https://lists.apache.org/thread/sfkzqzgwr4wvvwfdcyhqgymhx6lmv24h


Re: [RESULT] Subversion 1.14.3 up for testing/signing

2023-12-28 Thread Daniel Sahlberg
Den tors 28 dec. 2023 kl 06:14 skrev Nathan Hartman <
hartman.nat...@gmail.com>:

> Summary of the vote to release Subversion 1.14.3:
>
> We have five +1 to release, no 0, and no -1.
>
> The five +1 to release are from:
> * kotkov (Windows) [1]
> * stsp (*nix) [2]
> * jcorvel (Windows) [3]
> * dsahlberg (*nix) [4]
> * hartmannathan (*nix) [5]
>
> Thank you to everyone who took time out of the holiday season to test
> this release, and for your encouragement and support throughout this
> process!
>
> I'll proceed with the next steps shortly...
>
> Cheers,
> Nathan
>

Congratulations Nathan for your first Subversion release and a big Thank
You for the effort!

Kind regards,
Daniel


> References:
> [1] https://lists.apache.org/thread/1j77sxsjfo4570ho08j104cddhq1pxhw
> [2] https://lists.apache.org/thread/l72oq14x27lqqg8gzff5jlwyky6k18vp
> [3] https://lists.apache.org/thread/vyjmc7nll0xx911pfn5onwnpw05cyy9s
> [4] https://lists.apache.org/thread/ogq4h4wf2wzhgd02wd0n03nw8b7q7cb0
> [5] https://lists.apache.org/thread/ftj0kkpc5bh7ykfog5kbz2kpno3hh5cs
>


Re: svn commit: r1914961 - /subversion/site/staging/docs/community-guide/releasing.part.html

2023-12-28 Thread Daniel Sahlberg
Den tors 28 dec. 2023 kl 06:41 skrev :

> Author: hartmannathan
> Date: Thu Dec 28 05:41:35 2023
> New Revision: 1914961
>
> URL: http://svn.apache.org/viewvc?rev=1914961=rev
> Log:
> In site/staging:
>
> * docs/community-guide/releasing.part.html:
>   (#releasing-upload): Fix typo.
>
> Modified:
> subversion/site/staging/docs/community-guide/releasing.part.html
>
> Modified: subversion/site/staging/docs/community-guide/releasing.part.html
> URL:
> http://svn.apache.org/viewvc/subversion/site/staging/docs/community-guide/releasing.part.html?rev=1914961=1914960=1914961=diff
>
> ==
> --- subversion/site/staging/docs/community-guide/releasing.part.html
> (original)
> +++ subversion/site/staging/docs/community-guide/releasing.part.html Thu
> Dec 28 05:41:35 2023
> @@ -1222,7 +1222,7 @@ This moves the tarballs, signatures and
>  >^/dev/subversion
>  to https://dist.apache.org/repos/dist/release/subversion;
>  >^/release/subversion.
> -It takes the mirrors up to 24 hours to get pick up the new release.
> +It takes the mirrors up to 24 hours to pick up the new release.
>

I'm not sure the 24 hours apply anymore. In 2021, ASF moved to a download
content delivery network (DLCDN), see [1], and I was told that mirrors
should pick up the release within 15 minutes. Maybe we can remove this
grace period?


>  The archive will also automatically pick up the release.  While
>  running move-to-dist will move the signature files, committers are
>  still able to commit new signatures to ^/release/subversion;
>
>
>
Kind regards,
Daniel


[1] https://lists.apache.org/thread/305dcdxpoxhpoyqj4d1sfvg39olfjv6g


Fix issue SVN-4622 (Was: Re: "svn revert -R ." outputs spurious Reverted messages)

2023-12-26 Thread Daniel Sahlberg
Hi,

Prompted by Vincent Lefevre's e-mail (below, see thread at [1]), I took at
look at SVN-4622, or at least the second half of it.

TLDR; svn status is verifying read-only status for each file (based on
svn:needs-lock). Under Windows there is a separate read-only flag so this
is easily done and fixed. Under *nix the code will check for the normal
permission bits, first on user (if current user is owner), then on group
(if current user's group is the owner group) and finally on world. A file
with 664 permissions not owned by the current user/group will be seen as
read-only and the code will try to fix this (failing with Permissions
denied but still reporting Reverted).

The permissions code is in svn_io__is_finfo_read_only().
The code trying to "fix" the permissions is in revert_wc_data().

One way to solve this would be to add a separate argument to
svn_io__is_finfo_read_only() signalling if the file is not owned by the
current user (directly or via a group). This could then be picked up by
revert_wc_data() to output a separate message "Incorrect
permissions/ownership on XX" instead of the "Reverted".

There are some additional uses of svn_io__is_finfo_read_onl() but they can
easily fixed. As far as I understand, this function is a private function
so it doesn't require a changed API.

What do you think?

Kind regards,
Daniel

[1] https://lists.apache.org/thread/p1ky889bxwy8okqly7h1lgckxfpldnxs


Den sön 24 dec. 2023 kl 01:35 skrev Vincent Lefevre :

> On 2023-12-23 22:44:51 +0100, Daniel Sahlberg wrote:
> > Thank you for your report and also for finding the root cause. I can
> > confirm this is the same on my machine. I'd like to take a closer look
> but
> > I'm not quite sure when I will find the time.
> >
> > This has been reported before:
> > https://issues.apache.org/jira/browse/SVN-4622
>
> Indeed. Not for the original bug report, but what is said in the
> comments.
>
> I'm wondering why svn wants to change the permissions, at least if
> there is nothing to revert for the concerned file. I would see this
> as a (separate, but related) bug. FYI, in my working copies, I set
> some files as read-only in order to make sure I won't modify them
> by mistake (even though there would be no data loss with a revert,
> except the timestamp). So I think that this is a bad behavior if
> "svn revert" changes the write permission.
>
> --
> Vincent Lefèvre  - Web: <https://www.vinc17.net/>
> 100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
> Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)
>


Re: "svn revert -R ." outputs spurious Reverted messages

2023-12-23 Thread Daniel Sahlberg
Den lör 23 dec. 2023 kl 04:10 skrev Vincent Lefevre :

> On 2023-12-23 03:49:04 +0100, Vincent Lefevre wrote:
> > In one of my working copies:
> >
> > qaa% svn st
> > qaa% svn revert -R .
> > Reverted 'etc/apache2/conf-available/javascript-common.conf'
> > Reverted 'etc/apache2/mods-available/dnssd.load'
> > Reverted 'etc/apache2/mods-available/dnssd.conf'
> > qaa% svn revert -R .
> > Reverted 'etc/apache2/mods-available/dnssd.load'
> > Reverted 'etc/apache2/mods-available/dnssd.conf'
> > Reverted 'etc/apache2/conf-available/javascript-common.conf'
> [...]
>
> I've eventually found the cause: these files has owner root.
> Perhaps a mistake I did when I copied the files (but I am
> wondering how I could have done that). I've now fixed the
> ownership and the messages have disappeared.
>
> However, I don't see why I get such Reverted messages in the case
> of an incorrect owner.
>
> --
> Vincent Lefèvre  - Web: <https://www.vinc17.net/>
> 100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
> Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)
>

Thank you for your report and also for finding the root cause. I can
confirm this is the same on my machine. I'd like to take a closer look but
I'm not quite sure when I will find the time.

This has been reported before:
https://issues.apache.org/jira/browse/SVN-4622

Kind regards,
Daniel Sahlberg


Re: Subversion 1.14.3 up for testing/signing

2023-12-23 Thread Daniel Sahlberg
Den lör 9 dec. 2023 kl 16:50 skrev Nathan Hartman :

> The 1.14.3 release artifacts are now available for testing/signing.
> Please get the tarballs from
>   https://dist.apache.org/repos/dist/dev/subversion
> and add your signatures there.
>
> Thanks!
>



Summary
---
+1 to release (Linux)

Platform

Ubuntu 23.11

Verified

Signature and sha512 for subversion-1.14.3.tar.gz and
subversion-1.14.3.tar.bz2.

Contents of files are identical to tags/1.14.3,
and to branches/1.14.x@1914484 (except for expected differences in
svn_version.h and svnpubsub, svnwcsub and nominate.pl (symlinks vs. file
contents), and generated files).

Tested
--
[ Release build ] x [ fsfs ] x [ file | svn | http ]
javahl
swig-python

Results
---
All tests pass.

Dependencies

httpd 2.4.57
apr 1.7.2
apr-util 1.6.3
openssl 3.0.10
serf 1.3.10
sqlite 3.42.0
zlib 1.2.13
python 3.11.6
py3c 1.4
swig 4.1.0
lz4 1.9.4-1
(bundled utf8proc 2.1.0)

Other tools
---
perl 5.36.0
python 3.11.6
OpenJDK 17.0.9
junit 4.13.2

Signature
-
subversion-1.14.3.tar.bz2
-BEGIN PGP SIGNATURE-

iQGzBAABCgAdFiEET/y1XA0Nk0PPtGEfKNtHMpz/3GMFAmWHR2oACgkQKNtHMpz/
3GMgggwAoerQHK4Kh4TtFiIXTlqr1BGZAvLjHyPgEJ/zzxrAh7OM7W28HDrT/Si5
DZoKdW74w459AH6VUeRhTUdDEYnziZMicnGLHSck1p56tzDb/2MBlqMmCRywU9Zi
r4SEBxcJK7wWIRho0a1BjdGnuZwGpXF1pzkQwmgxsg8PMpZqxp35RtAPA87YQaNu
KqDSJjOCeJn+ioXBSWEVhNmbTaaQtniXhPyGA41KBbctFwjUTPDMb8ZmqXtIesAj
kQD2dWKAyUPXuSJ9Y2O0OflIk7OBSDCMGnkaseaXiOXTDfD5GssYRKOhF9/jStNI
t5T58sZX7vLAX1pf8hzIxn3caIk3x41uP4AzS9khEl8sF0WC4oRmAe34NhFwJwfc
W46SqqaCt3AwHTEbTkaFtCEyl7MSDhptOSOfXywz3M4+B06X1fexSkGY8jVFpazU
8zPnNmr3xPBwSzC+E6nD3grSjnGU2s0vxL2MRK9vH5VwcKmoeHA7Hm8DHi7/Bs3G
zNp9WKBv
=piyH
-END PGP SIGNATURE-

subversion-1.14.3.tar.gz
-BEGIN PGP SIGNATURE-

iQGzBAABCgAdFiEET/y1XA0Nk0PPtGEfKNtHMpz/3GMFAmWHR2oACgkQKNtHMpz/
3GMRGAv/RH3r/qLbfmfz5Q8eT/zXVI4uPss8crRfyzcoVbkqXn6/A66b8JafcKRG
QFVWhB8LRZecPbyC8pXQZRsGb5PxqfjEjI44L+ZwH34Q0nUDMdZmab+aBM3QnYqY
pJ0xuAnvNKfd2tcyB3Wbi1ps4qI8U1itHtnKBrHSVpEy6bK83y2xEWd5vK5mG9GE
Ty0NKzJjhY10lCDnbKZEcB4eZEpZXwdPiEwm7i7FdO4AXTjZImsvPLrv4G0ScqAF
AUbRuemmB5iqnIdZeOsOrgsHk94IQpK2rQI/+jX477xxg9n8ndJ/kKwHSt8kSNlW
vp3WqpqEHYEu2rVkKPlm3L+AV3ai3cUhalp6AcZWxXeHhJnAKsCrZwMYRwSQ0vIu
04CwjKasMJeDZnNvh57o8GHBoeME9anuxxV7dA29fsEjVEQJppNod1QTfjzO74UR
xSEiFeBDesvtCd2ci0sK2CX34QstCoJh73c3SnlDKMnsFveWvz6HDug9UoAy+wW2
EhjRUgnh
=YmrU
-END PGP SIGNATURE-

Kind regards,
Daniel


Re: Subversion 1.14.3 up for testing/signing

2023-12-22 Thread Daniel Sahlberg
Den fre 22 dec. 2023 kl 10:04 skrev Johan Corveleyn :

> On Fri, Dec 22, 2023 at 3:07 AM Yasuhito FUTATSUKI 
> wrote:
> >
> > Hi,
> >
> > On 2023/12/22 10:49, Johan Corveleyn wrote:
> ...
> > > Finally, did some tweaks to get gen_win_dependencies.py to run, but
> > > then ran into following error:
> > > [[[
> > > python gen-make.py --release
> > > --with-swig=C:\research\swigwin-4.1.1 --with-py3c=C:\research\py3c-1.4
> > > --with-junit=C:\research\svn\dev\deps64\src\junit-4.13.2\junit.jar
> > > --with-jdk="C:\Program Files\Java\jdk-21" --with-httpd=C:\Apache2.4.58
> > > --with-serf=C:\research\svn\dev\deps64\src\serf-1.3.10
> > > --with-openssl=C:\research\svn\dev\deps64\src\openssl-3.0.12
> > >
> --with-sqlite=C:\research\svn\dev\deps64\src\sqlite-amalgamation-3.44.2.0
> > > --with-zlib=C:\research\svn\dev\deps64\src\zlib-1.3
> > > --vsnet-version=2019 -t vcproj 2>&1 | tee log.gen-make
> > >
> C:\research\svn\dev\subversion-1.14.3\build\generator\gen_win_dependencies.py:233:
> > > SyntaxWarning: invalid escape sequence '\.'
> > >   if val == '2002' or re.match('^7(\.\d+)?$', val):
> > >
> C:\research\svn\dev\subversion-1.14.3\build\generator\gen_win_dependencies.py:238:
> > > SyntaxWarning: invalid escape sequence '\.'
> > >   elif val == '2003' or re.match('^8(\.\d+)?$', val):
> > >
> C:\research\svn\dev\subversion-1.14.3\build\generator\gen_win_dependencies.py:243:
> > > SyntaxWarning: invalid escape sequence '\.'
> > >   elif val == '2005' or re.match('^9(\.\d+)?$', val):
> > >
> C:\research\svn\dev\subversion-1.14.3\build\generator\gen_win_dependencies.py:248:
> > > SyntaxWarning: invalid escape sequence '\.'
> > >   elif val == '2008' or re.match('^10(\.\d+)?$', val):
> > >
> C:\research\svn\dev\subversion-1.14.3\build\generator\gen_win_dependencies.py:283:
> > > SyntaxWarning: invalid escape sequence '\d'
> > >   elif re.match('^20\d+$', val):
> > >
> C:\research\svn\dev\subversion-1.14.3\build\generator\gen_win_dependencies.py:290:
> > > SyntaxWarning: invalid escape sequence '\d'
> > >   elif re.match('^1\d+$', val):
> > > C:\research\svn\dev\subversion-1.14.3\build\transform_sql.py:53:
> > > SyntaxWarning: invalid escape sequence '\('
> > >   re_statement = re.compile('-- *STMT_([A-Z_0-9]+)( +\(([^\)]*)\))?')
> > > Generating for Visual Studio 2019
> > > ]]]
> > >
> > > That is using Python 3.12.1.
> > >
> > > When downgrading to Python 3.9 those errors are gone. Just wanted to
> > > let you guys know ...
> >
> > Should we backport r1912632?
>
> Ah, thanks. I missed that. Thanks for pointing it out.
>
> Agreed that we should probably backport this to 1.14.x (but as Daniel
> said: not a blocker for this release).
>
> --
> Johan
>

Nominated as r1914846.

Kind regards,
Daniel


Re: Subversion 1.14.3 up for testing/signing

2023-12-21 Thread Daniel Sahlberg
Den fre 22 dec. 2023 kl 02:49 skrev Johan Corveleyn :

> On Tue, Dec 19, 2023 at 1:30 PM Johan Corveleyn  wrote:
> >
> > On Mon, Dec 18, 2023 at 11:13 PM Nathan Hartman
> >  wrote:
> > >
> > > On Sat, Dec 9, 2023 at 10:50 AM Nathan Hartman <
> hartmannat...@apache.org> wrote:
> > > >
> > > > The 1.14.3 release artifacts are now available for testing/signing.
> > > > Please get the tarballs from
> > > >   https://dist.apache.org/repos/dist/dev/subversion
> > > > and add your signatures there.
> > > >
> > > > Thanks!
> > >
> > >
> > > Hi all,
> > >
> > > Just to help plan ahead a little bit: Who else is planning to
> > > test/sign the 1.14.3 tarballs, and approximately how much time do you
> > > need?
> > >
> > > (If it's possible to be done with testing/signing toward the
> > > middle/end of this week, I will have a good opportunity to finish up
> > > the release work, but please let me know...)
> >
> > I still plan to go for it (on Windows 10). Not sure if middle of this
> > week (i.e. Wednesday night or so) is still realistic, but I'll try. No
> > guarantees though, so if you have enough sigs feel free to go ahead.
> >
> > I started with some initial steps last weekend, but got stranded along
> > the way. Basically I realized that OpenSSL 1.1.1 is totally EOL, and I
> > should really go for OpenSSL 3 now, which forces me to rebuild serf
> > and httpd (fortunately, others have already done work on serf 1.3.10
> > to make it work with OpenSSL 3, so I hope that will work out smoothly
> > -- as for httpd, that's always a big adventure on Windows). Oh and I'd
> > like the latest version of APR 1.7 as well, which includes a relevant
> > fix for Windows junctions / subst / something. So far, I've just
> > collected everything I need, but have not run a single buildscript
> > yet. To be continued ...
>
> Still working on it.
>
> Succesfully built zlib-1.3, openssl-3.0.12, apr-1.7.4, apr-util-1.6.3,
> httpd-2.4.58 (and pcre-8.45 and expat-2.2.9) and serf 1.3.10.
>
> Finally, did some tweaks to get gen_win_dependencies.py to run, but
> then ran into following error:
> [[[
> python gen-make.py --release
> --with-swig=C:\research\swigwin-4.1.1 --with-py3c=C:\research\py3c-1.4
> --with-junit=C:\research\svn\dev\deps64\src\junit-4.13.2\junit.jar
> --with-jdk="C:\Program Files\Java\jdk-21" --with-httpd=C:\Apache2.4.58
> --with-serf=C:\research\svn\dev\deps64\src\serf-1.3.10
> --with-openssl=C:\research\svn\dev\deps64\src\openssl-3.0.12
> --with-sqlite=C:\research\svn\dev\deps64\src\sqlite-amalgamation-3.44.2.0
> --with-zlib=C:\research\svn\dev\deps64\src\zlib-1.3
> --vsnet-version=2019 -t vcproj 2>&1 | tee log.gen-make
>
> C:\research\svn\dev\subversion-1.14.3\build\generator\gen_win_dependencies.py:233:
> SyntaxWarning: invalid escape sequence '\.'
>   if val == '2002' or re.match('^7(\.\d+)?$', val):
>
> C:\research\svn\dev\subversion-1.14.3\build\generator\gen_win_dependencies.py:238:
> SyntaxWarning: invalid escape sequence '\.'
>   elif val == '2003' or re.match('^8(\.\d+)?$', val):
>
> C:\research\svn\dev\subversion-1.14.3\build\generator\gen_win_dependencies.py:243:
> SyntaxWarning: invalid escape sequence '\.'
>   elif val == '2005' or re.match('^9(\.\d+)?$', val):
>
> C:\research\svn\dev\subversion-1.14.3\build\generator\gen_win_dependencies.py:248:
> SyntaxWarning: invalid escape sequence '\.'
>   elif val == '2008' or re.match('^10(\.\d+)?$', val):
>
> C:\research\svn\dev\subversion-1.14.3\build\generator\gen_win_dependencies.py:283:
> SyntaxWarning: invalid escape sequence '\d'
>   elif re.match('^20\d+$', val):
>
> C:\research\svn\dev\subversion-1.14.3\build\generator\gen_win_dependencies.py:290:
> SyntaxWarning: invalid escape sequence '\d'
>   elif re.match('^1\d+$', val):
> C:\research\svn\dev\subversion-1.14.3\build\transform_sql.py:53:
> SyntaxWarning: invalid escape sequence '\('
>   re_statement = re.compile('-- *STMT_([A-Z_0-9]+)( +\(([^\)]*)\))?')
> Generating for Visual Studio 2019
> ]]]
>
> That is using Python 3.12.1.
>
> When downgrading to Python 3.9 those errors are gone. Just wanted to
> let you guys know ...
>
> Then, regardless of the above error, when I start building with
> msbuild, I run into following error:
> [[[
>
> C:\research\svn\dev\deps64\src\sqlite-amalgamation-3.44.2.0\sqlite3.c(34597,42):
> error C4013: 'sqlite3PagerWalSystemErrno' undefined; assuming extern
> returning
>  int
> [C:\research\svn\dev\subversion-1.14.3\build\win32\vcnet-vcproj\libsvn_subr.vcxproj]
> ]]]
>

The declaration is guarded by:
#if defined(SQLITE_USE_SEH) && !defined(SQLITE_OMIT_WAL)

While the actual use seems to be guarded by
#ifdef SQLITE_USE_SEH

In our code we have
$ grep -r SQLITE_OMIT_WAL
subversion/libsvn_subr/sqlite3wrapper.c:#  define SQLITE_OMIT_WAL 1

I assume we trigger some bug in SQLite but I don't have time to dig into
the SQLite source code. It's not the first time we've hit errors with
OMIT_WAL


> I ran out of time for tonight, will continue tomorrow. If anybody
> knows what 

Re: Subversion 1.14.3 up for testing/signing

2023-12-21 Thread Daniel Sahlberg
Den fre 22 dec. 2023 kl 03:08 skrev Yasuhito FUTATSUKI :

> Hi,
>
> On 2023/12/22 10:49, Johan Corveleyn wrote:
> > On Tue, Dec 19, 2023 at 1:30 PM Johan Corveleyn 
> wrote:
> >>
> >> On Mon, Dec 18, 2023 at 11:13 PM Nathan Hartman
> >>  wrote:
> >>>
> >>> On Sat, Dec 9, 2023 at 10:50 AM Nathan Hartman <
> hartmannat...@apache.org> wrote:
> 
>  The 1.14.3 release artifacts are now available for testing/signing.
>  Please get the tarballs from
>    https://dist.apache.org/repos/dist/dev/subversion
>  and add your signatures there.
> 
>  Thanks!
> >>>
> >>>
> >>> Hi all,
> >>>
> >>> Just to help plan ahead a little bit: Who else is planning to
> >>> test/sign the 1.14.3 tarballs, and approximately how much time do you
> >>> need?
> >>>
> >>> (If it's possible to be done with testing/signing toward the
> >>> middle/end of this week, I will have a good opportunity to finish up
> >>> the release work, but please let me know...)
> >>
> >> I still plan to go for it (on Windows 10). Not sure if middle of this
> >> week (i.e. Wednesday night or so) is still realistic, but I'll try. No
> >> guarantees though, so if you have enough sigs feel free to go ahead.
> >>
> >> I started with some initial steps last weekend, but got stranded along
> >> the way. Basically I realized that OpenSSL 1.1.1 is totally EOL, and I
> >> should really go for OpenSSL 3 now, which forces me to rebuild serf
> >> and httpd (fortunately, others have already done work on serf 1.3.10
> >> to make it work with OpenSSL 3, so I hope that will work out smoothly
> >> -- as for httpd, that's always a big adventure on Windows). Oh and I'd
> >> like the latest version of APR 1.7 as well, which includes a relevant
> >> fix for Windows junctions / subst / something. So far, I've just
> >> collected everything I need, but have not run a single buildscript
> >> yet. To be continued ...
> >
> > Still working on it.
> >
> > Succesfully built zlib-1.3, openssl-3.0.12, apr-1.7.4, apr-util-1.6.3,
> > httpd-2.4.58 (and pcre-8.45 and expat-2.2.9) and serf 1.3.10.
> >
> > Finally, did some tweaks to get gen_win_dependencies.py to run, but
> > then ran into following error:
> > [[[
> > python gen-make.py --release
> > --with-swig=C:\research\swigwin-4.1.1 --with-py3c=C:\research\py3c-1.4
> > --with-junit=C:\research\svn\dev\deps64\src\junit-4.13.2\junit.jar
> > --with-jdk="C:\Program Files\Java\jdk-21" --with-httpd=C:\Apache2.4.58
> > --with-serf=C:\research\svn\dev\deps64\src\serf-1.3.10
> > --with-openssl=C:\research\svn\dev\deps64\src\openssl-3.0.12
> > --with-sqlite=C:\research\svn\dev\deps64\src\sqlite-amalgamation-3.44.2.0
> > --with-zlib=C:\research\svn\dev\deps64\src\zlib-1.3
> > --vsnet-version=2019 -t vcproj 2>&1 | tee log.gen-make
> >
> C:\research\svn\dev\subversion-1.14.3\build\generator\gen_win_dependencies.py:233:
> > SyntaxWarning: invalid escape sequence '\.'
> >   if val == '2002' or re.match('^7(\.\d+)?$', val):
> >
> C:\research\svn\dev\subversion-1.14.3\build\generator\gen_win_dependencies.py:238:
> > SyntaxWarning: invalid escape sequence '\.'
> >   elif val == '2003' or re.match('^8(\.\d+)?$', val):
> >
> C:\research\svn\dev\subversion-1.14.3\build\generator\gen_win_dependencies.py:243:
> > SyntaxWarning: invalid escape sequence '\.'
> >   elif val == '2005' or re.match('^9(\.\d+)?$', val):
> >
> C:\research\svn\dev\subversion-1.14.3\build\generator\gen_win_dependencies.py:248:
> > SyntaxWarning: invalid escape sequence '\.'
> >   elif val == '2008' or re.match('^10(\.\d+)?$', val):
> >
> C:\research\svn\dev\subversion-1.14.3\build\generator\gen_win_dependencies.py:283:
> > SyntaxWarning: invalid escape sequence '\d'
> >   elif re.match('^20\d+$', val):
> >
> C:\research\svn\dev\subversion-1.14.3\build\generator\gen_win_dependencies.py:290:
> > SyntaxWarning: invalid escape sequence '\d'
> >   elif re.match('^1\d+$', val):
> > C:\research\svn\dev\subversion-1.14.3\build\transform_sql.py:53:
> > SyntaxWarning: invalid escape sequence '\('
> >   re_statement = re.compile('-- *STMT_([A-Z_0-9]+)( +\(([^\)]*)\))?')
> > Generating for Visual Studio 2019
> > ]]]
> >
> > That is using Python 3.12.1.
> >
> > When downgrading to Python 3.9 those errors are gone. Just wanted to
> > let you guys know ...
>
> Should we backport r1912632?
>

Yes, it is probably a good idea to nominate it, but I don't think we need
to hold off the release for this. It is only a warning at the moment
(although it will be an error in a later Python release, see the relase
notes for Python 3.12).


> Cheers,
> --
> Yasuhito FUTATSUKI /
>

 Kind regards,
Daniel


Re: Subversion 1.14.3 up for testing/signing

2023-12-19 Thread Daniel Sahlberg
Den tis 19 dec. 2023 kl 13:31 skrev Johan Corveleyn :

> On Mon, Dec 18, 2023 at 11:13 PM Nathan Hartman
>  wrote:
> >
> > On Sat, Dec 9, 2023 at 10:50 AM Nathan Hartman 
> wrote:
> > >
> > > The 1.14.3 release artifacts are now available for testing/signing.
> > > Please get the tarballs from
> > >   https://dist.apache.org/repos/dist/dev/subversion
> > > and add your signatures there.
> > >
> > > Thanks!
> >
> >
> > Hi all,
> >
> > Just to help plan ahead a little bit: Who else is planning to
> > test/sign the 1.14.3 tarballs, and approximately how much time do you
> > need?
> >
> > (If it's possible to be done with testing/signing toward the
> > middle/end of this week, I will have a good opportunity to finish up
> > the release work, but please let me know...)
>
> I still plan to go for it (on Windows 10). Not sure if middle of this
> week (i.e. Wednesday night or so) is still realistic, but I'll try. No
> guarantees though, so if you have enough sigs feel free to go ahead.
>
> I started with some initial steps last weekend, but got stranded along
> the way. Basically I realized that OpenSSL 1.1.1 is totally EOL, and I
> should really go for OpenSSL 3 now, which forces me to rebuild serf
> and httpd (fortunately, others have already done work on serf 1.3.10
> to make it work with OpenSSL 3, so I hope that will work out smoothly
> -- as for httpd, that's always a big adventure on Windows). Oh and I'd
> like the latest version of APR 1.7 as well, which includes a relevant
> fix for Windows junctions / subst / something. So far, I've just
> collected everything I need, but have not run a single buildscript
> yet. To be continued ...
>
> --
> Johan
>

I've also been working on the Windows build. Tried OpenSSL 3.1 but that
build failed with some error from the OpenSSL test suite. OpenSSL 3.0
seemed to work but I run out of time and will try to pick up tonight.

/Daniel


Re: Backport bot not running?

2023-12-18 Thread Daniel Sahlberg
tis 19 dec. 2023 kl. 02:14 skrev Daniel Shahaf :

> Daniel Sahlberg wrote on Mon, 18 Dec 2023 10:44 +00:00:
> > Den mån 18 dec. 2023 kl 09:40 skrev Daniel Shahaf <
> d...@daniel.shahaf.name>:
> >> To prevent recurrence, options include (1) make the cron job use the .py
> >> implementation; (2) add a regression test to backport_tests.py [sic] and
> >> then fix backport.pl's parsing.
> >>
> >> Glad to see backport.py being used :-)
> >>
> >
> > I'm considering to switch to backport.py after the release of 1.14.5
> > (should be discussed on dev@ first of course) for the reasons mentioned
> in
> > the readme: "written in a language that many more active developers are
> > comfortable with"). If we could add the missing functions (Reviewing
> STATUS
> > and Adding new entries) or decide that those ore not needed anymore we
> > could then remove backport.pl and have one way of doing stuff.
>
> And even if we don't implement F3 and F4 in backport.py, deploying the
> already-existing backport.py implementation of F1 in production would
> still be a step in the right direction.
>
> It's not clear to me whether F2 runs in production currently or not.


It still runs

https://ci2.apache.org/#/builders/109

(Well… not for 1.9.x obviously)


>
> Cheers,
>
> Daniel
>
> [Terms from backport.py:
> > F1. Auto-merge bot; the nightly svn-role commits.
> > F2. Conflicts detector bot; the svn-backport-conflicts-1.9.x buildbot
> task.
> > F3. Reviewing STATUS nominations and casting votes.
> > F4. Adding new entries to STATUS.
> ]
>


Re: Backport bot not running?

2023-12-18 Thread Daniel Sahlberg
Den mån 18 dec. 2023 kl 09:40 skrev Daniel Shahaf :

> Daniel Sahlberg wrote on Thu, 30 Nov 2023 07:00 +00:00:
> > Den ons 29 nov. 2023 kl 17:25 skrev Nathan Hartman <
> hartman.nat...@gmail.com
> >>:
> >
> >> On Wed, Nov 29, 2023 at 8:40 AM Daniel Sahlberg
> >>  wrote:
> >> >
> >> > Den ons 29 nov. 2023 kl 06:55 skrev Daniel Sahlberg <
> >> daniel.l.sahlb...@gmail.com>:
> >> >>
> >> >>
> >> >> ons 29 nov. 2023 kl. 05:57 skrev Nathan Hartman <
> >> hartman.nat...@gmail.com>:
> >> >>>
> >> >>> The backport bot (svn-role) normally runs nightly but the most
> recent
> >> >>> backport approval has been waiting in 1.14.x/STATUS for a couple of
> >> >>> days now.
> >> >>>
> >> >>> I went ahead and merged it manually (with
> >> >>> tools/dist/merge-approved-backports.py). This did the right thing,
> so
> >> >>> I assume there wasn't any syntax error in STATUS.
> >> >>>
>
> That was r1914201, I take it.
>

Correct



>
> >> >>> I don't have access to svn-qavm1 so I can't check why it didn't
> happen
> >> >>> automatically. Maybe someone with access could check if the machine
> is
> >> >>> at least running...
> >> >>>
> >> >>> Thanks,
> >> >>> Nathan
> >> >>
> >> >>
> >> >> I’ll check later today
> >> >>
> >> >
> >> > Now I've spent some time looking.
> >> >
> >> > The backports is a cron job running at 04.00 UTC so it isn't really a
> >> bot that is running in the background. As far as I could see it was
> started
> >> successfully every day for the last week, but there were no real logs
> >> around what happened. It SHOULD have succeeded as far as I can tell.
> >> >
> >> > One difference is that the backport "bot" is using backport.pl
> instead
> >> of the Python backport implementation. Don't know if there was a subtle
> >> difference in STATUS that caused backport.pl to barf while packport.py
> >> succeeded.
> >> >
> >> > Lets keep our eyes open for the next backport.
> >> >
> >> > Kind regards,
> >> > Daniel
> >>
> >>
> >> Thanks for checking!
> >>
> >> Based on when upcoming.part.html was last updated, I assume
> >> site/tools/upcoming.py is run by another cron job at 04.15 UTC; it
> >> looks like I manually merged the backport a little bit after it ran
> >> last night, so I'll watch to see if it shows up in upcoming.part.html
> >> tonight...
> >>
> >> Thanks again,
> >> Nathan
> >>
> >
> > Upcoming worked well tonight so I guess there might been something in the
> > STATUS file that prevented automated backport. If it fails again tonight
> > (with the new nominations), I'd like to check running packport.pl
> manually.
>
> It was probably the «*» at the start of line 2.
>

That might be the thing, yes. Thanks for the explaination.


>
> To prevent recurrence, options include (1) make the cron job use the .py
> implementation; (2) add a regression test to backport_tests.py [sic] and
> then fix backport.pl's parsing.
>
> Glad to see backport.py being used :-)
>
> Daniel
>

I'm considering to switch to backport.py after the release of 1.14.5
(should be discussed on dev@ first of course) for the reasons mentioned in
the readme: "written in a language that many more active developers are
comfortable with"). If we could add the missing functions (Reviewing STATUS
and Adding new entries) or decide that those ore not needed anymore we
could then remove backport.pl and have one way of doing stuff.

Kind regards,
Daniel


Re: svn commit: r1914679 - /subversion/trunk/tools/hook-scripts/mailer/mailer.py

2023-12-15 Thread Daniel Sahlberg
Den fre 15 dec. 2023 kl 10:45 skrev :

> Author: gstein
> Date: Fri Dec 15 09:44:03 2023
> New Revision: 1914679
>
> URL: http://svn.apache.org/viewvc?rev=1914679=rev
> Log:
> class DifflibDiffContent does not work, and maybe never did. There is
> no .next() method on the unified_diff() result object. While it would
> be possible to use the next() function, it is better to just remove
> this entirely and require the host OS to have a diff executable (not a
> hard requirement).
>

I don't completely understand the technical details, but the Swig bindings
([1], lines 201 and below) use svn.diff.file_diff_2 to create a diff.

Would it make sense to use this instead of a hard requirement on a diff
executable?

Kind regards,
Daniel


[1] subversion/bindings/swig/python/svn/fs.py


>
> * tools/hook-scripts/mailer/mailer.py:
>   (DiffGenerator.__getitem__): remove OSError exception, as the diff
> executable must be present.
>   (class DifflibDiffContent): removed.
>
> Modified:
> subversion/trunk/tools/hook-scripts/mailer/mailer.py
>
> Modified: subversion/trunk/tools/hook-scripts/mailer/mailer.py
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/tools/hook-scripts/mailer/mailer.py?rev=1914679=1914678=1914679=diff
>
> ==
> --- subversion/trunk/tools/hook-scripts/mailer/mailer.py (original)
> +++ subversion/trunk/tools/hook-scripts/mailer/mailer.py Fri Dec 15
> 09:44:03 2023
> @@ -1016,17 +1016,13 @@ class DiffGenerator:
>  if binary:
>content = src_fname = dst_fname = None
>  else:
> -  src_fname, dst_fname = diff.get_files()
> -  try:
> +src_fname, dst_fname = diff.get_files()
>  content = DiffContent(self.cfg.get_diff_cmd(self.group, {
>'label_from' : label1,
>'label_to' : label2,
>'from' : src_fname,
>'to' : dst_fname,
>}))
> -  except OSError:
> -# diff command does not exist, try difflib.unified_diff()
> -content = DifflibDiffContent(label1, label2, src_fname,
> dst_fname)
>
># return a data item for this diff
>return _data(
> @@ -1101,36 +1097,6 @@ class DiffContent:
>raise IndexError
>
>  line, ltype, self.seen_change = _classify_diff_line(line,
> self.seen_change)
> -return _data(
> -  raw=line,
> -  text=line[1:-1],  # remove indicator and newline
> -  type=ltype,
> -  )
> -
> -
> -class DifflibDiffContent():
> -  "This is a generator-like object returning annotated lines of a diff."
> -
> -  def __init__(self, label_from, label_to, from_file, to_file):
> -import difflib
> -self.seen_change = False
> -fromlines = open(from_file, 'U').readlines()
> -tolines = open(to_file, 'U').readlines()
> -self.diff = difflib.unified_diff(fromlines, tolines,
> - label_from, label_to)
> -
> -  def __nonzero__(self):
> -# we always have some items
> -return True
> -
> -  def __getitem__(self, idx):
> -
> -try:
> -  line = self.diff.next()
> -except StopIteration:
> -  raise IndexError
> -
> -line, ltype, self.seen_change = _classify_diff_line(line,
> self.seen_change)
>  return _data(
>raw=line,
>text=line[1:-1],  # remove indicator and newline
>
>
>


Re: Python 3 compatibility issue in tools/hook-scripts/mailer/mailer.py

2023-12-08 Thread Daniel Sahlberg
Hi,

Den fre 8 dec. 2023 kl 05:40 skrev Yasuhito FUTATSUKI <
futat...@yf.bsdclub.org>:

>
>
> On 2023/12/07 19:33, Daniel Sahlberg wrote:
> > Den tors 7 dec. 2023 kl 09:51 skrev Ruediger Pluem :
> >
> >> I stumbled accross a Python 3 compatibility issue in
> >> tools/hook-scripts/mailer/mailer.py.
> >>
> >> The call of self.cfg.which_groups in line 565 passes an empty string as
> >> first parameter.
> >> In which_groups this empty string is passed to to_str in line 1489.
> >> In line 88 to_str does x.decode('utf-8')
> >> In Python 2 str objects have a decode method at least in later Python 3
> >> versions they have not. Hence I propose the following patch which fixes
> the
> >> issue for me:
> 
>
> > There was some work done on mailer.py by Greg Stein, started here:
> > https://lists.apache.org/thread/v5bh1j8qz8kk0jv1tlctxqt1k454tz0h
> > However I don't think that touched these parts of the code.
>
> Yes, I also think the issue has existed before it.
>
> > I think there is more to this than that simple fix, but I'm no expert in
> > Python or in the Python bindings. There are two more calls to
> > which_groups():
> >
> > Line 491, called with the return from
> > svn.repos.ChangeCollector(...).get_changes().items() (first argument in a
> > tuple). I don't know which encoding this uses.
>
> We decided that for all C APIs which returns char * values, their Python 3
> wrapper functions return them as bytes object, and also we don't convert
> those values into str within helper functions in svn modules. Thus their
> path elements should be bytes object, and as they are internal paths,
> their encoding is UTF-8, regardless of locale.
>
> > Line 663, called with the return from sys.stdin.buffer.readlines(),
> already
> > processed by to_str() once.
> >
> > In particular the call on 663 should be double decoded unless I'm missing
> > something.
>
> Yes, there is inconsistency here.
>
> > So depending on what get_changes() return, maybe we should remove the
> > to_str() from which_groups() instead?
>
> As Config.which_group() call from Commit.__init__() has worked correctly,
> the pathes as matching pattern in which_group() are expected as str.
> So if remove to_str() from which_groups(), path argment for
> Config.which_group() should be a str object.
>
> > Note that we are still somewhat supporting Python 2 so we need to make
> sure
> > any changes does work on Python 2 as well.
>
> Then, how about this patch? It at least mailer-t1.sh passed both
> on python=python2.7 and on python=python3.9.
>
> [[[
> Fix inconsistency in path argment on Config.which_group()
>
> Previously, we call Config.which_group() with path as bytes in
> Commit.__init__() and with path as str in Lock.__init__() and
> in PropChange.__init__(), but Config.which_group handled path
> argment as bytes. To fix it we only use str as path argment on
> Config.wich_group().
>
> * tools/hook-scripts/mailer/mailer.py
>   (Config.which_groups): Treat path as str.
>   (Commit.__init__): convert bytes path into str before calling above.
>
> Found by: Ruediger Pluem (rpluem {_AT_} apache.org)
>
> Index: tools/hook-scripts/mailer/mailer.py
> ===
> --- tools/hook-scripts/mailer/mailer.py (revision 1913728)
> +++ tools/hook-scripts/mailer/mailer.py (working copy)
> @@ -488,7 +488,7 @@
>  # collect the set of groups and the unique sets of params for the
> options
>  self.groups = { }
>  for path, change in self.changelist:
> -  for (group, params) in self.cfg.which_groups(path, log):
> +  for (group, params) in self.cfg.which_groups(to_str(path), log):
>  # turn the params into a hashable object and stash it away
>  param_list = sorted(params.items())
>  # collect the set of paths belonging to this group
> @@ -1486,9 +1486,9 @@
>  "Return the path's associated groups."
>  groups = []
>  for group, pattern, exclude_pattern, repos_params, search_logmsg_re
> in self._group_re:
> -  match = pattern.match(to_str(path))
> +  match = pattern.match(path)
>if match:
> -if exclude_pattern and exclude_pattern.match(to_str(path)):
> +if exclude_pattern and exclude_pattern.match(path):
>continue
>  params = repos_params.copy()
>  params.update(match.groupdict())
> ]]]
>
> Cheers,
> --
> Yasuhito FUTATSUKI 
>

This looks good to me! Thanks for the detailed explaination!

Kind regards,
Daniel Sahlberg


Re: Python 3 compatibility issue in tools/hook-scripts/mailer/mailer.py

2023-12-07 Thread Daniel Sahlberg
Den tors 7 dec. 2023 kl 09:51 skrev Ruediger Pluem :

> I stumbled accross a Python 3 compatibility issue in
> tools/hook-scripts/mailer/mailer.py.
>
> The call of self.cfg.which_groups in line 565 passes an empty string as
> first parameter.
> In which_groups this empty string is passed to to_str in line 1489.
> In line 88 to_str does x.decode('utf-8')
> In Python 2 str objects have a decode method at least in later Python 3
> versions they have not. Hence I propose the following patch which fixes the
> issue for me:
>
> Index: mailer.py
> ===
> --- mailer.py   (revision 1914422)
> +++ mailer.py   (working copy)
> @@ -562,7 +562,7 @@
>
>  # collect the set of groups and the unique sets of params for the
> options
>  self.groups = { }
> -for (group, params) in self.cfg.which_groups('', None):
> +for (group, params) in self.cfg.which_groups(b'', None):
># turn the params into a hashable object and stash it away
>param_list = sorted(params.items())
>self.groups[group, tuple(param_list)] = params
>
>
> If I would get a go ahead here on list I would commit.
>
> Regards
>
> Rüdiger
>

There was some work done on mailer.py by Greg Stein, started here:
https://lists.apache.org/thread/v5bh1j8qz8kk0jv1tlctxqt1k454tz0h
However I don't think that touched these parts of the code.

I think there is more to this than that simple fix, but I'm no expert in
Python or in the Python bindings. There are two more calls to
which_groups():

Line 491, called with the return from
svn.repos.ChangeCollector(...).get_changes().items() (first argument in a
tuple). I don't know which encoding this uses.

Line 663, called with the return from sys.stdin.buffer.readlines(), already
processed by to_str() once.

In particular the call on 663 should be double decoded unless I'm missing
something.

So depending on what get_changes() return, maybe we should remove the
to_str() from which_groups() instead?

Note that we are still somewhat supporting Python 2 so we need to make sure
any changes does work on Python 2 as well.

Kind regards,
Daniel


Re: 1.14.3 release planning

2023-12-05 Thread Daniel Sahlberg
Hi

Sounds great! Good work!

I’m working on preparations for signing on my end, will try to get
everything going on Windows and Ubuntu 23.10 this time.

Kind regards
Daniel
Daniel

mån 4 dec. 2023 kl. 18:34 skrev Nathan Hartman :

> Hi all,
>
> Just a heads up, I plan to roll tarballs this week.
>
> Hopefully all goes well and we'll be able to make the release soon...
>
> Cheers,
> Nathan
>


Re: Backport bot not running?

2023-11-29 Thread Daniel Sahlberg
Den ons 29 nov. 2023 kl 17:25 skrev Nathan Hartman :

> On Wed, Nov 29, 2023 at 8:40 AM Daniel Sahlberg
>  wrote:
> >
> > Den ons 29 nov. 2023 kl 06:55 skrev Daniel Sahlberg <
> daniel.l.sahlb...@gmail.com>:
> >>
> >>
> >> ons 29 nov. 2023 kl. 05:57 skrev Nathan Hartman <
> hartman.nat...@gmail.com>:
> >>>
> >>> The backport bot (svn-role) normally runs nightly but the most recent
> >>> backport approval has been waiting in 1.14.x/STATUS for a couple of
> >>> days now.
> >>>
> >>> I went ahead and merged it manually (with
> >>> tools/dist/merge-approved-backports.py). This did the right thing, so
> >>> I assume there wasn't any syntax error in STATUS.
> >>>
> >>> I don't have access to svn-qavm1 so I can't check why it didn't happen
> >>> automatically. Maybe someone with access could check if the machine is
> >>> at least running...
> >>>
> >>> Thanks,
> >>> Nathan
> >>
> >>
> >> I’ll check later today
> >>
> >
> > Now I've spent some time looking.
> >
> > The backports is a cron job running at 04.00 UTC so it isn't really a
> bot that is running in the background. As far as I could see it was started
> successfully every day for the last week, but there were no real logs
> around what happened. It SHOULD have succeeded as far as I can tell.
> >
> > One difference is that the backport "bot" is using backport.pl instead
> of the Python backport implementation. Don't know if there was a subtle
> difference in STATUS that caused backport.pl to barf while packport.py
> succeeded.
> >
> > Lets keep our eyes open for the next backport.
> >
> > Kind regards,
> > Daniel
>
>
> Thanks for checking!
>
> Based on when upcoming.part.html was last updated, I assume
> site/tools/upcoming.py is run by another cron job at 04.15 UTC; it
> looks like I manually merged the backport a little bit after it ran
> last night, so I'll watch to see if it shows up in upcoming.part.html
> tonight...
>
> Thanks again,
> Nathan
>

Upcoming worked well tonight so I guess there might been something in the
STATUS file that prevented automated backport. If it fails again tonight
(with the new nominations), I'd like to check running packport.pl manually.

Kind regards,
Daniel


Re: svn commit: r1914221 - /subversion/branches/1.14.x/STATUS

2023-11-29 Thread Daniel Sahlberg
Den tors 30 nov. 2023 kl 05:57 skrev Nathan Hartman <
hartman.nat...@gmail.com>:

> On Wed, Nov 29, 2023 at 11:52 PM  wrote:
> >
> > Author: hartmannathan
> > Date: Thu Nov 30 04:52:20 2023
> > New Revision: 1914221
> >
> > URL: http://svn.apache.org/viewvc?rev=1914221=rev
> > Log:
> > * STATUS: Nominate r1914220.
> >
> > Modified:
> > subversion/branches/1.14.x/STATUS
> >
> > Modified: subversion/branches/1.14.x/STATUS
> > URL:
> http://svn.apache.org/viewvc/subversion/branches/1.14.x/STATUS?rev=1914221=1914220=1914221=diff
> >
> ==
> > --- subversion/branches/1.14.x/STATUS (original)
> > +++ subversion/branches/1.14.x/STATUS Thu Nov 30 04:52:20 2023
> > @@ -22,6 +22,13 @@ Candidate changes:
> > votes:
> >   +1: rhuijben
> >
> > + * r1914220
> > +   Update copyright year to 2023.
> > +   Justification:
> > + To show correct year in upcoming release.
> > +   Votes:
> > + +1: hartmannathan
>
>
> I don't know whether this really needs a nomination and vote -- I
> looked for precedents and found that sometimes the year bump was
> committed directly on a A.B.x branch, sometimes committed to trunk and
> merged without a nomination/vote to a A.B.x branch, and sometimes done
> via a vote... So I went ahead and took the most conservative approach,
> but if it's overkill, please let me know for the future.
>
> Thanks,
> Nathan
>

It doesn't hurt I guess. I'm +1 for changing the year ;-) so I went ahead
and approved. Not sure if it would have needed three +1 but I don't think
anyone can veto...

/Daniel


Re: New release

2023-11-29 Thread Daniel Sahlberg
Great work! :-)

/Daniel

Den tors 30 nov. 2023 kl 06:21 skrev Nathan Hartman <
hartman.nat...@gmail.com>:

> I have kept myself busy studying the information in HACKING [1], in
> Mark's SVN Release Management repo [2], in tools/dist/release.py, etc.
>
> A quick test run with release.py seems to work (mostly) correctly. It
> successfully installs the needed Autoconf, Libtool, and SWIG. It is
> able to generate tarballs and Windows zips (though I haven't tested
> these at all; they're just throwaways right now).
>
> There were a few warnings while rolling these throwaways:
>
> * Copyright year is stale; I committed r1914220 and r1914221 to address
>   that.
>
> * CHANGES has unmerged revisions; that's expected because I haven't
>   updated CHANGES yet. That's the next task on my list.
>
> * It complained that COMMITTERS file is not found, therefore
>   make-keys.sh failed. I remember seeing something about this in Mark's
>   svnrm repo notes. Need to revisit that...
>
> Also a couple of questions:
>
> Do we notify translators before a patch release? (i.e., should I send
> out an email as described in #notify-translators [3]?)
>
> I need to guesstimate the release date. If I can get to the point of
> rolling tarballs within the next few days, how much time will be needed
> by those who plan to test?
>
> Thanks,
> Nathan
>
> [1]
> https://subversion.apache.org/docs/community-guide/community-guide.html#release-creating
>
> [2] https://github.com/markphip/svnrm
>
> [3]
> https://subversion.apache.org/docs/community-guide/community-guide.html#notify-translators
>


Re: Is there a write opposite to "svn cat"?

2023-11-29 Thread Daniel Sahlberg
Den mån 27 nov. 2023 kl 20:29 skrev Daniel Sahlberg <
daniel.l.sahlb...@gmail.com>:
[...]

> I looked into apr/misc/unix/getopt.c to see if there was any way of making
> "-" a legal argument but I couldn't find anything. Thus I'm suggesting
> making the help text a little bit more clear:
>
> [[[
> Index: subversion/svnmucc/svnmucc.c
> ===
> --- subversion/svnmucc/svnmucc.c(revision 1914148)
> +++ subversion/svnmucc/svnmucc.c(working copy)
> @@ -286,7 +286,9 @@
>"  mv SRC-URL DST-URL : move SRC-URL to DST-URL\n"
>"  rm URL : delete URL\n"
>"  put SRC-FILE URL   : add or modify file URL with contents
> copied from\n"
> -  "   SRC-FILE (use \"-\" to read from
> standard input)\n"
> +  "   SRC-FILE (use \"-\" to read from
> standard input,\n"
> +  "   but only if preceeded by -- and in
> which case no\n"
> +  "   options can follow. \"-\" only makes
> sense once)\n"
>"  propset NAME VALUE URL : set property NAME on URL to VALUE\n"
>"  propsetf NAME FILE URL : set property NAME on URL to value read
> from FILE\n"
>"  propdel NAME URL   : delete property NAME from URL\n"
> ]]]
>
> The "\"-\" only makes sense once" is because stdin can't be rewinded and
> thus can't be put into more than once file. If several put actions (with -
> as SRC-FILE) are used, the second file will be empty.
>
> Does this make sense (in particular to our native english speakers)?
>

Nathan suggested a slightly simpler way of wording the help message in [1].
I've committed that exact help text in r1914222 and nominated for backport
in 1914223.

Kind regards,
Daniel

[1] https://lists.apache.org/thread/xrvwfdgqmjzqhv3q5sv1jhbcry8cdsly


Re: Backport bot not running?

2023-11-29 Thread Daniel Sahlberg
Den ons 29 nov. 2023 kl 06:55 skrev Daniel Sahlberg <
daniel.l.sahlb...@gmail.com>:

>
> ons 29 nov. 2023 kl. 05:57 skrev Nathan Hartman  >:
>
>> The backport bot (svn-role) normally runs nightly but the most recent
>> backport approval has been waiting in 1.14.x/STATUS for a couple of
>> days now.
>>
>> I went ahead and merged it manually (with
>> tools/dist/merge-approved-backports.py). This did the right thing, so
>> I assume there wasn't any syntax error in STATUS.
>>
>> I don't have access to svn-qavm1 so I can't check why it didn't happen
>> automatically. Maybe someone with access could check if the machine is
>> at least running...
>>
>> Thanks,
>> Nathan
>
>
> I’ll check later today
>
>
Now I've spent some time looking.

The backports is a cron job running at 04.00 UTC so it isn't really a bot
that is running in the background. As far as I could see it was started
successfully every day for the last week, but there were no real logs
around what happened. It SHOULD have succeeded as far as I can tell.

One difference is that the backport "bot" is using backport.pl instead of
the Python backport implementation. Don't know if there was a subtle
difference in STATUS that caused backport.pl to barf while packport.py
succeeded.

Lets keep our eyes open for the next backport.

Kind regards,
Daniel


Re: Backport bot not running?

2023-11-28 Thread Daniel Sahlberg
ons 29 nov. 2023 kl. 05:57 skrev Nathan Hartman :

> The backport bot (svn-role) normally runs nightly but the most recent
> backport approval has been waiting in 1.14.x/STATUS for a couple of
> days now.
>
> I went ahead and merged it manually (with
> tools/dist/merge-approved-backports.py). This did the right thing, so
> I assume there wasn't any syntax error in STATUS.
>
> I don't have access to svn-qavm1 so I can't check why it didn't happen
> automatically. Maybe someone with access could check if the machine is
> at least running...
>
> Thanks,
> Nathan


I’ll check later today


Kind regards
Daniel

>
>


Re: Is there a write opposite to "svn cat"?

2023-11-27 Thread Daniel Sahlberg
Den mån 27 nov. 2023 kl 20:29 skrev Daniel Sahlberg <
daniel.l.sahlb...@gmail.com>:
[...]

> In the mail to users@, Graham suggested a better error message indicating
> which option was invalid. I will make a separate suggestion and send this
> to the APR project.
>

I've sent this to d...@apr.apache.org:
https://lists.apache.org/thread/x84938n846xzj5g2ffyc3jq7ryf757lf

With the suggested patch, the error message would have been

[[[
[root@seawitch postgres]# cat db.sql | svnmucc put -
file:///var/lib/svn/db/db.sql
svnmucc: invalid option: -
Type 'svnmucc --help' for usage.
]]]

Notice the added "-" after the first line, compared to the original below:

Den sön 26 nov. 2023 kl 22:51 skrev Graham Leggett via users <
>>> us...@subversion.apache.org>:
>>>
>>>> [root@seawitch postgres]# cat db.sql | svnmucc put -
>>>> file:///var/lib/svn/db/db.sql
>>>>
>>>> svnmucc: invalid option:
>>>>
>>>> Type 'svnmucc --help' for usage.
>>>>
>>>> Alas the error message mentions an invalid option, but doesn’t say
>>>> which option, or why it is invalid.
>>>>
>>>
Kind regards,
Daniel


Re: Is there a write opposite to "svn cat"?

2023-11-27 Thread Daniel Sahlberg
Den mån 27 nov. 2023 kl 21:35 skrev Johan Corveleyn :

> >>> The documentation say:
> >>> [[[
> >>>   put SRC-FILE URL   : add or modify file URL with contents copied
> from
> >>>SRC-FILE (use "-" to read from standard
> input)
> >>> ]]]
>
> Just chiming in here from the sideline, without ability to test right
> now, but: has anyone tried to use a full "-" (double quote, dash,
> double quote) as argument? As in:
>
> svnmucc put "-" URL
>
> Maybe that's what that help text is actually trying to say.
>

Good thinking, never thought about that. I've tested now but unfortunately
it doesn't make a difference, still the same error message. (Tested on
Windows with 1.8 and 1.14 and on Linux with 1.14).

Kind regards,
Daniel


Re: Is there a write opposite to "svn cat"?

2023-11-27 Thread Daniel Sahlberg
Den mån 27 nov. 2023 kl 12:18 skrev Daniel Sahlberg <
daniel.l.sahlb...@gmail.com>:

> Moving a discussion from users@ to here. Original message below for
> reference.
>
> TL;DR: According to documentation, svnmucc put - URL should add or modify
> file URL reading the new contents from standard input. It doesn't, instead
> it output an error message "svnmucc: invalid option:".
>

I acknowledge the workaround posted by Lorenz in a separate message. It has
the downside that -- disables options/arguments parsing so anything that
follows must be strictly actions (put, cp, mkdir etc) and action arguments
(REV, SRC-FILE, URL etc.).


> I've tried to dig into subversion/svnmucc/svnmucc.c to figure out what is
> going wrong. The handling of "-" as stdin was added in r873014 (In the "put
> SRC_FILE URL" action read from stdin when SRC_FILE is "-") in 2008. I can't
> find anything significant having changed since it was added.
>
> I'm sending this off half-baked since I'm about to board a plane, in case
> anyone else would like to have a look.
>

Some investigations and tests later I can't see if this has ever worked. As
suggested by Graham Leggett in the thread at users@, this is because of
code in apr_getopt_long:

# misc/unix/getopt.c, row 274
[[[
if (*p == '\0')/* Bare "-" is illegal */
return serr(os, "invalid option", p, APR_BADCH);
]]]
(Sorry for cutting out a lot of context but there is related to processing
'-' characters, I think the comment was the only interested thing). This
file has not been touched since apr_1.0.x (save some whitespace fixes).

I have a collection of binaries, SlikSVN 1.5.5 doesn't contain the "- is
stdin" option, it still fails:
[[[
svnmucc: invalid option:
svnmucc: getopt failure: Bad character specified on command line
]]]

TortoiseSVN 1.8.12 has the option and fails with an actual error number
(maybe Subversion 1.5 just didn't print the E number on the console?):
[[[
svnmucc: invalid option:
svnmucc: E070012: getopt failure: Bad character specified on command line
]]]

TortoiseSVN 1.14.5 (based on Subversion 1.14.2) fails with a message
directing the user to the --help page.
[[[
svnmucc: invalid option:
Type 'svnmucc --help' for usage.
]]]

In all messages above, the first line is from apr/misc/unix/getopt.c and
the second line from svnmucc.c

As for a fix... I first tried switching of the "interleave option"
(opts->interleave = 1; on row 547) but that has about the same effect as
using -- on the command line and it breaks for example the test suite and
would break any scripts having commands (for example -m "logmessage") after
any arguments.

I looked into apr/misc/unix/getopt.c to see if there was any way of making
"-" a legal argument but I couldn't find anything. Thus I'm suggesting
making the help text a little bit more clear:

[[[
Index: subversion/svnmucc/svnmucc.c
===
--- subversion/svnmucc/svnmucc.c(revision 1914148)
+++ subversion/svnmucc/svnmucc.c(working copy)
@@ -286,7 +286,9 @@
   "  mv SRC-URL DST-URL : move SRC-URL to DST-URL\n"
   "  rm URL : delete URL\n"
   "  put SRC-FILE URL   : add or modify file URL with contents
copied from\n"
-  "   SRC-FILE (use \"-\" to read from
standard input)\n"
+  "   SRC-FILE (use \"-\" to read from
standard input,\n"
+  "   but only if preceeded by -- and in which
case no\n"
+  "   options can follow. \"-\" only makes
sense once)\n"
   "  propset NAME VALUE URL : set property NAME on URL to VALUE\n"
   "  propsetf NAME FILE URL : set property NAME on URL to value read
from FILE\n"
   "  propdel NAME URL   : delete property NAME from URL\n"
]]]

The "\"-\" only makes sense once" is because stdin can't be rewinded and
thus can't be put into more than once file. If several put actions (with -
as SRC-FILE) are used, the second file will be empty.

Does this make sense (in particular to our native english speakers)?

In the mail to users@, Graham suggested a better error message indicating
which option was invalid. I will make a separate suggestion and send this
to the APR project.

Kind regards,
Daniel


Den mån 27 nov. 2023 kl 07:47 skrev Daniel Sahlberg <
> daniel.l.sahlb...@gmail.com>:
>
>> Den sön 26 nov. 2023 kl 22:51 skrev Graham Leggett via users <
>> us...@subversion.apache.org>:
>>
>>> On 25 Nov 2023, at 13:40, Pavel Lyalyakin 
>>> wrote:
>>>
>>> `svnmucc put` p

Re: Is there a write opposite to "svn cat"?

2023-11-27 Thread Daniel Sahlberg
Moving a discussion from users@ to here. Original message below for
reference.

TL;DR: According to documentation, svnmucc put - URL should add or modify
file URL reading the new contents from standard input. It doesn't, instead
it output an error message "svnmucc: invalid option:".

I've tried to dig into subversion/svnmucc/svnmucc.c to figure out what is
going wrong. The handling of "-" as stdin was added in r873014 (In the "put
SRC_FILE URL" action read from stdin when SRC_FILE is "-") in 2008. I can't
find anything significant having changed since it was added.

I'm sending this off half-baked since I'm about to board a plane, in case
anyone else would like to have a look.

Kind regards,
Daniel

Den mån 27 nov. 2023 kl 07:47 skrev Daniel Sahlberg <
daniel.l.sahlb...@gmail.com>:

> Den sön 26 nov. 2023 kl 22:51 skrev Graham Leggett via users <
> us...@subversion.apache.org>:
>
>> On 25 Nov 2023, at 13:40, Pavel Lyalyakin 
>> wrote:
>>
>> `svnmucc put` perhaps?
>> https://svnbook.red-bean.com/en/1.8/svn.ref.svnmucc.re.html
>>
>>
>> From reading the manual it looks perfect, but I’m having no luck:
>>
>> [root@seawitch postgres]# cat db.sql | svnmucc put -
>> file:///var/lib/svn/db/db.sql
>>
>> svnmucc: invalid option:
>>
>> Type 'svnmucc --help' for usage.
>>
>> Alas the error message mentions an invalid option, but doesn’t say which
>> option, or why it is invalid.
>>
>
> The documentation say:
> [[[
>   put SRC-FILE URL   : add or modify file URL with contents copied from
>SRC-FILE (use "-" to read from standard input)
> ]]]
>
> So as far as I'm reading the documentation, the command you are using
> should be supported. I would say this is a bug, either in the argument
> handling or in the documentation.
>
> Can you instead try:
> $ svnmucc put db.sql file:///var/lib/svn/db/db.sql
>
> I will bring this discussion to the dev@subversion.apache.org to figure
> out if we need to change the documentation or if we can fix the code.
>
> Kind regards,
> Daniel
>


Re: svn merge issue with two working copy paths

2023-11-13 Thread Daniel Sahlberg
Den mån 13 nov. 2023 kl 13:41 skrev Daniel Sahlberg <
daniel.l.sahlb...@gmail.com>:

> Den mån 13 nov. 2023 kl 13:26 skrev Ontonator :
>
>> Hi,
>>
>> I've experienced an issue with merging two working copy paths in which
>> the behaviour of `svn` doesn't seem to match up with the documentation.
>>
>> My system Subversion version is:
>>
>> > svn, version 1.14.2 (r1899510)
>>
>> >compiled Oct 23 2023, 15:43:14 on x86_64-pc-linux-gnu
>>
>> but I have also reproduced it on revision 1913725 built from source.
>>
>> The only reference I could find to this issue was this users@ mailing
>> list post from almost a decade ago with no responses:
>>
>> https://lists.apache.org/thread/16vf8rr4w7nfkbjlsgfrnrfh0ly4mk41
>>
>> The relevant sections of the documentation are (from `svn help merge`):
>>
>> > usage: 1. merge SOURCE[@REV] [TARGET_WCPATH]
>>
>> >   (the 'complete' merge)
>>
>> >2. merge [-c M[,N...] | -r N:M ...] SOURCE[@REV] [TARGET_WCPATH]
>>
>> >   (the 'cherry-pick' merge)
>>
>> >3. merge SOURCE1[@REV1] SOURCE2[@REV2] [TARGET_WCPATH]
>>
>> >   (the '2-URL' merge)
>>
>> and (regarding form 1):
>>
>> >  SOURCE is usually a URL. The optional '@REV' specifies both the peg
>>
>> >  revision of the URL and the latest revision that will be considered
>>
>> >  for merging; if REV is not specified, the HEAD revision is
>> assumed. If
>>
>> >  SOURCE is a working copy path, the corresponding URL of the path is
>>
>> >  used, and the default value of 'REV' is the base revision (usually
>> the
>>
>> >  revision last updated to).
>>
>> However, running a command like `svn merge a b` results in:
>>
>> > svn: E195002: Invalid merge source 'a'; a working copy path can only be
>> used with a repository revision (a number, a date, or head)
>>
>> I have a reproduction `sh` script:
>>
>> #! /usr/bin/env sh
>>
>> set -e
>>
>> mkdir scratch
>>
>> cd scratch
>>
>> svnadmin create repos
>>
>> svn checkout "file://$(pwd)/repos" working
>>
>> cd working
>>
>> svn mkdir ^/a --message='Created a'
>>
>> svn copy ^/a ^/b --message='Created b'
>>
>> svn update
>>
>> set +e
>>
>> printf '\n=== Merging with two working copy paths (explicit target) ===\n'
>>
>> svn merge a b
>>
>> printf '\n=== Merging with two working copy paths (inside directory
>> target) ===\n'
>>
>> cd b
>>
>> svn merge ../a .
>>
>> cd ..
>>
>> printf '\n=== Merging with a single working copy path (inferred target)
>> ===\n'
>>
>> cd b
>>
>> svn merge ../a
>>
>> cd ..
>>
>> (End of reproduction)
>>
>> The output of this script is:
>>
>> > Checked out revision 0.
>>
>> > Committing transaction...
>>
>> > Committed revision 1.
>>
>> > Committing transaction...
>>
>> > Committed revision 2.
>>
>> > Updating '.':
>>
>> > Aa
>>
>> > Ab
>>
>> > Updated to revision 2.
>>
>> >
>>
>> > === Merging with two working copy paths (explicit target) ===
>>
>> > svn: E195002: Invalid merge source 'a'; a working copy path can only be
>> used with a repository revision (a number, a date, or head)
>>
>> >
>>
>> > === Merging with two working copy paths (inside directory target) ===
>>
>> > svn: E195002: Invalid merge source '../a'; a working copy path can only
>> be used with a repository revision (a number, a date, or head)
>>
>> >
>>
>> > === Merging with a single working copy path (inferred target) ===
>>
>> > --- Recording mergeinfo for merge of r2 into '.':
>>
>> >  U   .
>>
>> As can be seen in the reproduction, with an explicitly specified
>> `TARGET_WCPATH` there is an error, but when it is omitted, it is assumed to
>> be `.` and succeeds.
>>
>> From a brief search of the code, I believe the error lies at
>> `subversion/svn/merge-cmd.c:334` (I have revision 1913725 checked out). For
>> the lazy, it looks like:
>>
>> >   if (targets->nelts <= 1)
>>
>> > {
>>
>> >   two_sources_specified = FALSE;
>>
>> > }
>>
>>

Re: svn merge issue with two working copy paths

2023-11-13 Thread Daniel Sahlberg
Den mån 13 nov. 2023 kl 13:26 skrev Ontonator :

> Hi,
>
> I've experienced an issue with merging two working copy paths in which the
> behaviour of `svn` doesn't seem to match up with the documentation.
>
> My system Subversion version is:
>
> > svn, version 1.14.2 (r1899510)
>
> >compiled Oct 23 2023, 15:43:14 on x86_64-pc-linux-gnu
>
> but I have also reproduced it on revision 1913725 built from source.
>
> The only reference I could find to this issue was this users@ mailing
> list post from almost a decade ago with no responses:
>
> https://lists.apache.org/thread/16vf8rr4w7nfkbjlsgfrnrfh0ly4mk41
>
> The relevant sections of the documentation are (from `svn help merge`):
>
> > usage: 1. merge SOURCE[@REV] [TARGET_WCPATH]
>
> >   (the 'complete' merge)
>
> >2. merge [-c M[,N...] | -r N:M ...] SOURCE[@REV] [TARGET_WCPATH]
>
> >   (the 'cherry-pick' merge)
>
> >3. merge SOURCE1[@REV1] SOURCE2[@REV2] [TARGET_WCPATH]
>
> >   (the '2-URL' merge)
>
> and (regarding form 1):
>
> >  SOURCE is usually a URL. The optional '@REV' specifies both the peg
>
> >  revision of the URL and the latest revision that will be considered
>
> >  for merging; if REV is not specified, the HEAD revision is assumed.
> If
>
> >  SOURCE is a working copy path, the corresponding URL of the path is
>
> >  used, and the default value of 'REV' is the base revision (usually
> the
>
> >  revision last updated to).
>
> However, running a command like `svn merge a b` results in:
>
> > svn: E195002: Invalid merge source 'a'; a working copy path can only be
> used with a repository revision (a number, a date, or head)
>
> I have a reproduction `sh` script:
>
> #! /usr/bin/env sh
>
> set -e
>
> mkdir scratch
>
> cd scratch
>
> svnadmin create repos
>
> svn checkout "file://$(pwd)/repos" working
>
> cd working
>
> svn mkdir ^/a --message='Created a'
>
> svn copy ^/a ^/b --message='Created b'
>
> svn update
>
> set +e
>
> printf '\n=== Merging with two working copy paths (explicit target) ===\n'
>
> svn merge a b
>
> printf '\n=== Merging with two working copy paths (inside directory
> target) ===\n'
>
> cd b
>
> svn merge ../a .
>
> cd ..
>
> printf '\n=== Merging with a single working copy path (inferred target)
> ===\n'
>
> cd b
>
> svn merge ../a
>
> cd ..
>
> (End of reproduction)
>
> The output of this script is:
>
> > Checked out revision 0.
>
> > Committing transaction...
>
> > Committed revision 1.
>
> > Committing transaction...
>
> > Committed revision 2.
>
> > Updating '.':
>
> > Aa
>
> > Ab
>
> > Updated to revision 2.
>
> >
>
> > === Merging with two working copy paths (explicit target) ===
>
> > svn: E195002: Invalid merge source 'a'; a working copy path can only be
> used with a repository revision (a number, a date, or head)
>
> >
>
> > === Merging with two working copy paths (inside directory target) ===
>
> > svn: E195002: Invalid merge source '../a'; a working copy path can only
> be used with a repository revision (a number, a date, or head)
>
> >
>
> > === Merging with a single working copy path (inferred target) ===
>
> > --- Recording mergeinfo for merge of r2 into '.':
>
> >  U   .
>
> As can be seen in the reproduction, with an explicitly specified
> `TARGET_WCPATH` there is an error, but when it is omitted, it is assumed to
> be `.` and succeeds.
>
> From a brief search of the code, I believe the error lies at
> `subversion/svn/merge-cmd.c:334` (I have revision 1913725 checked out). For
> the lazy, it looks like:
>
> >   if (targets->nelts <= 1)
>
> > {
>
> >   two_sources_specified = FALSE;
>
> > }
>
> >   else if (targets->nelts == 2)
>
> > {
>
> >   if (svn_path_is_url(sourcepath1) && !svn_path_is_url(sourcepath2))
> // <-- This one
>
> > two_sources_specified = FALSE;
>
> > }
>
> In particular, it checks that `sourcepath1` is a URL, which doesn't seem
> right to me. It seems like this code was introduced in revision 868224,
> whose log message makes no mention of intending to change this behaviour
> (although I can't verify that the behaviour was introduced here as it uses
> Python 2 as part of the build process).
>
> While I can't be sure that this behaviour is not intended, it does not
> match the documentation, so at least one of the implementation and the
> documentation must be wrong. I would assume that there is an error in the
> implementation given the inconsistency between passing the second argument
> explicitly vs implicitly.
>
> I haven't reported any issues to Subversion before, so please tell me if
> there is anything I have neglected to do or should have done differently.
>

Thank you for your report, it is excellent, especially the reproduction
script is super useful.

I've only skimmed the details but I agree with you that it looks like the
behaviour doesn't match the documentation. I'll try to dig down into the
history of this code, in particular how the sourcepath[12] and

Re: "which" vs "-which" in Makefile.svn?

2023-11-12 Thread Daniel Sahlberg
sön 12 nov. 2023 kl. 19:02 skrev Nathan Hartman :

> In my adventures with tools/dev/unix-build/Makefile.svn, I saw two places
> that call '-which', that is, 'which' preceded by a dash.
>
> One of these is in the recipe for gettext's configure. It was changed from
> 'which' to '-which' in r1577186.
>
> I'm not familiar with this. What does '-which' do, and how is it different
> from 'which'?
>

Maybe it is just a make-ism to ignore errors? See the gnu make manual at
[1]:
“To ignore errors in a recipe line, write a ‘-’ at the beginning of the
line’s text (after the initial tab). The ‘-’ is discarded before the line
is passed to the shell for execution.”

Kind regards
Daniel


[1] https://www.gnu.org/software/make/manual/html_node/Errors.html


Re: New release

2023-11-12 Thread Daniel Sahlberg
Den sön 12 nov. 2023 kl 07:18 skrev Nathan Hartman :

> On Fri, Nov 10, 2023 at 9:16 AM Daniel Sahlberg
>  wrote:
> >
> > Den fre 10 nov. 2023 kl 15:09 skrev Nathan Hartman <
> hartman.nat...@gmail.com>:
> >>
> >> On Fri, Nov 10, 2023 at 8:02 AM Johan Corveleyn 
> wrote:
> >>>
> >>> Hi Nathan,
> >>>
> >>> Sorry I haven't helped so far ... I should be able to say something
> >>> useful here, since I'm a java dev in my dayjob. But I have been
> >>> drowning a bit in that same dayjob lately, so I can barely find the
> >>> time to scan these mails, and then forget about them because my
> >>> attention is needed elsewhere.
> >>>
> >>> Anyway, I'll try to find some more time to take a closer look at the
> >>> errors you get.
> >>>
> >>> Just as a first shot: if there is no RunTests.class in the location
> >>> where it should be (one of the directories in the classpath), then it
> >>> probably has not been compiled (or it has not been put in the right
> >>> location). My first guess would also be, like Daniel mentioned, that
> >>> you'll need the --with-junit option (referring to a junit-X.Y.jar
> >>> somewhere on your system) and then the build scripts should compile
> >>> the test classes.
> >>>
> >>> But I guess you already tried that, after Daniel's reply, and you're
> >>> still running into problems. I'll take a closer look when you send
> >>> your detailed mail then.
> >>>
> >>> --
> >>> Johan
> >>
> >>
> >>
> >> Thanks Johan. Apologies for not sending the detailed mail yet. I wanted
> to test a few specific theories but didn't get that far last night because
> I hit a problem unrelated to SVN.
> >>
> >> One thing I'm struggling with is to understand which part of the build
> system is supposed to compile RunTests, i.e., how/where the Java compiler
> is being invoked on it. I'm not seeing it in the build logs, though I do
> see the other Java files being compiled, so that's certainly interesting.
> >
> >
> > I'm fairly sure that RunTests is compiled only when --with-junit is
> present (don't know if "present" is enough or if it has to point to a valid
> JAR file). Next question is if check-javahl should be available at all if
> --with-junit is missing or if check-javahl should make sure RunTests is
> built. Either way, I think it is a bug in the build system. This question
> should probably be carved out to its own e-mail thread. I'll try to figure
> something out and start a new thread.
> >
> > Kind regards,
> > Daniel
>
> Hi all,
>
> Success! I have finally seen the JavaHL tests build and run!
>
> I'd like to say a great big Thank You to everyone for helping me get
> this far!
>
>
[...]


>
> (3) Worked around a problem with serf-1.3.10 that DSahlberg saw also.
>
>
[...]


> Regarding #3:
>
> When building serf-1.3.10, this warning foreshadows problems to come
> later:
>
> buckets/ssl_buckets.c:1193:9: warning: implicit declaration of
> function 'CRYPTO_malloc_init'; did you mean 'CRYPTO_malloc'?
> [-Wimplicit-function-declaration]
>
> Later, when trying to link SVN, this linker error happens:
>
> /usr/bin/ld: /home/nathan/ramdrive/svndev/prefix/serf/lib/libserf-1.so.1:
> undefined reference to `CRYPTO_malloc_init'
> collect2: error: ld returned 1 exit status
> make[1]: *** [build-outputs.mk:1140: subversion/svn/svn] Error 1
> make[1]: Leaving directory '/home/nathan/ramdrive/svndev/svn-1.14.x'
> make: *** [Makefile:1845:
> /home/nathan/ramdrive/svndev/objdir/svn-1.14.x/.compiled] Error 2
>
> For several years, I have been self-building SVN with serf-1.4.x and
> running the test suite successfully with that. I'm referring to serf's
> branches/1.4.x, last changed rev r1845547.
>

Note that serf 1.4 is build using CMake so things might behave differently
compared to SCons.


> IIRC this was suggested by Brane and it's on my TODO list to locate
> that conversation in the archives to see whether that was for this same
> reason.
>
> Perhaps something can be backported to serf-1.3.x to solve this. It
> manifests on recent Debian (DSahlberg reproduced this also [3]).
>

I did some more digging this morning and Nathan was onto something on IRC
[1] when he disabled the merge of the serf/branches/1.3.x-sslbuild branch.

It seems this branch mess things up when building on Debian. Some parts of
the branch are probably needed (including some updates to SConstruct to
handle building with Python3 when building 1.3.9) but it seems that some
changes conflict with improvements in later versions of Serf. I plan to dig
deeper (including setting up OpenBSD and test with LibreSSL) to see if we
can simplify the -sslbuild branch or even upstream some of the changes.

Kind regards,
Daniel

[1] https://colabti.org/irclogger/irclogger_log/svn-dev?date=2023-11-12


Re: Building with tools/dev/unix-build/Makefile.svn - problems linking with OpenSSL

2023-11-11 Thread Daniel Sahlberg
Den lör 11 nov. 2023 kl 17:14 skrev Stefan Sperling :

> On Sat, Nov 11, 2023 at 05:08:28PM +0100, Daniel Sahlberg wrote:
> > ne_openssl.o isn't built, the build fails already when running configure
> on
> > neon.
>
> Can you show the cofigure log? E.g. via paste.apache.org?
> I am also in #svn-dev on Libera IRC if you want to chat there.
>

The discussion between Stefan and myself continued over on Libera IRC but
to tie up the loose end.

It was an error on my system, I was missing pkg-config. After installing
this, the neon configure worked well and I'm back on track again (or at
least, stuck on other thinkgs that i need to look at).

Thanks Stefan!

Kind regards,
Daniel


Re: Building with tools/dev/unix-build/Makefile.svn - problems linking with OpenSSL

2023-11-11 Thread Daniel Sahlberg
Den lör 11 nov. 2023 kl 16:43 skrev Stefan Sperling :

> On Sat, Nov 11, 2023 at 04:34:07PM +0100, Stefan Sperling wrote:
> > On Sat, Nov 11, 2023 at 04:23:52PM +0100, Daniel Sahlberg wrote:
> > > As for the original problem, updating to the latest version of the Neon
> > > library doesn't help, I still get the same linking error. I''m sure
> there
> > > is something fishy with my Debian install...
> >
> > Which libssl is your build trying to use?
>

The libssl-dev package, which is OpenSSL version 1.1.1w-0+deb11u1.


> >
> > There is nothing in Makefile.svn that would try to pick a particular
> > version of OpenSSL. It just expects that a compatible version is
> > already installed.
> >
> > Maybe you have multiple versions installed and that confuses things?
>

Don't think so. There is only one libssl.a (and libssl.so) on my system.
Doing nm -g | grep OPENSSL_init_ssl (which is not the function in the error
message but as I've written before SSL_library_init is just a #define for
OPENSSL_init_ssl, with the appropriate define for OPENSSL_API_COMPAT) finds:

00270 T OPENSSL_init_ssl


> What is the output of this command on your system (in the ~/svn
> build dir):
>
>  nm objdir/neon-0.32.5/src/ne_openssl.o | grep -i SSL_library_init
>
> This doesn't print anything for me. If this shows the symbol
> on your system that would indicate a wrong OPENSSL_VERSION macro
> being used during the build.
>

ne_openssl.o isn't built, the build fails already when running configure on
neon.

Kind regards,
Daniel


Re: Building with tools/dev/unix-build/Makefile.svn - problems linking with OpenSSL

2023-11-11 Thread Daniel Sahlberg
Den lör 11 nov. 2023 kl 15:54 skrev Stefan Sperling :

> On Sat, Nov 11, 2023 at 02:22:55PM +0100, Daniel Sahlberg wrote:
> > Hi,
> >
> > I'm separating this out to a new thread since it doesn't relate to the
> > upcoming release, but I'm doing the work trying to reproduce Nathan's
> > problem.
> >
> > I've installed Debian Bullseye (on Hyper-V, but that shouldn't matter I
> > think). When I try to do the build, I end up having trouble building
> neon.
> > The error message is "could not find library containing
> SSL_library_init".
> > I have checked that the package libssl-dev is installed (with version
> > 1.1.1w-0+deb11u1).
> >
> > I've tried a very simple file:
> > #include 
> > int main(void)
> > {
> > SSL_library_init();
> > return 0;
> > }
> >
> > $ gcc -lssl -lcrypto -DOPENSSL_API_COMPAT=0x1000L ssl.c
> > [..] undefined reference to OPENSSL_init_ssl [...]
> >
> > This makes sense, since ssl.h defines SSL_library_init() as
> > OPENSSL_init_ssl(0, NULL) (protected by an #if checking
> OPENSSL_API_COMPAT
> > < 0x1010L).
> >
>
> This function first appeared in OpenSSL 1.1.0.
> Which means trying to call it on earlier version of OpenSSL is an
> expected failure.
>

I've got OpenSSL 1.1.1w. The code is calling SSL_library_init which is a
define pointing to OPENSSL_init_ssl().

The code compiles just fine, but I don't understand why the code isn't
linking.

As for the original problem, updating to the latest version of the Neon
library doesn't help, I still get the same linking error. I''m sure there
is something fishy with my Debian install...

Cheers,
Daniel


Building with tools/dev/unix-build/Makefile.svn - problems linking with OpenSSL

2023-11-11 Thread Daniel Sahlberg
Hi,

I'm separating this out to a new thread since it doesn't relate to the
upcoming release, but I'm doing the work trying to reproduce Nathan's
problem.

I've installed Debian Bullseye (on Hyper-V, but that shouldn't matter I
think). When I try to do the build, I end up having trouble building neon.
The error message is "could not find library containing SSL_library_init".
I have checked that the package libssl-dev is installed (with version
1.1.1w-0+deb11u1).

I've tried a very simple file:
#include 
int main(void)
{
SSL_library_init();
return 0;
}

$ gcc -lssl -lcrypto -DOPENSSL_API_COMPAT=0x1000L ssl.c
[..] undefined reference to OPENSSL_init_ssl [...]

This makes sense, since ssl.h defines SSL_library_init() as
OPENSSL_init_ssl(0, NULL) (protected by an #if checking OPENSSL_API_COMPAT
< 0x1010L).

I've tried changing the order of the libraries (-lcrypto -lssl) with the
same result.

I'm sure I'm missing something simple but I've spent the better half of a
day figuring out what.

Kind regards,
Daniel


Re: New release

2023-11-10 Thread Daniel Sahlberg
Den fre 10 nov. 2023 kl 15:09 skrev Nathan Hartman :

> On Fri, Nov 10, 2023 at 8:02 AM Johan Corveleyn  wrote:
>
>> Hi Nathan,
>>
>> Sorry I haven't helped so far ... I should be able to say something
>> useful here, since I'm a java dev in my dayjob. But I have been
>> drowning a bit in that same dayjob lately, so I can barely find the
>> time to scan these mails, and then forget about them because my
>> attention is needed elsewhere.
>>
>> Anyway, I'll try to find some more time to take a closer look at the
>> errors you get.
>>
>> Just as a first shot: if there is no RunTests.class in the location
>> where it should be (one of the directories in the classpath), then it
>> probably has not been compiled (or it has not been put in the right
>> location). My first guess would also be, like Daniel mentioned, that
>> you'll need the --with-junit option (referring to a junit-X.Y.jar
>> somewhere on your system) and then the build scripts should compile
>> the test classes.
>>
>> But I guess you already tried that, after Daniel's reply, and you're
>> still running into problems. I'll take a closer look when you send
>> your detailed mail then.
>>
>> --
>> Johan
>
>
>
> Thanks Johan. Apologies for not sending the detailed mail yet. I wanted to
> test a few specific theories but didn't get that far last night because I
> hit a problem unrelated to SVN.
>
> One thing I'm struggling with is to understand which part of the build
> system is supposed to compile RunTests, i.e., how/where the Java compiler
> is being invoked on it. I'm not seeing it in the build logs, though I do
> see the other Java files being compiled, so that's certainly interesting.
>

I'm fairly sure that RunTests is compiled only when --with-junit is present
(don't know if "present" is enough or if it has to point to a valid JAR
file). Next question is if check-javahl should be available at all if
--with-junit is missing or if check-javahl should make sure RunTests is
built. Either way, I think it is a bug in the build system. This question
should probably be carved out to its own e-mail thread. I'll try to figure
something out and start a new thread.

Kind regards,
Daniel


Re: Deleting /tools/dev/iz/

2023-11-03 Thread Daniel Sahlberg
Den fre 3 nov. 2023 kl 13:43 skrev Greg Stein :

> I'm with Mark on this one: whack it. With prejudice :-)
>

Says the original author :-)

With three +1 (counting myself) and such old code, I'm went berserk and
axed without waiting for everyone to have their say - if anyone disagree
down the line we can restore it.

/Daniel


Deleting /tools/dev/iz/

2023-11-03 Thread Daniel Sahlberg
Hi,

I don't know about the policy for deleting unmaintained / no longer
relevant code, so I'm asking here first.

/tools/dev/iz/ (
https://svn.apache.org/repos/asf/subversion/trunk/tools/dev/iz/) seems to
be a tool to extract issues statistics from IssueZilla (which I as far as I
understand was a CollabNet(?) customised(?) BugZilla instance). Last
relevant update was 2004, after that only Python3 and similar fixes.

Since we don't use BugZilla anymore: Anyone against deleting tools/dev/iz/

I think keeping old code for accessing old systems around will only cause
additional work for someone to look at "do we need to update this code
becaus of XX". If someone needs it, I heard someone mentioning version
control :-)

Kind regards,
Daniel


Re: New release

2023-11-03 Thread Daniel Sahlberg
Den fre 3 nov. 2023 kl 10:00 skrev Stefan Sperling :

> On Fri, Nov 03, 2023 at 12:00:32AM -0400, Nathan Hartman wrote:
>
[...]

> >
> >
> > Hi all,
> >
> > Previously I mentioned I plan to RM for the upcoming 1.14.3 release.
> > This being my first time, I need to solve some issues first.
> >
> > One of these, which has been a stumbling block for me since the
> > beginning, is getting the JavaHL bindings to build and test
> > successfully.
> >
> > I have made progress on this, but not a complete breakthrough yet.
> >
> > The progress was discovering that I need to have $JAVA_HOME set in my
> > environment; without this, our build system (in build/ac-macros/java.m4)
> > fails to find javac and assigns 'none' to JAVAC; then, much later,
> > 'make check-javahl' tries to call this 'none', which unsurprisingly
> > doesn't work. (I was surprised that the other logic in this AC_DEFUN
> > couldn't find javac without $JAVA_HOME being set, but that's a subject
> > for another thread.) The clue was the use of $JAVA_HOME there.
> >
> > I tried various values for $JAVA_HOME but nothing that seemed sensible
> > worked; based on a StackOverflow answer [1], I ended up setting
> > $JAVA_HOME as follows:
> >
> > $ export JAVA_HOME=$(readlink -f /usr/bin/javac | sed "s:/bin/javac::")
> >
> > On my system, this sets $JAVA_HOME to:
> > /usr/lib/jvm/java-17-openjdk-amd64
>

In my case (Ubuntu 23.04), I need to use the --with-jdk argument to
configure:
[[[
$ ./configure --enable-javahl --with-jdk=/usr/lib/jvm/java-17-openjdk-amd64/
]]]
otherwise I get the following error message:
[[[
configure: error: Cannot compile JavaHL without a suitable JDK.
  Please specify a suitable JDK using the --with-jdk option.
]]]
Don't know if the build on MacOS is different.


> >
> > This value seems a little strange IMHO but it finally got 'configure' to
> > find the JDK successfully and got me past the longstanding 'none'
> > problem!
>

Great! There are some MacOS specific instructions in the building
instructions for JavaHL [2]. Don't know if this refers to your problem or
not,


> >
> > But then I promptly ran into the next issue:
> >
> > [[[
> >
> > $ make check-javahl
> > (snip)
> > /usr/lib/jvm/java-17-openjdk-amd64/bin/java -Xcheck:jni
> >
> "-Dtest.rootdir=/home/nathan/ramdrive/svndev/svn-1.14.x/subversion/bindings/javahl/test-work"
> >
> "-Dtest.srcdir=/home/nathan/ramdrive/svndev/svn-1.14.x/subversion/bindings/javahl"
> > "-Dtest.rooturl=" "-Dtest.fstype="
> > "-Djava.library.path=:/home/nathan/ramdrive/svndev/prefix/svn-1.14.x/lib"
> > -classpath
> "subversion/bindings/javahl/classes:/home/nathan/ramdrive/svndev/svn-1.14.x/subversion/bindings/javahl/src:"
> > "-Dtest.cleanup=" "-Dtest.tests="
> > org.apache.subversion.javahl.RunTests
> > Error: Could not find or load main class
> org.apache.subversion.javahl.RunTests
> > Caused by: java.lang.ClassNotFoundException:
> > org.apache.subversion.javahl.RunTests
> > make: *** [Makefile:530: check-apache-javahl] Error 1
> >
> > ]]]
> >
> > If I understand correctly, there should be a file called
> > 'RunTests.class' in one of the directories specified with -classpath
> > above. Well, there are other .class files to be found there, but not
> > RunTests.class. A search for RunTests with any (or no) extension in the
> > 1.14.x tree gives these two RunTests.java (not .class) files:
> >
> > $ find . -name RunTests\*
> >
> ./subversion/bindings/javahl/tests/org/apache/subversion/javahl/RunTests.java
> >
> ./subversion/bindings/javahl/tests/org/tigris/subversion/javahl/RunTests.java
> >
> > Forgive me if this is something obvious, but I know almost nothing about
> > the Java ecosystem. :-)
>

A .class is basically a .o file but for the JVM. So I guess RunTests.java
was never compiled.


> >
> > I'll have to continue this exploration tomorrow, but I'm at least glad
> > to finally have made progress on a longstanding stumbling block.
> >
> > Meanwhile, any clues would be appreciated!
> >
> > [1] https://stackoverflow.com/a/29622512
> >
> > Cheers,
> > Nathan
> >
>
> In my build script (in Subversion's tree at tools/dev/unix-build/) I pass
> the
> following flags to Subversion's configure script if JavaHL is enabled:
>
> --enable-javahl=yes --with-jdk --with-jikes=no \
> --with-junit=$(DISTDIR)/$(JUNIT_DIST)
>
> Did you also pass --with-junit?
>

I did some tests and it seems that the --with-junit=/path/to/junit-X.Y.jar
is causing RunTests.java to be built, I can reproduce Nathan's error if I
don't add this option to configure.


>
> For some reason I don't recall, my build script is fetching a self-hosted
> junit JAR file from here: https://stsp.name/distfiles/junit-4.10.jar
> (sha256 :
> 36a747ca1e0b86f6ea88055b8723bb87030d627766da6288bf077afdeeb0f75a)
>

The build instructions for javahl [2] say that you should download JUnit
from https://junit.org/ ("4.11 has been tested"). I assume you've optimised
away 

Re: Pootle Server Update

2023-11-01 Thread Daniel Sahlberg
Den ons 1 nov. 2023 kl 20:24 skrev Dave Fisher :

>
> On Nov 1, 2023, at 12:14 PM, Daniel Sahlberg 
> wrote:
>
> *dropping jmeter/couchdb/oo and adding mechtilde*
>
> Den ons 1 nov. 2023 kl 19:37 skrev Dave Fisher :
>
>> The OpenOffice project wishes to update the Pootle server (
>> translate.Apache.org <http://translate.apache.org/>) that we share to
>> translation software that is maintained. Pootle development stopped over
>> four years ago.
>>
>> Please let OpenOffice know if you still use the service and if you do
>> would you like to cooperate on the upgrade?
>>
>
> This seems to be an initiative from a user mechti...@apache.org to update
> the german translation. Last updated 4 years ago. Mechtilde unfortunately
> doesn't seem to be a committer in Subversion nor subscribed to the dev@
> list.
>
> I downloaded quickly and there seems to be some translations of the help
> commands (svn help etc.) that doesn't exist in our own repository, I think
> it would be a nice addition but someone fluent in german should verify and
> it would be great if Mechtilde would be involved in submitting a patch.
>
>
> Mechtilde is on the OpenOffice PMC and has been running the Pootle server
> for years.for AOO’s many translations. If the limit of Subversion use is
> her translations then there is no real concern.
>

Great. Still, if she indeed made translation of the help messages, it would
be nice if we could add them to our repository.

/Daniel


Re: Pootle Server Update

2023-11-01 Thread Daniel Sahlberg
*dropping jmeter/couchdb/oo and adding mechtilde*

Den ons 1 nov. 2023 kl 19:37 skrev Dave Fisher :

> The OpenOffice project wishes to update the Pootle server (
> translate.Apache.org) that we share to translation software that is
> maintained. Pootle development stopped over four years ago.
>
> Please let OpenOffice know if you still use the service and if you do
> would you like to cooperate on the upgrade?
>

This seems to be an initiative from a user mechti...@apache.org to update
the german translation. Last updated 4 years ago. Mechtilde unfortunately
doesn't seem to be a committer in Subversion nor subscribed to the dev@
list.

I downloaded quickly and there seems to be some translations of the help
commands (svn help etc.) that doesn't exist in our own repository, I think
it would be a nice addition but someone fluent in german should verify and
it would be great if Mechtilde would be involved in submitting a patch.

Kind regards,
Daniel Sahlberg


Re: Windows Buildbot failures

2023-10-30 Thread Daniel Sahlberg
Hi,

Coming back to an old e-mail. Those workers now seem to be offline
according to the status in https://ci2.apache.org/#/workers

I don't like to remove them but if they don't work and we can't fix them we
should probably consider this.

Kind regards,
Daniel



Den tis 19 sep. 2023 kl 23:58 skrev Daniel Sahlberg <
daniel.l.sahlb...@gmail.com>:

> Hi,
>
> There are two buildbot jobs svn-windows-local and svn-windows-ra that
> currently fail. See for example
> https://ci2.apache.org/#/builders/106/builds/104
>
> Relevant part of error message, it seems that one Python component is
> missing:
>
> error in RunProcess._startCommand (spawnProcess not available since pywin32 
> is not installed.)
> Traceback (most recent call last):
>   File "C:\Program 
> Files\Python310\lib\site-packages\buildbot_worker\runprocess.py", line 460, 
> in start
> self._startCommand()
>   File "C:\Program 
> Files\Python310\lib\site-packages\buildbot_worker\runprocess.py", line 584, 
> in _startCommand
> self.process = self._spawnProcess(
>   File "C:\Program 
> Files\Python310\lib\site-packages\buildbot_worker\runprocess.py", line 618, 
> in _spawnProcess
> return self._spawnAsBatch(processProtocol, executable, args, env,
>   File "C:\Program 
> Files\Python310\lib\site-packages\buildbot_worker\runprocess.py", line 656, 
> in _spawnAsBatch
> return reactor.spawnProcess(processProtocol, executable, argv, env,
>   File "C:\Program 
> Files\Python310\lib\site-packages\twisted\internet\posixbase.py", line 419, 
> in spawnProcess
> raise NotImplementedError(
> NotImplementedError: spawnProcess not available since pywin32 is not 
> installed.
>
>
> Does anyone have access to this server to install pywin32?
>
> Kind regards,
> Daniel Sahlberg
>


Re: svn commit: r1913094 - /subversion/branches/1.14.x/STATUS

2023-10-18 Thread Daniel Sahlberg
Hi,

I might have been a bit harsh here to Veto this merge, after actually
providing a positive note. But I investigated why the merge didn't go
through and I realised there is a digit missing in the nomination for the
first three revisions. I'm fairly sure it should be r1912501, r1912502,
r1912503  instead (missing the third digit).

I'd like someone (Yasuhito?) to crosscheck, if you agree feel free to
modify my vote back to the old +0 with comment and move to approved.

Kind regards,
Daniel

Den tors 19 okt. 2023 kl 02:02 skrev :

> Author: dsahlberg
> Date: Thu Oct 19 00:02:48 2023
> New Revision: 1913094
>
> URL: http://svn.apache.org/viewvc?rev=1913094=rev
> Log:
> In branches/1.14.x:
>
> * STATUS
>   Remove my nomination for the swig-py fixes. Explained on dev@
>
> Modified:
> subversion/branches/1.14.x/STATUS
>
> Modified: subversion/branches/1.14.x/STATUS
> URL:
> http://svn.apache.org/viewvc/subversion/branches/1.14.x/STATUS?rev=1913094=1913093=1913094=diff
>
> ==
> --- subversion/branches/1.14.x/STATUS (original)
> +++ subversion/branches/1.14.x/STATUS Thu Oct 19 00:02:48 2023
> @@ -39,13 +39,14 @@ Candidate changes:
>  Veto-blocked changes:
>  =
>
> -Approved changes:
> -=
> -
>   * r192501, r192502, r192503, r1912500, r1912515, r1912517, r1912691
> swig-py: Use pure Python objects as edit/parse_fns3 and decendant
> batons.
> Justification:
>   Bug fix. Issue #4916, #4917, #4918
> Votes:
>   +1: futatuki
> - +0: dsahlberg (not enough experience for +1, but looks good)
> + -1: dsahlberg Nominated revision numbers doesn't make sense, see dev@
> +
> +Approved changes:
> +=
> +
>
>
>


Re: svn commit: r1913041 - /subversion/site/staging/packages.html

2023-10-17 Thread Daniel Sahlberg
Den tis 17 okt. 2023 kl 14:06 skrev Nathan Hartman :

> On Tue, Oct 17, 2023 at 2:50 AM  wrote:
>
>> Author: dsahlberg
>> Date: Tue Oct 17 06:50:02 2023
>> New Revision: 1913041
>>
>> URL: http://svn.apache.org/viewvc?rev=1913041=rev
>> Log:
>> In site/staging:
>>
>> WANdisco is renaming itself to Cirata. Update links accordingly.
>>
>
[...]

>
> I wonder if it makes sense to write "Cirata (formerly WANdisco)".
>
> This could give some context to those who recognize the name WANdisco but
> don't know enough about the company to know about the new name.
>

Good point. I'll do that when I get some feedback on the MacOS clients.

/Daniel


Re: New release

2023-10-16 Thread Daniel Sahlberg
Den sön 15 okt. 2023 kl 06:50 skrev Nathan Hartman :

> On Fri, Oct 13, 2023 at 1:58 PM Johan Corveleyn  wrote:
>
>> On Fri, Oct 13, 2023 at 5:35 PM Stefan Sperling  wrote:
>> >
>> > On Fri, Oct 13, 2023 at 08:43:59AM +0200, Daniel Sahlberg wrote:
>> > > Hi,
>> > >
>> > > There are quite a number of improvements waiting to be released. Can
>> we
>> > > muster the energy to do a new release?
>> > >
>> > > In trunk there are a lot of changes that warrant a 1.15, but before
>> doing
>> > > that I think we should also go back to the discussion of changing the
>> /
>> > > adding a checksum algorithm in the WC. That deserves a separate
>> thread and
>> > > I'm half way through summarising the previous discussions so I'd like
>> to
>> > > hold back 1.15 for the moment.
>> > >
>> > > Still in 1.14 there have been a number of bugfixes that might be good
>> to
>> > > get released. Maybe doing 1.14.3 first could set us up to do 1.15 a
>> little
>> > > later?
>> >
>> > The first problem to solve before the ball starts rolling would be
>> > finding a release manager. I don't have enough spare time to play RM
>> > this time around but I would support the RM as far as my time allows
>> for.
>> >
>> > Doing 1.14.3 first sounds like a good plan. This would help potential
>> > new releases managers to get bootstrapped into the process. A major
>> > release tends to involve a little bit more effort because some new
>> problems
>> > may only show up on specific operating system platforms during -rc
>> testing.
>>
>> +1
>>
>> --
>> Johan
>
>
>
> Hi all,
>
> I agree that a 1.14.3 release makes sense for a first-time RM, to be
> followed by a 1.15.0 at a later stage. (Also, there are some unresolved
> issues regarding 1.15.0, such as the above-mentioned question of checksum
> algorithms, that may require more development, but 1.14.3 should be much
> more straightforward.)
>
> At this moment I cannot promise to volunteer for this cycle but I will try
> to do so.
>
> Before I do, I need to get my system bootstrapped properly. I can build
> SVN and run most of the testsuite, but one pesky longstanding issue I have
> had is with the JavaHL bindings. That will require a separate thread and
> I'll describe the issues there. I'll check my notes to see what else might
> be a showstopper on my end.
>

I've successfully built and tested the JavaHL bindings at one point and I
think I can dig out my notes to help you if needed.


>
> If I can get set up and solve the problems I've encountered previously in
> a reasonable time frame (and everyone's help is appreciated of course) I'll
> volunteer.
>

That would be great! I'd volunteer to do the next release after this.

There were some Docker scripts floating around (Both from CMike and from
Mark). Maybe we should figure out if these can be included in /tools as a
help to set up the environment and to execute the necessary steps.

Kind regards,
Daniel


New release

2023-10-13 Thread Daniel Sahlberg
Hi,

There are quite a number of improvements waiting to be released. Can we
muster the energy to do a new release?

In trunk there are a lot of changes that warrant a 1.15, but before doing
that I think we should also go back to the discussion of changing the /
adding a checksum algorithm in the WC. That deserves a separate thread and
I'm half way through summarising the previous discussions so I'd like to
hold back 1.15 for the moment.

Still in 1.14 there have been a number of bugfixes that might be good to
get released. Maybe doing 1.14.3 first could set us up to do 1.15 a little
later?

What do you think?

Kind regards,
Daniel Sahlberg


Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `

2023-10-09 Thread Daniel Sahlberg
Den mån 9 okt. 2023 kl 10:30 skrev Yasuhito FUTATSUKI <
futat...@yf.bsdclub.org>:

>
>
> On 2023/10/09 4:14, Daniel Sahlberg wrote:
> > Den lör 7 okt. 2023 kl 06:52 skrev Yasuhito FUTATSUKI <
> > futat...@yf.bsdclub.org>:
>
> >> If svn is called with relative paths, it can only see
> >> cwd as a realpath, and absolute paths of argment paths
> >> are composed from it.  So the paths in error message we
> >> can expect here is absolute real paths or relative paths
> >> only (It assumes that relative paths passed from the
> >> command line don't contain symlinks).
>
> > I was able to reproduce the issue and thanks to Yasuhito's hints I was
> also
> > able to fix it by using absolute paths when running the svn command. I've
> > committed r1912826 with a simple fix which seems to work for me.
>
> I don't think r1912826 is a correct fix. For the main purpose of
> the test, checking SVN-4913 is surely fixed, it is correct,
> but then how the error message should be when the arguments are
> passed in the form of relative paths ?
>

Just so I understand your question, are you asking what the ACTUAL error
message should be when passed a relative path (which may contain a symlink)?

If this is the question, then it goes deep within the library where the
relative path is resolved to an absolute path. The error message has always
returned the absolute path (libsvn_client/copy.c, approx 3100). Maybe it
would have been better to return a path relative to the wc root?

Kind regards,
Daniel


Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `

2023-10-09 Thread Daniel Sahlberg
Den mån 9 okt. 2023 kl 07:05 skrev Nathan Hartman :

> On Sun, Oct 8, 2023 at 3:14 PM Daniel Sahlberg
>  wrote:
> > I was able to reproduce the issue and thanks to Yasuhito's hints I was
> also able to fix it by using absolute paths when running the svn command.
> I've committed r1912826 with a simple fix which seems to work for me.
> >
> > Nathan, can you verify if it also works for you?
>
> Thanks Daniel and Yasuhito for your help!
>
> With r1912826, the test passes now.
>

Great, thanks for confirmation!


>
> I had trouble determining what's different about this test as compared
> to others with expected errors containing wc paths. As I described
> previously, I experimented with different ways to construct the
> expected_error, but I hadn't considered to call the svn command itself
> with absolute paths!!
>
> Just to tie up all the loose ends, replying to some earlier questions:
>
> On Fri, Oct 6, 2023 at 3:33 PM Daniel Sahlberg
>  wrote:
> > From the test:
> >
> > [[[
> >   was_cwd = os.getcwd()
> >   from_path = os.path.abspath(sbox.ospath(''))
> >   to_path = os.path.abspath(sbox.ospath('F/B'))
> >   os.chdir(wc_dir)
> > ]]]
> >
> > Would be interesting to know the values of was_cwd, return value of
> sbox.ospath("") and how that translates with os.path.abspath() and the same
> for "F/B".
>
> Even though this is a moot point now, before you wrote your reply I
> instrumented the test and printed these out, so for completeness:
>
> was_cwd = /home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline
>
> from_path:
> sbox.ospath('') = svn-test-work/working_copies/copy_tests-17
> os.path.abspath(sbox.ospath('')) =
>
> /home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svn-test-work/working_copies/copy_tests-17
>
> to_path:
> sbox.ospath('F/B') = svn-test-work/working_copies/copy_tests-17/F/B
> os.path.abspath(sbox.ospath('F/B')) =
>
> /home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svn-test-work/working_copies/copy_tests-17/F/B
>
> Expected:
> svn: E27: Cannot move path
>
> '/home/nathan/ramdrive/svndev/svn\-trunk/subversion/tests/cmdline/svn\-test\-work/working_copies/copy_tests\-17'
> into its own child
>
> '/home/nathan/ramdrive/svndev/svn\-trunk/subversion/tests/cmdline/svn\-test\-work/working_copies/copy_tests\-17/F/B'
>
> Actual:
> svn: E27: Cannot move path
>
> '/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17'
> into its own child
>
> '/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17/F/B'
>
> >> I am assuming that in the expected path, 'svn-test-work' is probably a
> >> symlink to ~/ramdrive/svndev/ramdisk. I can't check this at the moment
> >> but I will try rerunning later and I'll inspect these locations.
>
> Yes, svn-test-work (subversion/tests/cmdline/svn-test-work) is indeed
> a symlink in my case.
>

I was able to get the test cases to fail with the exact same setup. It
seems the \- in the "expected" was a red herring, it obviously doesn't
affect the Expected/Actual check so I guess it is added in the printout
somehow.

Starting the test cases directly and adding the --development option was
very helpful in debugging:

[[[
cd subversion/tests/cmdline/
./copy_tests.py 17 --development
]]]


>
> > The symlink being an artefact from your custom build system? Sounds like
> you might be on to something.
>
> The tools/dev/unix-build/Makefile.svn (svn-check-prepare-ramdisk
> target) constructs this if you specify RAMDISK in the call to 'make',
> e.g.,:
>
> $ make svn-check RAMDISK=$SVN_DEV/ramdisk
>

Are the custom targets in Makefile.svn something that should be added to
the repository? I was blown away by the performance running the tests on a
ramdisk, would be really useful to have, but I had to manipulate each test
to replace the work folders with symlinks before running make check.

Kind regards,
Daniel


Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `

2023-10-08 Thread Daniel Sahlberg
Den lör 7 okt. 2023 kl 06:52 skrev Yasuhito FUTATSUKI <
futat...@yf.bsdclub.org>:

> Hi,
>
> On 2023/10/07 4:33, Daniel Sahlberg wrote:
> > Den fre 6 okt. 2023 kl 19:34 skrev Nathan Hartman <
> hartman.nat...@gmail.com
>
> >> The interesting thing, though, is that many tests in the test suite
> >> compare expected error strings to actual error strings, involving
> >> paths etc., and those are passing.
> >>
> >> What I also find interesting, besides the added escaping, is that the
> >> expected path is very different than the actual path. Showing these
> >> here without escaping for illustration:
> >>
> >> Expected:
> >>
> '/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svn-test-work/working_copies/copy_tests\-17'
> >> Actual:
> >>
>  '/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17'
> >>
> >> The actual path is correct.
>
> In the copy_tests.wc_move_parent_into_child, it executes
> 'svn mv . F/B', which specified paths as relative paths,
> but its error message uses absolute real paths.
>
> On the other hand, expected paths are composed from
> sbox.wc_dir which may contain symlinks, i.e. it may not
> be real paths.
>
> If svn is called with relative paths, it can only see
> cwd as a realpath, and absolute paths of argment paths
> are composed from it.  So the paths in error message we
> can expect here is absolute real paths or relative paths
> only (It assumes that relative paths passed from the
> command line don't contain symlinks).
>
>
I was able to reproduce the issue and thanks to Yasuhito's hints I was also
able to fix it by using absolute paths when running the svn command. I've
committed r1912826 with a simple fix which seems to work for me.

Nathan, can you verify if it also works for you?

Kind regards,
Daniel Sahlberg


Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `

2023-10-06 Thread Daniel Sahlberg
Den fre 6 okt. 2023 kl 19:34 skrev Nathan Hartman :

> On Fri, Oct 6, 2023 at 10:50 AM Daniel Sahlberg
>  wrote:
> >
> > Den fre 6 okt. 2023 kl 06:26 skrev Nathan Hartman <
> hartman.nat...@gmail.com>:
> (snip)
> >> This is on trunk, as of r1912743 (the latest right now), Linux Debian,
> >> Python 3.7.5, building SVN with --enable-maintainer-mode (this might
> >> account for some extra text in the actual error message?).
> >
> >
> > I also build with --enable-maintainer-mode and it has not caused
> problems for me previously.
>
> Thanks. In retrospect I don't think this is an issue.
>
> Replying inline below...
>
> >> [[[
> >> W: Unexpected output
> >> W: EXPECTED STDERR (regexp, match_all=False):
> >> W: | svn: E27: Cannot move path
> >>
> '/home/nathan/ramdrive/svndev/svn\-trunk/subversion/tests/cmdline/svn\-test\-work/working_copies/copy_tests\-17'
> >> into its own child
> >>
> '/home/nathan/ramdrive/svndev/svn\-trunk/subversion/tests/cmdline/svn\-test\-work/working_copies/copy_tests\-17/F/B'
> >
> >
> > Note the "\-", ie escaping of the - character. Can you check if removing
> the re.escape (reverting r1910129) helps? Since you are on *nix, the
> re.escape should not be needed (it was only there to account for the \ path
> separator in Windows).
>
> Ok I will check this.
>
> >> W: ACTUAL STDERR:
> >> W: | subversion/svn/move-cmd.c:102,
> >> W: | subversion/svn/util.c:557,
> >> W: | subversion/libsvn_client/copy.c:3495,
> >> W: | subversion/libsvn_client/copy.c:3093:
> (apr_err=SVN_ERR_UNSUPPORTED_FEATURE)
> >> W: | svn: E27: Cannot move path
> >>
> '/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17'
> >> into its own child
> >>
> '/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17/F/B'
> >> W: CWD:
> /home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17
> >
> >
> > Note the extra "/ramdisk" here, between svndev and svn-trunk, missing
> from the expected error previously.
> >
> > What is the actual layout of your system? Do you have any specific
> arguments when starting the test suite affecting the location of the
> test-repo/wc:s?
>
> Yes, this ramdrive/.../ramdisk oddity does exist. I am using a
> (modified to fit my system) version of the Makefile.svn in
> tools/dev/unix-build, which builds all the dependencies and makes
> running tests easy with [local|svn|serf] x [bdb|fsfs|fsx]. This
> makefile can take a RAMDISK variable, e.g.,:
>
> $ make svn-check RAMDISK=$SVN_DEV/ramdisk JAVA=no BRANCH=1.14.x
>
> and then it puts all the test suite's temporary working copies in the
> specified directory.
>
> In the past, the whole tree was on Flash storage and I was using the
> RAMDISK variable as above for testing. More recently, to avoid Flash
> wear from lots of development activities, I started mounting a ramdisk
> on a mountpoint in my home directory (/home/nathan/ramdrive). $SVN_DEV
> points to /home/nathan/ramdrive/svndev. And inside there, for
> historical reasons, is a directory called ramdisk, which is still
> being passed in the RAMDISK variable. It is just a directory, not a
> mountpoint for a second ramdisk. So, yes, it's stupid, but there is in
> fact ~/ramdrive/svndev/ramdisk/...!! (And inconsistent naming:
> ramdrive, ramdisk...)
>
> The interesting thing, though, is that many tests in the test suite
> compare expected error strings to actual error strings, involving
> paths etc., and those are passing.
>
> What I also find interesting, besides the added escaping, is that the
> expected path is very different than the actual path. Showing these
> here without escaping for illustration:
>
> Expected:
> '/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svn-test-work/working_copies/copy_tests\-17'
> Actual:
>  '/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17'
>
> The actual path is correct.
>

>From the test:

[[[
  was_cwd = os.getcwd()
  from_path = os.path.abspath(sbox.ospath(''))
  to_path = os.path.abspath(sbox.ospath('F/B'))
  os.chdir(wc_dir)
]]]

Would be interesting to know the values of was_cwd, return value of
sbox.ospath("") and how that translates with os.path.abspath() and the same
for "F/B".

I am assuming that in the expected path, 'svn-test-work' is probably a
> symlink to ~/ramdrive/svndev/ramdisk. I can't check this at the moment
> but I will try rerunning later and I'll inspect these locations.
>

The symlink being an artefact from your custom build system? Sounds like
you might be on to something.


>
> Is there a way to single-step through Python code that I could use to
> follow the test suite logic?
>

I've said it before; I'm no Python expert but maybe
https://docs.python.org/3/library/pdb.html? No idea if that will work with
the build system.

Kind regards,
Daniel


Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `

2023-10-06 Thread Daniel Sahlberg
Den fre 6 okt. 2023 kl 06:26 skrev Nathan Hartman :

> On Mon, May 29, 2023 at 10:47 PM Jun Omae  wrote:
> >
> > On 2023/05/29 20:09, Daniel Sahlberg wrote:
>
[...]

> > >
> > > The following patch could fix it and verified (applying
> `re.escape` to the paths).
>
[...]

>
> I'm running into a strange failure with the above test (copy_tests.py
> 17: move WC WC/subdir). The paths we construct for expected_error are
> not matching the actual error in my system. I tried various things but
> nothing seems to help.
>
> Looking around the testsuite, I tried changing '%s' to '.*%s'; also I
> tried removing os.path.abspath when constructing from_path and
> to_path; I made various other attempts, all aimed at minimally
> matching the known portion of the path while ignoring the extra stuff.
> Unfortunately I was not successful.
>
> This is on trunk, as of r1912743 (the latest right now), Linux Debian,
> Python 3.7.5, building SVN with --enable-maintainer-mode (this might
> account for some extra text in the actual error message?).
>

I also build with --enable-maintainer-mode and it has not caused problems
for me previously.


>
> All other tests are passing for me, for all [local|svn|serf] x [fsfs].
>
> Fail log:
>
> [[[
> W: Unexpected output
> W: EXPECTED STDERR (regexp, match_all=False):
> W: | svn: E27: Cannot move path
>
> '/home/nathan/ramdrive/svndev/svn\-trunk/subversion/tests/cmdline/svn\-test\-work/working_copies/copy_tests\-17'
> into its own child
>
> '/home/nathan/ramdrive/svndev/svn\-trunk/subversion/tests/cmdline/svn\-test\-work/working_copies/copy_tests\-17/F/B'
>

Note the "\-", ie escaping of the - character. Can you check if removing
the re.escape (reverting r1910129) helps? Since you are on *nix, the
re.escape should not be needed (it was only there to account for the \ path
separator in Windows).

W: ACTUAL STDERR:
> W: | subversion/svn/move-cmd.c:102,
> W: | subversion/svn/util.c:557,
> W: | subversion/libsvn_client/copy.c:3495,
> W: | subversion/libsvn_client/copy.c:3093:
> (apr_err=SVN_ERR_UNSUPPORTED_FEATURE)
> W: | svn: E27: Cannot move path
>
> '/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17'
> into its own child
>
> '/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17/F/B'
> W: CWD:
> /home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17
>

Note the extra "/ramdisk" here, between svndev and svn-trunk, missing from
the expected error previously.

What is the actual layout of your system? Do you have any specific
arguments when starting the test suite affecting the location of the
test-repo/wc:s?


> W: EXCEPTION: SVNUnmatchedError
> Traceback (most recent call last):
>   File
> "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svntest/main.py",
> line 1996, in run
>

No "ramdisk" here...


> rc = self.pred.run(sandbox)
>   File
> "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svntest/testcase.py",
> line 178, in run
> result = self.func(sandbox)
>   File
> "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/copy_tests.py",
> line 1299, in wc_move_parent_into_child
> '.', 'F/B')
>   File
> "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svntest/actions.py",
> line 340, in run_and_verify_svn
> expected_exit, *varargs)
>   File
> "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svntest/actions.py",
> line 380, in run_and_verify_svn2
> expected_stdout, expected_stderr)
>   File
> "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svntest/verify.py",
> line 532, in verify_outputs
> compare_and_display_lines(message, label, expected, actual, raisable)
>   File
> "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svntest/verify.py",
> line 505, in compare_and_display_lines
> raise raisable
> svntest.main.SVNUnmatchedError
> FAIL:  copy_tests.py 17: move WC WC/subdir
> ]]]
>
> Any ideas?
>
> Cheers,
> Nathan
>

Kind regards,
Daniel


Re: svn commit: r1912743 - in /subversion/trunk/subversion/bindings/swig/python: svn/fs.py tests/fs.py

2023-10-05 Thread Daniel Sahlberg
Den tors 5 okt. 2023 kl 14:18 skrev Nathan Hartman :

> Thanks for taking care of that. Looks good to me.
>
> I am considering nominating this for backport to 1.14.x. (At least, I
> don't see a reason not to, since Python2 remains supported.)
>

Why not, since we went the way of keeping Python2 support. I've nominated
in r1912747. It should be able to go in with only two votes so if you would
like to to the honors?

Kind regards,
Daniel


Re: Close SVN-1778

2023-10-05 Thread Daniel Sahlberg
Den tors 5 okt. 2023 kl 07:10 skrev Yasuhito FUTATSUKI <
futat...@yf.bsdclub.org>:

> On 2023/10/05 5:28, Nathan Hartman wrote:
> > On Wed, Oct 4, 2023 at 10:59 AM Yasuhito FUTATSUKI <
> futat...@yf.bsdclub.org>
> > wrote:
>
> >> Nothing but I don't want that we actively drop Python 2 support,
> >> at least SWIG Python bindings even in trunk, at least now.
> >
> >
> > I agree we shouldn't break Python 2 support if it's currently working. At
> > least, let's delay breaking it until 1.14.x is EOL, to avoid breakage on
> > very long term support OS. (I think that if we start removing Py2 compat
> on
> > trunk now, we risk mistakenly backporting the changes to 1.14.x at some
> > later time.)
>
> More over, I know and perhaps we all know one large site that is still
> using
> Python 2 bindings through ViewVC 1.1.x, although I don't know what version
> of Subversion it uses.
>
> > However, I see the point that it would be nice to make the code clear and
> > not require us to remember that OSError needs to change to
> > FileNotFoundError in the future.
> >
> > We could add a Python2 check and handle it differently. In the future
> when
> > we decide to actively remove Python2 support, we can grep for all the
> > Python2 checks and remove that code. I know it makes the code
> long-winded.
> > :-/
>
> Or just note it in a comment, then it does not need extra cost in
> execution.
>
> Clean up of Python2 support code is like a conversion from pure Python 2
> code
> to Python 2/Python 3 dual support code, especially in SWIG Python bindings,
> because it uses py3c, Python C API wrapper to absorb difference between for
> Python 2 and for Python 3. To remove py3c dependency, we should use native
> Python 3 C APIs. Anyways it need a whole code review, not just grep the
> markers, although the markers will help us greatly.
>

Agreed that it is a big undertaking. I do think it should be done but we
should probably announce it well ahead to let anyone be prepared to update.

When I get some time, I would like to start a new thread about the future
development (release schedule, features for each release) and then we can
publish something of what should go into each release.

Anyway, thanks for the review and feedback. I've commtted r1912743 catching
OSError and checking ENOENT, with a comment to change it when we remove
Python 2 support.

Kind regards,
Daniel Sahlberg


Re: Close SVN-1778

2023-10-04 Thread Daniel Sahlberg
Den ons 4 okt. 2023 kl 06:58 skrev Nathan Hartman :

> Thanks for the review! Committed in r1912724. More below:
>

Great!


> I see only one issue: FileNotFoundError is new in Python 3, so Python 2
> will fail with a NameError when it sees that. However: On Python 3,
> FileNotFoundError inherits from OSError, OSError exists in Python 2, and
> OSError in both Pythons has the strerror attribute. So (unless I'm missing
> something) we should catch OSError instead of FileNotFoundError here for
> compatibility.
>

Good point. If we catch OSError we should check for err.errno =
errno.ENOENT (as is done in subversion/bindings/swig/python/tests/fs.py).

I don't think anything has formally been decided regarding Python 2
support, we have tried hard to keep Python 2 compatibility in 1.14 but for
/trunk (and a coming 1.15 release) my opinion is that we should remove it.
This should probably be broken out to a separate thread and documented
somewhere.

I think catching FileNotFoundError is cleaner than OSError + check for
ENOENT. Also there is no immediate need to backport to 1.14 (this is just a
better error message). With that in mind, I'm leaning towards keeping
FileNotFoundError (we should probably update tests/fs.py to follow the same
pattern).

Yasuhito / Jun: Since you are Python experts, do you have any comments?

Kind regards,
Daniel

>


Re: Close SVN-1778

2023-10-03 Thread Daniel Sahlberg
Den tis 3 okt. 2023 kl 02:44 skrev Nathan Hartman :

> On Mon, Oct 2, 2023 at 5:39 AM Daniel Sahlberg
>  wrote:
> > Den sön 1 okt. 2023 kl 21:50 skrev Nathan Hartman <
> hartman.nat...@gmail.com>:
> (snip)
> >> This (somewhat platform-specific) "gotcha" isn't completely obvious
> from a first glance at the code.
> >>
> >> Your explanation above makes the failure mode and its reasons more
> clear.
> >>
> >> So, to close SVN-1778, I would suggest only to add some documentation
> of the above fact to the function. I'll be happy to compose a suggested
> docstring.
> >
> >
> > Please do!
>
> Suggested docstring (also attached as a patch):
>
> """Perform diff and return a file object from which the output can
> be read.
>
> When DIFFOPTIONS is None (the default), use svn's internal diff.
>
> With any other DIFFOPTIONS, exec the external diff found on PATH,
> passing it DIFFOPTIONS. On Windows, exec diff.exe rather than
> diff. If a diff utility is not installed or found on PATH, throws
> FileNotFoundError. Caveat: On some systems, including Windows, an
> external diff may not be available unless installed and added to
> PATH manually.
> """
>
> More below:
>

This looks good to me!



>
> >> I don't know how it fails (with a cryptic traceback message?) but if we
> want to get fancy, perhaps a failure could give some user-friendly hint
> about things to check for (such as whether a diff tool is available and
> diffoptions are correct).
> >
> >
> > It fails with a FileNotFoundError error so it should be trivial to
> catch. I don't know the proper conventions on error handling in the
> bindings (or in Python in general), I guess we should still throw an error,
> hoping that whoever use the FileDiff class will catch it and handle in an
> intelligent way.
> >
> > Another way would be to use the internal diff in case of an exception,
> but it would be less apparent (I would rather raise an error, even if it
> just means ignoring the FileNotFoundError and documenting the cause).
>
> In this case, I would rather leave the implementation as it is
> currently. I think that calling the internal diff when an external
> diff is expected (with some unknown options) would be worse than
> failing with FileNotFoundError here. Even if the exception is not
> caught, the docstring should provide the useful hint. :-)
>

Agreed! Being fancy, how about:

[[[
Index: subversion/bindings/swig/python/svn/fs.py
===
--- subversion/bindings/swig/python/svn/fs.py   (revision 1912696)
+++ subversion/bindings/swig/python/svn/fs.py   (working copy)
@@ -170,8 +170,13 @@
 + [self.tempfile1, self.tempfile2]

   # open the pipe, and return the file object for reading from the
child.
-  p = _subprocess.Popen(cmd, stdout=_subprocess.PIPE, bufsize=-1,
-close_fds=_sys.platform != "win32")
+  try:
+p = _subprocess.Popen(cmd, stdout=_subprocess.PIPE, bufsize=-1,
+  close_fds=_sys.platform != "win32")
+  except FileNotFoundError as e:
+e.strerror = "External diff command not found in PATH"
+raise
+
   return _PopenStdoutWrapper(p)

 else:
]]]

In my testing that gave me a FileNotFoundError with a custom error message
instead of the generic "No such file or directory".

Kind regards,
Daniel


Re: svn commit: r1912500 - in /subversion/trunk/subversion/bindings/swig: python/libsvn_swig_py/swigutil_py.c python/libsvn_swig_py/swigutil_py.h python/svn/delta.py python/svn/repos.py svn_delta.i sv

2023-10-02 Thread Daniel Sahlberg
mån 2 okt. 2023 kl. 16:54 skrev Yasuhito FUTATSUKI :

>
> > I've added my vote for the backports, because of my inexperience I've
> only
> > set +0 but binding fixes should be OK to backport with only one +1 and
> one
> > +0. Any fixes from [1] can be added to the same backport (as I read it
> > documentation fixes can be backported without votes).
>
> Thank you for the review and tests. All your suggestinos look reasonable,
> so I commited the fix in r1912691, and added it to backport nomination
> group r1912500.
>

I think we can even move it to approved. Would you like the honor?

Kind regards,
Daniel


Re: Close SVN-1778

2023-10-02 Thread Daniel Sahlberg
Den sön 1 okt. 2023 kl 21:50 skrev Nathan Hartman :

> On Sun, Oct 1, 2023 at 9:58 AM Daniel Sahlberg <
> daniel.l.sahlb...@gmail.com> wrote:
> >
> > Hi,
> >
> > Issue SVN-1778 [1] is about the Python bindings, class FileDiff, where
> get_pipe() called an external program diff (under Windows: diff.exe). Since
> diff.exe has to be installed (and added to PATH) manually on Windows the
> function would fail. The same is true on other os:es but diff is probably
> more commonly present by default.
> >
> > r1824410 added support for running the internal Subversion diff
> functions (svn.diff.file_diff_2 and svn.diff.file_output_unified4) as long
> as the argument diffoptions is None (the default). Since diffoptions is
> passed as arguments to the diff command, it seems reasonable to require the
> presence of diff (or diff.exe) if one uses diffoptions.
> >
> > I'm suggesting to close this issue - it is not completely solved but
> when using diffoptions,it is probably expected that this arguments is
> passed on to whatever diff/diff.exe executable is present on the system and
> thus it is not unreasonable to require the end user to install diff.exe
> >
> > Kind regards,
> >
> > Daniel Sahlberg
> >
> > [1] https://issues.apache.org/jira/browse/SVN-1778
>
>
> This (somewhat platform-specific) "gotcha" isn't completely obvious from a
> first glance at the code.
>
> Your explanation above makes the failure mode and its reasons more clear.
>
> So, to close SVN-1778, I would suggest only to add some documentation of
> the above fact to the function. I'll be happy to compose a suggested
> docstring.
>

Please do!


>
> I don't know how it fails (with a cryptic traceback message?) but if we
> want to get fancy, perhaps a failure could give some user-friendly hint
> about things to check for (such as whether a diff tool is available and
> diffoptions are correct).
>

It fails with a FileNotFoundError error so it should be trivial to catch. I
don't know the proper conventions on error handling in the bindings (or in
Python in general), I guess we should still throw an error, hoping that
whoever use the FileDiff class will catch it and handle in an intelligent
way.

Another way would be to use the internal diff in case of an exception, but
it would be less apparent (I would rather raise an error, even if it just
means ignoring the FileNotFoundError and documenting the cause).


>
> I noticed that there aren't docstrings on most of the functions in
> subversion/bindings/swig/python/svn/fs.py, and I don't know whether that's
> deliberate. If it is, then the suggested docstring could be a comment
> instead.
>

There are docstrings in the other py files so I suppose this was just
overlooked.

Kind regards,
Daniel


Re: svn commit: r1912500 - in /subversion/trunk/subversion/bindings/swig: python/libsvn_swig_py/swigutil_py.c python/libsvn_swig_py/swigutil_py.h python/svn/delta.py python/svn/repos.py svn_delta.i sv

2023-10-01 Thread Daniel Sahlberg
Den sön 24 sep. 2023 kl 12:55 skrev Yasuhito FUTATSUKI <
futat...@yf.bsdclub.org>:

> Hi,
>
> On 2023/09/24 17:53, Daniel Sahlberg wrote:
> > Den sön 24 sep. 2023 kl 07:06 skrev :
> >
> >> Author: futatuki
> >> Date: Sun Sep 24 05:06:08 2023
> >> New Revision: 1912500
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1912500=rev
> >> Log:
> >> swig-py: Use pure Python objects as edit/parse_fns3 and their decendant
> >> batons.
> >>
> >> Former implementation of the wrappers for svn_delta_editor_t and
> >> svn_repos_parse_fns3_t C APIs had some leakage of Python reference
> counts:
> >>
> >
> > [...]
> >
> > I didn't review but I hope to find some time to do this later today. The
> > issues fixed sounds like a very good improvement, thank you for your work
> > with this!
>
> Thank you for reviewing it. Now I've done a series of commits related to
> it, r1912500, r192501, r192502, r192503, r1912515, and r1912517.
>

I have reviewed these commits with only minor nits in one of them (see
separate e-mail [1]).

I have run the test suite with all the commits and it completes
successfully. I have also run it with each of r192501-r1912517 and verified
that the checks fail without r1912500. I must admit that I both Python and
SWIG are relatively new to me so I don't understand this fully but I think
the basic ideas are sound.

I've added my vote for the backports, because of my inexperience I've only
set +0 but binding fixes should be OK to backport with only one +1 and one
+0. Any fixes from [1] can be added to the same backport (as I read it
documentation fixes can be backported without votes).

Kind regards,
Daniel Sahlberg

[1] https://lists.apache.org/thread/m428zfspmm1j66606mjv7ohfq9kkr1zv


Kind regards,
Daniel Sahlberg


Close SVN-1778

2023-10-01 Thread Daniel Sahlberg
Hi,

Issue SVN-1778 [1] is about the Python bindings, class FileDiff, where
get_pipe() called an external program diff (under Windows: diff.exe). Since
diff.exe has to be installed (and added to PATH) manually on Windows the
function would fail. The same is true on other os:es but diff is probably
more commonly present by default.

r1824410 added support for running the internal Subversion diff functions
(svn.diff.file_diff_2 and svn.diff.file_output_unified4) as long as the
argument diffoptions is None (the default). Since diffoptions is passed as
arguments to the diff command, it seems reasonable to require the presence
of diff (or diff.exe) if one uses diffoptions.

I'm suggesting to close this issue - it is not completely solved but when
using diffoptions,it is probably expected that this arguments is passed on
to whatever diff/diff.exe executable is present on the system and thus it
is not unreasonable to require the end user to install diff.exe

Kind regards,

Daniel Sahlberg

[1] https://issues.apache.org/jira/browse/SVN-1778


Re: svn commit: r1912515 - /subversion/trunk/subversion/bindings/swig/python/tests/repository.py

2023-10-01 Thread Daniel Sahlberg
Hi,

Some minor nits in this commit:

Den sön 24 sep. 2023 kl 11:02 skrev :

> Author: futatuki
> Date: Sun Sep 24 09:02:05 2023
> New Revision: 1912515
>

[...]

> @@ -318,6 +388,36 @@ class SubversionRepositoryTestCase(unitt
>  del subpool
>  self.assertEqual(None, editor_ref())
>
> +  def test_replay_batons_refcounts(self):
> +"""Issue SVN-4917: check ref-count of batons created and used in call
> backs"""
>

I think we mostly write "callback(s)" as a single word. (The only other
cases in the code are in the bindings - also fix? - and when "call" is used
as a verb).


> +root = fs.revision_root(self.fs, self.rev)
> +editor = BatonCollector(self.fs, root)
> +e_ptr, e_baton = delta.make_editor(editor)
> +repos.replay(root, e_ptr, e_baton)
> +for baton in editor.batons:
> +  self.assertEqual(sys.getrefcount(baton[2]), 2,
> +   "leak on baton %s after replay without errors"
> +   % repr(baton))
> +del e_baton
> +self.assertEqual(sys.getrefcount(e_ptr), 2,
> + "leak on editor baton after replay without errors")
> +
> +editor = BatonCollectorErrorOnClose(self.fs, root,
> +error_path=b'branches/v1x')
> +e_ptr, e_baton = delta.make_editor(editor)
> +self.assertRaises(SubversionException, repos.replay, root, e_ptr,
> e_baton)
> +batons= editor.batons
>

Add a space to the left of the equal sign for readability?


> +# As svn_repos_replay calls neigher close_edit callback nor abort_edit
>

Spell fix: "neither"


Kind regards,
Daniel Sahlberg


Re: Invalid escape sequences in Python scripts (was Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `)

2023-09-30 Thread Daniel Sahlberg
On 2023/07/20 04:32:43 Yasuhito FUTATSUKI wrote:
> Jun, could you please commit the patch?

I have committed this patch now, r1912632, crediting Jun.

Thanks Jun for the patch and thanks Yasuhito for review.

Kind regards,
Daniel Sahlberg


Re: build/run_tests.py, error on line 990 getattr()

2023-09-30 Thread Daniel Sahlberg
On 2023/07/30 15:22:48 Daniel Sahlberg wrote:
> Den fre 28 juli 2023 kl 06:31 skrev Jun Omae :
[...]
> > [[[
> > Index: build/run_tests.py
> > ===
> > --- build/run_tests.py  (revision 1911320)
> > +++ build/run_tests.py  (working copy)
[...]
> > ]]]
> >
> 
> Thanks for the explanation and link. I understand the code now and it all
> looks good to me (I also like that you undated main.py at the same time to
> have the same pattern).
> 
> Can you commit this?

I have committed Jun's patch in r1912626.

Kind regards,
Daniel Sahlberg


Re: svn commit: r1912500 - in /subversion/trunk/subversion/bindings/swig: python/libsvn_swig_py/swigutil_py.c python/libsvn_swig_py/swigutil_py.h python/svn/delta.py python/svn/repos.py svn_delta.i sv

2023-09-24 Thread Daniel Sahlberg
Den sön 24 sep. 2023 kl 07:06 skrev :

> Author: futatuki
> Date: Sun Sep 24 05:06:08 2023
> New Revision: 1912500
>
> URL: http://svn.apache.org/viewvc?rev=1912500=rev
> Log:
> swig-py: Use pure Python objects as edit/parse_fns3 and their decendant
> batons.
>
> Former implementation of the wrappers for svn_delta_editor_t and
> svn_repos_parse_fns3_t C APIs had some leakage of Python reference counts:
>

[...]

I didn't review but I hope to find some time to do this later today. The
issues fixed sounds like a very good improvement, thank you for your work
with this!

In the meantime, is this a fix that should be backported to 1.14?

Kind regards,
Daniel





>


Re: 1.10.x, 1.14.x and beyond

2023-09-22 Thread Daniel Sahlberg
Den fre 22 sep. 2023 kl 12:25 skrev Nathan Hartman :

> I guess [2] is an inconsistency with the site, since 1.10.x doesn't have
> more updates planned and 1.10.8 was the final release in that line. The
> inconsistency likely exists because we used to support two LTS lines in a
> staggered manner. That became too burdensome to maintain and 1.10.x went
> quite beyond its planned 4 years of support. I guess that at the very least
> the wording should change to reflect that, or maybe 1.10.8 links need to be
> taken down entirely. Thanks for pointing that out. I'll take a look when I
> get to a computer.
>

A lot of this is already in site-staging so it is just a question of
merging. I'm kind of busy this weekend but I can also try to check it some
time next week. Last time it was discussed it was agreed to also do an
announce@ email. Now it might make sense to wait for the 1.15 release and
do it all at the same time?

Kind regards,
Daniel


Re: 1.10.x, 1.14.x and beyond

2023-09-22 Thread Daniel Sahlberg
Den fre 22 sep. 2023 kl 11:50 skrev Osipov, Michael (IN IT IN) via dev <
dev@subversion.apache.org>:

> Guys,
>
> I'd like to understand what your current planning is. According to "How
> We Plan Releases" [1] Subversion 1.10.x should have been long gone, but
> [2] says otherwise.
> What is your way forward, do you plan 1.15.x, 1.14.x will obsolete
> 1.10.x finally?
>
>
There has been a discussion on the list last year and the conclusion was
that 1.10.x should be considered EOL and I was tasked to do the
announcement, but for several reasons I didn't get to it. Together with the
announcement, 1.10.x would be removed from the download page.

There is a number of backported bugfixes in the 1.14.x line and I
personally think it is likely there will be a last 1.14.x relase.

There is a significant amount of development done in /trunk that won't be
backported but only released as 1.15.x. It seems mostly completed so we
only need it tested and someone to volounteer as release manager. Is this
something you would be interested in helping with?

Kind regards,
Daniel Sahlberg


Windows Buildbot failures

2023-09-19 Thread Daniel Sahlberg
Hi,

There are two buildbot jobs svn-windows-local and svn-windows-ra that
currently fail. See for example
https://ci2.apache.org/#/builders/106/builds/104

Relevant part of error message, it seems that one Python component is
missing:

error in RunProcess._startCommand (spawnProcess not available since
pywin32 is not installed.)
Traceback (most recent call last):
  File "C:\Program
Files\Python310\lib\site-packages\buildbot_worker\runprocess.py", line
460, in start
self._startCommand()
  File "C:\Program
Files\Python310\lib\site-packages\buildbot_worker\runprocess.py", line
584, in _startCommand
self.process = self._spawnProcess(
  File "C:\Program
Files\Python310\lib\site-packages\buildbot_worker\runprocess.py", line
618, in _spawnProcess
return self._spawnAsBatch(processProtocol, executable, args, env,
  File "C:\Program
Files\Python310\lib\site-packages\buildbot_worker\runprocess.py", line
656, in _spawnAsBatch
return reactor.spawnProcess(processProtocol, executable, argv, env,
  File "C:\Program
Files\Python310\lib\site-packages\twisted\internet\posixbase.py", line
419, in spawnProcess
raise NotImplementedError(
NotImplementedError: spawnProcess not available since pywin32 is not installed.


Does anyone have access to this server to install pywin32?

Kind regards,
Daniel Sahlberg


Re: Subversion Exception - assertion failed

2023-09-14 Thread Daniel Sahlberg
Den tis 12 sep. 2023 kl 12:19 skrev Matthias Klose :

> Hi,
>
> I want to list a failing assertion.
>
> What have I done?:
> Preparing a merge on a directory with Doxygen configuration files (maybe
> not usefull, but I did it).
> choosing options: Merge a range of revisions / all revisions / Compare
> whitespaces
> then running [Test merge]
>

Can you reproduce this error using the command line client? You can install
it from the TortoiseSVN installer by choosing the Command line tools.

You should probably do some variation of svn merge possibly with the
--dry-run option.


> -> this leads into the following exception:
>
> ---
> Subversion Exception!
> ---
> Subversion encountered a serious problem.
> Please take the time to report this on the Subversion mailing list
> with as much information as possible about what
> you were trying to do.
> But please first search the mailing list archives for the error message
> to avoid reporting the same problem repeatedly.
> You can find the mailing list archives at
> https://subversion.apache.org/mailing-lists.html


Despite the advice in the error message, it is usually best to report to
the TortoiseSVN mailing list unless you are also able to reproduce it with
the command line tool. In most cases it has turned out that TortoiseSVN is
calling the Subversion library with faulty and/or inconsistent options.See
https://groups.google.com/g/tortoisesvn


>
> Subversion reported the following
> (you can copy the content of this dialog
> to the clipboard using Ctrl-C):
>
> In file
>
>  
> 'D:\Development\SVN\Releases\TortoiseSVN-1.14.5\ext\subversion\subversion\libsvn_client\merge.c'
>  line 4994: assertion failed (*gap_start < *gap_end)
>

The assert you have hit has the following comment:
  /* ### Issue #4132:
 ### This assertion triggers [...]
 ### when a node is replaced by an older copy of itself. */

Does this make sense to you when you look at your repository?

I tried to create a naive repository and I don't trigger this assertion so
obviously the repository has to have some certain properties.


> ---
> OK
> ---
>
> Remark:
>
> if I run [Merge] instead of [Test Merge]
> then NO exception appears
> and Merge ends in my case with the following message:
> "Error: '...foo_1' must
> Error:  be ancestrally related to
> Error:  '...foo_2'  "
>

Are you trying to merge changes from one path that is not related (ie,
copied) to the other?

I was able to create a repository where I got the same error message from
the [Merge] button, while I got a completely different error message from
the [Test Merge] button. This lead me to believe that [Merge] and [Test
Merge] is calling the Subversion libraries with different arguments.
However I don't have time to investigate this further at the moment.


>
> Hope this helps to fix the failing assertion.
>

If you can give some more details about your repository including previous
merges (even better if you can write a reproduction receipt) that would be
great. And consider reporting this on the TortoiseSVN mailing list if you
think that it is a TortoiseSVN-only problem.

Kind regards,
Daniel Sahlberg


  1   2   3   4   5   >