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
