libsvn_cmdline (was: svn_opt_args_to_target_array/svn_client_args_to_target_array issue)

[Timofei Zhakov <ti...@chemodax.net>]

>
> See? SEE!!?? I *told* you it was a mess. :D
>
> This stuff should all be in libsvn_subr/cmdline.c. I don't even remember
> any more how this happened. I think that at one point I was even
> considering creating a new library that would contain only the features
> that command-line programs need; basically, command line conversion to
> UTF-8 and the argument parser. But it's tied in with libsvn_subr anyway,
> and that's already linked everywhere.
>
> I suggest the goal should be to move this out of libsvn_cmdline and get
> rid of the duplicates. Before that, some judicious use of svn blame and svn
> log might give you an idea why it's currently done that way. I really don't
> know, but we should, before we change anything.
>

Hi,

I really liked this idea and I would like to discuss it more.

(I guess this is a reason for re-threading)

Subversion is a very organised project. This is why I like developing it.
However, it is true that cmdline code is messy.

I think it would be cool to completely reorganise it and put it into a
separate library. Similar to libsvn_diff or libsvn_delta (but I guess these
were separate from the early days).

This will be also a reason to review the original problem with
svn_client_args_to_target_array(), since there would be a good place to
design the single version.

I see it like the following files might go to the new library:

- most of subversion\libsvn_subr\opt.c, except svn_opt_revision_t stuff
- subversion\libsvn_subr\opt.h as a dependency
- subversion\libsvn_subr\cmdline.c entirely
- subversion\libsvn_client\cmdline.c

Unfortunately, we can't do this with utf8 related code, since there are a
few usages in our main libraries.

This will be really beneficial for us due to a bunch of reasons:

1. The code would be more modular, which is good.
2. Command line related code would not be loaded in case subversion is
consumed as a library.
3. It will be clear which api to use when developing a program.
4. It opens up an opportunity to separate some of those files onto a few
more. For example, something like svn_editor.c can be simply factored out
and maybe simplify the code.

However, there is one huge problem. Due to backward compatibility policy,
libsvn_subr still must export those symbols, even if they are deprecated.
The thing is we can't code simple one-line functions that wrap
libsvn_cmdline into libsvn_subr, since libsvn_subr should not link with
libsvn_cmdline. I don't see any resolutions yet...

That's it for now...

-- 
Timofei Zhakov

Re: libsvn_cmdline

[Timofei Zhakov <ti...@chemodax.net>]

On Wed, May 28, 2025 at 10:32 AM Branko Čibej <br...@apache.org> wrote:

> On 28. 5. 25 00:11, Timofei Zhakov wrote:
>
> See? SEE!!?? I *told* you it was a mess. :D
>>
>> This stuff should all be in libsvn_subr/cmdline.c. I don't even remember
>> any more how this happened. I think that at one point I was even
>> considering creating a new library that would contain only the features
>> that command-line programs need; basically, command line conversion to
>> UTF-8 and the argument parser. But it's tied in with libsvn_subr anyway,
>> and that's already linked everywhere.
>>
>> I suggest the goal should be to move this out of libsvn_cmdline and get
>> rid of the duplicates. Before that, some judicious use of svn blame and svn
>> log might give you an idea why it's currently done that way. I really don't
>> know, but we should, before we change anything.
>>
>
> Hi,
>
> I really liked this idea and I would like to discuss it more.
>
> (I guess this is a reason for re-threading)
>
> Subversion is a very organised project. This is why I like developing it.
> However, it is true that cmdline code is messy.
>
> I think it would be cool to completely reorganise it and put it into a
> separate library. Similar to libsvn_diff or libsvn_delta (but I guess these
> were separate from the early days).
>
> This will be also a reason to review the original problem with
> svn_client_args_to_target_array(), since there would be a good place to
> design the single version.
>
> I see it like the following files might go to the new library:
>
> - most of subversion\libsvn_subr\opt.c, except svn_opt_revision_t stuff
> - subversion\libsvn_subr\opt.h as a dependency
> - subversion\libsvn_subr\cmdline.c entirely
> - subversion\libsvn_client\cmdline.c
>
> Unfortunately, we can't do this with utf8 related code, since there are a
> few usages in our main libraries.
>
> This will be really beneficial for us due to a bunch of reasons:
>
> 1. The code would be more modular, which is good.
> 2. Command line related code would not be loaded in case subversion is
> consumed as a library.
> 3. It will be clear which api to use when developing a program.
> 4. It opens up an opportunity to separate some of those files onto a few
> more. For example, something like svn_editor.c can be simply factored out
> and maybe simplify the code.
>
> However, there is one huge problem. Due to backward compatibility policy,
> libsvn_subr still must export those symbols, even if they are deprecated.
> The thing is we can't code simple one-line functions that wrap
> libsvn_cmdline into libsvn_subr, since libsvn_subr should not link with
> libsvn_cmdline. I don't see any resolutions yet...
>
>
>
> Exactly, this is the real issue here. It's also probably the reason for
> having two "to_target_array" functions: the one in libsvn_client does
> client-type checks, like finding the repository root. So even if we moved
> that to a libsvn_cmdline, it would still have to link with libsvn_client
> (but not the other way around). We'd have to split, for example, the
> resolution of repos-relative URLs out of that function to avoid linking
> with libsvn_client, and that opens another can of worms.
>
> All the private functions (like svn_opt__*) can be moved to the new
> library, but there's another problem: Existing users of our libraries,
> like, I don't know, such irrelevant things like TortoiseSVN ... would have
> to load the new library if we decided to move those functions. Or link with
> its static version.
>
> Anyway, it's a huge amount of work for, as you say, a lot of potential
> benefit for our tools, but extra issues for integrators of our code. And
> it's not really urgent.
>
> -- Brane
>

Exactly!

I'd like to suggest another crazy idea. What if we separate the whole
libsvn_subr into many separate libraries for each functionality it is
doing. For example, one for core (pools, strings), one for IO operation,
encoding stuff, checksums/compression, and etc... This would follow that
the whole subr library could be deprecated and wrap those small libraries
for backward compatibility reasons.

Yeah, it's a bit stupid...

By the way, this is what apr should be designed like.

-- 
Timofei Zhakov

Re: libsvn_cmdline

[Branko Čibej <br...@apache.org>]

On 28. 5. 25 00:11, Timofei Zhakov wrote:
>
>     See? SEE!!?? I *told* you it was a mess. :D
>
>     This stuff should all be in libsvn_subr/cmdline.c. I don't even
>     remember any more how this happened. I think that at one point I
>     was even considering creating a new library that would contain
>     only the features that command-line programs need; basically,
>     command line conversion to UTF-8 and the argument parser. But it's
>     tied in with libsvn_subr anyway, and that's already linked everywhere.
>
>     I suggest the goal should be to move this out of libsvn_cmdline
>     and get rid of the duplicates. Before that, some judicious use of
>     svn blame and svn log might give you an idea why it's currently
>     done that way. I really don't know, but we should, before we
>     change anything.
>
>
> Hi,
>
> I really liked this idea and I would like to discuss it more.
>
> (I guess this is a reason for re-threading)
>
> Subversion is a very organised project. This is why I like developing 
> it. However, it is true that cmdline code is messy.
>
> I think it would be cool to completely reorganise it and put it into a 
> separate library. Similar to libsvn_diff or libsvn_delta (but I guess 
> these were separate from the early days).
>
> This will be also a reason to review the original problem with 
> svn_client_args_to_target_array(), since there would be a good place 
> to design the single version.
>
> I see it like the following files might go to the new library:
>
> - most of subversion\libsvn_subr\opt.c, except svn_opt_revision_t stuff
> - subversion\libsvn_subr\opt.h as a dependency
> - subversion\libsvn_subr\cmdline.c entirely
> - subversion\libsvn_client\cmdline.c
>
> Unfortunately, we can't do this with utf8 related code, since there 
> are a few usages in our main libraries.
>
> This will be really beneficial for us due to a bunch of reasons:
>
> 1. The code would be more modular, which is good.
> 2. Command line related code would not be loaded in case subversion is 
> consumed as a library.
> 3. It will be clear which api to use when developing a program.
> 4. It opens up an opportunity to separate some of those files onto a 
> few more. For example, something like svn_editor.c can be simply 
> factored out and maybe simplify the code.
>
> However, there is one huge problem. Due to backward compatibility 
> policy, libsvn_subr still must export those symbols, even if they are 
> deprecated. The thing is we can't code simple one-line functions that 
> wrap libsvn_cmdline into libsvn_subr, since libsvn_subr should not 
> link with libsvn_cmdline. I don't see any resolutions yet...


Exactly, this is the real issue here. It's also probably the reason for 
having two "to_target_array" functions: the one in libsvn_client does 
client-type checks, like finding the repository root. So even if we 
moved that to a libsvn_cmdline, it would still have to link with 
libsvn_client (but not the other way around). We'd have to split, for 
example, the resolution of repos-relative URLs out of that function to 
avoid linking with libsvn_client, and that opens another can of worms.

All the private functions (like svn_opt__*) can be moved to the new 
library, but there's another problem: Existing users of our libraries, 
like, I don't know, such irrelevant things like TortoiseSVN ... would 
have to load the new library if we decided to move those functions. Or 
link with its static version.

Anyway, it's a huge amount of work for, as you say, a lot of potential 
benefit for our tools, but extra issues for integrators of our code. And 
it's not really urgent.

-- Brane