You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@apache.org> on 2018/01/08 13:31:06 UTC

A rational svn shelve/checkpoint CLI

Towards a more rational shelving-with-checkpoints CLI.

   svn shelf-{save,shelve,unshelve,shelves,log,drop,diff}
     [SHELF_SPEC...] [PATH...]

SHELF_SPEC:

   --shelf=[SHELF][.VERSION]
   @[SHELF][.VERSION]

     Specify shelf SHELF (name, 'all', 'newest'), version VERSION
     (number, date, 'all', 'newest'). Defaults are per command.

   --version=VERSION

     Specify the default version for shelf specs in the command.

Aliases:

   svn save            -> svn shelf-save
   svn shelve          -> svn shelf-shelve
   svn unshelve        -> svn shelf-unshelve
   svn shelves         -> svn shelf-shelves

Main commands:

   svn save | shelf-save      [@SHELF] [PATH...]    # save only
   svn shelve | shelf-shelve  [@SHELF] [PATH...]    # save & revert

     defaults: SHELF:newest; PATH:'.'
     Save/Shelve to shelf SHELF the changes within a PATH.

   svn unshelve | shelf-unshelve  [@[SHELF][.VERSION]...] [PATH...]

     defaults: SHELF:newest; VERSION:newest; PATH:'.'
     Unshelve each shelf-version SHELF.VERSION
     just the paths within PATH...
     (Warn if that omits other paths in the shelf-version.)

Ancillary commands:

   svn shelves | shelf-shelves  [@SHELF...] [PATH...]

     defaults: SHELF:all; PATH:'.'
     List each shelf SHELF that has changes within a PATH.
     Same as 'shelf-log --version=newest ...'.

   svn shelf-log      [@[SHELF][.VERSION]...] [PATH...]

     defaults: SHELF:newest; VERSION:all; PATH:'.'
     List each version in a shelf SHELF that has changes inside a PATH.

   svn shelf-drop     [@SHELF...]

     defaults: SHELF:newest
     Delete each shelf SHELF.

   svn shelf-diff     [@[SHELF][.VERSION]...] [PATH...]

     defaults: SHELF:newest; VERSION:newest; PATH:'.'
     Print a diff of the changes inside PATH... in each shelf-version.
     (Note: not the difference between two shelves or versions.)


Thoughts:

The use of a distinctive shelf specifier is intended to avoid the 
ambiguity in the first optional argument to a syntax like "svn shelve 
[NAME] [PATH...]". There are some other possible ways to resolve the 
ambiguity, such as interpreting the first optional argument as NAME 
unless it contains a '/' or '\' (if we don't allow '/' or '\' in NAME).

Using one-word subcommands like "shelf-drop" rather than "shelf drop" or 
"shelf --delete" is intended to be better for consistency, for the help 
system, and for extending existing scripting that assumes one-word 
subcommands (the bash-completion script is one example). But note the 
ugly repetition in "shelf-shelve", "shelve-unshelve", and "shelf-shelves".

A common syntax for referring to a particular shelved change is helpful. 
"--shelf=" is OK for a start but the repetition of "shelf" here after 
the subcommand name is ugly again, hence suggestion of "@" instead, 
although using "@" to introduce the name rather than the version is 
inconsistent with peg revision specifiers.

Perhaps we can take this further and achieve a syntax like "svn log 
--shelf=NAME" and "svn diff --shelf=NAME[@VERSION]" in which a 
shelf-spec is used with a regular existing subcommand, eliminating the 
repetition in those commands. Then 'svn shelve' and 'unshelve' and 
'shelves' become primary subcommands, but they still need a shelf specifier.


- Julian

Re: A rational svn shelve/checkpoint CLI

Posted by Julian Foad <ju...@apache.org>.
Julian Foad wrote:
> [...] Only the svn:log revprop is stored in the shelf format so far; it 
> will be easy to extend to support all revprops.

http://svn.apache.org/r1820714 does that.

- Julian

Re: A rational svn shelve/checkpoint CLI

Posted by Julian Foad <ju...@apache.org>.
Julian Foad wrote on Tue, 09 Jan 2018 13:31 +0000:
> Anyway the main point here is: we have in principle a set of tree
> snapshots (no matter that each one is stored in the form of a difference
> against the base); and we use revision specifiers to refer to the
> working and base versions:

