From: Derrick Stolee <derrickstolee@github.com>
To: Jonathan Nieder <jrnieder@gmail.com>, Taylor Blau <me@ttaylorr.com>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
git@vger.kernel.org, gitster@pobox.com, larsxschneider@gmail.com,
tytso@mit.edu
Subject: Re: [PATCH v5 00/17] cruft packs
Date: Wed, 25 May 2022 15:59:24 -0400 [thread overview]
Message-ID: <7f5a6a6a-c554-c659-72a8-404bc39e08c7@github.com> (raw)
In-Reply-To: <Yo3gl5Wv82mTZQb2@google.com>
On 5/25/2022 3:53 AM, Jonathan Nieder wrote:
> Taylor Blau wrote:
>> On Tue, May 24, 2022 at 11:55:02PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>>>> Moreover, I can't seem to find any formats that _don't_ use that
>>>> convention.
>>>
>>> It's used in the reftable format.
The use in reftable is the only one I can find and that implementation
is not idiomatic. Specifically, the way the four-byte header was
implemented is not easy to extract and share in other formats.
This series does the good work of extracting oid_version() as a
common method across these formats so it is easier to share.
> It's also used in the formats described in
> Documentation/technical/hash-function-transition.
It documents things that have not been implemented, such as the v3
pack-index format:
Pack index (.idx) files use a new v3 format that supports multiple
hash functions. They have the following format (all integers are in
network byte order):
(...)
* 4-byte number of object formats in this pack index: 2
* For each object format:
** 4-byte format identifier (e.g., 'sha1' for SHA-1)
** 4-byte length in bytes of shortened object names. This is the
shortest possible length needed to make names in the shortened
object name table unambiguous.
** 4-byte integer, recording where tables relating to this format
are stored in this index file, as an offset from the beginning.
This was added in your 752414ae431 (technical doc: add a design doc
for hash function transition, 2017-09-27), but has not been acted upon
yet.
> [...]
>> Sounds good. Unless others have a very strong opinion, let's leave it as
>> is.
>
> File formats are one of those things where a little time early can save
> a lot of work later. If there were a strong reason to use "1" and "2"
> here then I'd be okay with living with it --- I'm a pragmatic person.
> But in general, using the magic numbers instead of a sequential value is
> really helpful both in making the file formats more self-explanatory and
> in making it possible to experiment with multiple new hash_algos at the
> same time.
>
> The main argument I'm hearing for using "1" and "2" is "because some
> other formats got that wrong". That reason is the opposite of
> compelling to me: it makes me suspect that as a project we should more
> eagerly break the old bad habits and form new ones. I guess this
> qualifies as a very strong opinion.
Either way, these are magic numbers. One happens to somewhat spell
out something when looking at the file in a hex editor with ASCII
previews, but that doesn't change the fact that it is most important
that the hash function is correctly indicated by the file format and
parsed by the Git executable (not a human).
I'd much rather have a consistent and proven way of specifying the
hash value (using the oid_version() helper) than to try and make a
new mechanism.
Thanks,
-Stolee
next prev parent reply other threads:[~2022-05-25 19:59 UTC|newest]
Thread overview: 201+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-29 22:25 [PATCH 00/17] cruft packs Taylor Blau
2021-11-29 22:25 ` [PATCH 01/17] Documentation/technical: add cruft-packs.txt Taylor Blau
2021-12-02 14:33 ` Derrick Stolee
2021-12-03 21:53 ` Taylor Blau
2021-12-04 22:20 ` Elijah Newren
2021-12-04 23:32 ` Taylor Blau
2021-11-29 22:25 ` [PATCH 02/17] pack-mtimes: support reading .mtimes files Taylor Blau
2021-12-02 15:06 ` Derrick Stolee
2021-12-02 22:32 ` brian m. carlson
2021-12-03 22:24 ` Taylor Blau
2022-01-07 19:41 ` Taylor Blau
2021-11-29 22:25 ` [PATCH 03/17] pack-write: pass 'struct packing_data' to 'stage_tmp_packfiles' Taylor Blau
2021-11-29 22:25 ` [PATCH 04/17] chunk-format.h: extract oid_version() Taylor Blau
2021-12-02 15:22 ` Derrick Stolee
2021-12-03 22:40 ` Taylor Blau
2021-12-06 17:33 ` Derrick Stolee
2021-11-29 22:25 ` [PATCH 05/17] pack-mtimes: support writing pack .mtimes files Taylor Blau
2021-12-02 15:36 ` Derrick Stolee
2021-12-03 23:04 ` Taylor Blau
2021-11-29 22:25 ` [PATCH 06/17] t/helper: add 'pack-mtimes' test-tool Taylor Blau
2021-12-06 21:16 ` Derrick Stolee
2022-02-23 22:24 ` Taylor Blau
2021-11-29 22:25 ` [PATCH 07/17] builtin/pack-objects.c: return from create_object_entry() Taylor Blau
2021-11-29 22:25 ` [PATCH 08/17] builtin/pack-objects.c: --cruft without expiration Taylor Blau
2021-12-06 21:44 ` Derrick Stolee
2022-03-01 2:48 ` Taylor Blau
2021-12-07 15:17 ` Derrick Stolee
2022-02-23 23:34 ` Taylor Blau
2021-11-29 22:25 ` [PATCH 09/17] reachable: add options to add_unseen_recent_objects_to_traversal Taylor Blau
2021-11-29 22:25 ` [PATCH 10/17] reachable: report precise timestamps from objects in cruft packs Taylor Blau
2021-11-29 22:25 ` [PATCH 11/17] builtin/pack-objects.c: --cruft with expiration Taylor Blau
2021-12-07 15:30 ` Derrick Stolee
2022-02-23 23:35 ` Taylor Blau
2021-11-29 22:25 ` [PATCH 12/17] builtin/repack.c: support generating a cruft pack Taylor Blau
2021-12-05 20:46 ` Junio C Hamano
2022-03-01 2:00 ` Taylor Blau
2021-12-07 15:38 ` Derrick Stolee
2022-02-23 23:37 ` Taylor Blau
2021-11-29 22:25 ` [PATCH 13/17] builtin/repack.c: allow configuring cruft pack generation Taylor Blau
2021-11-29 22:25 ` [PATCH 14/17] builtin/repack.c: use named flags for existing_packs Taylor Blau
2021-11-29 22:25 ` [PATCH 15/17] builtin/repack.c: add cruft packs to MIDX during geometric repack Taylor Blau
2021-11-29 22:25 ` [PATCH 16/17] builtin/gc.c: conditionally avoid pruning objects via loose Taylor Blau
2021-11-29 22:25 ` [PATCH 17/17] sha1-file.c: don't freshen cruft packs Taylor Blau
2021-12-03 19:51 ` [PATCH 00/17] " Junio C Hamano
2021-12-03 20:08 ` Taylor Blau
2021-12-03 20:47 ` Taylor Blau
2022-03-02 0:57 ` [PATCH v2 " Taylor Blau
2022-03-02 0:58 ` [PATCH v2 01/17] Documentation/technical: add cruft-packs.txt Taylor Blau
2022-03-02 0:58 ` [PATCH v2 02/17] pack-mtimes: support reading .mtimes files Taylor Blau
2022-03-02 20:22 ` Derrick Stolee
2022-03-02 21:33 ` Taylor Blau
2022-03-02 0:58 ` [PATCH v2 03/17] pack-write: pass 'struct packing_data' to 'stage_tmp_packfiles' Taylor Blau
2022-03-02 0:58 ` [PATCH v2 04/17] chunk-format.h: extract oid_version() Taylor Blau
2022-03-02 0:58 ` [PATCH v2 05/17] pack-mtimes: support writing pack .mtimes files Taylor Blau
2022-03-02 0:58 ` [PATCH v2 06/17] t/helper: add 'pack-mtimes' test-tool Taylor Blau
2022-03-02 0:58 ` [PATCH v2 07/17] builtin/pack-objects.c: return from create_object_entry() Taylor Blau
2022-03-02 0:58 ` [PATCH v2 08/17] builtin/pack-objects.c: --cruft without expiration Taylor Blau
2022-03-02 0:58 ` [PATCH v2 09/17] reachable: add options to add_unseen_recent_objects_to_traversal Taylor Blau
2022-03-02 20:19 ` Derrick Stolee
2022-03-02 21:28 ` Taylor Blau
2022-03-02 0:58 ` [PATCH v2 10/17] reachable: report precise timestamps from objects in cruft packs Taylor Blau
2022-03-02 0:58 ` [PATCH v2 11/17] builtin/pack-objects.c: --cruft with expiration Taylor Blau
2022-03-02 7:42 ` Junio C Hamano
2022-03-02 15:54 ` Taylor Blau
2022-03-02 19:57 ` Derrick Stolee
2022-03-02 0:58 ` [PATCH v2 12/17] builtin/repack.c: support generating a cruft pack Taylor Blau
2022-03-02 0:58 ` [PATCH v2 13/17] builtin/repack.c: allow configuring cruft pack generation Taylor Blau
2022-03-02 0:58 ` [PATCH v2 14/17] builtin/repack.c: use named flags for existing_packs Taylor Blau
2022-03-02 0:58 ` [PATCH v2 15/17] builtin/repack.c: add cruft packs to MIDX during geometric repack Taylor Blau
2022-03-02 0:58 ` [PATCH v2 16/17] builtin/gc.c: conditionally avoid pruning objects via loose Taylor Blau
2022-03-02 0:58 ` [PATCH v2 17/17] sha1-file.c: don't freshen cruft packs Taylor Blau
2022-03-02 20:23 ` [PATCH v2 00/17] " Derrick Stolee
2022-03-02 21:36 ` Taylor Blau
2022-03-03 0:20 ` [PATCH v3 " Taylor Blau
2022-03-03 0:20 ` [PATCH v3 01/17] Documentation/technical: add cruft-packs.txt Taylor Blau
2022-03-07 18:03 ` Jonathan Nieder
2022-03-22 1:16 ` Taylor Blau
2022-03-22 21:45 ` Jonathan Nieder
2022-03-22 22:02 ` Taylor Blau
2022-03-22 23:04 ` Jonathan Nieder
2022-03-23 1:01 ` Taylor Blau
2022-03-28 18:46 ` Taylor Blau
2022-03-28 20:55 ` Junio C Hamano
2022-03-28 21:21 ` Taylor Blau
2022-03-29 15:59 ` Junio C Hamano
2022-03-30 2:23 ` Taylor Blau
2022-03-30 13:37 ` Junio C Hamano
2022-03-30 17:30 ` Taylor Blau
2022-03-03 0:20 ` [PATCH v3 02/17] pack-mtimes: support reading .mtimes files Taylor Blau
2022-03-03 0:20 ` [PATCH v3 03/17] pack-write: pass 'struct packing_data' to 'stage_tmp_packfiles' Taylor Blau
2022-03-03 0:20 ` [PATCH v3 04/17] chunk-format.h: extract oid_version() Taylor Blau
2022-03-03 16:30 ` Ævar Arnfjörð Bjarmason
2022-03-03 23:32 ` Taylor Blau
2022-03-04 0:16 ` Junio C Hamano
2022-03-03 0:20 ` [PATCH v3 05/17] pack-mtimes: support writing pack .mtimes files Taylor Blau
2022-03-03 16:45 ` Ævar Arnfjörð Bjarmason
2022-03-03 23:35 ` Taylor Blau
2022-03-04 10:40 ` Ævar Arnfjörð Bjarmason
2022-03-03 0:20 ` [PATCH v3 06/17] t/helper: add 'pack-mtimes' test-tool Taylor Blau
2022-03-03 0:21 ` [PATCH v3 07/17] builtin/pack-objects.c: return from create_object_entry() Taylor Blau
2022-03-03 0:21 ` [PATCH v3 08/17] builtin/pack-objects.c: --cruft without expiration Taylor Blau
2022-03-03 0:21 ` [PATCH v3 09/17] reachable: add options to add_unseen_recent_objects_to_traversal Taylor Blau
2022-03-03 0:21 ` [PATCH v3 10/17] reachable: report precise timestamps from objects in cruft packs Taylor Blau
2022-03-03 0:21 ` [PATCH v3 11/17] builtin/pack-objects.c: --cruft with expiration Taylor Blau
2022-03-03 0:21 ` [PATCH v3 12/17] builtin/repack.c: support generating a cruft pack Taylor Blau
2022-03-03 0:21 ` [PATCH v3 13/17] builtin/repack.c: allow configuring cruft pack generation Taylor Blau
2022-03-03 0:21 ` [PATCH v3 14/17] builtin/repack.c: use named flags for existing_packs Taylor Blau
2022-03-03 0:21 ` [PATCH v3 15/17] builtin/repack.c: add cruft packs to MIDX during geometric repack Taylor Blau
2022-03-03 0:21 ` [PATCH v3 16/17] builtin/gc.c: conditionally avoid pruning objects via loose Taylor Blau
2022-03-03 0:21 ` [PATCH v3 17/17] sha1-file.c: don't freshen cruft packs Taylor Blau
2022-03-03 1:29 ` [PATCH v3 00/17] " Derrick Stolee
2022-05-18 23:10 ` [PATCH v4 " Taylor Blau
2022-05-18 23:10 ` [PATCH v4 01/17] Documentation/technical: add cruft-packs.txt Taylor Blau
2022-05-19 14:04 ` Junio C Hamano
2022-05-18 23:10 ` [PATCH v4 02/17] pack-mtimes: support reading .mtimes files Taylor Blau
2022-05-19 10:40 ` Ævar Arnfjörð Bjarmason
2022-05-19 15:21 ` Junio C Hamano
2022-05-20 7:32 ` Ævar Arnfjörð Bjarmason
2022-05-20 22:37 ` Taylor Blau
2022-05-18 23:10 ` [PATCH v4 03/17] pack-write: pass 'struct packing_data' to 'stage_tmp_packfiles' Taylor Blau
2022-05-18 23:11 ` [PATCH v4 04/17] chunk-format.h: extract oid_version() Taylor Blau
2022-05-19 11:44 ` Ævar Arnfjörð Bjarmason
2022-05-18 23:11 ` [PATCH v4 05/17] pack-mtimes: support writing pack .mtimes files Taylor Blau
2022-05-18 23:11 ` [PATCH v4 06/17] t/helper: add 'pack-mtimes' test-tool Taylor Blau
2022-05-18 23:11 ` [PATCH v4 07/17] builtin/pack-objects.c: return from create_object_entry() Taylor Blau
2022-05-18 23:11 ` [PATCH v4 08/17] builtin/pack-objects.c: --cruft without expiration Taylor Blau
2022-05-19 10:04 ` Junio C Hamano
2022-05-19 15:16 ` Junio C Hamano
2022-05-20 22:52 ` Taylor Blau
2022-05-18 23:11 ` [PATCH v4 09/17] reachable: add options to add_unseen_recent_objects_to_traversal Taylor Blau
2022-05-18 23:11 ` [PATCH v4 10/17] reachable: report precise timestamps from objects in cruft packs Taylor Blau
2022-05-18 23:11 ` [PATCH v4 11/17] builtin/pack-objects.c: --cruft with expiration Taylor Blau
2022-05-18 23:11 ` [PATCH v4 12/17] builtin/repack.c: support generating a cruft pack Taylor Blau
2022-05-19 11:29 ` Ævar Arnfjörð Bjarmason
2022-05-20 22:39 ` Taylor Blau
2022-05-18 23:11 ` [PATCH v4 13/17] builtin/repack.c: allow configuring cruft pack generation Taylor Blau
2022-05-18 23:11 ` [PATCH v4 14/17] builtin/repack.c: use named flags for existing_packs Taylor Blau
2022-05-18 23:11 ` [PATCH v4 15/17] builtin/repack.c: add cruft packs to MIDX during geometric repack Taylor Blau
2022-05-19 11:32 ` Ævar Arnfjörð Bjarmason
2022-05-20 22:42 ` Taylor Blau
2022-05-18 23:11 ` [PATCH v4 16/17] builtin/gc.c: conditionally avoid pruning objects via loose Taylor Blau
2022-05-18 23:11 ` [PATCH v4 17/17] sha1-file.c: don't freshen cruft packs Taylor Blau
2022-05-18 23:48 ` [PATCH v4 00/17] " Derrick Stolee
2022-05-20 23:19 ` Junio C Hamano
2022-05-20 23:30 ` Taylor Blau
2022-05-19 11:42 ` [RFC PATCH 0/2] Utility functions for duplicated pack(write) code Ævar Arnfjörð Bjarmason
2022-05-19 11:42 ` [RFC PATCH 1/2] packfile API: add and use a pack_name_to_ext() utility function Ævar Arnfjörð Bjarmason
2022-05-19 15:40 ` Junio C Hamano
2022-05-19 11:42 ` [RFC PATCH 2/2] hash API: add and use a hash_short_id_by_algo() function Ævar Arnfjörð Bjarmason
2022-05-19 15:50 ` Junio C Hamano
2022-05-19 19:07 ` Ævar Arnfjörð Bjarmason
2022-05-19 15:31 ` [RFC PATCH 0/2] Utility functions for duplicated pack(write) code Junio C Hamano
2022-05-19 11:54 ` [PATCH v4 00/17] cruft packs Ævar Arnfjörð Bjarmason
2022-05-20 23:17 ` [PATCH v5 " Taylor Blau
2022-05-20 23:17 ` [PATCH v5 01/17] Documentation/technical: add cruft-packs.txt Taylor Blau
2022-05-20 23:17 ` [PATCH v5 02/17] pack-mtimes: support reading .mtimes files Taylor Blau
2022-05-24 19:32 ` Jonathan Nieder
2022-05-24 19:44 ` rsbecker
2022-05-24 22:25 ` Taylor Blau
2022-05-24 23:24 ` rsbecker
2022-05-25 0:07 ` Taylor Blau
2022-05-25 0:20 ` rsbecker
2022-05-25 9:11 ` adding new 32-bit on-disk (unsigned) timestamp formats (was: [PATCH v5 02/17] pack-mtimes: support reading .mtimes files) Ævar Arnfjörð Bjarmason
2022-05-25 13:30 ` Derrick Stolee
2022-05-25 21:13 ` Taylor Blau
2022-05-26 0:02 ` Ævar Arnfjörð Bjarmason
2022-05-26 0:12 ` Taylor Blau
2022-05-24 22:21 ` [PATCH v5 02/17] pack-mtimes: support reading .mtimes files Taylor Blau
2022-05-25 7:48 ` Jonathan Nieder
2022-05-25 21:36 ` Taylor Blau
2022-05-25 21:58 ` rsbecker
2022-05-25 22:59 ` Taylor Blau
2022-05-25 23:02 ` Taylor Blau
2022-05-26 0:30 ` Junio C Hamano
2023-06-01 13:01 ` Andreas Schwab
2022-05-20 23:17 ` [PATCH v5 03/17] pack-write: pass 'struct packing_data' to 'stage_tmp_packfiles' Taylor Blau
2022-05-20 23:17 ` [PATCH v5 04/17] chunk-format.h: extract oid_version() Taylor Blau
2022-05-20 23:17 ` [PATCH v5 05/17] pack-mtimes: support writing pack .mtimes files Taylor Blau
2022-05-20 23:17 ` [PATCH v5 06/17] t/helper: add 'pack-mtimes' test-tool Taylor Blau
2022-05-20 23:17 ` [PATCH v5 07/17] builtin/pack-objects.c: return from create_object_entry() Taylor Blau
2022-05-20 23:17 ` [PATCH v5 08/17] builtin/pack-objects.c: --cruft without expiration Taylor Blau
2022-05-20 23:17 ` [PATCH v5 09/17] reachable: add options to add_unseen_recent_objects_to_traversal Taylor Blau
2022-05-20 23:17 ` [PATCH v5 10/17] reachable: report precise timestamps from objects in cruft packs Taylor Blau
2022-05-20 23:18 ` [PATCH v5 11/17] builtin/pack-objects.c: --cruft with expiration Taylor Blau
2022-05-20 23:18 ` [PATCH v5 12/17] builtin/repack.c: support generating a cruft pack Taylor Blau
2022-05-20 23:18 ` [PATCH v5 13/17] builtin/repack.c: allow configuring cruft pack generation Taylor Blau
2022-05-20 23:18 ` [PATCH v5 14/17] builtin/repack.c: use named flags for existing_packs Taylor Blau
2022-05-20 23:18 ` [PATCH v5 15/17] builtin/repack.c: add cruft packs to MIDX during geometric repack Taylor Blau
2022-05-20 23:18 ` [PATCH v5 16/17] builtin/gc.c: conditionally avoid pruning objects via loose Taylor Blau
2022-06-19 5:38 ` René Scharfe
2022-06-21 15:58 ` Junio C Hamano
2022-05-20 23:18 ` [PATCH v5 17/17] sha1-file.c: don't freshen cruft packs Taylor Blau
2022-05-21 11:17 ` [PATCH v5 00/17] " Ævar Arnfjörð Bjarmason
2022-05-24 19:39 ` Jonathan Nieder
2022-05-24 21:50 ` Taylor Blau
2022-05-24 21:55 ` Ævar Arnfjörð Bjarmason
2022-05-24 22:12 ` Taylor Blau
2022-05-25 7:53 ` Jonathan Nieder
2022-05-25 19:59 ` Derrick Stolee [this message]
2022-05-25 21:09 ` Taylor Blau
2022-05-26 0:06 ` Ævar Arnfjörð Bjarmason
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7f5a6a6a-c554-c659-72a8-404bc39e08c7@github.com \
--to=derrickstolee@github.com \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
--cc=larsxschneider@gmail.com \
--cc=me@ttaylorr.com \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).