'svnadmin load-revprops' as first-level command?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

'svnadmin load-revprops' as first-level command?

Daniel Shahaf-2
I was going through the 1.10 release notes and I found the new
load-revprops / dump-revprops subcommands:

    https://subversion.apache.org/docs/release-notes/1.10#svnadmin-revprops
    New svnadmin dump-revprops and svnadmin load-revprops subcommands
   
    […] svnadmin dump-revprops will save the current values of all
    revision properties to a dump file. svnadmin load-revprops can be
    used to set revision properties in a repository to the values saved
    in a dump file.

I don't dispute the need for such functionality, but is a separate
top-level command the right home for it?  I would have expected
'load-revprops' to be spelled "svnadmin load --only-revprops" or even
"svndumpfilter revprops-only | svnadmin load".

The rationale is that 'load' and 'load-revprops' take exactly the same
data format and manipulate it in exactly the same way.

WDYT?

Daniel
(I smoke-tested this on IRC and Karl agreed.)
Reply | Threaded
Open this post in threaded view
|

RE: 'svnadmin load-revprops' as first-level command?

Markus Schaber
Hi,

From: Daniel Shahaf [mailto:[hidden email]]

> I was going through the 1.10 release notes and I found the new load-revprops
> / dump-revprops subcommands:
>
>     https://subversion.apache.org/docs/release-notes/1.10#svnadmin-revprops
>     New svnadmin dump-revprops and svnadmin load-revprops subcommands
>
>     […] svnadmin dump-revprops will save the current values of all
>     revision properties to a dump file. svnadmin load-revprops can be
>     used to set revision properties in a repository to the values saved
>     in a dump file.
>
> I don't dispute the need for such functionality, but is a separate top-level
> command the right home for it?  I would have expected 'load-revprops' to be
> spelled "svnadmin load --only-revprops" or even "svndumpfilter revprops-only
> | svnadmin load".
>
> The rationale is that 'load' and 'load-revprops' take exactly the same data
> format and manipulate it in exactly the same way.

I guess that, especially for the "dump" case, "dump-revprops" is (or should be)
much more efficient than a "dump" piped through "svndumpfilter". For "load",
the additional overhead will be smaller, but still there. So I think this
functionality should be implemented in svnadmin itself.

Note, however, that I wouldn't mind it being an "--only-revprops" option to
the existing "load" and "dump" subcommands instead of a new subcommand.

And I also don't oppose the idea of "svndumpfilter revprops-only", which
clearly has its own use cases. :-)


Best regards

Markus Schaber

CODESYS® a trademark of 3S-Smart Software Solutions GmbH

Inspiring Automation Solutions

3S-Smart Software Solutions GmbH
Dipl.-Inf. Markus Schaber | Product Development Core Technology
Memminger Str. 151 | 87439 Kempten | Germany
Tel. +49-831-54031-979 | Fax +49-831-54031-50

E-Mail: [hidden email] | Web: http://www.codesys.com | CODESYS store: http://store.codesys.com
CODESYS forum: http://forum.codesys.com

Managing Directors: Dipl.Inf. Dieter Hess, Dipl.Inf. Manfred Werner | Trade register: Kempten HRB 6186 | Tax ID No.: DE 167014915

This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received
this e-mail in error) please notify the sender immediately and destroy this e-mail. Any unauthorised copying, disclosure
or distribution of the material in this e-mail is strictly forbidden.

Reply | Threaded
Open this post in threaded view
|

RE: 'svnadmin load-revprops' as first-level command?

Bert Huijben-5

Perhaps the revprop load operation was designed to support some kind of 'refresh' of the revprops after an earlier dump/sync. In that case it might make sense to have a specific operation, as that would really change the operation to something completely else.

Bert


From: Markus Schaber <[hidden email]>
Sent: Wednesday, April 26, 2017 8:25:58 AM
To: Daniel Shahaf; [hidden email]
Subject: RE: 'svnadmin load-revprops' as first-level command?
 
Hi,