Initial implementation: http://svn.apache.org/r1820696 "On the 
'shelve-checkpoint' branch: Let revision specifiers access shelves."

I think this proves the principle of using revision specifier syntax to 
refer to shelves.

$ svn propset -r foo --revprop svn:log 'New log msg.'
$ svn propget -r foo --revprop svn:log
New log msg.

where 'foo' is a shelf. 'propedit' and 'proplist' and 'propdel' work 
too. Only the svn:log revprop is stored in the shelf format so far; it 
will be easy to extend to support all revprops.

We may well prefer some other syntax than simply "-r foo", such as "-r 
shelf:foo" or "-r [foo]" or whatever, but that's not the main point.

The main point is how the rev spec is converted to an (extended) 
svn_opt_revision_t object and then passed in to the libsvn_client APIs, 
and to what extent we would need to modify the APIs to properly support 
this. Already I found, for example, that the revprop APIs take an 
svn_opt_revision_t structure as input but they also output a plain 
svn_revnum_t to tell what number a date or 'head' revspec was resolved 
to; with a shelved change, the output revision specifier cannot be 
represented as svn_revnum_t, and I set it to -1 which causes some ugly 
output:

$ svn proplist --revprop -r foo
Unversioned properties on revision -1:
   svn:log

So we would want to change this to svn_opt_revision_t. This is just the 
first example I have come across of a change needed; some other APIs 
will likely require more extensive changes.

Only revprop access is implemented so far, as it was the easiest part to 
implement. Providing unified access to a 'tree snapshot' view of the 
shelved version (svn list/cat/propget (versioned properties), etc.) and 
making modifications (svn add/cp/mv/rm/propset, etc.) and a 'difference' 
view (svn diff/status) will be cumbersome to implement, especially with 
patch file as the storage, although relatively straightforward in 
principle. This could be the trigger for a change in storage form.

- Julian

Re: A rational svn shelve/checkpoint CLI

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Tue, 09 Jan 2018 13:31 +0000:
> Anyway the main point here is: we have in principle a set of tree 
> snapshots (no matter that each one is stored in the form of a difference 
> against the base); and we use revision specifiers to refer to the 
> working and base versions:
...
> The big advantage of some (any) such scheme is that existing 
> libsvn_client APIs for operating on a working copy change or state (such 
> as cat, diff, status, prop*, revprop_get, revprop_set, etc.) can then 
> potentially be extended to support operating on a shelved change or 
> state without any change to the API function declaration itself.

This resembles the FS API, where svn_fs_root_t can be either a revision
root or a txn root.

Re: A rational svn shelve/checkpoint CLI

Posted by Julian Foad <ju...@apache.org>.
Julian Foad wrote:
> [[[
> $ svn savepoint log 'qq'
> version 1: 52 days ago
>   0 files changed
> 
> version 2: 51 days ago
>   hello.txt |    1 +
>   1 file changed, 1 insertion(+)
> 
> version 3: 32 days ago
>   D5/file   |    2 ++
>   hello.txt |    1 +
>   2 files changed, 3 insertions(+)
> 
> 
> $ svn savepoint restore qq 2
> U         hello.txt
> restored 'qq' version 2 and deleted 1 newer versions
> 
> ]]]
> 
> Now I'm talking about *syntax* for accessing these.

Shelved changes are parallel to working changes.

   WC-base
   \ \ \ \__ WC-working
    \ \ \__ shelf 'foo' v1
     \ \__ shelf 'foo' v2
      \__ shelf 'bar' v1

The 'working' state and each 'shelved' state can be considered as a tree 
snapshot, and can also be considered as a diff relative to WC-base. In 
the near future hopefully we'll be able to track a different base for 
each shelf-version; for now they are all (implicitly) considered 
relative to the current WC base (even though the patch files already 
contain the base revision number of each file; currently we ignore those).

