Johannes Schindelin [off-list ref] writes:
If you changed your stance on the patch Sverre and I sent to fix this, we
could get a non-partial fix for this.
This is long time ago so I may be misremembering the details, but I
thought the original patch was (ab)using object flags to mark "this
was explicitly asked for, even though some other range operation may
have marked it uninteresting". Because it predated the introduction
of the rev_cmdline_info mechanism to record what was mentioned on
the command line separately from what objects are uninteresting
(i.e. object flags), it may have been one convenient way to record
this information, but it still looked unnecessarily ugly hack to me,
in that it allocated scarce object flag bits to represent a narrow
special case (iirc, only a freestanding "A" on the command line but
not "A" spelled in "B..A", or something), making it more expensive
to record other kinds of command line information in a way
consistent with the approach chosen (we do not want to waste object
flag bits in order to record "this was right hand side tip of the
symmetric difference range" and such).
If you are calling "do not waste object flags to represent one
special case among endless number of possibilities, as it will make
it impossible to extend it" my stance, that hasn't changed.
We added rev_cmdline_info since then so that we can tell what refs
were given from the command line in what way, and I thought that we
applied a patch from Sverre that uses it instead of the object
flags. Am I misremembering things?
On Mon, Nov 26, 2012 at 6:56 PM, Junio C Hamano [off-list ref] wrote:
Johannes Schindelin [off-list ref] writes:
quoted
If you changed your stance on the patch Sverre and I sent to fix this, we
could get a non-partial fix for this.
This is long time ago so I may be misremembering the details, but I
thought the original patch was (ab)using object flags to mark "this
was explicitly asked for, even though some other range operation may
have marked it uninteresting". Because it predated the introduction
of the rev_cmdline_info mechanism to record what was mentioned on
the command line separately from what objects are uninteresting
(i.e. object flags), it may have been one convenient way to record
this information, but it still looked unnecessarily ugly hack to me,
in that it allocated scarce object flag bits to represent a narrow
special case (iirc, only a freestanding "A" on the command line but
not "A" spelled in "B..A", or something), making it more expensive
to record other kinds of command line information in a way
consistent with the approach chosen (we do not want to waste object
flag bits in order to record "this was right hand side tip of the
symmetric difference range" and such).
If you are calling "do not waste object flags to represent one
special case among endless number of possibilities, as it will make
it impossible to extend it" my stance, that hasn't changed.
The problem with those patches is that they were doing many things at
the same time.
You are correct that one of the problems being solved was the fact
that we wanted to differentiate B from A in B..A independently of the
object, because it might have been referenced by ^C. My latest patch
series deals with that by using rev cmdline_info.
But there's another problem that series tried to fix: weather or not A
was exported by fast-export, which is not strictly the same as SHOWN.
This becomes a non-issue if my patch series is applied because it
properly identifies when an object has been marked or not. But it's
not when marks are not used.
For example:
% git branch A v1
% git branch B v0
% git branch C v0
% git branch D v1
% git fast-export B..A ^C D
A would be updated through a 'commit refs/heads/A' command, D would be
updated through 'reset refs/heads/D'.
But what if C points to v1? The code will assume A will be exported,
and it will be skipped, and there will be only one reset: 'reset
refs/heads/D'. Either way it doesn't matter, because the reset would
be to mark :0, so even if there was a 'reset refs/heads/A' (because A
was never exported), a mark :0 would be useless.
When marks are used my patch fixes the problem because it doesn't care
if A was exporeted or not; by knowing it was marked, it knows it was
never intended to be exported, so we get resets for both A and D, with
real marks.
We added rev_cmdline_info since then so that we can tell what refs
were given from the command line in what way, and I thought that we
applied a patch from Sverre that uses it instead of the object
flags. Am I misremembering things?
No, the patch from Sverre was never merged.
--
Felipe Contreras
Hi Junio,
On Mon, 26 Nov 2012, Junio C Hamano wrote:
Johannes Schindelin [off-list ref] writes:
quoted
If you changed your stance on the patch Sverre and I sent to fix this,
we could get a non-partial fix for this.
This is long time ago so I may be misremembering the details, but I
thought the original patch was (ab)using object flags to mark "this was
explicitly asked for, even though some other range operation may have
marked it uninteresting". Because it predated the introduction of the
rev_cmdline_info mechanism to record what was mentioned on the command
line separately from what objects are uninteresting (i.e. object flags),
it may have been one convenient way to record this information, but it
still looked unnecessarily ugly hack to me, in that it allocated scarce
object flag bits to represent a narrow special case (iirc, only a
freestanding "A" on the command line but not "A" spelled in "B..A", or
something), making it more expensive to record other kinds of command
line information in a way consistent with the approach chosen (we do not
want to waste object flag bits in order to record "this was right hand
side tip of the symmetric difference range" and such).
Good to know. I will find some time to look at rev_cmdline_info and patch
my patch.
If you are calling "do not waste object flags to represent one
special case among endless number of possibilities, as it will make
it impossible to extend it" my stance, that hasn't changed.
We added rev_cmdline_info since then so that we can tell what refs
were given from the command line in what way, and I thought that we
applied a patch from Sverre that uses it instead of the object
flags. Am I misremembering things?
It does sound so familiar that I am intended to claim that you remember
things correctly.
Ciao,
Johannes
On Mon, Nov 26, 2012 at 11:26 AM, Johannes Schindelin
[off-list ref] wrote:
quoted
We added rev_cmdline_info since then so that we can tell what refs
were given from the command line in what way, and I thought that we
applied a patch from Sverre that uses it instead of the object
flags. Am I misremembering things?
It does sound so familiar that I am intended to claim that you remember
things correctly.
FWIW, I implemented that in
http://thread.gmane.org/gmane.comp.version-control.git/184874 but
didn't do the work to get it merged.
--
Cheers,
Sverre Rabbelier