diff mbox series

git-repack doc: remove discussion of ancient caveat

Message ID patch-1.1-363f84a3fa7-20220715T075114Z-avarab@gmail.com (mailing list archive)
State New, archived
Headers show
Series git-repack doc: remove discussion of ancient caveat | expand

Commit Message

Ævar Arnfjörð Bjarmason July 15, 2022, 7:51 a.m. UTC
The backwards compatibility caveat discussed her hasn't been a concern
for anyone after Git v1.6.0. Let's simply remove it, I daresay
nobody's concerned these days with having git clients running older
than 2008-era git.

See b6945f570ac (git-repack: repo.usedeltabaseoffset, 2006-10-13) and
9f17688d93c (update git-repack documentation wrt
repack.UseDeltaBaseOffset, 2010-02-04) for the commits that previously
introduced and adjusted this documentation.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-repack.txt | 10 ----------
 1 file changed, 10 deletions(-)

Comments

Junio C Hamano July 15, 2022, 4:53 p.m. UTC | #1
Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> The backwards compatibility caveat discussed her hasn't been a concern
> for anyone after Git v1.6.0. Let's simply remove it, I daresay
> nobody's concerned these days with having git clients running older
> than 2008-era git.

For the purpose of this message, Git 1.6.0 has no significance.
Anything newer than 1.4.4 should be able to understand packfiles
that use delta-base-offset, even though they may not have used
delta-base-offset when writing a new one.

That's all academic.  I wouldn't have written the above if the
proposed log message stopped here.  But with the rest of proposed
log message that hints that the above statement is backed by a solid
study of history, it is wrong to write a wrong version number there.

I agree that it is safe to say that anything before Git 2.0.0 is
irrelevant at this point (I would actually say before Git 2.16.0,
i.e. anything older than 3-4 years).

> See b6945f570ac (git-repack: repo.usedeltabaseoffset, 2006-10-13) and
> 9f17688d93c (update git-repack documentation wrt
> repack.UseDeltaBaseOffset, 2010-02-04) for the commits that previously
> introduced and adjusted this documentation.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  Documentation/git-repack.txt | 10 ----------
>  1 file changed, 10 deletions(-)
>
> diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt
> index 0bf13893d81..fe1eac37090 100644
> --- a/Documentation/git-repack.txt
> +++ b/Documentation/git-repack.txt
> @@ -218,16 +218,6 @@ CONFIGURATION
>  Various configuration variables affect packing, see
>  linkgit:git-config[1] (search for "pack" and "delta").
>  
> -By default, the command passes `--delta-base-offset` option to
> -'git pack-objects'; this typically results in slightly smaller packs,
> -but the generated packs are incompatible with versions of Git older than
> -version 1.4.4. If you need to share your repository with such ancient Git
> -versions, either directly or via the dumb http protocol, then you
> -need to set the configuration variable `repack.UseDeltaBaseOffset` to
> -"false" and repack. Access from old Git versions over the native protocol
> -is unaffected by this option as the conversion is performed on the fly
> -as needed in that case.
> -
>  Delta compression is not used on objects larger than the
>  `core.bigFileThreshold` configuration variable and on files with the
>  attribute `delta` set to false.
Ævar Arnfjörð Bjarmason July 16, 2022, 7:59 a.m. UTC | #2
On Fri, Jul 15 2022, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> The backwards compatibility caveat discussed her hasn't been a concern
>> for anyone after Git v1.6.0. Let's simply remove it, I daresay
>> nobody's concerned these days with having git clients running older
>> than 2008-era git.
>
> For the purpose of this message, Git 1.6.0 has no significance.
> Anything newer than 1.4.4 should be able to understand packfiles
> that use delta-base-offset, even though they may not have used
> delta-base-offset when writing a new one.
>
> That's all academic.  I wouldn't have written the above if the
> proposed log message stopped here.  But with the rest of proposed
> log message that hints that the above statement is backed by a solid
> study of history, it is wrong to write a wrong version number there.
>
> I agree that it is safe to say that anything before Git 2.0.0 is
> irrelevant at this point (I would actually say before Git 2.16.0,
> i.e. anything older than 3-4 years).

1.4.4 is the version that introduced the flag, but the paragraphs I'm
removing is relevant to 1.6.0, as it discusses concerns with that
new-in-1.4.4 --delta-base-offset flag being turned on by default.

So this documentation matches e.g. the RelNotes update you made in
e903b4095a0 (Update draft release notes for 1.6.0, 2008-07-01).

I.e. 1.4.4 was released in late 2006, 1.6.0 in mid-2008. Hence the
mention of "git clients older than 2008-era git" not being a concern.

I.e. the target audience for this bit of documentatino is someone
running 1.6.0. or newer that's also concerned about pre-1.4.4.

>> See b6945f570ac (git-repack: repo.usedeltabaseoffset, 2006-10-13) and
>> 9f17688d93c (update git-repack documentation wrt
>> repack.UseDeltaBaseOffset, 2010-02-04) for the commits that previously
>> introduced and adjusted this documentation.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>  Documentation/git-repack.txt | 10 ----------
>>  1 file changed, 10 deletions(-)
>>
>> diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt
>> index 0bf13893d81..fe1eac37090 100644
>> --- a/Documentation/git-repack.txt
>> +++ b/Documentation/git-repack.txt
>> @@ -218,16 +218,6 @@ CONFIGURATION
>>  Various configuration variables affect packing, see
>>  linkgit:git-config[1] (search for "pack" and "delta").
>>  
>> -By default, the command passes `--delta-base-offset` option to
>> -'git pack-objects'; this typically results in slightly smaller packs,
>> -but the generated packs are incompatible with versions of Git older than
>> -version 1.4.4. If you need to share your repository with such ancient Git
>> -versions, either directly or via the dumb http protocol, then you
>> -need to set the configuration variable `repack.UseDeltaBaseOffset` to
>> -"false" and repack. Access from old Git versions over the native protocol
>> -is unaffected by this option as the conversion is performed on the fly
>> -as needed in that case.
>> -
>>  Delta compression is not used on objects larger than the
>>  `core.bigFileThreshold` configuration variable and on files with the
>>  attribute `delta` set to false.
Junio C Hamano July 18, 2022, 5:14 a.m. UTC | #3
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

>> That's all academic.  I wouldn't have written the above if the
>> proposed log message stopped here.  But with the rest of proposed
>> log message that hints that the above statement is backed by a solid
>> study of history, it is wrong to write a wrong version number there.
>>
>> I agree that it is safe to say that anything before Git 2.0.0 is
>> irrelevant at this point (I would actually say before Git 2.16.0,
>> i.e. anything older than 3-4 years).
>
> 1.4.4 is the version that introduced the flag, but the paragraphs I'm
> removing is relevant to 1.6.0, as it discusses concerns with that
> new-in-1.4.4 --delta-base-offset flag being turned on by default.
>
> So this documentation matches e.g. the RelNotes update you made in
> e903b4095a0 (Update draft release notes for 1.6.0, 2008-07-01).
>
> I.e. 1.4.4 was released in late 2006, 1.6.0 in mid-2008. Hence the
> mention of "git clients older than 2008-era git" not being a concern.
>
> I.e. the target audience for this bit of documentatino is someone
> running 1.6.0. or newer that's also concerned about pre-1.4.4.

Step back and think why the 1.6.0 carried the mention in the release
notes, and added the "use of delta-base-offset is incompatible with
versions of Git that are older than 1.4.4" to the documentation.

Back in 1.6.0 days, some of the users still had to refrain from
using the delta-base-offset feature by explicitly disabling it,
because the version of Git running on the other side of their
connection were before eb32d236 (introduce delta objects with offset
to base, 2006-09-21)---1.4.4 was the first version of Git that
learned how to produce packs with delta-base-offset, and how to read
objects out of such packs.

If 1.4.4 were too distant in the past to matter and we were sure
nobody is running such an ancient version back then, we wouldn't
have written such a notice there in the document when we turned it
on by default in 1.6.0.

Or, step back and think what we would have taken into account, if
the patch in question were suggested during the development cycle
toward Git 1.7.0, and what the reason for rejection would have been.
Even if there were tons of folks who were running pre-1.6.0 versions
of Git, as long as these old versions were all newer than 1.4.4,
then we would have said that the warning would not matter.  The
reason to reject such a patch in 1.7.0 cycle would not be because of
the age of 1.6.0 being too young, but because of the age of 1.4.4
being not old enough.

Both are based on how distant in the past 1.4.4 had happened.  As
the removed paragraph says, generated packs with delta-base-offset
are "incompatible with versions of Git older than 1.4.4".  If 1.4.4
and older died out, nobody needs to worry about use of d-b-o.  If
1.4.4 and older are still used, we would need to.  How ancient 1.6.0
does not matter.