Anyway the main point here is: we have in principle a set of tree 
snapshots (no matter that each one is stored in the form of a difference 
against the base); and we use revision specifiers to refer to the 
working and base versions:

   WC-base                      svn_opt_revision_base     "-r BASE"
   \ \ \ \__ WC-working         svn_opt_revision_working  (no CLI syntax)
    \ \ \__ shelf 'foo' v1
     \ \__ shelf 'foo' v2
      \__ shelf 'bar' v1


  (svn_opt_revision_working, _base, etc.).

We could extend the revision specifiers to refer to shelved versions:

   WC-base                      {svn_opt_revision_base}
   \ \ \ \__ WC-working         {svn_opt_revision_working}
    \ \ \__ shelf 'foo' v1      {svn_opt_revision_shelf,'foo',1}
     \ \__ shelf 'foo' v2       {svn_opt_revision_shelf,'foo',2}
      \__ shelf 'bar' v1        {svn_opt_revision_shelf,'bar',1}

with CLI syntax for example:

   svn cat -r shelf:foo path/to/file
   svn cat path/to/file@shelf:foo
   svn diff -c shelf:foo

The big advantage of some (any) such scheme is that existing 
libsvn_client APIs for operating on a working copy change or state (such 
as cat, diff, status, prop*, revprop_get, revprop_set, etc.) can then 
potentially be extended to support operating on a shelved change or 
state without any change to the API function declaration itself.

Just letting you know where my thoughts are going.

- Julian

Re: A rational svn shelve/checkpoint CLI

Posted by Julian Foad <ju...@apache.org>.
Stefan Sperling wrote:
>>> 	svn shelve
>>> 	svn unshelve
>>> 	svn shelf --list
>>> 	svn shelf --drop
>>> 	svn shelf --diff/--show
>>
>> That's a totally reasonable approach, and is similar to (not exactly the
>> same as) what's currently implemented on trunk and on shelve-checkpoint
>> branch.
>>
>> FWIW you missed out 'svn save' (create a checkpoint, don't revert from WC).
> 
> That could be 'svn shelve --keep'

Yes it could...

>> The discussion of syntax for referring to a particular shelf and version is
>> perhaps the more interesting part of the whole discussion; I would welcome
>> any thoughts on that part.
> 
> We could assign numbers to them, display those in 'svn shelf --list',
> and let all other shelf-manipulating options expect a number.

Sure, that's implemented already:

[[[
$ svn savepoint log 'qq'
version 1: 52 days ago
  0 files changed

version 2: 51 days ago
  hello.txt |    1 +
  1 file changed, 1 insertion(+)

version 3: 32 days ago
  D5/file   |    2 ++
  hello.txt |    1 +
  2 files changed, 3 insertions(+)


$ svn savepoint restore qq 2
U         hello.txt
restored 'qq' version 2 and deleted 1 newer versions

]]]

Now I'm talking about *syntax* for accessing these. I proposed some 
syntaxes in the initial email in this thread.

> Additionally, or instead, we could have user-provided shelf entry names
> (UTF-8 strings), and allow them to be used for identification.

I don't like that idea, but thanks for suggesting it anyway; it may 
spark some other thoughts.

- Julian


Re: A rational svn shelve/checkpoint CLI

Posted by Stefan Sperling <st...@apache.org>.
On Mon, Jan 08, 2018 at 03:29:45PM +0000, Julian Foad wrote:
> > How about a combination of new subcommands for actions which
> > tranfer working copy state from/to the shelf (used frequently),
> > and just one subcommand with options for management of the
> > shelf itself (used less frequently), like:
> > 
> > 	svn shelve
> > 	svn unshelve
> > 	svn shelf --list
> > 	svn shelf --drop
> > 	svn shelf --diff/--show
> 
> That's a totally reasonable approach, and is similar to (not exactly the
> same as) what's currently implemented on trunk and on shelve-checkpoint
> branch.
> 
> FWIW you missed out 'svn save' (create a checkpoint, don't revert from WC).

That could be 'svn shelve --keep'

> The discussion of syntax for referring to a particular shelf and version is
> perhaps the more interesting part of the whole discussion; I would welcome
> any thoughts on that part.

We could assign numbers to them, display those in 'svn shelf --list',
and let all other shelf-manipulating options expect a number.

Additionally, or instead, we could have user-provided shelf entry names
(UTF-8 strings), and allow them to be used for identification.

Re: A rational svn shelve/checkpoint CLI

Posted by Julian Foad <ju...@apache.org>.
Stefan Sperling wrote:
> On Mon, Jan 08, 2018 at 02:34:31PM +0000, Julian Foad wrote:
>>>>     svn save            -> svn shelf-save
>>>>     svn shelve          -> svn shelf-shelve
>>>>     svn unshelve        -> svn shelf-unshelve
>>>>     svn shelves         -> svn shelf-shelves
>>>
>>>
>>> This naming doesn't make any sense to me. Using a word root in the same
>>> command as both a noun and a verb runs counter to my notions of proper
>>> grammar. It doesn't look right. The following would be better:
>>>
>>>       shelf-save, shelf-restore, shelf-list, shelf-log, shelf-drop,
>>> shelf-diff.[*]
>>
>> Thanks for your thoughts.
>>
>> You missed out the 'shelve' command (which is like 'save' but reverts the
>> modifications) which is probably the most difficult one to accommodate. What
>> do you propose for that? If you intend 'save' to include the revert then
>> what do propose for the form that doesn't revert (that is, form that creates
>> another save-point/check-point)?
>>
>> I considered using the word 'restore'. It is fine in the context of
>> 'shelf-restore' but because of considering aliases I went for 'unshelve' as
>> the better one-word name, and let the two-word construction be ugly. I also
>> considered 'svn unshelve' as an alias with 'svn shelf-restore' as the
>> two-word form.
>>
>> Similarly for 'shelves' vs. 'list', with an additional consideration here
>> that 'svn list' is already a command with a different meaning, a meaning
>> that could also usefully be applied to a shelf.
> 
> How about a combination of new subcommands for actions which
> tranfer working copy state from/to the shelf (used frequently),
> and just one subcommand with options for management of the
> shelf itself (used less frequently), like:
> 
> 	svn shelve
> 	svn unshelve
> 	svn shelf --list
> 	svn shelf --drop
> 	svn shelf --diff/--show