From: Daniel Shahaf [[hidden email]]
> I was going through the 1.10 release notes and I found the new load-revprops
> / dump-revprops subcommands:
>
>     https://subversion.apache.org/docs/release-notes/1.10#svnadmin-revprops
>     New svnadmin dump-revprops and svnadmin load-revprops subcommands
>
>     […] svnadmin dump-revprops will save the current values of all
>     revision properties to a dump file. svnadmin load-revprops can be
>     used to set revision properties in a repository to the values saved
>     in a dump file.
>
> I don't dispute the need for such functionality, but is a separate top-level
> command the right home for it?  I would have expected 'load-revprops' to be
> spelled "svnadmin load --only-revprops" or even "svndumpfilter revprops-only
> | svnadmin load".
>
> The rationale is that 'load' and 'load-revprops' take exactly the same data
> format and manipulate it in exactly the same way.

I guess that, especially for the "dump" case, "dump-revprops" is (or should be)
much more efficient than a "dump" piped through "svndumpfilter". For "load",
the additional overhead will be smaller, but still there. So I think this
functionality should be implemented in svnadmin itself.

Note, however, that I wouldn't mind it being an "--only-revprops" option to
the existing "load" and "dump" subcommands instead of a new subcommand.

And I also don't oppose the idea of "svndumpfilter revprops-only", which
clearly has its own use cases. :-)


Best regards

Markus Schaber

CODESYS® a trademark of 3S-Smart Software Solutions GmbH

Inspiring Automation Solutions

3S-Smart Software Solutions GmbH
Dipl.-Inf. Markus Schaber | Product Development Core Technology
Memminger Str. 151 | 87439 Kempten | Germany
Tel. +49-831-54031-979 | Fax +49-831-54031-50

E-Mail: [hidden email] | Web: http://www.codesys.com | CODESYS store: http://store.codesys.com
CODESYS forum: http://forum.codesys.com

Managing Directors: Dipl.Inf. Dieter Hess, Dipl.Inf. Manfred Werner | Trade register: Kempten HRB 6186 | Tax ID No.: DE 167014915

This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received
this e-mail in error) please notify the sender immediately and destroy this e-mail. Any unauthorised copying, disclosure
or distribution of the material in this e-mail is strictly forbidden.

Reply | Threaded
Open this post in threaded view
|

Re: 'svnadmin load-revprops' as first-level command?

Daniel Shahaf-2
Bert Huijben wrote on Wed, Apr 26, 2017 at 13:03:46 +0000:
> Perhaps the revprop load operation was designed to support some kind
> of 'refresh' of the revprops after an earlier dump/sync. In that case
> it might make sense to have a specific operation, as that would really
> change the operation to something completely else.

Ah, yes, that's exactly how it works: 'load-revprops' changes
already-committed revisions, whereas 'load' would commit new ones.
I agree that this difference warrants a separate subcommand for
'load-revprops'.

Is the help text clear?

  {"load-revprops", subcommand_load_revprops, {0}, N_
   ("usage: svnadmin load-revprops REPOS_PATH\n\n"
    "Read a 'dumpfile'-formatted stream from stdin, setting the revision\n"
    "properties in the repository's filesystem.  Revisions not found in the\n"
    "repository will cause an error.  Progress feedback is sent to stdout.\n"
    "If --revision is specified, limit the loaded revisions to only those\n"
    "in the dump stream whose revision numbers match the specified range.\n"),

Markus Schaber <[hidden email]>:
> I guess that, especially for the "dump" case, "dump-revprops" is (or should be)
> much more efficient than a "dump" piped through "svndumpfilter". For "load",
> the additional overhead will be smaller, but still there. So I think this
> functionality should be implemented in svnadmin itself.

Agreed about efficiency of dump-revprops: as an svnadmin command, it
won't need to combine and generate deltas.

The remaining question is whether 'dump-revprops' should remain
a top-level command or become an option under the 'dump' command.  
I can't say I feel strongly about this...

Cheers,

Daniel