Perhaps rein in the temptation to "attack back" in spinal reflex
when somebody bothers to take time to point out where you are
mistaken?  They are often not attacking you, but merely offering
improvements.
Ævar Arnfjörð Bjarmason July 18, 2022, 7:35 a.m. UTC | #4
On Sun, Jul 17 2022, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>> That's all academic.  I wouldn't have written the above if the
>>> proposed log message stopped here.  But with the rest of proposed
>>> log message that hints that the above statement is backed by a solid
>>> study of history, it is wrong to write a wrong version number there.
>>>
>>> I agree that it is safe to say that anything before Git 2.0.0 is
>>> irrelevant at this point (I would actually say before Git 2.16.0,
>>> i.e. anything older than 3-4 years).
>>
>> 1.4.4 is the version that introduced the flag, but the paragraphs I'm
>> removing is relevant to 1.6.0, as it discusses concerns with that
>> new-in-1.4.4 --delta-base-offset flag being turned on by default.
>>
>> So this documentation matches e.g. the RelNotes update you made in
>> e903b4095a0 (Update draft release notes for 1.6.0, 2008-07-01).
>>
>> I.e. 1.4.4 was released in late 2006, 1.6.0 in mid-2008. Hence the
>> mention of "git clients older than 2008-era git" not being a concern.
>>
>> I.e. the target audience for this bit of documentatino is someone
>> running 1.6.0. or newer that's also concerned about pre-1.4.4.
>
> Step back and think why the 1.6.0 carried the mention in the release
> notes, and added the "use of delta-base-offset is incompatible with
> versions of Git that are older than 1.4.4" to the documentation.
>
> Back in 1.6.0 days, some of the users still had to refrain from
> using the delta-base-offset feature by explicitly disabling it,
> because the version of Git running on the other side of their
> connection were before eb32d236 (introduce delta objects with offset
> to base, 2006-09-21)---1.4.4 was the first version of Git that
> learned how to produce packs with delta-base-offset, and how to read
> objects out of such packs.
>
> If 1.4.4 were too distant in the past to matter and we were sure
> nobody is running such an ancient version back then, we wouldn't
> have written such a notice there in the document when we turned it
> on by default in 1.6.0.
>
> Or, step back and think what we would have taken into account, if
> the patch in question were suggested during the development cycle
> toward Git 1.7.0, and what the reason for rejection would have been.
> Even if there were tons of folks who were running pre-1.6.0 versions
> of Git, as long as these old versions were all newer than 1.4.4,
> then we would have said that the warning would not matter.  The
> reason to reject such a patch in 1.7.0 cycle would not be because of
> the age of 1.6.0 being too young, but because of the age of 1.4.4
> being not old enough.
>
> Both are based on how distant in the past 1.4.4 had happened.  As
> the removed paragraph says, generated packs with delta-base-offset
> are "incompatible with versions of Git older than 1.4.4".  If 1.4.4
> and older died out, nobody needs to worry about use of d-b-o.  If
> 1.4.4 and older are still used, we would need to.  How ancient 1.6.0
> does not matter.

The removed documentation covers two distinct topics:

 1. For >= 1.6.0 that the default changed to include --delta-base-offset.
 2. For < 1.4.4 that providing --delta-base-offset produces
    incompatibilties.

With "older than 2008-era git" and v1.6.0. I was referring to #1, not
#2. You're pointing out that we should cover #2 more prominently, I'll
re-roll to do that.

> Perhaps rein in the temptation to "attack back" in spinal reflex
> when somebody bothers to take time to point out where you are
> mistaken?  They are often not attacking you, but merely offering
> improvements.

That's really not the case here. The reason I replied with a
clarification was this part of your upthread
<xmqqsfn21fmt.fsf@gitster.g>:

	But with the rest of proposed log message that hints that the
	above statement is backed by a solid study of history, it is
	wrong to write a wrong version number there.

Which I may have misread, but I understood as going beyond suggesting
that we cover #2 over (or in addition to) #1, and into speculation that
the change being suggested here was suspect because I hadn't carried out
a "solid study of history".

Which I would agree with if I had mixed the two up. After all if someone
proposes to remove documentation where 1.4.4 v.s. 1.6.0 is critical what
else have they missed, and can the change be trusted? Perhaps there's
some major caveat that the referenced older commits would reveal that
the submitter didn't bother to study.

Which is why I replied rather than re-rolling. I.e. if that was the case
I don't think a mere re-roll would address all of your concerns, I'd
just be copy/pasting and rephrasing what you suggested, but I still
wouldn't have carried out that "study of history".

But that's not the case. I was just attempting to go for brevity
here. I.e. if I could establish that anything older than 1.6.0 wasn't
relevant (#1) I wouldn't have to cover (#2) at all, as it follows from
the chronology that if #1 isn't worth worrying about, neither is #2.

I'll re-roll with something I think addresses your comments here, and
perhaps I've just dug myself deeper into the hole you've pegged my in
with this reply, but hopefully this clarifies things & where I was
coming from.

Thanks.
Junio C Hamano July 19, 2022, 1:16 a.m. UTC | #5
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> Which I may have misread, but I understood as going beyond suggesting
> that we cover #2 over (or in addition to) #1, and into speculation that
> the change being suggested here was suspect because I hadn't carried out
> a "solid study of history".

OK, so there was a study of history, but the resulting commit did
not interpret and reflect what's significant in the history
correctly.  Sorry for mischaracterizing your mistake.

Lets put it this way.  Here is a statement:

    Since 1.6.0, people started to need to worry more about
    compatibility with 1.4.4 and older.

Now that statement, while it may be still correct, is irrelevant.
Why?

Even if there were tons of people who still use 1.6.0 (or 1.5.3 for
that matter, which happens to be one of my favorite releases in the
era), as long as nobody uses 1.4.4 or older, we can safely remove
such a statement from our end-user facing documentation set.  Some
archaeologists in us may care, but it is irrelevant to the general
public, as long as 1.4.4 or older have died out.  "As continued use
of 1.4.4 by people stopped being an issue long time ago, remove the
warning about interoperability" is the only thing we need to say
about this change.  We can add "that we needed to add in 1.6.0 era"
at the end but that is already too verbose.

Please do not be one of those folks we had to deal with in the past
who for whatever reason cannot admit that they were wrong.

Thanks.
Ævar Arnfjörð Bjarmason July 19, 2022, 9:07 a.m. UTC | #6
On Mon, Jul 18 2022, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> Which I may have misread, but I understood as going beyond suggesting
>> that we cover #2 over (or in addition to) #1, and into speculation that
>> the change being suggested here was suspect because I hadn't carried out
>> a "solid study of history".
>
> OK, so there was a study of history, but the resulting commit did
> not interpret and reflect what's significant in the history
> correctly.  Sorry for mischaracterizing your mistake.
>
> Lets put it this way.  Here is a statement:
>
>     Since 1.6.0, people started to need to worry more about
>     compatibility with 1.4.4 and older.
>
> Now that statement, while it may be still correct, is irrelevant.
> Why?
>
> Even if there were tons of people who still use 1.6.0 (or 1.5.3 for
> that matter, which happens to be one of my favorite releases in the
> era), as long as nobody uses 1.4.4 or older, we can safely remove
> such a statement from our end-user facing documentation set.  Some
> archaeologists in us may care, but it is irrelevant to the general
> public, as long as 1.4.4 or older have died out.  "As continued use
> of 1.4.4 by people stopped being an issue long time ago, remove the
> warning about interoperability" is the only thing we need to say
> about this change.  We can add "that we needed to add in 1.6.0 era"
> at the end but that is already too verbose.
>
> Please do not be one of those folks we had to deal with in the past
> who for whatever reason cannot admit that they were wrong.

I won't, and I think on the subject of documentation & commit messages
you (as in me, in this case) have already lost the "argument" if you're
having to explain what was meant by the text, which clearly should be
self-contained enough to resolve any such ambiguities by itself.

I was just replying to clarify what I was trying to go for in the v1,
which in my mind isn't an argument *for* that version, but just an
explanation. I.e. sometimes the solution is to more clearly phrase what
I was going for, and sometimes it's to take another approach entirely.

In in case, I we may have crossed in E-Mails, here's a re-rolled v2:

    https://lore.kernel.org/git/patch-v2-1.1-98b6de56019-20220719T000847Z-avarab@gmail.com/

I tried to make the dates & versions involved clear, and also to work in
your preference for v2.0.0 (or possibly later) as a cut-off.

It may not at all be what you had in mind, or maybe you're willing to
queue it as-is, just let me know & I'll adjust it.

Thanks a lot for the review, and sorry about the misunderstanding.
diff mbox series

Patch

diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt
index 0bf13893d81..fe1eac37090 100644
--- a/Documentation/git-repack.txt
+++ b/Documentation/git-repack.txt
@@ -218,16 +218,6 @@  CONFIGURATION
 Various configuration variables affect packing, see
 linkgit:git-config[1] (search for "pack" and "delta").
 
-By default, the command passes `--delta-base-offset` option to
-'git pack-objects'; this typically results in slightly smaller packs,
-but the generated packs are incompatible with versions of Git older than
-version 1.4.4. If you need to share your repository with such ancient Git
-versions, either directly or via the dumb http protocol, then you
-need to set the configuration variable `repack.UseDeltaBaseOffset` to
-"false" and repack. Access from old Git versions over the native protocol
-is unaffected by this option as the conversion is performed on the fly
-as needed in that case.
-
 Delta compression is not used on objects larger than the
 `core.bigFileThreshold` configuration variable and on files with the
 attribute `delta` set to false.