That's a totally reasonable approach, and is similar to (not exactly the 
same as) what's currently implemented on trunk and on shelve-checkpoint 
branch.

FWIW you missed out 'svn save' (create a checkpoint, don't revert from WC).

The discussion of syntax for referring to a particular shelf and version 
is perhaps the more interesting part of the whole discussion; I would 
welcome any thoughts on that part.

- Julian

Re: A rational svn shelve/checkpoint CLI

Posted by Stefan Sperling <st...@apache.org>.
On Mon, Jan 08, 2018 at 02:34:31PM +0000, Julian Foad wrote:
> > >    svn save            -> svn shelf-save
> > >    svn shelve          -> svn shelf-shelve
> > >    svn unshelve        -> svn shelf-unshelve
> > >    svn shelves         -> svn shelf-shelves
> > 
> > 
> > This naming doesn't make any sense to me. Using a word root in the same
> > command as both a noun and a verb runs counter to my notions of proper
> > grammar. It doesn't look right. The following would be better:
> > 
> >      shelf-save, shelf-restore, shelf-list, shelf-log, shelf-drop,
> > shelf-diff.[*]
> 
> Thanks for your thoughts.
> 
> You missed out the 'shelve' command (which is like 'save' but reverts the
> modifications) which is probably the most difficult one to accommodate. What
> do you propose for that? If you intend 'save' to include the revert then
> what do propose for the form that doesn't revert (that is, form that creates
> another save-point/check-point)?
> 
> I considered using the word 'restore'. It is fine in the context of
> 'shelf-restore' but because of considering aliases I went for 'unshelve' as
> the better one-word name, and let the two-word construction be ugly. I also
> considered 'svn unshelve' as an alias with 'svn shelf-restore' as the
> two-word form.
> 
> Similarly for 'shelves' vs. 'list', with an additional consideration here
> that 'svn list' is already a command with a different meaning, a meaning
> that could also usefully be applied to a shelf.

How about a combination of new subcommands for actions which
tranfer working copy state from/to the shelf (used frequently),
and just one subcommand with options for management of the
shelf itself (used less frequently), like:

	svn shelve
	svn unshelve
	svn shelf --list
	svn shelf --drop
	svn shelf --diff/--show

