Re: multiple targets for 'svnlook propget'

On Mon, Nov 19, 2012 at 3:43 AM, Johan Corveleyn <jcorvel_at_gmail.com> wrote:
> On Mon, Nov 19, 2012 at 5:32 AM, Paul Burba <ptburba_at_gmail.com> wrote:
>> On Fri, Nov 16, 2012 at 1:04 PM, Paul Burba <ptburba_at_gmail.com> wrote:
>>> On Fri, Nov 16, 2012 at 10:58 AM, Julian Foad
>>> <julianfoad_at_btopenworld.com> wrote:
>>>>
>>>> --
>>>> Subversion Live 2012 - 2 Days: training, networking, demos & more! http://bit.ly/NgUaRi
>>>> Certified & Supported Apache Subversion Downloads: http://www.wandisco.com/subversion/download
>>>>
>>>> ----- Original Message -----
>>>>> From: Johan Corveleyn <jcorvel_at_gmail.com>
>>>>> To: Julian Foad <julianfoad_at_btopenworld.com>
>>>>> Cc: Subversion Development <dev_at_subversion.apache.org>
>>>>> Sent: Friday, 16 November 2012, 8:54
>>>>> Subject: Re: multiple targets for 'svnlook propget'
>>>>>
>>>>> On Fri, Nov 16, 2012 at 1:19 PM, Julian Foad <julianfoad_at_btopenworld.com>
>>>>> wrote:
>>>>>> Johan Corveleyn wrote:
>>>>>>
>>>>>>> Right now, 'svnlook propget' supports only a single
>>>>> PATH_IN_REPOS
>>>>>>> argument:
>>>>>>>
>>>>>>> usage: 1. svnlook propget REPOS_PATH PROPNAME PATH_IN_REPOS
>>>>>>>
>>>>>>> I think it would be useful to support multiple targets, like 'svn
>>>>>>> propget'. That would make it much faster to gather all props set
>>>>>>> during a transaction, for instance in a pre-commit hook:
>>>>>>>
>>>>>>> svnlook propget -t $TXN $REPOS svn:eol-style `svnlook changed -t
>>>>>>> $TXN $REPOS`
>>>>>>>
>>>>>>> The only complication I see, is that the output for multiple targets
>>>>>>> has to write the node for which the properties are shown.
>>>>>>>
>>>>>>> Looking at 'svn propget' for inspiration, we see that it
>>>>> doesn't
>>>>>>> output the node when there is only one target. But it does when there
>>>>>>> are multiple targets. I guess 'svnlook propget' could implement
>>>>> the
>>>>>>> same behavior, which means that the output for single target remains
>>>>>>> backwards compatible (at the cost of slight increase of complexity for
>>>>>>> scripts that parse that output -> they have to know if they passed
>>>>> one
>>>>>>> or mutiple targets).
>>>>>>>
>>>>>>> Current output of 'svn propget':
>>>>>>>
>>>>>>> $ svn pg svn:eol-style ^/file.txt
>>>>>>> native
>>>>>>> $ svn pg svn:eol-style ^/file.txt ^/binary.exe ^/file2.txt
>>>>>>> http://my.svn.server/svn/file.txt - native
>>>>>>> http://my.svn.server/svn/file2.txt - native
>>>>>>>
>>>>>>> (As you can see, the it simply does not output the targets that
>>>>> don't
>>>>>>> have the property, and shows the actual target for every file that
>>>>>>> does)
>>>>>>>
>>>>>>> Or with working copy targets:
>>>>>>>
>>>>>>> $ svn pg svn:eol-style file.txt binary.exe file2.txt
>>>>>>> file.txt - native
>>>>>>> file2.txt - native
>>>>>>>
>>>>>>>
>>>>>>> Now, when we look at the output of 'svnlook propget':
>>>>>>>
>>>>>>> $ svnlook pg repos svn:eol-style file.txt
>>>>>>> native
>>>>>>>
>>>>>>> We could have following multi-target output:
>>>>>>>
>>>>>>> $ svnlook pg repos svn:eol-style file.txt binary.exe file2.txt
>>>>>>> file.txt - native
>>>>>>> file2.txt - native
>>>>>>>
>>>>>>>
>>>>>>> Thoughts?
>>>>>>>
>>>>>>>
>>>>>>> P.S.: This multi-target output isn't very parseable for multi-line
>>>>>>> properties. Currently, 'svn propget' also has this problem, but
>>>>> users
>>>>>>> can use --xml to get machine-readable output.
>>>>>>
>>>>>> I added a "svn propget --verbose" mode (in 1.7 I think) to
>>>>> resolve that problem. It uses the same format as "svn proplist
>>>>> --verbose", which is like this:
>>>>>>
>>>>>> [[[
>>>>>> $ svn pg -vR svn:ignore # selected bits of output are shown
>>>>>> Properties on 'subversion/bindings/ctypes-python/csvn/ext':
>>>>>> svn:ignore
>>>>>> *.pyc
>>>>>> Properties on 'subversion/mod_dav_svn/reports':
>>>>>> svn:ignore
>>>>>> .libs
>>>>>>
>>>>>> Properties on 'contrib/server-side/svnstsw/include':
>>>>>> svn:ignore
>>>>>> Makefile
>>>>>> Makefile.in
>>>>>>
>>>>>> ]]]
>>>>>>
>>>>>> If the email system hasn't mangled the white space, you should be able
>>>>> to see that the prop name is indented by two spaces and the value by four
>>>>> spaces, and newlines in the value are preserved (with exactly one LF added to
>>>>> ensure there is a line break in the output) so it's fully parseable.
>>>>>>
>>>>>>> Maybe 'svnlook propget'
>>>>>>> should eventually also grow a --xml option to handle this. But I'd
>>>>> say
>>>>>>> that's yet another feature. For now, I would be happy if svnlook
>>>>> could
>>>>>>> already do "plain" output for multiple targets, which can
>>>>> already be
>>>>>>> useful/parseable for single-line properties like svn:eol-style.
>>>>>>
>>>>>> Given that 'svnlook' is very much expected to be used by scripts
>>>>> ('svn' serves most human-interaction cases), I don't see
>>>>> implementing the "plain" output is a good idea.
>>>>>>
>>>>>> But +1 on implementing the "svn pg -v" format.
>>>>>
>>>>> Ok, but then you probably mean that I should add two things:
>>>>> 1) Add multi-target support to 'svnlook pg'
>>>>> 2) Add -v to 'svnlook pg', format like 'svn pg -v'
>>>>>
>>>>> The combination of 1 and 2 would then give us a parseable output for
>>>>> multiple targets.
>>>>>
>>>>> But I guess I shouldn't forbid the use of 1 without 2, right? I think
>>>>> it would be strange to say "cannot use multiple arguments without
>>>>> --verbose" ...
>>>>> So it seems I should also provide a format for the multi-arg case
>>>>> without -v ... like the current format of 'svn pg' without -v.
>>>>>
>>>>>
>>>>> I also note that the same enhancement can't easily be made for
>>>>> proplist. Mainly because 'svnlook pl' already has a --verbose option,
>>>>> so we can't change the output of 'svnlook pl --verbose' with a
>>>>> single
>>>>> target without breaking backwards compatibility.
>>>>>
>>>>> - 'svn pl' always shows "Properties on 'XXX':"
>>>>> (whether for single
>>>>> target or multiple targets, whether verbose or not)
>>>>> - 'svnlook pl' does not, and when used with --verbose it doesn't
>>>>> indent multiline properties like 'svn pl' does.
>>>>>
>>>>> [[[
>>>>> $ svn pl sources --verbose
>>>>> Properties on 'sources':
>>>>> svn:ignore
>>>>> .classpath
>>>>> .settings
>>>>> .project
>>>>>
>>>>> $ svnlook pl repos trunk/sources --verbose
>>>>> svn:ignore : .classpath
>>>>> .settings
>>>>> .project
>>>>> ]]]
>>>>>
>>>>> Currently I'm mainly focused on propget, so proplist doesn't bother me
>>>>> too much. But it would of course be even better if proplist behavior
>>>>> would match the propget behavior. If anyone has any ideas for this ...
>>>>
>>>> It seems to me that the 'svnlook pl --verbose' output is pretty useless. Well, OK, not useless, but poor. IIRC 'svn pl --verbose' was like that and I changed it to use the new format, at the same time as I changed 'svn pg --verbose' to use the new format. In terms of back-compat, however, 'svn pl --verbose' didn't exist before I added it, whereas I changed the output of 'svn pl --verbose' in an incompatible way. I simply decided that the latter was an acceptable change.
>>>>
>>>> Apart from backward compatibility concerns, I would totally go for making 'svn pg' and 'svn pl' and 'svnlook pg' and 'svnlook pl' all use the indented & parseable format, and not relate it to the '--verbose' option at all.
>>>
>>> +1
>>>
>>>> I dunno, add a '--parseable' (or should that be '--parsable') option to select the new format?
>>>
>>> I prefer the backward incompatible format change over a new option.
>>> It's a slippery slope if, every time we want to change the format of a
>>> command line program, we add a new option. What if we later change
>>> the format of the optioned output?
>>
>> Johan,
>>
>> Are you planning on making any of these changes? I'm curious because
>> obviously it would impact any work I do to add --show-inherited-props
>> support to svnlook [propget|proplist]
>
> Hi Paul,
>
> I'd like to do this, yes, but I have to admit the time I can spend on
> it is unpredictable (in between work, kids, and construction works on
> our house), so I'm not really sure when I'll get around to it. And I
> certainly don't want to hold you or anyone back on making other
> changes.
>
> Another option is, if you (or anyone) have some more time to spare, go
> ahead and make these changes yourself if you'd like. If you're in the
> code there anyway, for the inherited-props stuff, maybe it's easy to
> tackle these other things as well?

It is, and that's what I'll do. I just wanted to be sure we weren't
duplicating our efforts.

-- 
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba