* [PATCH v4 2/3] t/lib-httpd: stop using legacy crypt(3) for authentication
From: Patrick Steinhardt @ 2023-11-10 8:17 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1699596457.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2129 bytes --]
When setting up httpd for our tests, we also install a passwd and
proxy-passwd file that contain the test user's credentials. These
credentials currently use crypt(3) as the password encryption schema.
This schema can be considered deprecated nowadays as it is not safe
anymore. Quoting Apache httpd's documentation [1]:
> Unix only. Uses the traditional Unix crypt(3) function with a
> randomly-generated 32-bit salt (only 12 bits used) and the first 8
> characters of the password. Insecure.
This is starting to cause issues in modern Linux distributions. glibc
has deprecated its libcrypt library that used to provide crypt(3) in
favor of the libxcrypt library. This newer replacement provides a
compile time switch to disable insecure password encryption schemata,
which causes crypt(3) to always return `EINVAL`. The end result is that
httpd tests that exercise authentication will fail on distros that use
libxcrypt without these insecure encryption schematas.
Regenerate the passwd files to instead use the default password
encryption schema, which is md5. While it feels kind of funny that an
MD5-based encryption schema should be more secure than anything else, it
is the current default and supported by all platforms. Furthermore, it
really doesn't matter all that much given that these files are only used
for testing purposes anyway.
[1]: https://httpd.apache.org/docs/2.4/misc/password_encryptions.html
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/lib-httpd/passwd | 2 +-
t/lib-httpd/proxy-passwd | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/lib-httpd/passwd b/t/lib-httpd/passwd
index 99a34d64874..d9c122f3482 100644
--- a/t/lib-httpd/passwd
+++ b/t/lib-httpd/passwd
@@ -1 +1 @@
-user@host:xb4E8pqD81KQs
+user@host:$apr1$LGPmCZWj$9vxEwj5Z5GzQLBMxp3mCx1
diff --git a/t/lib-httpd/proxy-passwd b/t/lib-httpd/proxy-passwd
index 77c25138e07..2ad7705d9a3 100644
--- a/t/lib-httpd/proxy-passwd
+++ b/t/lib-httpd/proxy-passwd
@@ -1 +1 @@
-proxuser:2x7tAukjAED5M
+proxuser:$apr1$RxS6MLkD$DYsqQdflheq4GPNxzJpx5.
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v4 0/3] t: improve compatibility with NixOS
From: Patrick Steinhardt @ 2023-11-10 8:16 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1699428122.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 3367 bytes --]
Hi,
this is the fourth version of my patch series to improve compatibility
of our test suite with NixOS.
Three changes compared to v3:
- Switched from `test <expr> -a <expr>` to `test <expr> && test
<expr>`.
- Improved the commit message to explain why the new
runtime-detected paths are only used as a fallback.
- Rebased on top of 0e3b67e2aa (ci: add support for GitLab CI,
2023-11-09), which has been merged to `next` and caused conflicts.
Technically speaking this series also depends on 0763c3a2c4 (http:
update curl http/2 info matching for curl 8.3.0, 2023-09-15), without
which the tests will fail on NixOS machines with a recent libcurl.
Patrick
Patrick Steinhardt (3):
t/lib-httpd: dynamically detect httpd and modules path
t/lib-httpd: stop using legacy crypt(3) for authentication
t9164: fix inability to find basename(1) in Subversion hooks
t/lib-httpd.sh | 17 +++++++++++++----
t/lib-httpd/passwd | 2 +-
t/lib-httpd/proxy-passwd | 2 +-
t/t9164-git-svn-dcommit-concurrent.sh | 9 ++++++++-
4 files changed, 23 insertions(+), 7 deletions(-)
Range-diff against v3:
1: e4c75c492dd ! 1: 41b9dada2e0 t/lib-httpd: dynamically detect httpd and modules path
@@ Commit message
- The module directory can (in many cases) be derived via the
`HTTPD_ROOT` compile-time variable.
- Amend the code to do so. The new runtime-detected paths will only be
- used in case none of the hardcoded paths are usable.
+ Amend the code to do so.
+
+ Note that the new runtime-detected paths will only be used as a fallback
+ in case none of the hardcoded paths are usable. For the PATH lookup this
+ is because httpd is typically installed into "/usr/sbin", which is often
+ not included in the user's PATH variable. And the module path detection
+ relies on a configured httpd installation and may thus not work in all
+ cases, either.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
@@ t/lib-httpd.sh: fi
+ "$(command -v apache2)"
do
- if test -x "$DEFAULT_HTTPD_PATH"
-+ if test -n "$DEFAULT_HTTPD_PATH" -a -x "$DEFAULT_HTTPD_PATH"
++ if test -n "$DEFAULT_HTTPD_PATH" && test -x "$DEFAULT_HTTPD_PATH"
then
break
fi
@@ t/lib-httpd.sh: fi
'/usr/lib/apache2/modules' \
'/usr/lib64/httpd/modules' \
'/usr/lib/httpd/modules' \
-- '/usr/libexec/httpd'
-+ '/usr/libexec/httpd' \
+ '/usr/libexec/httpd' \
+- '/usr/lib/apache2'
++ '/usr/lib/apache2' \
+ "${DETECTED_HTTPD_ROOT:+${DETECTED_HTTPD_ROOT}/modules}"
do
- if test -d "$DEFAULT_HTTPD_MODULE_PATH"
-+ if test -n "$DEFAULT_HTTPD_MODULE_PATH" -a -d "$DEFAULT_HTTPD_MODULE_PATH"
++ if test -n "$DEFAULT_HTTPD_MODULE_PATH" && test -d "$DEFAULT_HTTPD_MODULE_PATH"
then
break
fi
2: 3dce76da477 = 2: 7d28c32feaa t/lib-httpd: stop using legacy crypt(3) for authentication
3: 6891e254155 = 3: 6a7773aadba t9164: fix inability to find basename(1) in Subversion hooks
base-commit: 0e3b67e2aa25edb7e1a5c999c87b52a7b3a7649a
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v4 1/3] t/lib-httpd: dynamically detect httpd and modules path
From: Patrick Steinhardt @ 2023-11-10 8:17 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1699596457.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2482 bytes --]
In order to set up the Apache httpd server, we need to locate both the
httpd binary and its default module path. This is done with a hardcoded
list of locations that we scan. While this works okayish with distros
that more-or-less follow the Filesystem Hierarchy Standard, it falls
apart on others like NixOS that don't.
While it is possible to specify these paths via `LIB_HTTPD_PATH` and
`LIB_HTTPD_MODULE_PATH`, it is not a nice experience for the developer
to figure out how to set those up. And in fact we can do better by
dynamically detecting both httpd and its module path at runtime:
- The httpd binary can be located via PATH.
- The module directory can (in many cases) be derived via the
`HTTPD_ROOT` compile-time variable.
Amend the code to do so.
Note that the new runtime-detected paths will only be used as a fallback
in case none of the hardcoded paths are usable. For the PATH lookup this
is because httpd is typically installed into "/usr/sbin", which is often
not included in the user's PATH variable. And the module path detection
relies on a configured httpd installation and may thus not work in all
cases, either.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/lib-httpd.sh | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 9ea74927c40..f69d0da51d1 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -55,22 +55,31 @@ fi
HTTPD_PARA=""
-for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'
+for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' \
+ '/usr/sbin/apache2' \
+ "$(command -v httpd)" \
+ "$(command -v apache2)"
do
- if test -x "$DEFAULT_HTTPD_PATH"
+ if test -n "$DEFAULT_HTTPD_PATH" && test -x "$DEFAULT_HTTPD_PATH"
then
break
fi
done
+if test -x "$DEFAULT_HTTPD_PATH"
+then
+ DETECTED_HTTPD_ROOT="$("$DEFAULT_HTTPD_PATH" -V | sed -n 's/^ -D HTTPD_ROOT="\(.*\)"$/\1/p')"
+fi
+
for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \
'/usr/lib/apache2/modules' \
'/usr/lib64/httpd/modules' \
'/usr/lib/httpd/modules' \
'/usr/libexec/httpd' \
- '/usr/lib/apache2'
+ '/usr/lib/apache2' \
+ "${DETECTED_HTTPD_ROOT:+${DETECTED_HTTPD_ROOT}/modules}"
do
- if test -d "$DEFAULT_HTTPD_MODULE_PATH"
+ if test -n "$DEFAULT_HTTPD_MODULE_PATH" && test -d "$DEFAULT_HTTPD_MODULE_PATH"
then
break
fi
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: [ANNOUNCE] Git v2.43.0-rc1
From: Andy Koppe @ 2023-11-10 7:59 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Linux Kernel, git-packagers
In-Reply-To: <xmqq8r785ev1.fsf@gitster.g>
On 08/11/2023 17:33, Junio C Hamano wrote:
> * "git log --format" has been taught the %(decorate) placeholder.
* "git log --format" has been taught the %(decorate) placeholder for
customizing the symbols used in ref decorations.
?
Regards,
Andy
^ permalink raw reply
* [PATCH v3 13/13] trailer doc: <token> is a <key> or <keyAlias>, not both
From: Teng Long @ 2023-11-10 6:33 UTC (permalink / raw)
To: gitgitgadget; +Cc: chriscool, git, linusa
In-Reply-To: <0b9525db5a0787fdc71834abad9151aa03acc497.1694125210.git.gitgitgadget@gmail.com>
Linus Arver <linusa@google.com> writes:
> diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt
> index 25433e1a1ff..418265f044d 100644
> --- a/Documentation/git-interpret-trailers.txt
> +++ b/Documentation/git-interpret-trailers.txt
> @@ -9,7 +9,7 @@ SYNOPSIS
> --------
> [verse]
> 'git interpret-trailers' [--in-place] [--trim-empty]
> - [(--trailer <token>[(=|:)<value>])...]
> + [(--trailer (<key>|<keyAlias>)[(=|:)<value>])...]
Maybe use 'key-alias' instead of 'keyAlias'? After checking the naming in some
other documents, it seems that they are basically in the former format, not
camel case format.
Thanks.
^ permalink raw reply
* Re: recover lost file
From: Junio C Hamano @ 2023-11-10 4:09 UTC (permalink / raw)
To: Malik Rumi; +Cc: git
In-Reply-To: <CAKd6oBxF8m09QnqZ46joVpcR3mrR4MRSO+kL8vELGwVhD=rgXg@mail.gmail.com>
Malik Rumi <malik.a.rumi@gmail.com> writes:
> I am looking for a lost file that was accidentally deleted about 18
> months ago. The docs I consulted are version 2.42.1. The git on my
> machine is version 2.34.1
>
> I followed the instructions on this page”
> https://git-scm.com/book/en/v2/Git-Tools-Searching , the section “Line
> Log Search”. But I got an error.
>
> malikarumi@Tetuoan2:~/Projects/hattie$ git grep titlesplit
> malikarumi@Tetuoan2:~/Projects/hattie$ git grep filesplit
> malikarumi@Tetuoan2:~/Projects/hattie$ git log -L :titlesplit
> fatal: -L argument not 'start,end:file' or ':funcname:file': :titlesplit
> malikarumi@Tetuoan2:~/Projects/hattie$ git log -L :titlesplit:
> fatal: -L argument not 'start,end:file' or ':funcname:file': :titlesplit:
>
> I don’t know the purpose of the colon. Is it a boundary marker? Does
> it belong at the front of the search object, the end, or both?
> 'start,end:file' sounds like the error message expects me to provide a
> start and an end, to which the obvious reply is - If I knew where it
> was, I wouldn’t be trying to find it.
Whatever documenttion that gave you "-L" perhaps did not give you a
good example.
$ git log -L"20,+5:helloworld.c"
$ git log -L"/^titlesplit(/,/^}/:helloworld.c"
$ git log -L":titlesplit:helloworld.c"
are ways to follow, how the range of lines in the helloworld.c file
of the CURRENTLY CHECKED OUT version, came about throughout the
history, going backwards. The way to specify the range in the
current version of helloworld.c file may be different (the examples
show "starting at line #20, for five lines", "starting at the line
that matches the pattern "^titlesplit(", and ending at the line that
matches the pattern "^}", or "the body of the 'titlesplit' function
(according to the xfuncname logic)"), but they share one important
detail that may make the feature unsuitable for your use case. You
need to exactly know where in the current version of which file the
line range you want to follow resides, but from your description I
am getting an impression that you do not even know the name of the
file.
You only said "a lost file" without giving any specifics, so it is
totally unclear to readers of your message how strings like
"titlesplit" and "filesplit" relate to what you are looking for. In
the above I randomly made a blind guess that it might be a function
name, but I may be totally off the mark.
- Do you mean "I think the file I removed had a name with either
titlesplit or filesplit in it?"
- Or do you mean "I know that the file had a definition of a
function whose name was either titlesplit or filesplit?"
- Or something completely different from the above two?
If I know all of the followings are true:
- I had the necessary contents committed in Git;
- I do not remember the filename at all, but I am sure I deleted it
and committed the deletion some time ago;
- I know the lost contents I am looking for had a string "frotz" in
it.
then I would probably try
$ git log -Sfrotz --diff-filter=D -p
which will look for all file deletions throughout the history,
limiting the output to those that had string "frotz" in them.
But again, it is unclear what useful clue you have to locate the
lost file from your description, so ...
^ permalink raw reply
* Re: recover lost file
From: Junio C Hamano @ 2023-11-10 5:27 UTC (permalink / raw)
To: Malik Rumi; +Cc: git
In-Reply-To: <xmqqmsvmw8oc.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> You only said "a lost file" without giving any specifics, so it is
> totally unclear to readers of your message how strings like
> "titlesplit" and "filesplit" relate to what you are looking for. In
> the above I randomly made a blind guess that it might be a function
> name, but I may be totally off the mark.
>
> - Do you mean "I think the file I removed had a name with either
> titlesplit or filesplit in it?"
>
> - Or do you mean "I know that the file had a definition of a
> function whose name was either titlesplit or filesplit?"
>
> - Or something completely different from the above two?
>
> If I know all of the followings are true:
>
> - I had the necessary contents committed in Git;
>
> - I do not remember the filename at all, but I am sure I deleted it
> and committed the deletion some time ago;
>
> - I know the lost contents I am looking for had a string "frotz" in
> it.
>
> then I would probably try
>
> $ git log -Sfrotz --diff-filter=D -p
>
> which will look for all file deletions throughout the history,
> limiting the output to those that had string "frotz" in them.
>
> But again, it is unclear what useful clue you have to locate the
> lost file from your description, so ...
If the scenario were
- I know the file were once committed in Git;
- I do not remember the filename, but I think its name had either
"frotz" or "nitfol" in it;
then I would try this instead:
$ git log --diff-filter=D --summary -- '*frotz*' '*nitfol*'
^ permalink raw reply
* Re: [PATCH] glossary: add definitions for dereference & peel
From: Junio C Hamano @ 2023-11-10 5:20 UTC (permalink / raw)
To: Victoria Dye via GitGitGadget; +Cc: git, ps, Victoria Dye
In-Reply-To: <xmqq1qcyxxri.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> I've never seen "peel" used for commits, though. So another
> improvement might be to say "peel" is "an act of dereferencing a
> tag" here.
I am reasonably sure I was the one who coined the term "peel", and
the picture I had in mind when I used it was to peel an onion, which
inherently was about unwrapping many levels repeatedly. I think
that is why it felt strange to see "peel" used in the context of
using a commit as a tree-ish, which (as your documentation update
clearly said) is doable only once.
^ permalink raw reply
* Re: [PATCH 1/7] chunk-format: introduce `pair_chunk_expect()` helper
From: Junio C Hamano @ 2023-11-10 4:55 UTC (permalink / raw)
To: Taylor Blau; +Cc: git, Jeff King
In-Reply-To: <af5fe3b7237caeba8f970e967933db96c83a230e.1699569246.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
> +static int pair_chunk_expect_fn(const unsigned char *chunk_start,
> + size_t chunk_size,
> + void *data)
> +{
> + struct pair_chunk_data *pcd = data;
> + if (chunk_size / pcd->record_size != pcd->record_nr)
> + return -1;
> + *pcd->p = chunk_start;
> + return 0;
> +}
I know one of the original places did the "divide the whole by
per-record size and see if it matches the number of records", the
same as we see above, but the check in the above could also be
if (chunk_size != st_mult(pcd->record_size, pcd->record_nr))
return -1;
which would also catch the case where chunk_size is not a multiple
of the record size. Your conversion of OOFF in midx.c loses this
protection as the original uses the multiplication-and-compare, but
the rewrite to call pair_chunk_expect would call the above and
checks with the truncating-divide-and-compare.
Does the distinction matter? I dunno. If the record/chunk
alignment is asserted elsewhere, then the distinction should not
matter, but even if it were, seeing a truncating division used in
any validation makes my skin tingle.
Other than that, the series was a pleasant read.
Thanks.
^ permalink raw reply
* recover lost file
From: Malik Rumi @ 2023-11-10 3:02 UTC (permalink / raw)
To: git
I am looking for a lost file that was accidentally deleted about 18
months ago. The docs I consulted are version 2.42.1. The git on my
machine is version 2.34.1
I followed the instructions on this page”
https://git-scm.com/book/en/v2/Git-Tools-Searching , the section “Line
Log Search”. But I got an error.
malikarumi@Tetuoan2:~/Projects/hattie$ git grep titlesplit
malikarumi@Tetuoan2:~/Projects/hattie$ git grep filesplit
malikarumi@Tetuoan2:~/Projects/hattie$ git log -L :titlesplit
fatal: -L argument not 'start,end:file' or ':funcname:file': :titlesplit
malikarumi@Tetuoan2:~/Projects/hattie$ git log -L :titlesplit:
fatal: -L argument not 'start,end:file' or ':funcname:file': :titlesplit:
I don’t know the purpose of the colon. Is it a boundary marker? Does
it belong at the front of the search object, the end, or both?
'start,end:file' sounds like the error message expects me to provide a
start and an end, to which the obvious reply is - If I knew where it
was, I wouldn’t be trying to find it.
What is the correct syntax?
Is there another search option?
Thanx
---
“None of you has faith until he loves for his brother or his neighbor
what he loves for himself.”
^ permalink raw reply
* Re: [PATCH 1/1] attr: enable attr pathspec magic for git-add and git-stash
From: Joanna Wang @ 2023-11-10 2:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Joanna Wang
whoops no comments. thank you for the review and making the fixes!
^ permalink raw reply
* Re: git-send-email: Send with mutt(1)
From: Alejandro Colomar @ 2023-11-10 0:51 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20231109180308.GA2711684@coredump.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 2469 bytes --]
Hi Jeff,
On Thu, Nov 09, 2023 at 01:03:08PM -0500, Jeff King wrote:
> On Thu, Nov 09, 2023 at 04:26:23PM +0100, Alejandro Colomar wrote:
>
> > I've tried something even simpler:
> >
> > ---8<---
> > #!/bin/sh
> >
> > mutt -H -;
> > --->8---
> >
> > I used it for sending a couple of patches to linux-man@, and it seems to
> > work. I don't have much experience with mutt, so maybe I'm missing some
> > corner cases. Do you expect it to not work for some case? Otherwise,
> > we might have a winner. :)
>
> Wow, I don't know how I missed that when I read the manual. That was
> exactly the feature I was thinking that mutt would need. ;)
>
> So yeah, that is obviously better than the "postponed" hackery I showed.
> I notice that "-H" even causes mutt to ignore "-i" (a sendmail flag that
> Git adds to sendemail.sendmailcmd). So you can just invoke it directly
> from your config like:
>
> git config sendemail.sendmailcmd "mutt -H -"
Having it directly in sendmailcmd causes some glitch: It repeats all CCs
in TO. See a log:
Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll): y
OK. Log says:
Sendmail: mutt -H - -i kevin@8t8.us mutt-dev@mutt.org alx@kernel.org e.sovetkin@gmail.com neomutt-devel@neomutt.org
From: Alejandro Colomar <alx@kernel.org>
To: Kevin McCarthy <kevin@8t8.us>,
mutt-dev@mutt.org
Cc: Alejandro Colomar <alx@kernel.org>,
Jenya Sovetkin <e.sovetkin@gmail.com>,
neomutt-devel@neomutt.org
Subject: [PATCH] send.c: Allow crypto operations in batch and mailx modes.
Date: Fri, 10 Nov 2023 01:41:24 +0100
Message-ID: <20231110004128.5972-2-alx@kernel.org>
X-Mailer: git-send-email 2.42.0
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Result: OK
The sent mail ended up being
From: Alejandro Colomar <alx@kernel.org>
To: Kevin McCarthy <kevin@8t8.us>, mutt-dev@mutt.org, alx@kernel.org,
e.sovetkin@gmail.com, neomutt-devel@neomutt.org
Cc: Alejandro Colomar <alx@kernel.org>,
Jenya Sovetkin <e.sovetkin@gmail.com>, neomutt-devel@neomutt.org
So maybe we need the wrapper script to ignore the arguments.
Cheers,
Alex
>
> Annoyingly, "-E" doesn't work when reading over stdin (I guess mutt
> isn't willing to re-open the tty itself). But if you're happy with not
> editing as they go through, then "-H" is then that's enough (in my
> workflow, I do the final proofread via mutt).
>
> -Peff
--
<https://www.alejandro-colomar.es/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] glossary: add definitions for dereference & peel
From: Junio C Hamano @ 2023-11-10 0:22 UTC (permalink / raw)
To: Victoria Dye via GitGitGadget; +Cc: git, ps, Victoria Dye
In-Reply-To: <pull.1610.git.1699574277143.gitgitgadget@gmail.com>
"Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com> writes:
> @@ -125,6 +124,24 @@ to point at the new commit.
> dangling object has no references to it from any
> reference or <<def_object,object>> in the <<def_repository,repository>>.
>
> +[[def_dereference]]dereference::
> + Referring to a <<def_symref,symbolic ref>>: the action of accessing the
> + <<def_ref,reference>> pointed at by a symbolic ref. Recursive
> + dereferencing involves repeating the aforementioned process on the
> + resulting ref until a non-symbolic reference is found.
> ++
> +Referring to a <<def_tag_object,tag object>>: the action of accessing the
> +<<def_object,object>> a tag points at. Tags are recursively dereferenced by
> +repeating the operation on the result object until the result has either a
> +specified <<def_object_type,object type>> (where applicable) or any non-"tag"
> +object type.
> ++
All of the above makes sense.
I would casually mention "peeling" here with cross reference,
if I were writing this section. There already is enough cross
reference in the other direction pointing this way.
> +Referring to a <<def_commit_object,commit object>>: the action of accessing
> +the commit's tree object. Commits cannot be dereferenced recursively.
I personally consider this is weird misuse of the verb and is rarely
used, but we see it in the description of tree-ish below.
> +Unless otherwise specified, "dereferencing" as it used in the context of Git
> +commands or protocols is implicitly recursive.
Nice to see this spelled out like this.
> @@ -444,6 +461,12 @@ exclude;;
> of the logical predecessor(s) in the line of development, i.e. its
> parents.
>
> +[[def_peel]]peel::
> + Synonym for object <<def_dereference,dereference>>. Most commonly used
> + in the context of tags, where it refers to the process of recursively
> + dereferencing a <<def_tag_object,tag object>> until the result object's
> + <<def_object_type,type>> is something other than "tag".
"object dereference" is not defined anywhere (yet). "Most commonly
used in the context of tags" implies that objects other than tags
can be "peeled" and "object dereference" is a word to refer to
peeling either "commit" or "tag", but we would want to be a bit more
clear and explicit. Let's either define "object dereference", or
better yet, avoid saying "object dereference" here and instead say
something like: "Synonym for dereference when used on tags and
commits".
I've never seen "peel" used for commits, though. So another
improvement might be to say "peel" is "an act of dereferencing a
tag" here.
Thanks.
^ permalink raw reply
* [PATCH] glossary: add definitions for dereference & peel
From: Victoria Dye via GitGitGadget @ 2023-11-09 23:57 UTC (permalink / raw)
To: git; +Cc: ps, Victoria Dye, Victoria Dye
From: Victoria Dye <vdye@github.com>
Add 'gitglossary' definitions for "dereference" (as it used for both symrefs
and objects) and "peel". These terms are used in options and documentation
throughout Git, but they are not clearly defined anywhere and the behavior
they refer to depends heavily on context. Provide explicit definitions to
clarify existing documentation to users and help contributors to use the
most appropriate terminology possible in their additions to Git.
Update other definitions in the glossary that use the term "dereference" to
link to 'def_dereference'.
Signed-off-by: Victoria Dye <vdye@github.com>
---
glossary: add definitions for dereference & peel
As promised in [1], this patch adds definitions for "peel" and
"dereference" in the glossary, based on how they're currently used
throughout Git. As a result, the definitions are somewhat broad
(although I did my best to explicitly describe the different contexts in
which they're used). My hope is that this will at least reduce confusion
around this terminology. These definitions can also serve as a starting
point if, in the future, another contributor wants to deprecate certain
usages of these terms to make them less ambiguous.
* Victoria
[1]
https://lore.kernel.org/git/21dfe606-39f5-4154-aaa4-695e5f6f784d@github.com/
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1610%2Fvdye%2Fvdye%2Fglossary-peel-dereference-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1610/vdye/vdye/glossary-peel-dereference-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1610
Documentation/glossary-content.txt | 50 +++++++++++++++++++++---------
1 file changed, 36 insertions(+), 14 deletions(-)
diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
index 65c89e7b3eb..41dd5721def 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -98,9 +98,8 @@ to point at the new commit.
revision.
[[def_commit-ish]]commit-ish (also committish)::
- A <<def_commit_object,commit object>> or an
- <<def_object,object>> that can be recursively dereferenced to
- a commit object.
+ A <<def_commit_object,commit object>> or an <<def_object,object>> that
+ can be recursively <<def_dereference,dereferenced>> to a commit object.
The following are all commit-ishes:
a commit object,
a <<def_tag_object,tag object>> that points to a commit
@@ -125,6 +124,24 @@ to point at the new commit.
dangling object has no references to it from any
reference or <<def_object,object>> in the <<def_repository,repository>>.
+[[def_dereference]]dereference::
+ Referring to a <<def_symref,symbolic ref>>: the action of accessing the
+ <<def_ref,reference>> pointed at by a symbolic ref. Recursive
+ dereferencing involves repeating the aforementioned process on the
+ resulting ref until a non-symbolic reference is found.
++
+Referring to a <<def_tag_object,tag object>>: the action of accessing the
+<<def_object,object>> a tag points at. Tags are recursively dereferenced by
+repeating the operation on the result object until the result has either a
+specified <<def_object_type,object type>> (where applicable) or any non-"tag"
+object type.
++
+Referring to a <<def_commit_object,commit object>>: the action of accessing
+the commit's tree object. Commits cannot be dereferenced recursively.
++
+Unless otherwise specified, "dereferencing" as it used in the context of Git
+commands or protocols is implicitly recursive.
+
[[def_detached_HEAD]]detached HEAD::
Normally the <<def_HEAD,HEAD>> stores the name of a
<<def_branch,branch>>, and commands that operate on the
@@ -444,6 +461,12 @@ exclude;;
of the logical predecessor(s) in the line of development, i.e. its
parents.
+[[def_peel]]peel::
+ Synonym for object <<def_dereference,dereference>>. Most commonly used
+ in the context of tags, where it refers to the process of recursively
+ dereferencing a <<def_tag_object,tag object>> until the result object's
+ <<def_object_type,type>> is something other than "tag".
+
[[def_pickaxe]]pickaxe::
The term <<def_pickaxe,pickaxe>> refers to an option to the diffcore
routines that help select changes that add or delete a given text
@@ -620,12 +643,11 @@ The most notable example is `HEAD`.
copies of) commit objects of the contained submodules.
[[def_symref]]symref::
- Symbolic reference: instead of containing the <<def_SHA1,SHA-1>>
- id itself, it is of the format 'ref: refs/some/thing' and when
- referenced, it recursively dereferences to this reference.
- '<<def_HEAD,HEAD>>' is a prime example of a symref. Symbolic
- references are manipulated with the linkgit:git-symbolic-ref[1]
- command.
+ Symbolic reference: instead of containing the <<def_SHA1,SHA-1>> id
+ itself, it is of the format 'ref: refs/some/thing' and when referenced,
+ it recursively <<def_dereference,dereferences>> to this reference.
+ '<<def_HEAD,HEAD>>' is a prime example of a symref. Symbolic references
+ are manipulated with the linkgit:git-symbolic-ref[1] command.
[[def_tag]]tag::
A <<def_ref,ref>> under `refs/tags/` namespace that points to an
@@ -661,11 +683,11 @@ The most notable example is `HEAD`.
<<def_tree,tree>> is equivalent to a <<def_directory,directory>>.
[[def_tree-ish]]tree-ish (also treeish)::
- A <<def_tree_object,tree object>> or an <<def_object,object>>
- that can be recursively dereferenced to a tree object.
- Dereferencing a <<def_commit_object,commit object>> yields the
- tree object corresponding to the <<def_revision,revision>>'s
- top <<def_directory,directory>>.
+ A <<def_tree_object,tree object>> or an <<def_object,object>> that can
+ be recursively <<def_dereference,dereferenced>> to a tree object.
+ Dereferencing a <<def_commit_object,commit object>> yields the tree
+ object corresponding to the <<def_revision,revision>>'s top
+ <<def_directory,directory>>.
The following are all tree-ishes:
a <<def_commit-ish,commit-ish>>,
a tree object,
base-commit: dadef801b365989099a9929e995589e455c51fed
--
gitgitgadget
^ permalink raw reply related
* Re: What's cooking in git.git (Nov 2023, #04; Thu, 9)
From: Junio C Hamano @ 2023-11-09 23:33 UTC (permalink / raw)
To: Taylor Blau, Johannes Schindelin; +Cc: Jeff King, git
In-Reply-To: <ZUzcmsfJe6jk4fTk@nand.local>
Taylor Blau <me@ttaylorr.com> writes:
> On Thu, Nov 09, 2023 at 02:40:28AM +0900, Junio C Hamano wrote:
>> * tb/merge-tree-write-pack (2023-10-23) 5 commits
> ...
> This series received a couple of LGTMs from you and Patrick:
>
> - https://lore.kernel.org/git/xmqqo7go7w63.fsf@gitster.g/#t
> - https://lore.kernel.org/git/ZTjKmcV5c_EFuoGo@tanuki/
Yup, I am aware of them.
> Johannes had posted some comments[1] about instead using a temporary
> object store where objects are written as loose that would extend to git
> replay....
I was hoping to hear from Johannes saying he agrees with the above.
It is not strictly required, but is much nice to have once we hear
"let's step back a bit---are we going in the right direction?" and
it has been responded.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/1] attr: enable attr pathspec magic for git-add and git-stash
From: Junio C Hamano @ 2023-11-09 23:27 UTC (permalink / raw)
To: Joanna Wang; +Cc: git
In-Reply-To: <xmqqsf5mgl0r.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Thanks. The compiled result from this version looks quite good. As
> you started a new round with typofix, let me start the final
> nitpicking.
> ...
> Here is what I ended up queuing.
>
> Thanks.
Any comments on the "counterproposal to fix nitpicks"? I'd like to
make sure the original author is either OK with the update (in which
case I can just move what I have in my tree forward) or unhappy with
it and plan to send in an update on their own (in which case I can
wait for the next iteration), in a case like this.
Thanks.
^ permalink raw reply
* Re: [PATCH v10 1/3] unit tests: Add a project plan document
From: Junio C Hamano @ 2023-11-09 23:15 UTC (permalink / raw)
To: Josh Steadmon; +Cc: git, phillip.wood123, oswald.buddenhagen, christian.couder
In-Reply-To: <f706ba9b682e7c4070d49086ad3af582bf269c79.1699555664.git.steadmon@google.com>
Josh Steadmon <steadmon@google.com> writes:
> In our current testing environment, we spend a significant amount of
> effort crafting end-to-end tests for error conditions that could easily
> be captured by unit tests (or we simply forgo some hard-to-setup and
> rare error conditions). Describe what we hope to accomplish by
> implementing unit tests, and explain some open questions and milestones.
> Discuss desired features for test frameworks/harnesses, and provide a
> comparison of several different frameworks. Finally, document our
> rationale for implementing a custom framework.
>
> Co-authored-by: Calvin Wan <calvinwan@google.com>
> Signed-off-by: Calvin Wan <calvinwan@google.com>
> Signed-off-by: Josh Steadmon <steadmon@google.com>
> ---
> Documentation/Makefile | 1 +
> Documentation/technical/unit-tests.txt | 240 +++++++++++++++++++++++++
> 2 files changed, 241 insertions(+)
> create mode 100644 Documentation/technical/unit-tests.txt
Looks good. I'll downcase "Add" on the title to match what I have
in my tree, but otherwise it looks OK to me. Let's see if we can
mark this round ready for 'next' and check if we hear complaints.
I have to make sure I do not forget about the other topic that
builds on top of this one.
Thanks.
^ permalink raw reply
* Re: [PATCH 2/4] contrib/subtree: stop using `-o` to test for number of args
From: Junio C Hamano @ 2023-11-09 23:02 UTC (permalink / raw)
To: Jeff King; +Cc: Patrick Steinhardt, git
In-Reply-To: <20231109185515.GD2711684@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> # Usage: process_subtree_split_trailer SPLIT_HASH MAIN_HASH [REPOSITORY]
>> process_subtree_split_trailer () {
>> - assert test $# = 2 -o $# = 3
>> + assert test $# -ge 2
>> + assert test $# -le 3
>
> It took me a minute to figure out why we were swapping "=" for "-ge". It
> is because we want to logical-OR the two conditions, but "assert"
> requires that we test one at a time. I think that is probably worth
> explaining in the commit message.
I wish we could write something like
assert test $# -ge 2 && test $# -le 3
(and I'd allow double quoting the whole thing after assert if
needed) but we cannot do so without tweaking the implementation of
assert.
>
>> @@ -916,7 +919,7 @@ cmd_split () {
>> if test $# -eq 0
>> then
>> rev=$(git rev-parse HEAD)
>> - elif test $# -eq 1 -o $# -eq 2
>> + elif test $# -eq 1 || test $# -eq 2
>
> OK, this one is a straight-forward use of "||".
Yes, but why not consistently use the range notation like the
earlier one here, or below?
elif test $# -ge 1 && test $# -le 2
>> cmd_merge () {
>> - test $# -eq 1 -o $# -eq 2 ||
>> + if test $# -lt 1 || test $# -gt 2
>> ...
> (I am OK with either, it just took me a minute to verify that your
> conversion was correct. But that is a one-time issue now while
> reviewing, and I think the code is readable going forward).
Yeah, the end result looks good.
Thanks, both.
^ permalink raw reply
* Re: git-send-email: Send with mutt(1)
From: Alejandro Colomar @ 2023-11-09 23:00 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20231109180308.GA2711684@coredump.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 1556 bytes --]
Hi Jeff,
On Thu, Nov 09, 2023 at 01:03:08PM -0500, Jeff King wrote:
> On Thu, Nov 09, 2023 at 04:26:23PM +0100, Alejandro Colomar wrote:
>
> > I've tried something even simpler:
> >
> > ---8<---
> > #!/bin/sh
> >
> > mutt -H -;
> > --->8---
> >
> > I used it for sending a couple of patches to linux-man@, and it seems to
> > work. I don't have much experience with mutt, so maybe I'm missing some
> > corner cases. Do you expect it to not work for some case? Otherwise,
> > we might have a winner. :)
>
> Wow, I don't know how I missed that when I read the manual. That was
> exactly the feature I was thinking that mutt would need. ;)
>
> So yeah, that is obviously better than the "postponed" hackery I showed.
> I notice that "-H" even causes mutt to ignore "-i" (a sendmail flag that
> Git adds to sendemail.sendmailcmd). So you can just invoke it directly
> from your config like:
>
> git config sendemail.sendmailcmd "mutt -H -"
Hmm great then! Definitely a winner. :)
>
> Annoyingly, "-E" doesn't work when reading over stdin (I guess mutt
> isn't willing to re-open the tty itself). But if you're happy with not
> editing as they go through, then "-H" is then that's enough (in my
> workflow, I do the final proofread via mutt).
Since git-send-email allows editing, I usually edit with that. Having
-E would be redundant (and in fact it felt like that to me with your
suggested mutt-as-mta.sh) for my use case.
Cheers,
Alex
>
> -Peff
--
<https://www.alejandro-colomar.es/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 1/4] global: convert trivial usages of `test <expr> -a/-o <expr>`
From: Junio C Hamano @ 2023-11-09 22:56 UTC (permalink / raw)
To: Jeff King; +Cc: Patrick Steinhardt, git
In-Reply-To: <20231109184843.GC2711684@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> But IMHO it is worth getting rid of all -o/-a regardless. Even
> non-ambiguous cases make reasoning about the code harder, and we don't
> want to encourage people to think they're OK to use.
You're 100% correct. "We do not have to worry about it" is a very
strong incentive to go after.
>
>> I do not need a subshell for grouping, either. Plain {} should do
>> (but you may need a LF or semicolon after the statement)..
>
> This I definitely agree with. :)
;-).
^ permalink raw reply
* [PATCH 7/7] midx: read `OOFF` chunk with `pair_chunk_expect()`
From: Taylor Blau @ 2023-11-09 22:34 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1699569246.git.me@ttaylorr.com>
The `OOFF` chunk can benefit from the new chunk-format API function
described in the previous commit. Convert it to `pair_chunk_expect()`
accordingly.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
midx.c | 17 +++--------------
t/t5319-multi-pack-index.sh | 1 -
2 files changed, 3 insertions(+), 15 deletions(-)
diff --git a/midx.c b/midx.c
index 66f34ed1a3..ca41748b74 100644
--- a/midx.c
+++ b/midx.c
@@ -86,19 +86,6 @@ static int midx_read_oid_fanout(const unsigned char *chunk_start,
return 0;
}
-static int midx_read_object_offsets(const unsigned char *chunk_start,
- size_t chunk_size, void *data)
-{
- struct multi_pack_index *m = data;
- m->chunk_object_offsets = chunk_start;
-
- if (chunk_size != st_mult(m->num_objects, MIDX_CHUNK_OFFSET_WIDTH)) {
- error(_("multi-pack-index object offset chunk is the wrong size"));
- return 1;
- }
- return 0;
-}
-
struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local)
{
struct multi_pack_index *m = NULL;
@@ -176,7 +163,9 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local
if (pair_chunk_expect(cf, MIDX_CHUNKID_OIDLOOKUP, &m->chunk_oid_lookup,
m->hash_len, m->num_objects))
die(_("multi-pack-index required OID lookup chunk missing or corrupted"));
- if (read_chunk(cf, MIDX_CHUNKID_OBJECTOFFSETS, midx_read_object_offsets, m))
+ if (pair_chunk_expect(cf, MIDX_CHUNKID_OBJECTOFFSETS,
+ &m->chunk_object_offsets, MIDX_CHUNK_OFFSET_WIDTH,
+ m->num_objects))
die(_("multi-pack-index required object offsets chunk missing or corrupted"));
pair_chunk(cf, MIDX_CHUNKID_LARGEOFFSETS, &m->chunk_large_offsets,
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index 2d68616c59..f1f6764efe 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -1111,7 +1111,6 @@ test_expect_success 'reader notices too-small object offset chunk' '
corrupt_chunk OOFF clear 00000000 &&
test_must_fail git log 2>err &&
cat >expect <<-\EOF &&
- error: multi-pack-index object offset chunk is the wrong size
fatal: multi-pack-index required object offsets chunk missing or corrupted
EOF
test_cmp expect err
--
2.43.0.rc0.39.g44bd344727
^ permalink raw reply related
* [PATCH 6/7] midx: read `OIDL` chunk with `pair_chunk_expect()`
From: Taylor Blau @ 2023-11-09 22:34 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1699569246.git.me@ttaylorr.com>
The `OIDL` chunk can benefit from the new chunk-format API function
described in the previous commit. Convert it to `pair_chunk_expect()`
accordingly.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
midx.c | 16 ++--------------
t/t5319-multi-pack-index.sh | 1 -
2 files changed, 2 insertions(+), 15 deletions(-)
diff --git a/midx.c b/midx.c
index 1d14661dad..66f34ed1a3 100644
--- a/midx.c
+++ b/midx.c
@@ -86,19 +86,6 @@ static int midx_read_oid_fanout(const unsigned char *chunk_start,
return 0;
}
-static int midx_read_oid_lookup(const unsigned char *chunk_start,
- size_t chunk_size, void *data)
-{
- struct multi_pack_index *m = data;
- m->chunk_oid_lookup = chunk_start;
-
- if (chunk_size != st_mult(m->hash_len, m->num_objects)) {
- error(_("multi-pack-index OID lookup chunk is the wrong size"));
- return 1;
- }
- return 0;
-}
-
static int midx_read_object_offsets(const unsigned char *chunk_start,
size_t chunk_size, void *data)
{
@@ -186,7 +173,8 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local
die(_("multi-pack-index required pack-name chunk missing or corrupted"));
if (read_chunk(cf, MIDX_CHUNKID_OIDFANOUT, midx_read_oid_fanout, m))
die(_("multi-pack-index required OID fanout chunk missing or corrupted"));
- if (read_chunk(cf, MIDX_CHUNKID_OIDLOOKUP, midx_read_oid_lookup, m))
+ if (pair_chunk_expect(cf, MIDX_CHUNKID_OIDLOOKUP, &m->chunk_oid_lookup,
+ m->hash_len, m->num_objects))
die(_("multi-pack-index required OID lookup chunk missing or corrupted"));
if (read_chunk(cf, MIDX_CHUNKID_OBJECTOFFSETS, midx_read_object_offsets, m))
die(_("multi-pack-index required object offsets chunk missing or corrupted"));
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index 313496c0cf..2d68616c59 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -1077,7 +1077,6 @@ test_expect_success 'reader notices too-small oid lookup chunk' '
corrupt_chunk OIDL clear 00000000 &&
test_must_fail git log 2>err &&
cat >expect <<-\EOF &&
- error: multi-pack-index OID lookup chunk is the wrong size
fatal: multi-pack-index required OID lookup chunk missing or corrupted
EOF
test_cmp expect err
--
2.43.0.rc0.39.g44bd344727
^ permalink raw reply related
* [PATCH 5/7] commit-graph: read `BIDX` chunk with `pair_chunk_expect()`
From: Taylor Blau @ 2023-11-09 22:34 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1699569246.git.me@ttaylorr.com>
The `BIDX` chunk can benefit from the new chunk-format API function
described in the previous commit. Convert it to `pair_chunk_expect()`
accordingly.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
commit-graph.c | 19 +++++--------------
1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/commit-graph.c b/commit-graph.c
index 3b456d199f..76d220508b 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -299,18 +299,6 @@ static int graph_read_oid_fanout(const unsigned char *chunk_start,
return 0;
}
-static int graph_read_bloom_index(const unsigned char *chunk_start,
- size_t chunk_size, void *data)
-{
- struct commit_graph *g = data;
- if (chunk_size / 4 != g->num_commits) {
- warning(_("commit-graph changed-path index chunk is too small"));
- return -1;
- }
- g->chunk_bloom_indexes = chunk_start;
- return 0;
-}
-
static int graph_read_bloom_data(const unsigned char *chunk_start,
size_t chunk_size, void *data)
{
@@ -437,8 +425,11 @@ struct commit_graph *parse_commit_graph(struct repo_settings *s,
}
if (s->commit_graph_read_changed_paths) {
- read_chunk(cf, GRAPH_CHUNKID_BLOOMINDEXES,
- graph_read_bloom_index, graph);
+ int res = pair_chunk_expect(cf, GRAPH_CHUNKID_BLOOMINDEXES,
+ &graph->chunk_bloom_indexes,
+ sizeof(uint32_t), graph->num_commits);
+ if (res && res != CHUNK_NOT_FOUND)
+ warning(_("commit-graph changed-path index chunk is too small"));
read_chunk(cf, GRAPH_CHUNKID_BLOOMDATA,
graph_read_bloom_data, graph);
}
--
2.43.0.rc0.39.g44bd344727
^ permalink raw reply related
* [PATCH 4/7] commit-graph: read `GDAT` chunk with `pair_chunk_expect()`
From: Taylor Blau @ 2023-11-09 22:34 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1699569246.git.me@ttaylorr.com>
The `GDAT` chunk can benefit from the new chunk-format API function
described in the previous commit. Convert it to `pair_chunk_expect()`
accordingly.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
commit-graph.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/commit-graph.c b/commit-graph.c
index 67ab0f2448..3b456d199f 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -299,16 +299,6 @@ static int graph_read_oid_fanout(const unsigned char *chunk_start,
return 0;
}
-static int graph_read_generation_data(const unsigned char *chunk_start,
- size_t chunk_size, void *data)
-{
- struct commit_graph *g = data;
- if (chunk_size / sizeof(uint32_t) != g->num_commits)
- return error(_("commit-graph generations chunk is wrong size"));
- g->chunk_generation_data = chunk_start;
- return 0;
-}
-
static int graph_read_bloom_index(const unsigned char *chunk_start,
size_t chunk_size, void *data)
{
@@ -433,8 +423,11 @@ struct commit_graph *parse_commit_graph(struct repo_settings *s,
&graph->chunk_base_graphs_size);
if (s->commit_graph_generation_version >= 2) {
- read_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA,
- graph_read_generation_data, graph);
+ if (pair_chunk_expect(cf, GRAPH_CHUNKID_GENERATION_DATA,
+ &graph->chunk_generation_data,
+ sizeof(uint32_t),
+ graph->num_commits))
+ error(_("commit-graph generations chunk is wrong size"));
pair_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA_OVERFLOW,
&graph->chunk_generation_data_overflow,
&graph->chunk_generation_data_overflow_size);
--
2.43.0.rc0.39.g44bd344727
^ permalink raw reply related
* [PATCH 3/7] commit-graph: read `CDAT` chunk with `pair_chunk_expect()`
From: Taylor Blau @ 2023-11-09 22:34 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1699569246.git.me@ttaylorr.com>
The `CDAT` chunk can benefit from the new chunk-format API function
described in the previous commit. Convert it to `pair_chunk_expect()`
accordingly.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
commit-graph.c | 15 +++------------
t/t5318-commit-graph.sh | 3 +--
2 files changed, 4 insertions(+), 14 deletions(-)
diff --git a/commit-graph.c b/commit-graph.c
index 6072c2a17f..67ab0f2448 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -299,16 +299,6 @@ static int graph_read_oid_fanout(const unsigned char *chunk_start,
return 0;
}
-static int graph_read_commit_data(const unsigned char *chunk_start,
- size_t chunk_size, void *data)
-{
- struct commit_graph *g = data;
- if (chunk_size / GRAPH_DATA_WIDTH != g->num_commits)
- return error(_("commit-graph commit data chunk is wrong size"));
- g->chunk_commit_data = chunk_start;
- return 0;
-}
-
static int graph_read_generation_data(const unsigned char *chunk_start,
size_t chunk_size, void *data)
{
@@ -431,8 +421,9 @@ struct commit_graph *parse_commit_graph(struct repo_settings *s,
error(_("commit-graph OID lookup chunk is the wrong size"));
goto free_and_return;
}
- if (read_chunk(cf, GRAPH_CHUNKID_DATA, graph_read_commit_data, graph)) {
- error(_("commit-graph required commit data chunk missing or corrupted"));
+ if (pair_chunk_expect(cf, GRAPH_CHUNKID_DATA, &graph->chunk_commit_data,
+ GRAPH_DATA_WIDTH, graph->num_commits)) {
+ error(_("commit-graph commit data chunk is wrong size"));
goto free_and_return;
}
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index b3e8af018d..fd5165bd07 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -550,7 +550,7 @@ test_expect_success 'detect missing OID lookup chunk' '
test_expect_success 'detect missing commit data chunk' '
corrupt_graph_and_verify $GRAPH_BYTE_COMMIT_DATA_ID "\0" \
- "commit-graph required commit data chunk missing or corrupted"
+ "commit-graph commit data chunk is wrong size"
'
test_expect_success 'detect incorrect fanout' '
@@ -875,7 +875,6 @@ test_expect_success 'reader notices too-small commit data chunk' '
check_corrupt_chunk CDAT clear 00000000 &&
cat >expect.err <<-\EOF &&
error: commit-graph commit data chunk is wrong size
- error: commit-graph required commit data chunk missing or corrupted
EOF
test_cmp expect.err err
'
--
2.43.0.rc0.39.g44bd344727
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox