Re: "svnlook pl --revprop" does not work on transactions?
On Mon, Mar 1, 2010 at 7:42 PM, C. Michael Pilato wrote:
> C. Michael Pilato wrote:
>> Ryan Schmidt wrote:
Did I miss something in the original problem description? Is that
precisely
what is being attempted here and yet it's not working?
>>> That is my understanding:
>>>
>>> On Fri, 2010-02-26 at 22:15 -0800, Alexey Neyman wrote:
It seems that I encountered a bug in 'svnlook pl --revprop': it fails with
the following message:
$ svnlook pl --revprop -t 10547-86b /svn/test-svn
svnlook: Invalid revision number '-1'
>>> I can replicate this error message with svn 1.6.9 on Mac OS X. "svnlook
>>> pl -t" works fine, but "svnlook pl --revprop -t" complains that revision
>>> number
>>> '-1' is invalid, though I specified a correct in-progress transaction
>>> number.
>>
>> Okay -- sorry for the noise, then. That was my fault for not parsing the
>> conversation correctly.
>>
>> Yes, this I believe this is a bug. (I'll even grant that using an option
>> named "--revprops" does nothing for the illusion that revisions aren't
>> really just "special transactions".)
>
> I think the attached patch fixes this, and I plan to commit as much after
> some more testing.
>
> --
> C. Michael Pilato
> CollabNet <> www.collab.net <> Distributed Development On Demand
>
> * subversion/svnlook/main.c
> (do_plist): Correctly handle invocations of 'svnlook plist --revprop' when
> used with '-t TXN_NAME' instead of '-r REV'.
>
> Reported by: Alexey Neyman
> --This line, and those below, will be ignored--
>
> Index: subversion/svnlook/main.c
> ===
> --- subversion/svnlook/main.c (revision 916775)
> +++ subversion/svnlook/main.c (working copy)
> @@ -1685,11 +1685,16 @@
> SVN_ERR(verify_path(&kind, root, path, pool));
> SVN_ERR(svn_fs_node_proplist(&props, root, path, pool));
> }
> - else
> + else if (c->is_revision)
> {
> SVN_ERR(svn_fs_revision_proplist(&props, c->fs, c->rev_id, pool));
> revprop = TRUE;
> }
> + else
> + {
> + SVN_ERR(svn_fs_txn_proplist(&props, c->txn, pool));
> + revprop = TRUE;
> + }
>
> if (xml)
> {
> @@ -1703,8 +1708,16 @@
> if (revprop)
> {
> /* "" */
> - svn_xml_make_open_tag(&sb, pool, svn_xml_normal, "revprops",
> - "rev", revstr, NULL);
> + if (c->is_revision)
> + {
> + svn_xml_make_open_tag(&sb, pool, svn_xml_normal, "revprops",
> + "rev", revstr, NULL);
> + }
> + else
> + {
> + svn_xml_make_open_tag(&sb, pool, svn_xml_normal, "revprops",
> + "txn", c->txn_name, NULL);
> + }
> }
He, thanks for also fixing the XML part. I was in fact, working on a
regression test, as none of the svnlook options seem to be tested on
transactions.
I'll commit the test when I'm finished and add it to your backport proposal.
Lieven
Re: "svnlook pl --revprop" does not work on transactions?
C. Michael Pilato wrote: > C. Michael Pilato wrote: >> Ryan Schmidt wrote: Did I miss something in the original problem description? Is that precisely what is being attempted here and yet it's not working? >>> That is my understanding: >>> >>> On Fri, 2010-02-26 at 22:15 -0800, Alexey Neyman wrote: It seems that I encountered a bug in 'svnlook pl --revprop': it fails with the following message: $ svnlook pl --revprop -t 10547-86b /svn/test-svn svnlook: Invalid revision number '-1' >>> I can replicate this error message with svn 1.6.9 on Mac OS X. "svnlook >>> pl -t" works fine, but "svnlook pl --revprop -t" complains that revision >>> number >>> '-1' is invalid, though I specified a correct in-progress transaction >>> number. >> Okay -- sorry for the noise, then. That was my fault for not parsing the >> conversation correctly. >> >> Yes, this I believe this is a bug. (I'll even grant that using an option >> named "--revprops" does nothing for the illusion that revisions aren't >> really just "special transactions".) > > I think the attached patch fixes this, and I plan to commit as much after > some more testing. Committed in r917640 and proposed for backport to 1.6.x. -- C. Michael Pilato CollabNet <> www.collab.net <> Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: "svnlook pl --revprop" does not work on transactions?
C. Michael Pilato wrote:
> Ryan Schmidt wrote:
>>> Did I miss something in the original problem description? Is that precisely
>>> what is being attempted here and yet it's not working?
>> That is my understanding:
>>
>> On Fri, 2010-02-26 at 22:15 -0800, Alexey Neyman wrote:
>>> It seems that I encountered a bug in 'svnlook pl --revprop': it fails with
>>> the following message:
>>>
>>> $ svnlook pl --revprop -t 10547-86b /svn/test-svn
>>> svnlook: Invalid revision number '-1'
>> I can replicate this error message with svn 1.6.9 on Mac OS X. "svnlook
>> pl -t" works fine, but "svnlook pl --revprop -t" complains that revision
>> number
>> '-1' is invalid, though I specified a correct in-progress transaction number.
>
> Okay -- sorry for the noise, then. That was my fault for not parsing the
> conversation correctly.
>
> Yes, this I believe this is a bug. (I'll even grant that using an option
> named "--revprops" does nothing for the illusion that revisions aren't
> really just "special transactions".)
I think the attached patch fixes this, and I plan to commit as much after
some more testing.
--
C. Michael Pilato
CollabNet <> www.collab.net <> Distributed Development On Demand
* subversion/svnlook/main.c
(do_plist): Correctly handle invocations of 'svnlook plist --revprop' when
used with '-t TXN_NAME' instead of '-r REV'.
Reported by: Alexey Neyman
--This line, and those below, will be ignored--
Index: subversion/svnlook/main.c
===
--- subversion/svnlook/main.c (revision 916775)
+++ subversion/svnlook/main.c (working copy)
@@ -1685,11 +1685,16 @@
SVN_ERR(verify_path(&kind, root, path, pool));
SVN_ERR(svn_fs_node_proplist(&props, root, path, pool));
}
- else
+ else if (c->is_revision)
{
SVN_ERR(svn_fs_revision_proplist(&props, c->fs, c->rev_id, pool));
revprop = TRUE;
}
+ else
+{
+ SVN_ERR(svn_fs_txn_proplist(&props, c->txn, pool));
+ revprop = TRUE;
+}
if (xml)
{
@@ -1703,8 +1708,16 @@
if (revprop)
{
/* "" */
- svn_xml_make_open_tag(&sb, pool, svn_xml_normal, "revprops",
-"rev", revstr, NULL);
+ if (c->is_revision)
+{
+ svn_xml_make_open_tag(&sb, pool, svn_xml_normal, "revprops",
+"rev", revstr, NULL);
+}
+ else
+{
+ svn_xml_make_open_tag(&sb, pool, svn_xml_normal, "revprops",
+"txn", c->txn_name, NULL);
+}
}
else
{
signature.asc
Description: OpenPGP digital signature
Re: "svnlook pl --revprop" does not work on transactions?
Ryan Schmidt wrote: >> Did I miss something in the original problem description? Is that precisely >> what is being attempted here and yet it's not working? > > That is my understanding: > > On Fri, 2010-02-26 at 22:15 -0800, Alexey Neyman wrote: >> It seems that I encountered a bug in 'svnlook pl --revprop': it fails with >> the following message: >> >> $ svnlook pl --revprop -t 10547-86b /svn/test-svn >> svnlook: Invalid revision number '-1' > > I can replicate this error message with svn 1.6.9 on Mac OS X. "svnlook > pl -t" works fine, but "svnlook pl --revprop -t" complains that revision > number > '-1' is invalid, though I specified a correct in-progress transaction number. Okay -- sorry for the noise, then. That was my fault for not parsing the conversation correctly. Yes, this I believe this is a bug. (I'll even grant that using an option named "--revprops" does nothing for the illusion that revisions aren't really just "special transactions".) -- C. Michael Pilato CollabNet <> www.collab.net <> Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: "svnlook pl --revprop" does not work on transactions?
On Mar 1, 2010, at 11:53, C. Michael Pilato wrote: > Ryan Schmidt wrote: >> So you're saying "svnlook pl --revprop -t" is not meant to work? If that's >> correct, how are we meant to know this from the output of "svnlook help pl" >> (which I'll show again) which clearly states that "--revprop" may be used >> with either "-r" or "-t"? > > No, I'm saying that 'svnlook pl --revprop -t TXN_NAME' is expected to work > only if TXN_NAME is the name of an uncommitted transaction. Well naturally. :) > Did I miss something in the original problem description? Is that precisely > what is being attempted here and yet it's not working? That is my understanding: On Fri, 2010-02-26 at 22:15 -0800, Alexey Neyman wrote: > It seems that I encountered a bug in 'svnlook pl --revprop': it fails with > the following message: > > $ svnlook pl --revprop -t 10547-86b /svn/test-svn > svnlook: Invalid revision number '-1' I can replicate this error message with svn 1.6.9 on Mac OS X. "svnlook pl -t" works fine, but "svnlook pl --revprop -t" complains that revision number '-1' is invalid, though I specified a correct in-progress transaction number. This pre-commit hook demonstrates the problem: #!/bin/sh REPO="$1" TXN="$2" SVNLOOK=/opt/local/bin/svnlook $SVNLOOK pl --revprop -t "$TXN" "$REPO" 1>&2 exit 1
Re: "svnlook pl --revprop" does not work on transactions?
Ryan Schmidt wrote:
> On Mar 1, 2010, at 11:20, C. Michael Pilato wrote:
>> Olivier Dehon wrote:
>>> On Sat, 2010-02-27 at 08:43 -0600, Ryan Schmidt wrote:
Why can't he use "svnlook pl --revprop -t" as he suggested? "svnlook help
pl" suggests this should work.
>>> Agreed, it might be a missing feature. I was just trying to suggest a
>>> workaround for what the OP wanted to achieve.
>> This is not an oversight. Subversion's public interfaces have always split
>> trees of changes in the repository into two non-overlapping segments:
>> revisions ("which are sets of tree changes that have been promoted into new,
>> numbered, revisions by the commit process"), and transactions ("which are
>> sets of tree changes not yet promoted into revisions"). We believe that the
>> fact that those "revisions" are just promoted "transactions" (meaning, that
>> you can still address them by their transaction name) is a bit of an
>> implementation detail that needn't be revealed through the public interfaces
>> to Subversion.
>
> So you're saying "svnlook pl --revprop -t" is not meant to work? If that's
> correct, how are we meant to know this from the output of "svnlook help pl"
> (which I'll show again) which clearly states that "--revprop" may be used
> with either "-r" or "-t"?
No, I'm saying that 'svnlook pl --revprop -t TXN_NAME' is expected to work
only if TXN_NAME is the name of an uncommitted transaction.
Did I miss something in the original problem description? Is that precisely
what is being attempted here and yet it's not working?
--
C. Michael Pilato
CollabNet <> www.collab.net <> Distributed Development On Demand
signature.asc
Description: OpenPGP digital signature
Re: "svnlook pl --revprop" does not work on transactions?
On Mar 1, 2010, at 11:20, C. Michael Pilato wrote:
> Olivier Dehon wrote:
>> On Sat, 2010-02-27 at 08:43 -0600, Ryan Schmidt wrote:
>>> Why can't he use "svnlook pl --revprop -t" as he suggested? "svnlook help
>>> pl" suggests this should work.
>>
>> Agreed, it might be a missing feature. I was just trying to suggest a
>> workaround for what the OP wanted to achieve.
>
> This is not an oversight. Subversion's public interfaces have always split
> trees of changes in the repository into two non-overlapping segments:
> revisions ("which are sets of tree changes that have been promoted into new,
> numbered, revisions by the commit process"), and transactions ("which are
> sets of tree changes not yet promoted into revisions"). We believe that the
> fact that those "revisions" are just promoted "transactions" (meaning, that
> you can still address them by their transaction name) is a bit of an
> implementation detail that needn't be revealed through the public interfaces
> to Subversion.
So you're saying "svnlook pl --revprop -t" is not meant to work? If that's
correct, how are we meant to know this from the output of "svnlook help pl"
(which I'll show again) which clearly states that "--revprop" may be used with
either "-r" or "-t"?
$ svnlook help pl
proplist (plist, pl): usage: 1. svnlook proplist REPOS_PATH PATH_IN_REPOS
2. svnlook proplist --revprop REPOS_PATH
List the properties of a path in the repository, or
with the --revprop option, revision properties.
With -v, show the property values too.
Valid options:
-r [--revision] ARG : specify revision number ARG
-t [--transaction] ARG : specify transaction name ARG
-v [--verbose] : be verbose
--revprop: operate on a revision property (use with -r or -t)
--xml: output in XML
Re: "svnlook pl --revprop" does not work on transactions?
Olivier Dehon wrote:
> On Sat, 2010-02-27 at 08:43 -0600, Ryan Schmidt wrote:
>> On Feb 27, 2010, at 08:36, Olivier Dehon wrote:
>>> On Fri, 2010-02-26 at 22:15 -0800, Alexey Neyman wrote:
It seems that I encountered a bug in 'svnlook pl --revprop': it fails with
the following message:
$ svnlook pl --revprop -t 10547-86b /svn/test-svn
svnlook: Invalid revision number '-1'
>>> Looks like you need to use "svnlook info -t" to access the values of the
>>> properties that are going to be set on the revision-to-be (svn:log,
>>> svn:author, ...)
>> Why can't he use "svnlook pl --revprop -t" as he suggested? "svnlook help
>> pl" suggests this should work.
>>
>
> Agreed, it might be a missing feature. I was just trying to suggest a
> workaround for what the OP wanted to achieve.
This is not an oversight. Subversion's public interfaces have always split
trees of changes in the repository into two non-overlapping segments:
revisions ("which are sets of tree changes that have been promoted into new,
numbered, revisions by the commit process"), and transactions ("which are
sets of tree changes not yet promoted into revisions"). We believe that the
fact that those "revisions" are just promoted "transactions" (meaning, that
you can still address them by their transaction name) is a bit of an
implementation detail that needn't be revealed through the public interfaces
to Subversion.
--
C. Michael Pilato
CollabNet <> www.collab.net <> Distributed Development On Demand
signature.asc
Description: OpenPGP digital signature
Re: "svnlook pl --revprop" does not work on transactions?
On Saturday 27 February 2010 11:35:24 am Lieven Govaerts wrote:
> > Note that it always uses svn_fs_revision_proplist(), even when '-t TXN'
> > is passed. In this case, c->rev_id == SVN_INVALID_REVNUM (-1), and
> > svn_fs_revision_proplist() rightfully fails.
> >
> > Shouldn't it be using svn_fs_txn_proplist() instead, just as
> > get_property() chooses between svn_fs_txn_prop() and
> > svn_fs_revision_prop()?
>
> Think so. Do you want to submit a patch to fix this issue?
Sure, attached.
Alexey.
Index: subversion/svnlook/main.c
===
--- subversion/svnlook/main.c (revision 917057)
+++ subversion/svnlook/main.c (working copy)
@@ -1687,7 +1687,14 @@
}
else
{
- SVN_ERR(svn_fs_revision_proplist(&props, c->fs, c->rev_id, pool));
+ if (! c->is_revision)
+ {
+ SVN_ERR(svn_fs_txn_proplist(&props, c->txn, pool));
+ }
+ else
+ {
+ SVN_ERR(svn_fs_revision_proplist(&props, c->fs, c->rev_id, pool));
+ }
revprop = TRUE;
}
Re: "svnlook pl --revprop" does not work on transactions?
Olivier, Actually, I tried to check the presence of some custom properties (say, foo:bar and foo:baz), so svnlook info does not print them. These properties are essentially boolean (they're either present or not). So, I tried to check them all in one run. Instead, I am doing several "svnlook pg --revprop -t TXN", which works. Regards, Alexey. On Saturday 27 February 2010 06:36:44 am Olivier Dehon wrote: > On Fri, 2010-02-26 at 22:15 -0800, Alexey Neyman wrote: > > Hi all, > > > > It seems that I encountered a bug in 'svnlook pl --revprop': it fails > > with the following message: > > > > $ svnlook pl --revprop -t 10547-86b /svn/test-svn > > svnlook: Invalid revision number '-1' > > Looks like you need to use "svnlook info -t" to access the values of the > properties that are going to be set on the revision-to-be (svn:log, > svn:author, ...) > > -Olivier >
Re: "svnlook pl --revprop" does not work on transactions?
On Sat, Feb 27, 2010 at 3:25 AM, Alexey Neyman wrote:
> Hi all,
>
> It seems that I encountered a bug in 'svnlook pl --revprop': it fails with
> the following message:
>
> $ svnlook pl --revprop -t 10547-86b /svn/test-svn
> svnlook: Invalid revision number '-1'
Reproducible with tr...@911289.
>
> Observed with Subversion 1.6.6. Looks like offending code is this block in
> do_plist():
>
> ==
> if (path != NULL)
> ...
> else
> {
> SVN_ERR(svn_fs_revision_proplist(&props, c->fs, c->rev_id, pool));
> revprop = TRUE;
> }
> ==
>
> Note that it always uses svn_fs_revision_proplist(), even when '-t TXN' is
> passed. In this case, c->rev_id == SVN_INVALID_REVNUM (-1), and
> svn_fs_revision_proplist() rightfully fails.
>
> Shouldn't it be using svn_fs_txn_proplist() instead, just as
> get_property() chooses between svn_fs_txn_prop() and
> svn_fs_revision_prop()?
Think so. Do you want to submit a patch to fix this issue?
Lieven
Re: "svnlook pl --revprop" does not work on transactions?
On Sat, 2010-02-27 at 08:43 -0600, Ryan Schmidt wrote: > On Feb 27, 2010, at 08:36, Olivier Dehon wrote: > > On Fri, 2010-02-26 at 22:15 -0800, Alexey Neyman wrote: > >> It seems that I encountered a bug in 'svnlook pl --revprop': it fails with > >> the following message: > >> > >> $ svnlook pl --revprop -t 10547-86b /svn/test-svn > >> svnlook: Invalid revision number '-1' > > > > Looks like you need to use "svnlook info -t" to access the values of the > > properties that are going to be set on the revision-to-be (svn:log, > > svn:author, ...) > > Why can't he use "svnlook pl --revprop -t" as he suggested? "svnlook help pl" > suggests this should work. > Agreed, it might be a missing feature. I was just trying to suggest a workaround for what the OP wanted to achieve. -Olivier
Re: "svnlook pl --revprop" does not work on transactions?
On Feb 27, 2010, at 08:36, Olivier Dehon wrote: > On Fri, 2010-02-26 at 22:15 -0800, Alexey Neyman wrote: >> It seems that I encountered a bug in 'svnlook pl --revprop': it fails with >> the following message: >> >> $ svnlook pl --revprop -t 10547-86b /svn/test-svn >> svnlook: Invalid revision number '-1' > > Looks like you need to use "svnlook info -t" to access the values of the > properties that are going to be set on the revision-to-be (svn:log, > svn:author, ...) Why can't he use "svnlook pl --revprop -t" as he suggested? "svnlook help pl" suggests this should work. $ svnlook help pl proplist (plist, pl): usage: 1. svnlook proplist REPOS_PATH PATH_IN_REPOS 2. svnlook proplist --revprop REPOS_PATH List the properties of a path in the repository, or with the --revprop option, revision properties. With -v, show the property values too. Valid options: -r [--revision] ARG : specify revision number ARG -t [--transaction] ARG : specify transaction name ARG -v [--verbose] : be verbose --revprop: operate on a revision property (use with -r or -t) --xml: output in XML
Re: "svnlook pl --revprop" does not work on transactions?
On Fri, 2010-02-26 at 22:15 -0800, Alexey Neyman wrote: > Hi all, > > It seems that I encountered a bug in 'svnlook pl --revprop': it fails with > the following message: > > $ svnlook pl --revprop -t 10547-86b /svn/test-svn > svnlook: Invalid revision number '-1' > Looks like you need to use "svnlook info -t" to access the values of the properties that are going to be set on the revision-to-be (svn:log, svn:author, ...) -Olivier
"svnlook pl --revprop" does not work on transactions?
Hi all,
It seems that I encountered a bug in 'svnlook pl --revprop': it fails with
the following message:
$ svnlook pl --revprop -t 10547-86b /svn/test-svn
svnlook: Invalid revision number '-1'
Observed with Subversion 1.6.6. Looks like offending code is this block in
do_plist():
==
if (path != NULL)
...
else
{
SVN_ERR(svn_fs_revision_proplist(&props, c->fs, c->rev_id, pool));
revprop = TRUE;
}
==
Note that it always uses svn_fs_revision_proplist(), even when '-t TXN' is
passed. In this case, c->rev_id == SVN_INVALID_REVNUM (-1), and
svn_fs_revision_proplist() rightfully fails.
Shouldn't it be using svn_fs_txn_proplist() instead, just as
get_property() chooses between svn_fs_txn_prop() and
svn_fs_revision_prop()?
This conditional is still in /trunk, so most likely, bug is also
reproducible with top-of-trunk Subversion.
Regards,
Alexey.
signature.asc
Description: This is a digitally signed message part.
"svnlook pl --revprop" does not work on transactions?
Hi all,
It seems that I encountered a bug in 'svnlook pl --revprop': it fails with
the following message:
$ svnlook pl --revprop -t 10547-86b /svn/test-svn
svnlook: Invalid revision number '-1'
Observed with Subversion 1.6.6. Looks like offending code is this block in
do_plist():
==
if (path != NULL)
...
else
{
SVN_ERR(svn_fs_revision_proplist(&props, c->fs, c->rev_id, pool));
revprop = TRUE;
}
==
Note that it always uses svn_fs_revision_proplist(), even when '-t TXN' is
passed. In this case, c->rev_id == SVN_INVALID_REVNUM (-1), and
svn_fs_revision_proplist() rightfully fails.
Shouldn't it be using svn_fs_txn_proplist() instead, just as
get_property() chooses between svn_fs_txn_prop() and
svn_fs_revision_prop()?
This conditional is still in /trunk, so most likely, bug is also
reproducible with top-of-trunk Subversion.
Regards,
Alexey.
