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