Re: A rational svn shelve/checkpoint CLI

Posted by Julian Foad <ju...@apache.org>.
Branko Čibej wrote:
> On 08.01.2018 14:31, Julian Foad wrote:
>> Towards a more rational shelving-with-checkpoints CLI.
>>
>>    svn shelf-{save,shelve,unshelve,shelves,log,drop,diff}
>>      [SHELF_SPEC...] [PATH...]
>>
>> SHELF_SPEC:
>>
>>    --shelf=[SHELF][.VERSION]
>>    @[SHELF][.VERSION]
>>
>>      Specify shelf SHELF (name, 'all', 'newest'), version VERSION
>>      (number, date, 'all', 'newest'). Defaults are per command.
>>
>>    --version=VERSION
>>
>>      Specify the default version for shelf specs in the command.
>>
>> Aliases:
>>
>>    svn save            -> svn shelf-save
>>    svn shelve          -> svn shelf-shelve
>>    svn unshelve        -> svn shelf-unshelve
>>    svn shelves         -> svn shelf-shelves
> 
> 
> This naming doesn't make any sense to me. Using a word root in the same
> command as both a noun and a verb runs counter to my notions of proper
> grammar. It doesn't look right. The following would be better:
> 
>      shelf-save, shelf-restore, shelf-list, shelf-log, shelf-drop,
> shelf-diff.[*]

Thanks for your thoughts.

You missed out the 'shelve' command (which is like 'save' but reverts 
the modifications) which is probably the most difficult one to 
accommodate. What do you propose for that? If you intend 'save' to 
include the revert then what do propose for the form that doesn't revert 
(that is, form that creates another save-point/check-point)?

I considered using the word 'restore'. It is fine in the context of 
'shelf-restore' but because of considering aliases I went for 'unshelve' 
as the better one-word name, and let the two-word construction be ugly. 
I also considered 'svn unshelve' as an alias with 'svn shelf-restore' as 
the two-word form.

Similarly for 'shelves' vs. 'list', with an additional consideration 
here that 'svn list' is already a command with a different meaning, a 
meaning that could also usefully be applied to a shelf.

> Adding a common prefix to existing commands without renaming the
> commands doesn't always make sense.

Totally agree.

> [*] It would be even better without the dash: 'shelf save', 'shelf
> restore', and so on; but I realise that would involve either custom
> command interpretation or some deep surgery in our command-line parser.

I already have an implementation without the dash ('shelf save', etc.) 
working on the 'shelve-checkpoint' branch. It suffers from the problems 
I mentioned towards the end of my mail. I agree it would be better from 
a human perspective. The problems of parsing and 'help' display could be 
overcome, but I don't think the work to do so is justified at present.

- Julian


Re: A rational svn shelve/checkpoint CLI

Posted by Branko Čibej <br...@apache.org>.
On 08.01.2018 14:31, Julian Foad wrote:
> Towards a more rational shelving-with-checkpoints CLI.
>
>   svn shelf-{save,shelve,unshelve,shelves,log,drop,diff}
>     [SHELF_SPEC...] [PATH...]
>
> SHELF_SPEC:
>
>   --shelf=[SHELF][.VERSION]
>   @[SHELF][.VERSION]
>
>     Specify shelf SHELF (name, 'all', 'newest'), version VERSION
>     (number, date, 'all', 'newest'). Defaults are per command.
>
>   --version=VERSION
>
>     Specify the default version for shelf specs in the command.
>
> Aliases:
>
>   svn save            -> svn shelf-save
>   svn shelve          -> svn shelf-shelve
>   svn unshelve        -> svn shelf-unshelve
>   svn shelves         -> svn shelf-shelves


This naming doesn't make any sense to me. Using a word root in the same
command as both a noun and a verb runs counter to my notions of proper
grammar. It doesn't look right. The following would be better:

    shelf-save, shelf-restore, shelf-list, shelf-log, shelf-drop,
shelf-diff.[*]

Adding a common prefix to existing commands without renaming the
commands doesn't always make sense.

[*] It would be even better without the dash: 'shelf save', 'shelf
restore', and so on; but I realise that would involve either custom
command interpretation or some deep surgery in our command-line parser.

-- Brane