You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2019/12/03 12:02:27 UTC
Eight 'info --show-item' issues
I just discovered that 'svn info --show-item' has some twisty logic for
_sometimes_ tacking on additional information at the end of a line:
[subversion/svn/info-cmd.c]
1332 receiver_baton.multiple_targets = (opt_state->depth > svn_depth_empty
1333 || targets->nelts > 1);
That logic is responsible for the output differences between the
following pairs of commands:
% svn info --show-item=url README
https://svn.apache.org/repos/asf/subversion/trunk/README
% svn info --show-item=url README -R
https://svn.apache.org/repos/asf/subversion/trunk/README README
% svn info --show-item=kind BUGS
file
% svn info --show-item=kind BUGS BUGS
file BUGS
file BUGS
However, it doesn't play nice with the new --show-item=foo,bar logic:
% svn info --show-item=url,kind README -R
https://svn.apache.org/repos/asf/subversion/trunk/README README file README
What's worse, in "file BUGS" the separator is spaces, not tabs, so
a consumer of «svn info --show-item=foo,kind -R README» wouldn't be able
to tab-split the one line of output they'll get, even if the name of the
target were printed only once.
There is also some special-case logic around the --show-item=repos-size,
designed to make it output nothing at all — not just an empty line, but
zero bytes of output — when the target is a directory or a local file:
% svn info --show-item=repos-size . BUGS ^/subversion/README
6651 https://svn.apache.org/repos/asf/subversion/README
%
Currently, that leads to these anomalies:
% svn info --show-item=repos-size,kind -R README
% svn info --show-item=repos-size --depth=infinity README
% svn info --show-item=repos-size --depth=infinity ./
% svn info --show-item=repos-size --depth=empty ./
% svn info --show-item=kind,repos-size,kind -R README | xxd
00000000: 6669 6c65 2020 2020 2020 2052 4541 444d file READM
00000010: 4509 E.
%
----
So, what to do about all this.
Let's start by naming the issues:
1. The output for a single file target differs between --depth=files and
--depth=infinity.
2. The output for a single file target has a different schema than the
output for two files targets.
3. When MULTIPLE_TARGETS is true and there are multiple items to show,
the final tab-split item also contains the name of the file.
4. When MULTIPLE_TARGETS is true and there are multiple items to show,
the name of the target appears multiple times in the output.
5. repos-size, when used on local file targets, outputs nothing
with no error. It should print the detranslated size.
6. repos-size, when used on local directory targets at depth empty,
outputs nothing with no error. Not even an empty string followed by a
newline; it inhibits the line entirely. [This makes it difficult to
extend «svn info --show-item=repos-size A/» either to multiple items or
to multiple targets. For example, it breaks the invariant that removing
one item from «svn info --show-item=foo,bar -- lorem ipsum» would not
change the number of lines in the output.]
7. repos-size, when used on local directory targets with depth infinity,
doesn't recurse.
8. «--show-item=kind,repos-size,kind» doesn't behave correctly: it
prints the kind just once, prints a trailing tab after the filename,
and doesn't print a final newline.
----
And proposed solutions. I think the following is fairly non-invasive:
(a). #1,#2: Leave as is, for compatibility. #3: Change the spaces to
a single tab; declare it an API erratum out of an abundance of caution.
#4: Fix it; it's new in trunk so no compatibility concerns. #5: It's
a feature request, I guess. Add it. #6: I don't see a way to fix this.
A --show-item=foo value that inhibits printing the newline that follows
whatever the value of foo is is inherently not composable. To be composable,
it would need to print an empty string followed by a newline,¹ but
that's not backwards compatible. #7: It's a bug. Fix it. #8: Not
sure. Review it after fixing the other issues?
However, I'm not happy with this. It would leave #1 and #2 unfixed, and
#6 would be either unfixed or a non-composable special case. The
semantics I'd like to have are:
- If -q was passed, MULTIPLE_TARGETS shall be false. Otherwise,
MULTIPLE_TARGETS shall be true. The number of targets and
operation depth shall not affect the value of MULTIPLE_TARGETS.
(fixes #1 and #2)
[Or maybe use -v rather than -q, _mutatis mutandis_]
- The target name, when printed, shall be preceded by a tab.
(fixes #3)
- The target name, when printed, shall be printed just once, and
will always be the final element in the line. (fixes #4; FSFS
doesn't allow filenames with embedded newlines anyway)
- repos-size shall have the following semantics: If the target is
a directory, print a single HYPHEN-MINUS (U+002D). If the target
is a file, print the length of the repository-normal form of the
file in octets. svn:special will files will not be treated
specially. (Hopefully fixes #5,#6,#7,#8 in one fell swoop, by
eliminating all the special cases.)
So, my alternative proposal is:
(b) Deprecate --show-item=, and add --show-item2= with the above
semantics. We can backport the fixes to #3,#4,#5,#7 to --show-item=
for the benefit of existing scripts.
----
I added multiple items support, so I'm happy to take ownership of #3
and #4. They should be fairly easy to implement: it looks like just a
matter of editing print_info_item_string() and print_info_item_revision(),
plus writing an API erratum, assuming we have consensus on the
spaces-to-tab change.
Cheers,
Daniel
¹ Just like «--show-item=kind» would if we made svn_node_kind_to_word()
return «""».
Re: Eight 'info --show-item' issues
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Tue, 03 Dec 2019 12:25 +00:00:
> Daniel Shahaf wrote:
> >> Let's start by naming the issues:
> [...]
> >> The semantics I'd like to have are:
> [...]
> > [I] didn't want this issue, while outstanding, to take up any
> > brainwidth.
>
> All sounds good. Thanks for documenting it. Looks like a good little
> non-invasive "starter" project for anybody.
Created SVN-4838 to track this idea.
Re: Eight 'info --show-item' issues
Posted by Julian Foad <ju...@apache.org>.
Daniel Shahaf wrote:
>> Let's start by naming the issues:
[...]
>> The semantics I'd like to have are:
[...]
> [I] didn't want this issue, while outstanding, to take up any
> brainwidth.
All sounds good. Thanks for documenting it. Looks like a good little
non-invasive "starter" project for anybody.
- Julian
Re: Eight 'info --show-item' issues
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Daniel Shahaf wrote on Tue, 03 Dec 2019 12:02 +00:00:
> ----
>
> Let's start by naming the issues:
9. MULTIPLE_TARGETS mode suppresses the "can't show in-repository size
of working copy file '%s'" error. I can't imagine why. If it's fine
to print nothing for a wc file when there are other targets, then print
nothing when it's the only target, too; and if it's an error, then it
should be an error whatever the other targets are.
> ----
>
> I added multiple items support, so I'm happy to take ownership of #3
> and #4. They should be fairly easy to implement: it looks like just a
> matter of editing print_info_item_string() and print_info_item_revision(),
> plus writing an API erratum, assuming we have consensus on the
> spaces-to-tab change.
I reverted my changes in r1870750, as a matter of ordinary routing. If
I have a fix for #3 and/or #4, then I'll post it or commit it, as per
usual. However, I have plenty of things to worry about IRL right now
and didn't want this issue, while outstanding, to take up any
brainwidth.
Cheers,
Daniel