You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "C. Michael Pilato" <cm...@collab.net> on 2007/02/26 15:43:23 UTC
Behavior check: svnadmin load of revisions with no datestamp
It appears that today if you 'svnadmin load' a dumpfile that contains
revision with no svn:date property, the resulting repository will, for those
revisions, have that property (set to the time of the commit of the loaded
revision).
I can't decide if this is a feature or a bug, but I lean toward the latter.
If revisions lack a datestamp, it is almost certainly not accidentally so.
So perhaps the load logic should be unsetting the svn:date property after
committing revisions which did not carry that property in the dumpstream.
Thoughts?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Behavior check: svnadmin load of revisions with no datestamp
Posted by Daniel Rall <dl...@collab.net>.
On Mon, 26 Feb 2007, Ben Collins-Sussman wrote:
> On 2/26/07, C. Michael Pilato <cm...@collab.net> wrote:
>
> >But I think you misunderstood my question. I am taking for granted that
> >the
> >ability to have a missing svn:date is *not* a bug. My question is, "In
> >such
> >a case, should a date suddenly appear just because you dump and load the
> >repository data?" I think not. *That's* the bug I'm talking about.
>
> I think you've identified a bug, yeah.
Given that missing/incomprehensible dates are allowed in the
repository, this is certainly a bug, +1.
Can someone clue me in to why we allow this state in the first place?
Given the impact on Subversion operations which take dates as input, I
don't see the ability to unset the date on a revision as a valuable
enough benefit to allowing missing/incomprehensible dates into the
repository in the first place.
> In that same vein, what if a revision has no svn:author property?
I doubt this is a problem for any existing Subversion operations.
Commits to a mod_dav_svn-fronted repository which isn't configured to
issue a HTTP basic auth challenge never set a user name. This
behavior seems okay to me.
FWIW, it is possible to set a (default) user name via a hook script,
and/or block removal of the user name revprop, if you want to prevent
this situation
> That's far more common (and less havoc-wreaking) than the 'no
> svn:date' scenario. In that case, might 'svnadmin load' (or even
> 'svnsync sync') be accidentally creating svn:author properties?
Good question, Ben. Has anyone tried this already?
Re: Behavior check: svnadmin load of revisions with no datestamp
Posted by "C. Michael Pilato" <cm...@collab.net>.
C. Michael Pilato wrote:
> Ben Collins-Sussman wrote:
>> On 2/26/07, C. Michael Pilato <cm...@collab.net> wrote:
>>
>>> But I think you misunderstood my question. I am taking for granted
>>> that the
>>> ability to have a missing svn:date is *not* a bug. My question is,
>>> "In such
>>> a case, should a date suddenly appear just because you dump and load the
>>> repository data?" I think not. *That's* the bug I'm talking about.
>> I think you've identified a bug, yeah.
>>
>> In that same vein, what if a revision has no svn:author property?
>> That's far more common (and less havoc-wreaking) than the 'no
>> svn:date' scenario. In that case, might 'svnadmin load' (or even
>> 'svnsync sync') be accidentally creating svn:author properties?
>
> No, a missing author is fine because the filesystem code doesn't assume
> there is an author for every commit. I'm sure that svn:date is a special
> case here because that property is the one property that Subversion creates
> itself at txn-start time, and rewrites itself at txn-commit time.
I've filed this as issue #2729, and will be supplying a fix and test case soon.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Behavior check: svnadmin load of revisions with no datestamp
Posted by "C. Michael Pilato" <cm...@collab.net>.
Ben Collins-Sussman wrote:
> On 2/26/07, C. Michael Pilato <cm...@collab.net> wrote:
>
>> But I think you misunderstood my question. I am taking for granted
>> that the
>> ability to have a missing svn:date is *not* a bug. My question is,
>> "In such
>> a case, should a date suddenly appear just because you dump and load the
>> repository data?" I think not. *That's* the bug I'm talking about.
>
> I think you've identified a bug, yeah.
>
> In that same vein, what if a revision has no svn:author property?
> That's far more common (and less havoc-wreaking) than the 'no
> svn:date' scenario. In that case, might 'svnadmin load' (or even
> 'svnsync sync') be accidentally creating svn:author properties?
No, a missing author is fine because the filesystem code doesn't assume
there is an author for every commit. I'm sure that svn:date is a special
case here because that property is the one property that Subversion creates
itself at txn-start time, and rewrites itself at txn-commit time.
I haven't tested this, but I suspect the patch is as simple as the following:
Index: subversion/libsvn_repos/load.c
===================================================================
--- subversion/libsvn_repos/load.c (revision 23433)
+++ subversion/libsvn_repos/load.c (working copy)
@@ -1219,11 +1219,13 @@
/* Grrr, svn_fs_commit_txn rewrites the datestamp property to the
current clock-time. We don't want that, we want to preserve
- history exactly. Good thing revision props aren't versioned! */
- if (rb->datestamp)
- SVN_ERR(svn_fs_change_rev_prop(pb->fs, *new_rev,
- SVN_PROP_REVISION_DATE, rb->datestamp,
- rb->pool));
+ history exactly. Good thing revision props aren't versioned!
+ Note that if rb->datestamp is NULL, that's fine -- if the dump
+ data doesn't carry a datestamp, we want to preserve that fact in
+ the load. */
+ SVN_ERR(svn_fs_change_rev_prop(pb->fs, *new_rev,
+ SVN_PROP_REVISION_DATE, rb->datestamp,
+ rb->pool));
if (*new_rev == rb->rev)
{
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Behavior check: svnadmin load of revisions with no datestamp
Posted by Ben Collins-Sussman <su...@red-bean.com>.
On 2/26/07, C. Michael Pilato <cm...@collab.net> wrote:
> But I think you misunderstood my question. I am taking for granted that the
> ability to have a missing svn:date is *not* a bug. My question is, "In such
> a case, should a date suddenly appear just because you dump and load the
> repository data?" I think not. *That's* the bug I'm talking about.
I think you've identified a bug, yeah.
In that same vein, what if a revision has no svn:author property?
That's far more common (and less havoc-wreaking) than the 'no
svn:date' scenario. In that case, might 'svnadmin load' (or even
'svnsync sync') be accidentally creating svn:author properties?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Support of missing/invalid dates
Posted by "C. Michael Pilato" <cm...@collab.net>.
Daniel Rall wrote:
>>> Subversion expects "svn:date" to be set -- it doesn't consider an
>>> empty date to be valid data.
>> Unfortunately, that's entirely untrue. You simply can't make use of
>> date-based lookups if the dates aren't present.
>
> Uhh Mike, you just filed a bug yourself (#2721) about 'log -r {DATE}'
> on an invalid date returning an error. "Entirely untrue" is
> inaccurrate and a bit harsh, don't'cha think?
That issue is about *bogus* dates, not *missing* ones. Subversion doesn't
care at all if a date property doesn't exist. But I sure doesn't like
trying to parse one that's misformatted.
Your statement is a little confusing in a discussion about Subversion
properties: Subversion does *not* expect svn:date to be set (NULL is fine),
but yes, it doesn't consider an *empty* date ("") to be valid data.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Support of missing/invalid dates
Posted by Greg Hudson <gh...@MIT.EDU>.
On Tue, 2007-02-27 at 15:08 -0800, Blair Zajac wrote:
> The algorithm that searches for revision from a date, does it always use
> all the dates in the repository, or just the dates associated with the
> revisions that modified the subtree you are looking in?
The first (sort of).
The algorithm which resolves a date to a revision does not take a path
input, and it can potentially use the date of any revision. In reality,
it does a binary search, so it only uses the date of some small set of
revisions, but any bogus, misordered, or missing date can presumably
throw it off.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Support of missing/invalid dates
Posted by Blair Zajac <bl...@orcaware.com>.
Daniel Rall wrote:
> On Mon, 26 Feb 2007, C. Michael Pilato wrote:
>
>> Daniel Rall wrote:
>>> On Mon, 26 Feb 2007, C. Michael Pilato wrote:
>>>
>>>> It appears that today if you 'svnadmin load' a dumpfile that contains
>>>> revision with no svn:date property, the resulting repository will, for those
>>>> revisions, have that property (set to the time of the commit of the loaded
>>>> revision).
>>>>
>>>> I can't decide if this is a feature or a bug, but I lean toward the latter.
>>> IMO, this is definitely a bug, since it will result in a repository
>>> with revision dates and numbers out of sequence, which impacts the
>>> behavior of some operations which take a date or date range argument
>>> (e.g. 'svn log -r {START_DATE}:{END_DATE}').
>>>
>>> Subversion expects "svn:date" to be set -- it doesn't consider an
>>> empty date to be valid data.
>> Unfortunately, that's entirely untrue. You simply can't make use of
>> date-based lookups if the dates aren't present.
>
> Uhh Mike, you just filed a bug yourself (#2721) about 'log -r {DATE}'
> on an invalid date returning an error. "Entirely untrue" is
> inaccurrate and a bit harsh, don't'cha think?
>
> If we want Subversion to fully support invalid/missing dates -- which
> I don't -- it's going to require some work.
>
>> And even if they *are*
>> present, that's not to say they are in a meaningful order. For example, if
>> you use cvs2svn to load projects one-at-a-time into a Subversion repository,
>> the dates will be completely screwy and overlapping.
> ...
>
> Yup, this is what happened to the ASF's repository. It's *very* annoying.
The algorithm that searches for revision from a date, does it always use
all the dates in the repository, or just the dates associated with the
revisions that modified the subtree you are looking in?
So if you're working in a subdirectory of the repository, where the
dates are monotonically increasing, the algorithm will work?
Regards,
Blair
--
Blair Zajac, Ph.D.
<bl...@orcaware.com>
Subversion training, consulting and support
http://www.orcaware.com/svn/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Support of missing/invalid dates
Posted by Daniel Rall <dl...@collab.net>.
On Mon, 26 Feb 2007, C. Michael Pilato wrote:
> Daniel Rall wrote:
> > On Mon, 26 Feb 2007, C. Michael Pilato wrote:
> >
> >> It appears that today if you 'svnadmin load' a dumpfile that contains
> >> revision with no svn:date property, the resulting repository will, for those
> >> revisions, have that property (set to the time of the commit of the loaded
> >> revision).
> >>
> >> I can't decide if this is a feature or a bug, but I lean toward the latter.
> >
> > IMO, this is definitely a bug, since it will result in a repository
> > with revision dates and numbers out of sequence, which impacts the
> > behavior of some operations which take a date or date range argument
> > (e.g. 'svn log -r {START_DATE}:{END_DATE}').
> >
> > Subversion expects "svn:date" to be set -- it doesn't consider an
> > empty date to be valid data.
>
> Unfortunately, that's entirely untrue. You simply can't make use of
> date-based lookups if the dates aren't present.
Uhh Mike, you just filed a bug yourself (#2721) about 'log -r {DATE}'
on an invalid date returning an error. "Entirely untrue" is
inaccurrate and a bit harsh, don't'cha think?
If we want Subversion to fully support invalid/missing dates -- which
I don't -- it's going to require some work.
> And even if they *are*
> present, that's not to say they are in a meaningful order. For example, if
> you use cvs2svn to load projects one-at-a-time into a Subversion repository,
> the dates will be completely screwy and overlapping.
...
Yup, this is what happened to the ASF's repository. It's *very* annoying.
Re: Behavior check: svnadmin load of revisions with no datestamp
Posted by "C. Michael Pilato" <cm...@collab.net>.
Daniel Rall wrote:
> On Mon, 26 Feb 2007, C. Michael Pilato wrote:
>
>> It appears that today if you 'svnadmin load' a dumpfile that contains
>> revision with no svn:date property, the resulting repository will, for those
>> revisions, have that property (set to the time of the commit of the loaded
>> revision).
>>
>> I can't decide if this is a feature or a bug, but I lean toward the latter.
>
> IMO, this is definitely a bug, since it will result in a repository
> with revision dates and numbers out of sequence, which impacts the
> behavior of some operations which take a date or date range argument
> (e.g. 'svn log -r {START_DATE}:{END_DATE}').
>
> Subversion expects "svn:date" to be set -- it doesn't consider an
> empty date to be valid data.
Unfortunately, that's entirely untrue. You simply can't make use of
date-based lookups if the dates aren't present. And even if they *are*
present, that's not to say they are in a meaningful order. For example, if
you use cvs2svn to load projects one-at-a-time into a Subversion repository,
the dates will be completely screwy and overlapping.
But I think you misunderstood my question. I am taking for granted that the
ability to have a missing svn:date is *not* a bug. My question is, "In such
a case, should a date suddenly appear just because you dump and load the
repository data?" I think not. *That's* the bug I'm talking about.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Behavior check: svnadmin load of revisions with no datestamp
Posted by Daniel Rall <dl...@collab.net>.
On Mon, 26 Feb 2007, C. Michael Pilato wrote:
> It appears that today if you 'svnadmin load' a dumpfile that contains
> revision with no svn:date property, the resulting repository will, for those
> revisions, have that property (set to the time of the commit of the loaded
> revision).
>
> I can't decide if this is a feature or a bug, but I lean toward the latter.
IMO, this is definitely a bug, since it will result in a repository
with revision dates and numbers out of sequence, which impacts the
behavior of some operations which take a date or date range argument
(e.g. 'svn log -r {START_DATE}:{END_DATE}').
Subversion expects "svn:date" to be set -- it doesn't consider an
empty date to be valid data.
> If revisions lack a datestamp, it is almost certainly not accidentally so.
> So perhaps the load logic should be unsetting the svn:date property after
> committing revisions which did not carry that property in the dumpstream.
What effect does an empty date have on operations which accept a date
or date range?
Setting the date to the next millisecond after the date of the
previous revision seems better to me. While it has the side-effect of
creating data which was not specified in the dump file (which is
apparently consistent with current behavior), it keeps the repository
data in the format expected by Subversion's own operations.