[RFC] fast-import: note deletion command

[RFC] fast-import: note deletion command

[Dmitry Ivankov]

Hi,

fast-import is able to write notes provided a command
N <note_content_data_specification> <commit_specification>

There is no documented command to delete a note. It can't be
easily replaced with Delete command on notes tree because
of notes fanout.

But fast-import interprets 0{40} data_specification as a note
deletion command. It is consistent with fast-import internals
where null_sha1 is used for absent/unknown object. But is
not consistent with file Modify command nor looks like a common
convention in git ui.

So the questions are:
- should 0{40} not be treated as deletion toggle? Downside is that
it is used as one in t/t9301-fast-import-notes.sh.
- how should a documented way to delete notes look like?
ND commit_sha1 # One may think there are also NC, NR, NM
N delete commit_sha1
# Only :mark, full-40-byte-sha1 and 'inline' are allowed currently.
# So no clashes arise, but then one may also want M 'delete' path
# command to work too.

Re: [RFC] fast-import: note deletion command

[Jonathan Nieder]

(+cc: Sverre for fast-import, Johan for notes, Thomas as a user of
 the note import feature, snipping less for their benefit)

Dmitry Ivankov wrote:

> Hi,
>
> fast-import is able to write notes provided a command
> N <note_content_data_specification> <commit_specification>
>
> There is no documented command to delete a note. It can't be
> easily replaced with Delete command on notes tree because
> of notes fanout.
>
> But fast-import interprets 0{40} data_specification as a note
> deletion command. It is consistent with fast-import internals
> where null_sha1 is used for absent/unknown object. But is
> not consistent with file Modify command nor looks like a common
> convention in git ui.

Right.

> So the questions are:
> - should 0{40} not be treated as deletion toggle? Downside is that
> it is used as one in t/t9301-fast-import-notes.sh.
> - how should a documented way to delete notes look like?
> ND commit_sha1 # One may think there are also NC, NR, NM
> N delete commit_sha1
> # Only :mark, full-40-byte-sha1 and 'inline' are allowed currently.
> # So no clashes arise, but then one may also want M 'delete' path
> # command to work too.

If I were doing it:

 - advertise "N 0{40} <commit>" as the historical way to delete a
   note, and make sure it keeps working

 - introduce 'ND' or 'notedelete <commit>' as a human-friendly
   synonym, keeping in mind that NC and NR could be introduced in the
   future if there is demand for them.

 - don't add any new magic "N <magic keyword> <commit>" commands,
   since it's hard to know when they will clash with some VCS's
   convention for object names.

I imagine there are plenty of other acceptable ways to accomplish
these things, though. :)  Thanks for looking into it.

Re: [RFC] fast-import: note deletion command

[Sverre Rabbelier]

Heya,

On Sun, Sep 18, 2011 at 22:35, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> ND commit_sha1 # One may think there are also NC, NR, NM
>> N delete commit_sha1
>> # Only :mark, full-40-byte-sha1 and 'inline' are allowed currently.
>> # So no clashes arise, but then one may also want M 'delete' path
>> # command to work too.
>
> If I were doing it:
>
>  - advertise "N 0{40} <commit>" as the historical way to delete a
>   note, and make sure it keeps working
>
>  - introduce 'ND' or 'notedelete <commit>' as a human-friendly
>   synonym, keeping in mind that NC and NR could be introduced in the
>   future if there is demand for them.

Is this perhaps a good moment to also think about branch deletion?
That came up earlier as well, and thinking about that might give us
some insights in how to deal with deletions uniformly.

-- 
Cheers,

Sverre Rabbelier

Re: [RFC] fast-import: note deletion command

[Dmitry Ivankov]

On Mon, Sep 19, 2011 at 2:39 AM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> Heya,
>
>>
>> If I were doing it:
>>
>>  - advertise "N 0{40} <commit>" as the historical way to delete a
>>   note, and make sure it keeps working
>>
>>  - introduce 'ND' or 'notedelete <commit>' as a human-friendly
>>   synonym, keeping in mind that NC and NR could be introduced in the
>>   future if there is demand for them.
>
> Is this perhaps a good moment to also think about branch deletion?
> That came up earlier as well, and thinking about that might give us
> some insights in how to deal with deletions uniformly.
Also maybe marks deletion, getting mark sha1,  resetting a mark with
explicit sha1. But most probably not tags deletion.

And going much further on commands, following look nice to have:
- 'ls' storing result to a mark (to allow us not to compute sha1/store
  object if we don't want to)
- marks namespaces (to keep ls mark separately, mark deletion will
  do too, if it's only a single temporary mark). Maybe like ::nr:mark.

>
> --
> Cheers,
>
> Sverre Rabbelier
>

fast-import wishlist (Re: [RFC] fast-import: note deletion command)

[Jonathan Nieder]

Dmitry Ivankov wrote:
> On Mon, Sep 19, 2011 at 2:39 AM, Sverre Rabbelier <srabbelier@gmail.com> wrote:

>> Is this perhaps a good moment to also think about branch deletion?

It might be a good temporal moment, but it's not quite the right
thread for it. :)  If someone wants to maintain a fast-import wishlist
in a wiki somewhere, it sounds like it could be useful to some people
(I prefer to read patches and stories about particular use cases,
myself :)).

>> That came up earlier as well, and thinking about that might give us
>> some insights in how to deal with deletions uniformly.
>
> Also maybe marks deletion, getting mark sha1,  resetting a mark with
> explicit sha1. But most probably not tags deletion.
>
> And going much further on commands, following look nice to have:
> - 'ls' storing result to a mark (to allow us not to compute sha1/store
>   object if we don't want to)
> - marks namespaces (to keep ls mark separately, mark deletion will
>   do too, if it's only a single temporary mark). Maybe like ::nr:mark.

Regards,
Jonathan

Branch deletion (Re: [RFC] fast-import: note deletion command)

[Jonathan Nieder]

Sverre Rabbelier wrote:

> Is this perhaps a good moment to also think about branch deletion?
> That came up earlier as well, and thinking about that might give us
> some insights in how to deal with deletions uniformly.

Sorry, my earlier reply missed the point.  To answer your question:
Dmitry's RFC was about deleting individual notes rather than entire
notes refs, so it is not too closely related to branch deletion.

A new "deleteref" command would be a very simple addition, and the UI
seems pretty obvious.  The main concern in adding such a thing is that
it destroys history, while currently "git fast-import" without -f does
not provide any commands that can do that.  So presumably one would
want to do one of the following:

 - only allow deletion of refs that were not present at the start of
   the import, or
 - add a command-line option to permit deletion of refs, so it doesn't
   happen by mistake when misparsing a stream.

It would also be interesting to see an example use (do you remember
the earlier thread where it came up?) to make sure the UI fits it
well.  In most typical cases I can imagine, direct use of "git
update-ref -d" would work better.  In the case of remote helpers, IIRC
there was already a need for the transport-helper to handle the final
ref updates so "git fetch" can write a nice notice about them to the
console.

Re: Branch deletion (Re: [RFC] fast-import: note deletion command)

[Sverre Rabbelier]

Heya,

On Sun, Sep 18, 2011 at 23:18, Jonathan Nieder <jrnieder@gmail.com> wrote:
> In the case of remote helpers, IIRC there was already a need for
> the transport-helper to handle the final ref updates so "git fetch"
> can write a nice notice about them to the console.

That book-keeping is trivial to do. The problem currently is that when
you try to "git push origin :experimental-branch", there is no way for
the transport-helper code to tell the helper to delete the ref.
Something like 'deleteref' sounds sane I suppose, and I agree that
there should be some sort of safety switch :). I think we need a new
feature announcing that we require support for it, and then the
importer can abort right away if the user doesn't want to have their
refs deleted.

-- 
Cheers,

Sverre Rabbelier

Re: Branch deletion (Re: [RFC] fast-import: note deletion command)

[Jonathan Nieder]

Sverre Rabbelier wrote:

> The problem currently is that when
> you try to "git push origin :experimental-branch", there is no way for
> the transport-helper code to tell the helper to delete the ref.

Ah!  Thanks for explaining.

It also occurs to me that

	reset refs/heads/experimental-branch

	... rest of the fast-import stream comes here ...

could be used as an especially non-self-explanatory way to express
this kind of thing. :)  No idea whether that works already.  A
deleteref command and deleteref feature documented to be meant for
this purpose sound handy to me.

By the way, what does the "export" command do in the following
situation?

	git push origin something-big:master

Does it assume the remote-tracking branch for master reflects what's
available on the other end and send a stream for
origin/master..something-big, or does it send the entire history of
something-big?

Re: Branch deletion (Re: [RFC] fast-import: note deletion command)

[Dmitry Ivankov]

On Mon, Sep 19, 2011 at 3:49 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Sverre Rabbelier wrote:
>
>> The problem currently is that when
>> you try to "git push origin :experimental-branch", there is no way for
>> the transport-helper code to tell the helper to delete the ref.
>
> Ah!  Thanks for explaining.
>
> It also occurs to me that
>
>        reset refs/heads/experimental-branch
>
>        ... rest of the fast-import stream comes here ...
>
> could be used as an especially non-self-explanatory way to express
> this kind of thing. :)  No idea whether that works already.
     feature force
     reset refs/heads/experimental-branch
Will be close. It'll overwrite the branch if any commits are done to
it. But will
keep it as is in case it's not touched in the rest of the fast-import stream.

> A deleteref command and deleteref feature documented to be meant for
> this purpose sound handy to me.
>
> By the way, what does the "export" command do in the following
> situation?
>
>        git push origin something-big:master
>
> Does it assume the remote-tracking branch for master reflects what's
> available on the other end and send a stream for
> origin/master..something-big, or does it send the entire history of
> something-big?
>

Re: Branch deletion (Re: [RFC] fast-import: note deletion command)

[Sverre Rabbelier]

Heya,

On Sun, Sep 18, 2011 at 23:49, Jonathan Nieder <jrnieder@gmail.com> wrote:
> By the way, what does the "export" command do in the following
> situation?
>
>        git push origin something-big:master
>
> Does it assume the remote-tracking branch for master reflects what's
> available on the other end and send a stream for
> origin/master..something-big, or does it send the entire history of
> something-big?

We have a test case for this actually. It goes like this:

checking known breakage:
	(cd clone &&
	 git push origin new-name:new-refspec
	) &&
	compare_refs clone HEAD server refs/heads/new-refspec

Everything up-to-date
fatal: Needed a single revision
not ok 16 - push new branch with old:new refspec # TODO known breakage


In other words, we don't handle it at all. What we do handle a case
similar to what you say, where we have already pushed part of the
history of some-branch, and in that case we do indeed only push the
needed objects.

-- 
Cheers,

Sverre Rabbelier