You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by cm...@collab.net on 2003/02/07 20:55:19 UTC
Re: svn commit: rev 4775 - in branches/issue-1037-uuids/subversion: include libsvn_repos
mbk@tigris.org writes:
> Author: mbk
> Date: 2003-02-06 21:57:37 -0600 (Thu, 06 Feb 2003)
> New Revision: 4775
>
> Modified:
> branches/issue-1037-uuids/subversion/include/svn_repos.h
> branches/issue-1037-uuids/subversion/libsvn_repos/load.c
> Log:
> Increment the dumpfile format version, and add dumpfile format
> verification on load, now that we've got an incompatible one.
>
> * subversion/include/svn_repos.h
> (SVN_REPOS_DUMPFILE_FORMAT_VERSION): Change to 2.
>
> * subversion/libsvn_repos/load.c
> (validate_format_version, parse_format_version): change name of function
> to reflect new semantics. Parses the first line of the dumpfile stream,
> validating (somewhat loosely) its lexical structure and extracting the
> actual version specified.
> (svn_repos_parse_dumpstream): change call to reflect new name.
Mark, maybe I missed some discussion, but why did you bump the dump
format version?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 4775 - in branches/issue-1037-uuids/subversion: include libsvn_repos
Posted by cm...@collab.net.
mark benedetto king <mb...@boredom.org> writes:
> It is my impression that it reads each block into a hash, and then
> processes the hash. This should make it ignore unknown headers.
> If I add a "Foo: bar." header after the "UUID" header, it seems
> to be ignored.
This is correct.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 4775 - in branches/issue-1037-uuids/subversion: include libsvn_repos
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
mark benedetto king <mb...@boredom.org> writes:
> The repository's UUID will be updated iff
> the dumpstream contains a UUID and
> @a uuid_action is not equal to @c svn_repos_load_uuid_ignore and
> either the repository contains no revisions or
> @a uuid_action is equal to @c svn_repos_load_uuid_force
Hmmm, I see. That doc is true as far as it goes, it just doesn't say
what will happen if the dumpstream does *not* contain a UUID but
uuid_action is force. Erroring would be compliant; so would silent
success.
> Unfortunately the user-facing documentation, svnadmin help load,
> is less clear:
>
> Read a 'dumpfile'-formatted stream from stdin, committing
> new revisions into the repository's filesystem. If the repository
> was previously empty, its UUID will, by default, be changed to the
> one specified in the stream. Progress feedback is sent to stdout.
>
> Valid options:
> --ignore-uuid : ignore the UUID specified in the file.
> --force-uuid : force the repository UUID to be updated.
>
> My preference would be to change the message to match the existing
> semantics, rather than the other way 'round.
Okay. I don't feel strongly about it either; I've changed the docs to
be clearer about the current behavior.
Thanks!
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 4775 - in branches/issue-1037-uuids/subversion: include libsvn_repos
Posted by mark benedetto king <mb...@boredom.org>.
On Tue, Feb 11, 2003 at 03:42:51PM -0600, Karl Fogel wrote:
>
> Am I correct in thinking that --force-uuid means "take the UUID found
> in this dump file and set the destination repository's UUID to it"?
>
That may be what it should mean, but what it actually means is
carefully explained in svn_repos.h:
The repository's UUID will be updated iff
the dumpstream contains a UUID and
@a uuid_action is not equal to @c svn_repos_load_uuid_ignore and
either the repository contains no revisions or
@a uuid_action is equal to @c svn_repos_load_uuid_force
Unfortunately the user-facing documentation, svnadmin help load,
is less clear:
Read a 'dumpfile'-formatted stream from stdin, committing
new revisions into the repository's filesystem. If the repository
was previously empty, its UUID will, by default, be changed to the
one specified in the stream. Progress feedback is sent to stdout.
Valid options:
--ignore-uuid : ignore the UUID specified in the file.
--force-uuid : force the repository UUID to be updated.
My preference would be to change the message to match the existing
semantics, rather than the other way 'round.
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 4775 - in branches/issue-1037-uuids/subversion: include libsvn_repos
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
mark benedetto king <mb...@boredom.org> writes:
> > Since it's not really a showstopper if the UUID is absent, personally
> > think we shouldn't error, even if the format version is 2. If we're
> > flexible about these things, then maybe we'll never have to upgrade
> > the format version number again :-), our loaders will just DTRT all
> > the time. (Of course, if someone passes --force-uuid and no UUID is
> > found, that has to be an error.)
>
> I disagree:
>
> $ rm foo
> rm: cannot remove `foo': No such file or directory
> $ rm -f foo
> $
Bogus analogy, I think. Just because the word "force" is used for the
Unix rm -f flag and in the Subversion "--force-uuid" flag, doesn't
mean they mean the same thing :-).
Am I correct in thinking that --force-uuid means "take the UUID found
in this dump file and set the destination repository's UUID to it"?
If that's what it means, then it might be appropriate to error if no
UUID is available. After all, the user thought they were setting
their repository's identity to some new UUID... but then it didn't
happen, and the user may never know it.
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 4775 - in branches/issue-1037-uuids/subversion: include libsvn_repos
Posted by mark benedetto king <mb...@boredom.org>.
On Mon, Feb 10, 2003 at 10:58:03PM -0600, Karl Fogel wrote:
> Just that
>
> 1. We should always error when there's a problem.
> 2. We should never error when there isn't a problem.
>
> Choking because of "UUIE" would violate (2).
>
Okeedokee.
> Choking because "UUID" is *absent* is a separate question. If UUID is
> required for dump format version 2, then we should error. But if it
> is not required, then we shouldn't.
It isn't required, and we don't.
> Since it's not really a showstopper if the UUID is absent, personally
> think we shouldn't error, even if the format version is 2. If we're
> flexible about these things, then maybe we'll never have to upgrade
> the format version number again :-), our loaders will just DTRT all
> the time. (Of course, if someone passes --force-uuid and no UUID is
> found, that has to be an error.)
I disagree:
$ rm foo
rm: cannot remove `foo': No such file or directory
$ rm -f foo
$
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 4775 - in branches/issue-1037-uuids/subversion: include libsvn_repos
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
mark benedetto king <mb...@boredom.org> writes:
> The way it currently works (which could be fixed by simply removing code)
> is that if it doesn't recognize any of the header fields in a particular
> record, it decides that the record is bogus. We could have it ignore
> these sorts of records, instead. If we do that, it will be less likely
> that we'll need to increment version numbers in the future (we'll be able
> to add new record types without fear).
In other words, we insist that there be at least one recognized header
(we don't care which one) in a record, and there can also be any
number of unrecognized headers?
Let's just drop that "one" to "zero" :-). If a record can't be
processed because some required header is missing, that's different --
then we must error. But if no headers in particular are required,
then there's no reason to error, even if no headers were recognized.
> I'm indifferent. What do you think?
Just that
1. We should always error when there's a problem.
2. We should never error when there isn't a problem.
Choking because of "UUIE" would violate (2).
Choking because "UUID" is *absent* is a separate question. If UUID is
required for dump format version 2, then we should error. But if it
is not required, then we shouldn't.
Since it's not really a showstopper if the UUID is absent, personally
think we shouldn't error, even if the format version is 2. If we're
flexible about these things, then maybe we'll never have to upgrade
the format version number again :-), our loaders will just DTRT all
the time. (Of course, if someone passes --force-uuid and no UUID is
found, that has to be an error.)
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 4775 - in branches/issue-1037-uuids/subversion: include libsvn_repos
Posted by mark benedetto king <mb...@boredom.org>.
On Mon, Feb 10, 2003 at 05:39:01PM -0600, Karl Fogel wrote:
>
> If changing "UUID" to "UUIE" makes it choke, then it sounds like it is
> not ignoring unknown headers, since "UUIE" should just be an unknown
> header name like "Foo" is.
>
> ? :-),
The way it currently works (which could be fixed by simply removing code)
is that if it doesn't recognize any of the header fields in a particular
record, it decides that the record is bogus. We could have it ignore
these sorts of records, instead. If we do that, it will be less likely
that we'll need to increment version numbers in the future (we'll be able
to add new record types without fear).
I'm indifferent. What do you think?
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 4775 - in branches/issue-1037-uuids/subversion: include libsvn_repos
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
mark benedetto king <mb...@boredom.org> writes:
> Yes, they should, because it's an unknown record type, which is why the
> incremented version number was necessary. I don't have an old loader
> around to test, though. If I change "UUID" to "UUIE", the new loader
> chokes.
>
> > However, our policy about ignoring unrecognized headers is official,
> > as I recall. So now that we have a repos-headers block, as well as
> > revision headers blocks, we can add new repos headers with impunity --
> > assuming the *current* code knows to ignore unknown headers. (Does
> > it?)
>
> It is my impression that it reads each block into a hash, and then
> processes the hash. This should make it ignore unknown headers.
> If I add a "Foo: bar." header after the "UUID" header, it seems
> to be ignored.
If changing "UUID" to "UUIE" makes it choke, then it sounds like it is
not ignoring unknown headers, since "UUIE" should just be an unknown
header name like "Foo" is.
? :-),
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 4775 - in branches/issue-1037-uuids/subversion: include libsvn_repos
Posted by mark benedetto king <mb...@boredom.org>.
On Mon, Feb 10, 2003 at 11:59:22AM -0600, Karl Fogel wrote:
> cmpilato@collab.net writes:
> > Our dumper and loader are *generally* written so that unrecognized
> > headers are ignored. That said, our loader does not ignore whole
> > header *blocks*, so I'm cool with your version increment. As you
> > stated, we'll just have a format that is:
> >
> > version-number
> >
> > repos-headers
> >
> > revision-headers
> > ...
>
> The question is whether old loaders will choke if they see the new
> UUID header -- and they do, right?
>
Yes, they should, because it's an unknown record type, which is why the
incremented version number was necessary. I don't have an old loader
around to test, though. If I change "UUID" to "UUIE", the new loader
chokes.
> However, our policy about ignoring unrecognized headers is official,
> as I recall. So now that we have a repos-headers block, as well as
> revision headers blocks, we can add new repos headers with impunity --
> assuming the *current* code knows to ignore unknown headers. (Does
> it?)
It is my impression that it reads each block into a hash, and then
processes the hash. This should make it ignore unknown headers.
If I add a "Foo: bar." header after the "UUID" header, it seems
to be ignored.
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 4775 - in branches/issue-1037-uuids/subversion: include libsvn_repos
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
cmpilato@collab.net writes:
> Our dumper and loader are *generally* written so that unrecognized
> headers are ignored. That said, our loader does not ignore whole
> header *blocks*, so I'm cool with your version increment. As you
> stated, we'll just have a format that is:
>
> version-number
>
> repos-headers
>
> revision-headers
> ...
The question is whether old loaders will choke if they see the new
UUID header -- and they do, right?
However, our policy about ignoring unrecognized headers is official,
as I recall. So now that we have a repos-headers block, as well as
revision headers blocks, we can add new repos headers with impunity --
assuming the *current* code knows to ignore unknown headers. (Does
it?)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 4775 - in branches/issue-1037-uuids/subversion: include libsvn_repos
Posted by cm...@collab.net.
mark benedetto king <mb...@boredom.org> writes:
> On Fri, Feb 07, 2003 at 02:55:19PM -0600, cmpilato@collab.net wrote:
> >
> > Mark, maybe I missed some discussion, but why did you bump the dump
> > format version?
> >
>
> I added a record type that would not be supported by older svnadmin
> loaders (the UUID: record). I suppose that I could have made the UUID
> part of the magic header, making it a multi-line record, but I
> liked the idea of having the version info in a record by itself.
Our dumper and loader are *generally* written so that unrecognized
headers are ignored. That said, our loader does not ignore whole
header *blocks*, so I'm cool with your version increment. As you
stated, we'll just have a format that is:
version-number
repos-headers
revision-headers
...
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 4775 - in branches/issue-1037-uuids/subversion: include libsvn_repos
Posted by mark benedetto king <mb...@boredom.org>.
On Fri, Feb 07, 2003 at 02:55:19PM -0600, cmpilato@collab.net wrote:
>
> Mark, maybe I missed some discussion, but why did you bump the dump
> format version?
>
I added a record type that would not be supported by older svnadmin
loaders (the UUID: record). I suppose that I could have made the UUID
part of the magic header, making it a multi-line record, but I
liked the idea of having the version info in a record by itself.
I consider the new dumpfile format to be
SVN-fs-dump-format-version: 2 (dumpfile metadata)
UUID: 84c10f92-bbb5-0310-a2d6-f82301255a32 (repo metadata)
Revision-number: 0 (actual dump)
...
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org