* Re: [PATCH v2 0/2] refs: introduce reftable backend
From: Junio C Hamano @ 2024-02-03 20:41 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Han-Wen Nienhuys, Karthik Nayak
In-Reply-To: <cover.1706862705.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> this is the second version of my patch series that introduces the
> reftable backend.
>
> This version addresses the feedback by Karthik. I don't think it really
> makes sense to spell out every change here -- the range diff should
> likely be easier to digest.
This, when merged to 'seen' (which also has "for-each-ref now thinks
an empty string is a signal to show ref-like things outside the
refs/ hierarchy" topic, kn/for-all-refs), seems to break *-reftable
CI tests, cf. https://github.com/git/git/actions/runs/7765401528
I'll tentatively eject the topic from 'seen', even though I have a
suspicion that it byitself would pass all the tests.
Thanks.
^ permalink raw reply
* Re: [RFC PATCH v2 0/6] test-tool: add unit test suite runner
From: Junio C Hamano @ 2024-02-03 18:52 UTC (permalink / raw)
To: Josh Steadmon; +Cc: git, johannes.schindelin, peff, phillip.wood
In-Reply-To: <cover.1706921262.git.steadmon@google.com>
Josh Steadmon <steadmon@google.com> writes:
> Please note: this series has been rebased onto jk/unit-tests-buildfix.
>
> For various reasons (see discussion at [1]) we would like an alternative
> to `prove` for running test suites (including the unit tests) on
> Windows.
>
> This series extends the existing `test-tool run-command testsuite` to
> support running unit tests. In addition, it includes some small
> cleanups:
> * move t-basic out of the unit-tests directory
> * don't hardcode the shell for running tests in `test-tool ... testsuite`
> * don't hardcode a test name filter in `test-tool ... testsuite`
> * add a test wrapper script to allow unit tests and the shell test suite
> to run in a single `prove` process
>
> Some known remaining bits of work:
> * We should investigate switching the Windows CI to use `test-tool`
> instead of prove. However, Windows CI seems broken on
> jk/unit-tests-buildfix, and I haven't had time to determine why.
Thanks to Dscho who figured this out, the jk/unit-tests-buildfix
topic in my tree has been updated to pass "win test (n)" jobs.
> * We should determine whether it is confusing or otherwise harmful to
> people's workflow to have the unit tests run in parallel with shell
> tests when using prove as the default test target.
I do not know much about "confusing" thing, but if the user
allocates, say, 16 jobs to run tests in parallel, and one of them
drives the "unit test suite runner" that wants to do its own
parallelism, we'd easily end up busting the resource limit the
end-user desires. It does not necessarily mean that we should limit
the parallelism of "unit test suite runner" to 1 under prove, though.
^ permalink raw reply
* Re: git-users: email list has become spam-drowned
From: Kristoffer Haugsbakk @ 2024-02-03 18:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqbk8xsba8.fsf@gitster.g>
On Sat, Feb 3, 2024, at 18:33, Junio C Hamano wrote:
> to find out who the owners are and how many members there are. I
> expect the UI redacts the e-mail addresses of them, but there should
> be a "Contact owners and managers" link, from which you can contact
> them, to let them know about the situation.
I tried that now. Turns out my Gmail account was already a member. I was
able to find the owner. But when sending the message it said something
like “couldn’t send message”.
--
Kristoffer Haugsbakk
^ permalink raw reply
* Re: git-users: email list has become spam-drowned
From: Junio C Hamano @ 2024-02-03 17:33 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: git
In-Reply-To: <339f17e1-c6a0-4859-aa5b-481dd6e0e91b@app.fastmail.com>
"Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:
> Hello
>
> I know that this list has got nothing to do with git-scm (per se). But I
> was recommended by Dscho to raise the issue here.[1]
>
> Maybe someone who happens to be affiliated with git-users happens to
> read this.
>
> † 1: from https://github.com/git/git-scm.com/issues/1828
>
> —
>
> According to https://git-scm.com/community
>
>> There is also [Git user mailing
>> list](https://groups.google.com/forum/?fromgroups#!forum/git-users) on
>> Google Groups which is a nice place for beginners to ask about
>> anything.
>
> I am not allowed to access it. Just as well because this is apparently
> what it looks like:
> https://www.mail-archive.com/git-users@googlegroups.com/
>
> The last legitimate thread (except for an email saying “bye, this list
> is too spammy”) was 2023-12-29.
I am not a member of that list so I cannot do this, but somebody who
is a member should be able to visit
https://groups.google.com/g/git-users/about
to find out who the owners are and how many members there are. I
expect the UI redacts the e-mail addresses of them, but there should
be a "Contact owners and managers" link, from which you can contact
them, to let them know about the situation.
What they could do after getting notified is not clear to me,
though. Volunteer themselves, or gather volunteer groups among the
members to moderate the traffic, if they want to run it in a useful
form, in ongoing basis might be too much, but at least they can flip
the "Anyone on the web can join group" bit a bit tighter, perhaps,
and then eject spammy group members as a one-time clean-up?
^ permalink raw reply
* Re: [PATCH 1/2] GitHub Actions: update to checkout@v4
From: Junio C Hamano @ 2024-02-03 17:18 UTC (permalink / raw)
To: Óscar Domínguez Celada; +Cc: git
In-Reply-To: <CADCFv=4Gjh5B6RFF3P--FO9T7R+uGg2dyN8hT4VR0yJtJt-UsA@mail.gmail.com>
Óscar Domínguez Celada <dominguez.celada@gmail.com> writes:
> I am adding non-html e-mail reply to keep track in git@vger.kernel.org:
>
> The switch to checkout@v4 for GitHub Actions looks good to me. I
> wonder if we should be updating other actions to v4 so they start
> using Node 20:
No need to wonder. I only noticed the ones involved in the main CI
job that triggered the warnings, and dealt only with the "easy" ones
;-) The primary reason why I CC'ed you was because I hoped you knew
better about the "container jobs are pinned at checkout@v1", which I
left as-is.
Help in updating other actions to newer versions, if needed, is greatly
appreciated.
^ permalink raw reply
* Re: git-users: email list has become spam-drowned
From: Thomas Guyot @ 2024-02-03 16:38 UTC (permalink / raw)
To: Kristoffer Haugsbakk, git
In-Reply-To: <339f17e1-c6a0-4859-aa5b-481dd6e0e91b@app.fastmail.com>
On 2024-02-03 10:02, Kristoffer Haugsbakk wrote:
> Hello
>
> I know that this list has got nothing to do with git-scm (per se). But I
> was recommended by Dscho to raise the issue here.[1]
>
> Maybe someone who happens to be affiliated with git-users happens to
> read this.
>
Hi,
This list is hidden in googlegroups, I'm guessing whoever owned it
probably got his account hijacked and the list may now be controlled by
spammers trying to push as much spam as possible to list users. You
could try reporting to Google but I wouldn't hold high hopes on any
outcome from that.
Maybe a better option would be to start an official git-users list,
hosted by vger, so control is actually retained by git developers?
There doesn't appear to be a lot of user-centric lists there, but there
are a few...
Regards,
--
Thomas
^ permalink raw reply
* Re: [PATCH v2 1/1] completion: dir-type optargs for am, format-patch
From: Rubén Justo @ 2024-02-03 15:13 UTC (permalink / raw)
To: Britton Leo Kerin, git
In-Reply-To: <d37781c3-6af2-409b-95a8-660a9b92d20b@smtp-relay.sendinblue.com>
On 08-ene-2024 15:53:03, Britton Leo Kerin wrote:
> Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
> ---
> contrib/completion/git-completion.bash | 37 ++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 185b47d802..2b2b6c9738 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1356,6 +1356,29 @@ __git_count_arguments ()
> printf "%d" $c
> }
>
> +# Complete actual dir (not pathspec), respecting any -C options.
> +#
> +# Usage: __git_complete_refs [<option>]...
> +# --cur=<word>: The current dir to be completed. Defaults to the current word.
> +__git_complete_dir ()
> +{
> + local cur_="$cur"
> +
> + while test $# != 0; do
> + case "$1" in
> + --cur=*) cur_="${1##--cur=}" ;;
> + *) return 1 ;;
> + esac
> + shift
> + done
> +
> + # This rev-parse invocation amounts to a pwd which respects -C options
> + local context_dir=$(__git rev-parse --show-toplevel --show-prefix 2>/dev/null | paste -s -d '/' 2>/dev/null)
> + [ -d "$context_dir" ] || return 1
> +
> + COMPREPLY=$(cd "$context_dir" 2>/dev/null && compgen -d -- "$cur_")
This assignment is problematic.
First, COMPREPLY is expected to be an array. Maybe a simple change can
do the trick:
- COMPREPLY=$(cd "$context_dir" 2>/dev/null && compgen -d -- "$cur_")
+ COMPREPLY=( $(cd "$context_dir" 2>/dev/null && compgen -d -- "$cur_") )
But, what happens with directories that have SP's in its name? We're
giving wrong options:
$ mkdir one
$ mkdir "one more dir"
$ git am --directory=o<TAB><TAB>
dir more one
Setting IFS can help us:
+ local IFS=$'\n'
Now we're returning correct options:
$ mkdir one
$ mkdir "one more dir"
$ git am --directory=o<TAB><TAB>
one one more dir
Here, the user might be expecting directory names with a trailing '/',
as Bash do. Again, a simple trick:
- COMPREPLY=( $(cd "$context_dir" 2>/dev/null && compgen -d -- "$cur_") )
+ COMPREPLY=( $(cd "$context_dir" 2>/dev/null && compgen -d -S / -- "$cur_") )
Now looks better, IMO:
$ git am --directory=o<TAB><TAB>
one/ one more dir/
But, after all of this, we're going to provoke a problematic completion due
to the SP:
$ mkdir "another one"
$ git am --directory=anot<TAB><TAB>
...
$ git am --directory=another one/
We should complete to:
$ git am --directory=another\ one/
Here we need a less simple trick:
+ # use a hack to enable file mode in bash < 4
+ # compopt -o filenames +o nospace 2>/dev/null ||
+ compgen -f /non-existing-dir/ >/dev/null ||
+ true
Some commits you may find interesting:
fea16b47b6 (git-completion.bash: add support for path completion, 2013-01-11)
3ffa4df4b2 (completion: add hack to enable file mode in bash < 4, 2013-04-27)
Well, so far, so good? I'm afraid, not: What happens with other
special characters like quotes '"'?
I suggest you take a look at how we are already doing all of
considerations for other commands, like git-add.
> +}
> +
> __git_whitespacelist="nowarn warn error error-all fix"
> __git_patchformat="mbox stgit stgit-series hg mboxrd"
> __git_showcurrentpatch="diff raw"
> @@ -1374,6 +1397,10 @@ _git_am ()
> __gitcomp "$__git_whitespacelist" "" "${cur##--whitespace=}"
> return
> ;;
> + --directory=*)
> + __git_complete_dir --cur="${cur##--directory=}"
> + return
> + ;;
> --patch-format=*)
> __gitcomp "$__git_patchformat" "" "${cur##--patch-format=}"
> return
> @@ -1867,7 +1894,17 @@ __git_format_patch_extra_options="
>
> _git_format_patch ()
> {
> + case "$prev,$cur" in
> + -o,*)
> + __git_complete_dir
> + return
> + ;;
> + esac
> case "$cur" in
> + --output-directory=*)
> + __git_complete_dir --cur="${cur##--output-directory=}"
> + return
> + ;;
Adding completion for these options, and possibly opening the way for
others, is a nice improvement.
Thank you for working on this.
^ permalink raw reply
* git-users: email list has become spam-drowned
From: Kristoffer Haugsbakk @ 2024-02-03 15:02 UTC (permalink / raw)
To: git
Hello
I know that this list has got nothing to do with git-scm (per se). But I
was recommended by Dscho to raise the issue here.[1]
Maybe someone who happens to be affiliated with git-users happens to
read this.
† 1: from https://github.com/git/git-scm.com/issues/1828
—
According to https://git-scm.com/community
> There is also [Git user mailing
> list](https://groups.google.com/forum/?fromgroups#!forum/git-users) on
> Google Groups which is a nice place for beginners to ask about
> anything.
I am not allowed to access it. Just as well because this is apparently
what it looks like:
https://www.mail-archive.com/git-users@googlegroups.com/
The last legitimate thread (except for an email saying “bye, this list
is too spammy”) was 2023-12-29.
--
Kristoffer Haugsbakk
^ permalink raw reply
* No one is aloud to make changes to dependency’s beside me- owner of domain and software!
From: ross nicholas Oneil thomas @ 2024-02-03 13:20 UTC (permalink / raw)
To: GitHub, Github email, GitHub Developer Support
Hello
I’m tired of this changing and the people using license for there own benifit GitHub is open source on the site no where else are they aloud to do this with license or patents or trademarks or copyrights. Are those even still secured?
We should charge a fin for changing this!
https://github.com/chromaui/chromatic-cli/actions/workflows/check-labels.yml
Ross Nicholas Oneil Thomas
ownership of:
www.github.com
www.coinbase.com
www.jsnull.com
(559-816-2950)
^ permalink raw reply
* [PATCH v3 2/2] add-patch: classify '@' as a synonym for 'HEAD'
From: Ghanshyam Thakkar @ 2024-02-03 11:25 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Ghanshyam Thakkar
In-Reply-To: <20240202150434.11256-1-shyamthakkar001@gmail.com>
Currently, (checkout, reset, restore) commands correctly take '@' as a
synonym for 'HEAD'. However, in patch mode (-p/--patch), for both '@' and
'HEAD', different prompts/messages are given by the commands mentioned
above. This is due to the literal and only string comparison with the word
'HEAD' in run_add_p(). Synonymity between '@' and 'HEAD' is obviously
desired, especially since '@' already resolves to 'HEAD'.
Therefore, make a new function user_meant_head() which takes the
revision string and compares it to 'HEAD' as well as '@'. However, in
builtin/checkout.c, there is a logic to convert all command line input
rev to the raw object name for underlying machinery (e.g., diff-index)
that does not recognize the <a>...<b> notation, but we'd need to leave
'HEAD' intact. Now we need to teach that '@' is a synonym to 'HEAD'
to that code and leave '@' intact, too.
There is one unintended side-effect/behavior change of this, even if
there exists a branch named '@', when providing '@' as a rev-source to
(checkout, reset, restore) commands in patch mode, it will consider it
as HEAD. This is due to the behavior of diff-index. However, naming a
branch '@' is an obvious foot-gun and there are many existing commands
which take '@' for 'HEAD' even if 'refs/heads/@' exists (e.g., 'git log
@', 'git push origin @' etc.). Therefore, this should be fine.
Also, add tests to check the above mentioned synonymity between 'HEAD'
and '@'.
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
---
add-patch.c | 11 +++++++---
builtin/checkout.c | 11 +++++-----
t/t2016-checkout-patch.sh | 46 ++++++++++++++++++++++-----------------
t/t2071-restore-patch.sh | 18 +++++++++------
t/t7105-reset-patch.sh | 10 +++++++++
5 files changed, 61 insertions(+), 35 deletions(-)
diff --git a/add-patch.c b/add-patch.c
index 68f525b35c..7d565dcb33 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -378,6 +378,11 @@ static int parse_hunk_header(struct add_p_state *s, struct hunk *hunk)
return 0;
}
+static inline int user_meant_head(const char *rev)
+{
+ return !strcmp(rev, "HEAD") || !strcmp(rev, "@");
+}
+
static int is_octal(const char *p, size_t len)
{
if (!len)
@@ -1729,21 +1734,21 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
if (mode == ADD_P_STASH)
s.mode = &patch_mode_stash;
else if (mode == ADD_P_RESET) {
- if (!revision || !strcmp(revision, "HEAD"))
+ if (!revision || user_meant_head(revision))
s.mode = &patch_mode_reset_head;
else
s.mode = &patch_mode_reset_nothead;
} else if (mode == ADD_P_CHECKOUT) {
if (!revision)
s.mode = &patch_mode_checkout_index;
- else if (!strcmp(revision, "HEAD"))
+ else if (user_meant_head(revision))
s.mode = &patch_mode_checkout_head;
else
s.mode = &patch_mode_checkout_nothead;
} else if (mode == ADD_P_WORKTREE) {
if (!revision)
s.mode = &patch_mode_checkout_index;
- else if (!strcmp(revision, "HEAD"))
+ else if (user_meant_head(revision))
s.mode = &patch_mode_worktree_head;
else
s.mode = &patch_mode_worktree_nothead;
diff --git a/builtin/checkout.c b/builtin/checkout.c
index a6e30931b5..79e208ee6d 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -539,12 +539,13 @@ static int checkout_paths(const struct checkout_opts *opts,
* recognized by diff-index), we will always replace the name
* with the hex of the commit (whether it's in `...` form or
* not) for the run_add_interactive() machinery to work
- * properly. However, there is special logic for the HEAD case
- * so we mustn't replace that. Also, when we were given a
- * tree-object, new_branch_info->commit would be NULL, but we
- * do not have to do any replacement, either.
+ * properly. However, there is special logic for the 'HEAD' and
+ * '@' case so we mustn't replace that. Also, when we were
+ * given a tree-object, new_branch_info->commit would be NULL,
+ * but we do not have to do any replacement, either.
*/
- if (rev && new_branch_info->commit && strcmp(rev, "HEAD"))
+ if (rev && new_branch_info->commit && strcmp(rev, "HEAD") &&
+ strcmp(rev, "@"))
rev = oid_to_hex_r(rev_oid, &new_branch_info->commit->object.oid);
if (opts->checkout_index && opts->checkout_worktree)
diff --git a/t/t2016-checkout-patch.sh b/t/t2016-checkout-patch.sh
index 747eb5563e..c4f9bf09aa 100755
--- a/t/t2016-checkout-patch.sh
+++ b/t/t2016-checkout-patch.sh
@@ -38,26 +38,32 @@ test_expect_success 'git checkout -p with staged changes' '
verify_state dir/foo index index
'
-test_expect_success 'git checkout -p HEAD with NO staged changes: abort' '
- set_and_save_state dir/foo work head &&
- test_write_lines n y n | git checkout -p HEAD &&
- verify_saved_state bar &&
- verify_saved_state dir/foo
-'
-
-test_expect_success 'git checkout -p HEAD with NO staged changes: apply' '
- test_write_lines n y y | git checkout -p HEAD &&
- verify_saved_state bar &&
- verify_state dir/foo head head
-'
-
-test_expect_success 'git checkout -p HEAD with change already staged' '
- set_state dir/foo index index &&
- # the third n is to get out in case it mistakenly does not apply
- test_write_lines n y n | git checkout -p HEAD &&
- verify_saved_state bar &&
- verify_state dir/foo head head
-'
+for opt in "HEAD" "@"
+do
+ test_expect_success "git checkout -p $opt with NO staged changes: abort" '
+ set_and_save_state dir/foo work head &&
+ test_write_lines n y n | git checkout -p $opt >output &&
+ verify_saved_state bar &&
+ verify_saved_state dir/foo &&
+ test_grep "Discard" output
+ '
+
+ test_expect_success "git checkout -p $opt with NO staged changes: apply" '
+ test_write_lines n y y | git checkout -p $opt >output &&
+ verify_saved_state bar &&
+ verify_state dir/foo head head &&
+ test_grep "Discard" output
+ '
+
+ test_expect_success "git checkout -p $opt with change already staged" '
+ set_state dir/foo index index &&
+ # the third n is to get out in case it mistakenly does not apply
+ test_write_lines n y n | git checkout -p $opt >output &&
+ verify_saved_state bar &&
+ verify_state dir/foo head head &&
+ test_grep "Discard" output
+ '
+done
test_expect_success 'git checkout -p HEAD^...' '
# the third n is to get out in case it mistakenly does not apply
diff --git a/t/t2071-restore-patch.sh b/t/t2071-restore-patch.sh
index b5c5c0ff7e..3dc9184b4a 100755
--- a/t/t2071-restore-patch.sh
+++ b/t/t2071-restore-patch.sh
@@ -44,13 +44,17 @@ test_expect_success PERL 'git restore -p with staged changes' '
verify_state dir/foo index index
'
-test_expect_success PERL 'git restore -p --source=HEAD' '
- set_state dir/foo work index &&
- # the third n is to get out in case it mistakenly does not apply
- test_write_lines n y n | git restore -p --source=HEAD &&
- verify_saved_state bar &&
- verify_state dir/foo head index
-'
+for opt in "HEAD" "@"
+do
+ test_expect_success PERL "git restore -p --source=$opt" '
+ set_state dir/foo work index &&
+ # the third n is to get out in case it mistakenly does not apply
+ test_write_lines n y n | git restore -p --source=$opt >output &&
+ verify_saved_state bar &&
+ verify_state dir/foo head index &&
+ test_grep "Discard" output
+ '
+done
test_expect_success PERL 'git restore -p --source=HEAD^' '
set_state dir/foo work index &&
diff --git a/t/t7105-reset-patch.sh b/t/t7105-reset-patch.sh
index 05079c7246..ec7f16dfb6 100755
--- a/t/t7105-reset-patch.sh
+++ b/t/t7105-reset-patch.sh
@@ -33,6 +33,16 @@ test_expect_success PERL 'git reset -p' '
test_grep "Unstage" output
'
+for opt in "HEAD" "@"
+do
+ test_expect_success PERL "git reset -p $opt" '
+ test_write_lines n y | git reset -p $opt >output &&
+ verify_state dir/foo work head &&
+ verify_saved_state bar &&
+ test_grep "Unstage" output
+ '
+done
+
test_expect_success PERL 'git reset -p HEAD^' '
test_write_lines n y | git reset -p HEAD^ >output &&
verify_state dir/foo work parent &&
--
2.43.0
^ permalink raw reply related
* [PATCH v3 1/2] add-patch: remove unnecessary NEEDSWORK comment
From: Ghanshyam Thakkar @ 2024-02-03 11:25 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Ghanshyam Thakkar
In-Reply-To: <20240202150434.11256-1-shyamthakkar001@gmail.com>
The comment suggested to compare commit objects instead of string
comparison to 'HEAD' for supporting more ways of saying 'HEAD' (e.g.
'@'). However, this approach would also count a non-checked out branch
pointing to same commit as HEAD, as HEAD. This would cause confusion to
the user.
Junio described it best as[1]:
"Users may consider 'HEAD' and '@' the same and may want them to behave
the same way, but the user, when explicitly naming '$branch', means they
want to "check contents out of that OTHER thing named '$branch', not the
current branch"; it may or may not happen to be pointing at the same
commit as HEAD, but if the user meant to say "check contents out of the
current commit, (partially) reverting the local changes I have", the user
would have said HEAD. After all, the user may not even be immediately
aware that '$branch' happens to point at the same commit as HEAD."
[1]: https://lore.kernel.org/git/xmqqmssohu69.fsf@gitster.g/
Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
---
add-patch.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/add-patch.c b/add-patch.c
index 79eda168eb..68f525b35c 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -1729,14 +1729,6 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
if (mode == ADD_P_STASH)
s.mode = &patch_mode_stash;
else if (mode == ADD_P_RESET) {
- /*
- * NEEDSWORK: Instead of comparing to the literal "HEAD",
- * compare the commit objects instead so that other ways of
- * saying the same thing (such as "@") are also handled
- * appropriately.
- *
- * This applies to the cases below too.
- */
if (!revision || !strcmp(revision, "HEAD"))
s.mode = &patch_mode_reset_head;
else
--
2.43.0
^ permalink raw reply related
* [PATCH v3 0/2] add-patch: '@' as a synonym for 'HEAD'
From: Ghanshyam Thakkar @ 2024-02-03 11:25 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Ghanshyam Thakkar
In-Reply-To: <20240202150434.11256-1-shyamthakkar001@gmail.com>
The v3 of the series updates the commit message for patch [2/2] and
replaces the function name from is_rev_head() to user_meant_head().
Patch [1/2] remains unchanged.
Ghanshyam Thakkar (2):
add-patch: remove unnecessary NEEDSWORK comment
add-patch: classify '@' as a synonym for 'HEAD'
add-patch.c | 19 +++++++---------
builtin/checkout.c | 11 +++++-----
t/t2016-checkout-patch.sh | 46 ++++++++++++++++++++++-----------------
t/t2071-restore-patch.sh | 18 +++++++++------
t/t7105-reset-patch.sh | 10 +++++++++
5 files changed, 61 insertions(+), 43 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: Git in GSoC 2024
From: Kaartic Sivaraam @ 2024-02-03 11:41 UTC (permalink / raw)
To: Patrick Steinhardt, Christian Couder
Cc: git, Taylor Blau, Junio C Hamano, Victoria Dye, Karthik Nayak
In-Reply-To: <Zbtmoo8qTmj-yt99@tanuki>
Hi Patrick, Karthik and Christian,
For the note, other project ideas and volunteer are always welcome. So,
feel free to chime in if you have any ideas or wish to volunteer as a
mentor to guide potential future contributors of the community. :-)
On 01/02/24 15:08, Patrick Steinhardt wrote:
> On Wed, Jan 31, 2024 at 11:27:13PM +0530, Kaartic Sivaraam wrote:
>>
>> I created a fairly rough SoC ideas page for now including a barebones
>> information about the unit test migration idea:
>>
>> https://git.github.io/SoC-2024-Ideas/
>>
>> Note well that the existing idea's description is far from complete and I
>> mostly just cooked it up to serve as a template for how the idea entry could
>> look like. Kindly contribute towards elaborating the same :-)
>>
>> Also, feel free to suggest ideas you have around refs and reftable backed,
>> Patrick. Those would be helpful.
>
> I'll have a the beginning of next week and will think about topics
> meanwhile.
>
Thanks, Patrick! It would be great if you could share the same as soon
as possible. The deadline for applying to GSoC is Feb 6 (18:00 UTC) and
we need the ideas page to be decent enough before we go ahead with
applying for this year.
If the elaborate project description could take time, feel free to share
a paragraph or two that are supplemented with a few references. That
should be sufficient for applying to GSoC.
Christian,
It would be great if you could look into and improve the detail for the
unit test migration idea. I just added a very terse description based on
what I could get my hands on. If you think the description we used for
the Outreachy round would do, kindly update the page with the same or
kindly share it here so that I could update the same in the ideas page :-)
>
> Yeah, as long as there is a co-mentor that can take over during my
> absence I'm happy to do it. Karthik said that he'd be willing to cover
> me, which I think would be a good fit given that he's already got quite
> a bit of exposure to the reftable backend internally at GitLab. Thanks!
>
Sounds good. Thank you for volunteering to co-mentor, Karthik!
--
Sivaraam
^ permalink raw reply
* Re: [PATCH 1/2] GitHub Actions: update to checkout@v4
From: Óscar Domínguez Celada @ 2024-02-03 11:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <CADCFv=5=uwp_NVpndTYFiRRK4hEwmMdA2At80cXuS91V5mKN2A@mail.gmail.com>
I am adding non-html e-mail reply to keep track in git@vger.kernel.org:
The switch to checkout@v4 for GitHub Actions looks good to me. I
wonder if we should be updating other actions to v4 so they start
using Node 20:
In coverity.yml
(https://github.com/git/git/blob/8838dd21e8a4ec1324377ffcfa90413844ca3674/.github/workflows/coverity.yml#L101C15-L101C39)
actions/cache/restore@v3 -> v4 (reference:
https://github.com/actions/cache/releases/tag/v4.0.0)
actions/cache/save@v3 -> v4 (reference:
https://github.com/actions/cache/releases/tag/v4.0.0)
In main.yml
actions/upload-artifact@v3 -> v4 (reference:
https://github.com/actions/upload-artifact/commit/aa5cae10db2b39d79f5244f6bc5084278993a3ae#diff-1243c5424efaaa19bd8e813c5e6f6da46316e63761421b3e5f5c8ced9a36e6b6R33)
actions/download-artifact@v3 -> v4 (reference:
https://github.com/actions/download-artifact/commit/88dadfbcfcdd10293192ac8ee1e3ffe61f7055ee#diff-1243c5424efaaa19bd8e813c5e6f6da46316e63761421b3e5f5c8ced9a36e6b6R28)
On Sat, 3 Feb 2024 at 12:31, Óscar Domínguez Celada
<dominguez.celada@gmail.com> wrote:
>
> The switch to checkout@v4 for GitHub Actions looks good to me. I wonder if we should be updating other actions to v4 so they start using Node 20:
>
> In coverity.yml
>
> actions/cache/restore@v3 -> v4 (reference: https://github.com/actions/cache/releases/tag/v4.0.0)
> actions/cache/save@v3 -> v4 (reference: https://github.com/actions/cache/releases/tag/v4.0.0)
>
> In main.yml
>
> actions/upload-artifact@v3 -> v4 (reference: https://github.com/actions/upload-artifact/commit/aa5cae10db2b39d79f5244f6bc5084278993a3ae#diff-1243c5424efaaa19bd8e813c5e6f6da46316e63761421b3e5f5c8ced9a36e6b6R33)
> actions/download-artifact@v3 -> v4 (reference: https://github.com/actions/download-artifact/commit/88dadfbcfcdd10293192ac8ee1e3ffe61f7055ee#diff-1243c5424efaaa19bd8e813c5e6f6da46316e63761421b3e5f5c8ced9a36e6b6R28)
>
>
> On Fri, 2 Feb 2024 at 21:39, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> We seem to be getting "Node.js 16 actions are deprecated." warnings
>> for jobs that use checkout@v3. Except for the i686 containers job
>> that is kept at checkout@v1 [*], update to checkout@v4, which is
>> said to use Node.js 20.
>>
>> [*] 6cf4d908 (ci(main): upgrade actions/checkout to v3, 2022-12-05)
>> refers to https://github.com/actions/runner/issues/2115 and
>> explains why container jobs are kept at checkout@v1. We may
>> want to check the current status of the issue and move it to the
>> same version as other jobs, but that is outside the scope of
>> this step.
>>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>> .github/workflows/check-whitespace.yml | 2 +-
>> .github/workflows/coverity.yml | 2 +-
>> .github/workflows/main.yml | 18 +++++++++---------
>> 3 files changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml
>> index a58e2dc8ad..a241a63428 100644
>> --- a/.github/workflows/check-whitespace.yml
>> +++ b/.github/workflows/check-whitespace.yml
>> @@ -19,7 +19,7 @@ jobs:
>> check-whitespace:
>> runs-on: ubuntu-latest
>> steps:
>> - - uses: actions/checkout@v3
>> + - uses: actions/checkout@v4
>> with:
>> fetch-depth: 0
>>
>> diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
>> index e5532d381b..a81a7566d1 100644
>> --- a/.github/workflows/coverity.yml
>> +++ b/.github/workflows/coverity.yml
>> @@ -38,7 +38,7 @@ jobs:
>> COVERITY_LANGUAGE: cxx
>> COVERITY_PLATFORM: overridden-below
>> steps:
>> - - uses: actions/checkout@v3
>> + - uses: actions/checkout@v4
>> - name: install minimal Git for Windows SDK
>> if: contains(matrix.os, 'windows')
>> uses: git-for-windows/setup-git-for-windows-sdk@v1
>> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
>> index 4d97da57ec..90973f9693 100644
>> --- a/.github/workflows/main.yml
>> +++ b/.github/workflows/main.yml
>> @@ -112,7 +112,7 @@ jobs:
>> group: windows-build-${{ github.ref }}
>> cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}
>> steps:
>> - - uses: actions/checkout@v3
>> + - uses: actions/checkout@v4
>> - uses: git-for-windows/setup-git-for-windows-sdk@v1
>> - name: build
>> shell: bash
>> @@ -173,10 +173,10 @@ jobs:
>> group: vs-build-${{ github.ref }}
>> cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}
>> steps:
>> - - uses: actions/checkout@v3
>> + - uses: actions/checkout@v4
>> - uses: git-for-windows/setup-git-for-windows-sdk@v1
>> - name: initialize vcpkg
>> - uses: actions/checkout@v3
>> + uses: actions/checkout@v4
>> with:
>> repository: 'microsoft/vcpkg'
>> path: 'compat/vcbuild/vcpkg'
>> @@ -297,7 +297,7 @@ jobs:
>> runs_on_pool: ${{matrix.vector.pool}}
>> runs-on: ${{matrix.vector.pool}}
>> steps:
>> - - uses: actions/checkout@v3
>> + - uses: actions/checkout@v4
>> - run: ci/install-dependencies.sh
>> - run: ci/run-build-and-tests.sh
>> - name: print test failures
>> @@ -317,7 +317,7 @@ jobs:
>> CC: clang
>> runs-on: ubuntu-latest
>> steps:
>> - - uses: actions/checkout@v3
>> + - uses: actions/checkout@v4
>> - run: ci/install-dependencies.sh
>> - run: ci/run-build-and-minimal-fuzzers.sh
>> dockerized:
>> @@ -342,7 +342,7 @@ jobs:
>> runs-on: ubuntu-latest
>> container: ${{matrix.vector.image}}
>> steps:
>> - - uses: actions/checkout@v3
>> + - uses: actions/checkout@v4
>> if: matrix.vector.jobname != 'linux32'
>> - uses: actions/checkout@v1
>> if: matrix.vector.jobname == 'linux32'
>> @@ -373,7 +373,7 @@ jobs:
>> group: static-analysis-${{ github.ref }}
>> cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}
>> steps:
>> - - uses: actions/checkout@v3
>> + - uses: actions/checkout@v4
>> - run: ci/install-dependencies.sh
>> - run: ci/run-static-analysis.sh
>> - run: ci/check-directional-formatting.bash
>> @@ -396,7 +396,7 @@ jobs:
>> artifact: sparse-20.04
>> - name: Install the current `sparse` package
>> run: sudo dpkg -i sparse-20.04/sparse_*.deb
>> - - uses: actions/checkout@v3
>> + - uses: actions/checkout@v4
>> - name: Install other dependencies
>> run: ci/install-dependencies.sh
>> - run: make sparse
>> @@ -411,6 +411,6 @@ jobs:
>> jobname: Documentation
>> runs-on: ubuntu-latest
>> steps:
>> - - uses: actions/checkout@v3
>> + - uses: actions/checkout@v4
>> - run: ci/install-dependencies.sh
>> - run: ci/test-documentation.sh
>> --
>> 2.43.0-522-g2a540e432f
>>
>
>
> --
> Óscar Domínguez Celada
--
Óscar Domínguez Celada
^ permalink raw reply
* What's cooking in git.git (Feb 2024, #02; Fri, 2)
From: Junio C Hamano @ 2024-02-03 8:23 UTC (permalink / raw)
To: git
Here are the topics that have been cooking in my tree. Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release). Commits prefixed with '-' are only in 'seen', and
aren't considered "accepted" at all and may be annotated with an URL
to a message that raises issues but they are no means exhaustive. A
topic without enough support may be discarded after a long period of
no activity (of course they can be resubmit when new interests
arise).
Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors. Some
repositories have only a subset of branches.
With maint, master, next, seen, todo:
git://git.kernel.org/pub/scm/git/git.git/
git://repo.or.cz/alt-git.git/
https://kernel.googlesource.com/pub/scm/git/git/
https://github.com/git/git/
https://gitlab.com/git-vcs/git/
With all the integration branches and topics broken out:
https://github.com/gitster/git/
Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):
git://git.kernel.org/pub/scm/git/git-htmldocs.git/
https://github.com/gitster/git-htmldocs.git/
Release tarballs are available at:
https://www.kernel.org/pub/software/scm/git/
--------------------------------------------------
[Cooking]
* jc/t0091-with-unknown-git (2024-01-30) 1 commit
(merged to 'next' on 2024-01-31 at 3dfcad1b18)
+ t0091: allow test in a repository without tags
The test did not work when Git was built from a repository without
tags.
Will merge to 'master'.
source: <xmqqv87aabk3.fsf@gitster.g>
* ps/reftable-backend (2024-02-02) 3 commits
- ci: add jobs to test with the reftable backend
- refs: introduce reftable backend
- Merge branch 'ps/tests-with-ref-files-backend' into ps/reftable-backend
(this branch uses ps/tests-with-ref-files-backend.)
Integrate the reftable code into the refs framework as a backend.
Needs review.
source: <cover.1706862705.git.ps@pks.im>
* js/win32-retry-pipe-write-on-enospc (2024-01-30) 1 commit
(merged to 'next' on 2024-01-31 at 60ad589fd0)
+ win32: special-case `ENOSPC` when writing to a pipe
Update to the code that writes to pipes on Windows.
Will merge to 'master'.
source: <pull.1648.git.1706650619950.gitgitgadget@gmail.com>
* jc/make-libpath-template (2024-01-31) 2 commits
(merged to 'next' on 2024-01-31 at 559d5138bc)
+ Makefile: simplify output of the libpath_template
+ Makefile: reduce repetitive library paths
The Makefile often had to say "-L$(path) -R$(path)" that repeats
the path to the same library directory for link time and runtime.
A Makefile template is used to reduce such repetition.
Will merge to 'master'.
source: <20240131174220.4160560-1-gitster@pobox.com>
* cb/use-freebsd-13-2-at-cirrus-ci (2024-01-31) 1 commit
(merged to 'next' on 2024-01-31 at f89dc8a289)
+ ci: update FreeBSD cirrus job
Cirrus CI jobs started breaking because we specified version of
FreeBSD that is no longer available, which has been corrected.
Will merge to 'master'.
source: <20240131191325.33228-1-carenas@gmail.com>
* cc/rev-list-allow-missing-tips (2024-02-01) 3 commits
- rev-list: add --allow-missing-tips to be used with --missing=...
- t6022: fix 'even though' typo in comment
- revision: clarify a 'return NULL' in get_reference()
"git rev-list --missing=print" have learned to optionally take
"--allow-missing-tips", which allows the objects at the starting
points to be missing.
Needs review.
source: <20240201115809.1177064-1-christian.couder@gmail.com>
* ps/reftable-iteration-perf (2024-02-01) 7 commits
- reftable/reader: add comments to `table_iter_next()`
- reftable/record: don't try to reallocate ref record name
- reftable/block: swap buffers instead of copying
- reftable/pq: allocation-less comparison of entry keys
- reftable/merged: skip comparison for records of the same subiter
- reftable/merged: allocation-less dropping of shadowed records
- reftable/record: introduce function to compare records by key
The code to iterate over refs with the reftable backend has seen
some optimization.
Needs review.
source: <cover.1706782841.git.ps@pks.im>
* ps/reftable-styles (2024-02-01) 9 commits
- reftable/record: improve semantics when initializing records
- reftable/merged: refactor initialization of iterators
- reftable/merged: refactor seeking of records
- reftable/stack: use `size_t` to track stack length
- reftable/stack: use `size_t` to track stack slices during compaction
- reftable/stack: index segments with `size_t`
- reftable/stack: fix parameter validation when compacting range
- reftable: introduce macros to allocate arrays
- reftable: introduce macros to grow arrays
Code clean-up in various reftable code paths.
Needs review.
source: <cover.1706772591.git.ps@pks.im>
* pb/imap-send-wo-curl-build-fix (2024-02-01) 1 commit
- imap-send: add missing "strbuf.h" include under NO_CURL
Build fix.
Will merge to 'next'.
source: <pull.1664.git.git.1706833113569.gitgitgadget@gmail.com>
* jc/github-actions-update (2024-02-02) 1 commit
- Merge branch 'jc/maint-github-actions-update' into jc/github-actions-update
(this branch uses jc/maint-github-actions-update.)
An evil merge of the other topic to a more modern codebase.
Will merge to 'next'?
* jc/maint-github-actions-update (2024-02-02) 2 commits
- GitHub Actions: update to github-script@v7
- GitHub Actions: update to checkout@v4
(this branch is used by jc/github-actions-update.)
Squelch node.js 16 deprecation warnings from GitHub Actions CI
by updating actions/github-script and actions/checkout that use
node.js 20.
Needs review.
source: <20240202203935.1240458-1-gitster@pobox.com>
* jh/sparse-index-expand-to-path-fix (2024-02-02) 1 commit
- sparse-index: pass string length to index_file_exists()
A caller called index_file_exists() that takes a string expressed
as <ptr, length> with a wrong length, which has been corrected.
Will merge to 'next'.
source: <pull.1649.git.1706897095273.gitgitgadget@gmail.com>
* jc/comment-style-fixes (2024-01-29) 3 commits
(merged to 'next' on 2024-01-30 at a58d48a9ce)
+ reftable/pq_test: comment style fix
+ merge-ort.c: comment style fix
+ builtin/worktree: comment style fixes
Rewrite //-comments to /* comments */ in files whose comments
prevalently use the latter.
Will merge to 'master'.
source: <20240129202839.2234084-1-gitster@pobox.com>
* jk/diff-external-with-no-index (2024-01-29) 1 commit
(merged to 'next' on 2024-01-30 at 30c3e9f91d)
+ diff: handle NULL meta-info when spawning external diff
"git diff --no-index file1 file2" segfaulted while invoking the
external diff driver, which has been corrected.
Will merge to 'next'.
source: <20240129015708.GA1762343@coredump.intra.peff.net>
* jk/unit-tests-buildfix (2024-02-02) 4 commits
(merged to 'next' on 2024-02-02 at 8838dd21e8)
+ t/Makefile: say the default target upfront
(merged to 'next' on 2024-01-31 at 00df31c4c8)
+ t/Makefile: get UNIT_TESTS list from C sources
+ Makefile: remove UNIT_TEST_BIN directory with "make clean"
+ Makefile: use mkdir_p_parent_template for UNIT_TEST_BIN
Build dependency around unit tests has been fixed.
Will merge to 'master'.
source: <20240130053714.GA165967@coredump.intra.peff.net>
source: <xmqqjznmtjr9.fsf@gitster.g>
* js/merge-tree-3-trees (2024-01-29) 1 commit
(merged to 'next' on 2024-01-30 at 0c77b04e59)
+ merge-tree: accept 3 trees as arguments
"git merge-tree" has learned that the three trees involved in the
3-way merge only need to be trees, not necessarily commits.
Will merge to 'master'.
source: <pull.1647.v2.git.1706474063109.gitgitgadget@gmail.com>
* jt/p4-spell-re-with-raw-string (2024-01-29) 1 commit
(merged to 'next' on 2024-01-30 at 42b03b58eb)
+ git-p4: use raw string literals for regular expressions
"git p4" update to squelch warnings from Python.
Will merge to 'master'.
source: <pull.1639.v2.git.1706312496608.gitgitgadget@gmail.com>
* kh/maintenance-use-xdg-when-it-should (2024-01-29) 1 commit
(merged to 'next' on 2024-01-30 at c449ac74bf)
+ config: add back code comment
Comment fix.
Will merge to 'master'.
source: <48d66e94ece3b763acbe933561d82157c02a5f58.1706466321.git.code@khaugsbakk.name>
* mh/credential-oauth-refresh-token-with-wincred (2024-01-29) 1 commit
- credential/wincred: store oauth_refresh_token
Teach wincred credential backend to support oauth refresh token the
same way as credential-cache and credential-libsecret backends.
Will merge to 'next'.
source: <pull.1534.v3.git.1706477103039.gitgitgadget@gmail.com>
* pb/complete-config (2024-01-29) 5 commits
- completion: add an use _ _git_compute_second_level_config_vars_for_section
- builtin/help: add --config-all-for-completion
- completion: add and use _ _git_compute_first_level_config_vars_for_section
- completion: complete 'submodule.*' config variables
- completion: add space after config variable names also in Bash 3
The command line completion script (in contrib/) learned to
complete configuration variable names better.
Needs review.
source: <pull.1660.v2.git.git.1706534881.gitgitgadget@gmail.com>
* rj/complete-reflog (2024-01-26) 4 commits
- completion: reflog show <log-options>
- completion: reflog with implicit "show"
- completion: introduce __git_find_subcommand
- completion: introduce __gitcomp_subcommand
The command line completion script (in contrib/) learned to
complete "git reflog" better.
Needs review.
source: <98daf977-dbad-4d3b-a293-6a769895088f@gmail.com>
* rj/test-with-leak-check (2024-01-29) 4 commits
(merged to 'next' on 2024-01-31 at 76e4596666)
+ t0080: mark as leak-free
+ test-lib: check for TEST_PASSES_SANITIZE_LEAK
+ t6113: mark as leak-free
+ t5332: mark as leak-free
More tests that are supposed to pass leak sanitizer are marked as such.
Will merge to 'master'.
source: <45eb0748-6415-4e52-a54f-8d4e5ad57dde@gmail.com>
* tb/pack-bitmap-drop-unused-struct-member (2024-01-29) 1 commit
(merged to 'next' on 2024-01-30 at f3749b15fc)
+ pack-bitmap: drop unused `reuse_objects`
Code clean-up.
Will merge to 'master'.
source: <0bbaf9a3591765161872fb71383263edb0c7ef83.1706328008.git.me@ttaylorr.com>
* ps/reftable-compacted-tables-permission-fix (2024-01-26) 1 commit
(merged to 'next' on 2024-01-29 at dbb06e1571)
+ reftable/stack: adjust permissions of compacted tables
Reftable bugfix.
Will merge to 'master'.
source: <a211818108053754aca002726d0206623a347952.1706263589.git.ps@pks.im>
* jc/index-pack-fsck-levels (2024-02-01) 2 commits
(merged to 'next' on 2024-02-02 at 0e4ef26aa1)
+ index-pack: --fsck-objects to take an optional argument for fsck msgs
+ index-pack: test and document --strict=<msg-id>=<severity>...
The "--fsck-objects" option of "git index-pack" now can take the
optional parameter to tweak severity of different fsck errors.
Will merge to 'master'.
source: <pull.1658.v4.git.git.1706751483.gitgitgadget@gmail.com>
* ps/reftable-multi-level-indices-fix (2024-02-01) 6 commits
- reftable: document reading and writing indices
- reftable/writer: fix writing multi-level indices
- reftable/writer: simplify writing index records
- reftable/writer: use correct type to iterate through index entries
- reftable/reader: be more careful about errors in indexed seeks
- Merge branch 'jc/reftable-core-fsync' into ps/reftable-multi-level-indices-fix
(this branch uses jc/reftable-core-fsync.)
Write multi-level indices for reftable has been corrected.
Needs review.
source: <cover.1706773842.git.ps@pks.im>
* jc/reftable-core-fsync (2024-01-30) 2 commits
(merged to 'next' on 2024-01-30 at c3a79b6172)
+ reftable/stack: fsync "tables.list" during compaction
(merged to 'next' on 2024-01-24 at cea12beddb)
+ reftable: honor core.fsync
(this branch is used by ps/reftable-multi-level-indices-fix.)
The write codepath for the reftable data learned to honor
core.fsync configuration.
Will merge to 'master'.
source: <7bdafc9bd7f53f38a24d69a563615b6ad484e1ba.1706592127.git.ps@pks.im>
* cp/unit-test-prio-queue (2024-01-22) 1 commit
(merged to 'next' on 2024-02-01 at 38aa6559b0)
+ tests: move t0009-prio-queue.sh to the new unit testing framework
Migrate priority queue test to unit testing framework.
Will merge to 'master'.
source: <pull.1642.v4.git.1705865326185.gitgitgadget@gmail.com>
* ml/log-merge-with-cherry-pick-and-other-pseudo-heads (2024-01-17) 2 commits
- revision: implement `git log --merge` also for rebase/cherry_pick/revert
- revision: ensure MERGE_HEAD is a ref in prepare_show_merge
"git log --merge" learned to pay attention to CHERRY_PICK_HEAD and
other kinds of *_HEAD pseudorefs.
Comments?
source: <xmqqzfxa9usx.fsf@gitster.g>
* kn/for-all-refs (2024-01-29) 4 commits
(merged to 'next' on 2024-01-30 at e7a9234a8b)
+ for-each-ref: avoid filtering on empty pattern
+ refs: introduce `refs_for_each_all_refs()`
+ refs: extract out `loose_fill_ref_dir_regular_file()`
+ refs: introduce `is_pseudoref()` and `is_headref()`
"git for-each-ref" filters its output with prefixes given from the
command line, but it did not honor an empty string to mean "pass
everything", which has been corrected.
Will merge to 'master'.
source: <20240129113527.607022-1-karthik.188@gmail.com>
* bk/complete-bisect (2024-01-29) 9 commits
- SQUASH???
- completion: add tests for git-bisect
- completion: bisect: recognize but do not complete view subcommand
- completion: bisect: complete log opts for visualize subcommand
- completion: log: use __git_complete_log_opts
- completion: new function __git_complete_log_opts
- completion: bisect: complete missing --first-parent and --no-checkout options
- completion: bisect: complete custom terms and related options
- completion: bisect: complete bad, new, old, and help subcommands
Command line completion support (in contrib/) has been
updated for "git bisect".
Comments?
cf. <ZaofJhHsFjRxx7a3@tanuki>
source: <20240128223447.342493-1-britton.kerin@gmail.com>
* bk/complete-dirname-for-am-and-format-patch (2024-01-12) 1 commit
- completion: dir-type optargs for am, format-patch
Command line completion support (in contrib/) has been
updated for a few commands to complete directory names where a
directory name is expected.
Needs review.
source: <d37781c3-6af2-409b-95a8-660a9b92d20b@smtp-relay.sendinblue.com>
* bk/complete-send-email (2024-01-12) 1 commit
- completion: don't complete revs when --no-format-patch
Command line completion support (in contrib/) has been taught to
avoid offering revision names as candidates to "git send-email" when
the command is used to send pre-generated files.
Needs review.
source: <a718b5ee-afb0-44bd-a299-3208fac43506@smtp-relay.sendinblue.com>
* la/trailer-api (2024-01-30) 10 commits
- trailer: introduce "template" term for readability
- trailer: delete obsolete argument handling code from API
- trailer: move arg handling to interpret-trailers.c
- trailer: prepare to move parse_trailers_from_command_line_args() to builtin
- trailer: spread usage of "trailer_block" language
- trailer: make trailer_info struct private
- sequencer: use the trailer iterator
- trailer: unify trailer formatting machinery
- trailer: move interpret_trailers() to interpret-trailers.c
- trailer: prepare to expose functions as part of API
Code clean-up.
Needs review.
cf. <xmqqa5ol409k.fsf@gitster.g>
source: <pull.1632.v3.git.1706664144.gitgitgadget@gmail.com>
* ps/tests-with-ref-files-backend (2024-01-29) 6 commits
(merged to 'next' on 2024-01-30 at 376b9c9c1b)
+ t: mark tests regarding git-pack-refs(1) to be backend specific
+ t5526: break test submodule differently
+ t1419: mark test suite as files-backend specific
+ t1302: make tests more robust with new extensions
+ t1301: mark test for `core.sharedRepository` as reffiles specific
+ t1300: make tests more robust with non-default ref backends
(this branch is used by ps/reftable-backend.)
Prepare existing tests on refs to work better with non-default
backends.
Will merge to 'master'.
source: <cover.1706525813.git.ps@pks.im>
* cp/apply-core-filemode (2023-12-26) 3 commits
- apply: code simplification
- apply: correctly reverse patch's pre- and post-image mode bits
- apply: ignore working tree filemode when !core.filemode
"git apply" on a filesystem without filemode support have learned
to take a hint from what is in the index for the path, even when
not working with the "--index" or "--cached" option, when checking
the executable bit match what is required by the preimage in the
patch.
Needs review.
source: <20231226233218.472054-1-gitster@pobox.com>
* ja/doc-placeholders-fix (2023-12-26) 2 commits
- doc: enforce placeholders in documentation
- doc: enforce dashes in placeholders
Docfix.
Will merge to 'next'.
source: <pull.1626.git.1703539287.gitgitgadget@gmail.com>
* jc/bisect-doc (2023-12-09) 1 commit
- bisect: document "terms" subcommand more fully
Doc update.
Needs review.
source: <xmqqzfyjmk02.fsf@gitster.g>
* tb/pair-chunk-expect (2023-11-10) 8 commits
- midx: read `OOFF` chunk with `pair_chunk_expect()`
- midx: read `OIDL` chunk with `pair_chunk_expect()`
- commit-graph: read `BIDX` chunk with `pair_chunk_expect()`
- commit-graph: read `GDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `CDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `OIDL` chunk with `pair_chunk_expect()`
- chunk-format: introduce `pair_chunk_expect()` helper
- Merge branch 'jk/chunk-bounds-more' into HEAD
Further code clean-up.
Needs review.
source: <cover.1699569246.git.me@ttaylorr.com>
* tb/path-filter-fix (2024-01-31) 16 commits
- bloom: introduce `deinit_bloom_filters()`
- commit-graph: reuse existing Bloom filters where possible
- object.h: fix mis-aligned flag bits table
- commit-graph: new Bloom filter version that fixes murmur3
- commit-graph: unconditionally load Bloom filters
- bloom: prepare to discard incompatible Bloom filters
- bloom: annotate filters with hash version
- repo-settings: introduce commitgraph.changedPathsVersion
- t4216: test changed path filters with high bit paths
- t/helper/test-read-graph: implement `bloom-filters` mode
- bloom.h: make `load_bloom_filter_from_graph()` public
- t/helper/test-read-graph.c: extract `dump_graph_info()`
- gitformat-commit-graph: describe version 2 of BDAT
- commit-graph: ensure Bloom filters are read with consistent settings
- revision.c: consult Bloom filters for root commits
- t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
The Bloom filter used for path limited history traversal was broken
on systems whose "char" is unsigned; update the implementation and
bump the format version to 2.
source: <cover.1706741516.git.me@ttaylorr.com>
* ak/color-decorate-symbols (2023-10-23) 7 commits
- log: add color.decorate.pseudoref config variable
- refs: exempt pseudorefs from pattern prefixing
- refs: add pseudorefs array and iteration functions
- log: add color.decorate.ref config variable
- log: add color.decorate.symbol config variable
- log: use designated inits for decoration_colors
- config: restructure color.decorate documentation
A new config for coloring.
Needs review.
source: <20231023221143.72489-1-andy.koppe@gmail.com>
* eb/hash-transition (2023-10-02) 30 commits
- t1016-compatObjectFormat: add tests to verify the conversion between objects
- t1006: test oid compatibility with cat-file
- t1006: rename sha1 to oid
- test-lib: compute the compatibility hash so tests may use it
- builtin/ls-tree: let the oid determine the output algorithm
- object-file: handle compat objects in check_object_signature
- tree-walk: init_tree_desc take an oid to get the hash algorithm
- builtin/cat-file: let the oid determine the output algorithm
- rev-parse: add an --output-object-format parameter
- repository: implement extensions.compatObjectFormat
- object-file: update object_info_extended to reencode objects
- object-file-convert: convert commits that embed signed tags
- object-file-convert: convert commit objects when writing
- object-file-convert: don't leak when converting tag objects
- object-file-convert: convert tag objects when writing
- object-file-convert: add a function to convert trees between algorithms
- object: factor out parse_mode out of fast-import and tree-walk into in object.h
- cache: add a function to read an OID of a specific algorithm
- tag: sign both hashes
- commit: export add_header_signature to support handling signatures on tags
- commit: convert mergetag before computing the signature of a commit
- commit: write commits for both hashes
- object-file: add a compat_oid_in parameter to write_object_file_flags
- object-file: update the loose object map when writing loose objects
- loose: compatibilty short name support
- loose: add a mapping between SHA-1 and SHA-256 for loose objects
- repository: add a compatibility hash algorithm
- object-names: support input of oids in any supported hash
- oid-array: teach oid-array to handle multiple kinds of oids
- object-file-convert: stubs for converting from one object format to another
Teach a repository to work with both SHA-1 and SHA-256 hash algorithms.
Needs review.
source: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
* jc/rerere-cleanup (2023-08-25) 4 commits
- rerere: modernize use of empty strbuf
- rerere: try_merge() should use LL_MERGE_ERROR when it means an error
- rerere: fix comment on handle_file() helper
- rerere: simplify check_one_conflict() helper function
Code clean-up.
Not ready to be reviewed yet.
source: <20230824205456.1231371-1-gitster@pobox.com>
^ permalink raw reply
* Re: What's cooking in git.git (Feb 2024, #01; Fri, 2)
From: Junio C Hamano @ 2024-02-03 5:22 UTC (permalink / raw)
To: git
In-Reply-To: <xmqqr0hutm1x.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> * jk/diff-external-with-no-index (2024-01-29) 1 commit
> (merged to 'next' on 2024-01-30 at 30c3e9f91d)
> + diff: handle NULL meta-info when spawning external diff
>
> "git diff --no-index file1 file2" segfaulted while invoking the
> external diff driver, which has been corrected.
>
> Reverted out of 'next' for now, seems to break "win test (n)" jobs.
> cf. <xmqqh6irwtkd.fsf@gitster.g>
> source: <20240129015708.GA1762343@coredump.intra.peff.net>
The above topic is marked as such by mistake. It is on track to be
merged to 'master'. What was temporarily reverted was the next one.
> * jk/unit-tests-buildfix (2024-01-31) 3 commits
> (merged to 'next' on 2024-01-31 at 00df31c4c8)
> + t/Makefile: get UNIT_TESTS list from C sources
> + Makefile: remove UNIT_TEST_BIN directory with "make clean"
> + Makefile: use mkdir_p_parent_template for UNIT_TEST_BIN
>
> Build dependency around unit tests has been fixed.
>
> Will merge to 'master'.
> source: <20240130053714.GA165967@coredump.intra.peff.net>
But Dscho helped to fix the breakage on "win test (n)" jobs so it
has again been queued to 'next'.
^ permalink raw reply
* Re: [PATCH v3 05/10] trailer: make trailer_info struct private
From: Junio C Hamano @ 2024-02-03 4:43 UTC (permalink / raw)
To: Linus Arver
Cc: Linus Arver via GitGitGadget, git, Christian Couder,
Emily Shaffer, Josh Steadmon, Randall S. Becker
In-Reply-To: <owlycyte1hhj.fsf@fine.c.googlers.com>
Linus Arver <linusa@google.com> writes:
>>> +size_t trailer_block_start(struct trailer_info *info);
>>> +size_t trailer_block_end(struct trailer_info *info);
>>> +int blank_line_before_trailer_block(struct trailer_info *info);
>>
>> And we need new accessors, which is a good change regardless of the
>> answer to the "do we really want an extra pointer dereference?
>> Shouldn't the existing 'private' and 'internal' label we see below
>> sufficient?" question.
>
> I am very surprised with your response here, because I was expecting the
> complete opposite reaction from reviewers --- something like
>
> Hmph, we have to write accessor functions now when before we could
> just reach inside the struct directly? Isn't this just adding
> needless verbosity?
>
> (perhaps with more dissatisfaction). Something tells me you meant "bad
> change" but accidentally wrote "good change". Am I correct?
I often make typoes and screw up my double negations, but not this
time. While I still suspect that the extra secrecy afforded by
using a pointer to an opaque structure is something unnecessary
between friends, as coding convention, it would be a good change to
introduce accessors and have them used by the API users, i.e. code
outside the API implementation.
You can still "git grep" for references to the ".trailer_private"
(or whatever the "private" members are named by convention) member
outside the trailer "client code" to see if there are people who are
too intimate than they should be that way, as they should all be
using the published accessors.
^ permalink raw reply
* Re: [PATCH v3 08/10] trailer: move arg handling to interpret-trailers.c
From: Linus Arver @ 2024-02-03 1:48 UTC (permalink / raw)
To: Junio C Hamano, Linus Arver via GitGitGadget
Cc: git, Christian Couder, Emily Shaffer, Josh Steadmon,
Randall S. Becker
In-Reply-To: <xmqqttmrx1ro.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> [...]
>
> Other than that, the main thrust of this step seems sensible.
>
> Thanks.
Quick update: I've broken down patches 3 and 4 into smaller pieces, and
am now ready to break this one down, too.
Hopefully sometime over the weekend I'll have some answers ready for the
many good questions you've raised. Thanks.
^ permalink raw reply
* Re: [PATCH v3 09/10] trailer: delete obsolete argument handling code from API
From: Linus Arver @ 2024-02-03 1:40 UTC (permalink / raw)
To: Junio C Hamano, Linus Arver via GitGitGadget
Cc: git, Christian Couder, Emily Shaffer, Josh Steadmon,
Randall S. Becker
In-Reply-To: <xmqqplxfx1nk.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> From: Linus Arver <linusa@google.com>
>>
>> This commit was not squashed with its parent in order to keep the diff
>> separate (to make the additive changes in the parent easier to read).
>
> Hopefully we won't see such an artificial separation in the new
> iteration. As we saw, being able to compare before and after images
> in a single patch is useful while reviewing a change that is supposed
> to be making things cleaner without altering what they do.
Totally agree (and I'm reaping the benefits of this as well in the
smaller preparatory refactoring changes I've made for the next reroll).
^ permalink raw reply
* Re: [PATCH v3 06/10] trailer: spread usage of "trailer_block" language
From: Linus Arver @ 2024-02-03 1:37 UTC (permalink / raw)
To: Junio C Hamano, Linus Arver via GitGitGadget
Cc: git, Christian Couder, Emily Shaffer, Josh Steadmon,
Randall S. Becker
In-Reply-To: <xmqqil38ypuk.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> From: Linus Arver <linusa@google.com>
>>
>> Deprecate the "trailer_info" struct name and replace it with
>> "trailer_block". The main reason is to help readability, because
>> "trailer_info" on the surface sounds like it's about a single trailer
>> when in reality it is a collection of contiguous lines, at least 25% of
>> which are trailers.
>
> Yup, "info" is usually a fairly meaningless word. At least "block"
> may imply it is a collection of trailers.
>
> The naming would not matter as much to the API users, if the thing
> is now opaque, though.
You make an interesting point (and I agree).
If we were to provide additional private structs named "trailer_<foo>"
at that point we should think about how these <foo> parts "interact"
with each other (if you will) in terms of names. For example we probably
would never want to name a struct "trailer_block_parser" if it has no
relationship to the existing "trailer_block" struct. More elaborate APIs
might design a particular naming scheme (with some rules about certain
suffixes, perhaps, for opaque pointers that all behave a certain way).
But such naming considerations would naturally come into view
independent of this public vs public struct discussion, so I think the
above paragraph is moot.
^ permalink raw reply
* Re: [PATCH v2 0/3] some unit-test Makefile polishing
From: Junio C Hamano @ 2024-02-03 1:32 UTC (permalink / raw)
To: Johannes Schindelin
Cc: git, Jeff King, Phillip Wood, SZEDER Gábor, Adam Dinwoodie,
Patrick Steinhardt
In-Reply-To: <b7b92f1a-9231-2f53-299e-ad58fc699284@gmx.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> The line 753 of that file (as can be seen at
> https://github.com/git/git/blob/38aa6559b0c513d755d6d5ccf32414ed63754726/config.mak.uname#L753)
Ouch. When it is laid out like this it is very obvious why this is
broken, and what its workaround should be.
Thanks. Let's queue this on top.
------- >8 ------------- >8 ------------- >8 ------------- >8 -------
Subject: [PATCH] t/Makefile: say the default target upfront
Similar to how 2731d048 (Makefile: say the default target upfront.,
2005-12-01) added the default target to the very beginning of the
main Makefile to prevent a random rule that happens to be defined
first in an included makefile fragments from becoming the default
target, protect this Makefile the same way.
This started to matter as we started to include config.mak.uname
and that included makefile fragment does more than defining Make
macros, unfortunately.
Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/Makefile | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/t/Makefile b/t/Makefile
index 281f4c3534..2d95046f26 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -1,3 +1,6 @@
+# The default target of this Makefile is...
+all::
+
# Import tree-wide shared Makefile behavior and libraries
include ../shared.mak
@@ -52,7 +55,7 @@ UNIT_TESTS = $(sort $(filter-out unit-tests/bin/t-basic%,$(UNIT_TEST_PROGRAMS)))
# scripts not to run the external "chainlint.pl" script themselves
CHAINLINTSUPPRESS = GIT_TEST_EXT_CHAIN_LINT=0 && export GIT_TEST_EXT_CHAIN_LINT &&
-all: $(DEFAULT_TEST_TARGET)
+all:: $(DEFAULT_TEST_TARGET)
test: pre-clean check-chainlint $(TEST_LINT)
$(CHAINLINTSUPPRESS) $(MAKE) aggregate-results-and-cleanup
--
2.43.0-522-g2a540e432f
^ permalink raw reply related
* Re: [PATCH v3 05/10] trailer: make trailer_info struct private
From: Linus Arver @ 2024-02-03 1:09 UTC (permalink / raw)
To: Junio C Hamano, Linus Arver via GitGitGadget
Cc: git, Christian Couder, Emily Shaffer, Josh Steadmon,
Randall S. Becker
In-Reply-To: <xmqqplxgyq7f.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> Make this struct private by putting its definition inside trailer.c.
>> This has two benefits:
>>
>> (1) it makes the surface area of the public facing
>> interface (trailer.h) smaller, and
>>
>> (2) external API users are unable to peer inside this struct (because
>> it is only ever exposed as an opaque pointer).
>
> At the cost of an extra pointer dereference every time the member of
> the struct is accessed, plus the cost of allocation/deallocation.
Added to commit message for v4, thanks.
> Which may not be a huge deal, but I wonder if the approach to name
> the member of the outer struct with "private" that seems to be used
> in other parts of the code (e.g. the .private_size member in the
> hashmap structure, the .refs_private member in the repository
> structure) or even a documented convention (e.g. raw_object_store),
> might be more appropriate here.
Having gotten my hands dirty with the pimpl idiom, I think possibly the
best thing about it is that the compiler can tell me exactly where
every Git subsystem outside of trailer.c is accessing members inside the
(newly private) struct. So it's great for checking existing usage of how
things are used already, day-to-day. Not being well-versed in shortlog
or sequencer internals, these warnings from the compiler for these
functions (trying to peer through the opaque pointer) have been very
informative.
I think it would be reasonable to drop the idiom if the additional
performance costs (pointer dereference, allocation/deallocation) are too
much to bear. For the trailer subsystem I don't see an immediate need to
focus on performance though, so I think it's fine for now.
> If Coccinelle works well (which we
> may be having some trouble with --- cf. <xmqq1q9ybsnl.fsf@gitster.g>),
> we should be able to catch external accesses without having to hide
> the implementation details via an extra pointer dereference.
That's true. But that would only work for trailer API users inside the
Git codebase. Eventually I envision projects external to Git using
<trailer.h>, and in that scenario we would _really_ not want to let
these external users peek inside structs with members named "private" or
"internal" in name only. Murphy's law suggests those external users will
(naughtily) depend on the internals, and that would place an undue
burden on us if we ever wanted to make large changes to these (public)
structs because we might break them.
Another thing to consider is that if our API users ever want to get more
flexibility out of it, they would immediately come to us if we used the
pimpl idiom (our users are bound by exactly how we define the API). If
the structs were public, then they would be free to implement new
functionality to extend the API on their own, without ever letting us
know. If there are folks making small (unofficial) extensions to the
API, I want that to happen _in_ this project, not outside.
In summary what I'm saying is that this idiom gives very strong guidance
for desining an API for both Git-internal and external-to-Git usage. I
think for at least the trailer subsystem it's worth trying out. I've
pointed out some of the same points in the cover letter also.
>> @@ -176,11 +176,12 @@ static void interpret_trailers(const struct process_trailer_options *opts,
>> strbuf_release(&trailer_block);
>>
>> free_trailers(&head);
>> - trailer_info_release(&info);
>>
>> /* Print the lines after the trailers as is */
>> if (!opts->only_trailers)
>> - fwrite(sb.buf + info.trailer_block_end, 1, sb.len - info.trailer_block_end, outfile);
>> + fwrite(sb.buf + trailer_block_end(info), 1, sb.len - trailer_block_end(info), outfile);
>> +
>> + trailer_info_release(info);
>
> Interesting. Is this an indenendent bugfix even if we decided not
> to take this patch?
Nice catch!
> No, I have not fully decided to be negative on
> the move this entire patch makes (even though I am leaning towards
> saying so). Just hypothetically, even if we wanted to keep "info"
> here as a structure and not a pointer to an opaque structure,
> doesn't this hunk fix a real bug?
>
> Well, technically, not quite, because the members referenced in that
> if (.only_trailers) block are still live in the info struct. But it
> still smells wrong to access info.* after calling _release() on it,
> and this fix should come before "info" is turned from an instance to
> a pointer, I would say.
I've promoted this change into a bugfix patch as the very first patch
for the next reroll. Thanks.
>> diff --git a/trailer.h b/trailer.h
>> index a7599067acc..e19ddf84e64 100644
>> --- a/trailer.h
>> +++ b/trailer.h
>> @@ -4,6 +4,8 @@
>> #include "list.h"
>> #include "strbuf.h"
>>
>> +struct trailer_info;
>> +
>> enum trailer_where {
>> WHERE_DEFAULT,
>> WHERE_END,
>> @@ -29,27 +31,6 @@ int trailer_set_where(enum trailer_where *item, const char *value);
>> int trailer_set_if_exists(enum trailer_if_exists *item, const char *value);
>> int trailer_set_if_missing(enum trailer_if_missing *item, const char *value);
>>
>> +size_t trailer_block_start(struct trailer_info *info);
>> +size_t trailer_block_end(struct trailer_info *info);
>> +int blank_line_before_trailer_block(struct trailer_info *info);
>
> And we need new accessors, which is a good change regardless of the
> answer to the "do we really want an extra pointer dereference?
> Shouldn't the existing 'private' and 'internal' label we see below
> sufficient?" question.
I am very surprised with your response here, because I was expecting the
complete opposite reaction from reviewers --- something like
Hmph, we have to write accessor functions now when before we could
just reach inside the struct directly? Isn't this just adding
needless verbosity?
(perhaps with more dissatisfaction). Something tells me you meant "bad
change" but accidentally wrote "good change". Am I correct?
^ permalink raw reply
* [RFC PATCH v2 6/6] t/Makefile: run unit tests alongside shell tests
From: Josh Steadmon @ 2024-02-03 0:50 UTC (permalink / raw)
To: git; +Cc: johannes.schindelin, peff, phillip.wood, gitster
In-Reply-To: <cover.1706921262.git.steadmon@google.com>
From: Jeff King <peff@peff.net>
Add a wrapper script to allow `prove` to run both shell tests and unit
tests from a single invocation. This avoids issues around running prove
twice in CI, as discussed in [1].
Additionally, this moves the unit tests into the main dev workflow, so
that errors can be spotted more quickly.
NEEDS WORK: as discussed in previous commits in this series, there's a
desire to avoid `prove` specifically and (IIUC) unnecessary
fork()/exec()ing in general on Windows. This change adds an extra exec()
for each shell and unit test execution, will that be a problem for
Windows?
[1] https://lore.kernel.org/git/pull.1613.git.1699894837844.gitgitgadget@gmail.com/
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Josh Steadmon <steadmon@google.com>
---
t/Makefile | 2 +-
t/run-test.sh | 13 +++++++++++++
2 files changed, 14 insertions(+), 1 deletion(-)
create mode 100755 t/run-test.sh
diff --git a/t/Makefile b/t/Makefile
index 6e6316c29b..6a67fc22d7 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -64,7 +64,7 @@ failed:
test -z "$$failed" || $(MAKE) $$failed
prove: pre-clean check-chainlint $(TEST_LINT)
- @echo "*** prove ***"; $(CHAINLINTSUPPRESS) $(PROVE) --exec '$(TEST_SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
+ @echo "*** prove (shell & unit tests) ***"; $(CHAINLINTSUPPRESS) $(PROVE) --exec ./run-test.sh $(GIT_PROVE_OPTS) $(T) $(UNIT_TESTS) :: $(GIT_TEST_OPTS)
$(MAKE) clean-except-prove-cache
$(T):
diff --git a/t/run-test.sh b/t/run-test.sh
new file mode 100755
index 0000000000..c29fef48dc
--- /dev/null
+++ b/t/run-test.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+# A simple wrapper to run shell tests via TEST_SHELL_PATH,
+# or exec unit tests directly.
+
+case "$1" in
+*.sh)
+ exec ${TEST_SHELL_PATH:-/bin/sh} "$@"
+ ;;
+*)
+ exec "$@"
+ ;;
+esac
--
2.43.0.594.gd9cf4e227d-goog
^ permalink raw reply related
* [RFC PATCH v2 5/6] unit tests: add rule for running with test-tool
From: Josh Steadmon @ 2024-02-03 0:50 UTC (permalink / raw)
To: git; +Cc: johannes.schindelin, peff, phillip.wood, gitster
In-Reply-To: <cover.1706921262.git.steadmon@google.com>
In the previous commit, we added support in test-tool for running
collections of unit tests. Now, add rules in t/Makefile for running in
this way.
This new rule can be executed from the top-level Makefile via
`make DEFAULT_UNIT_TEST_TARGET=unit-tests-test-tool unit-tests`, or by
setting DEFAULT_UNIT_TEST_TARGET in config.mak.
Signed-off-by: Josh Steadmon <steadmon@google.com>
---
Makefile | 2 +-
t/Makefile | 10 +++++++++-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index ba55d817ee..b0d1f04b4d 100644
--- a/Makefile
+++ b/Makefile
@@ -3870,5 +3870,5 @@ $(UNIT_TEST_PROGS): $(UNIT_TEST_BIN)/%$X: $(UNIT_TEST_DIR)/%.o $(UNIT_TEST_DIR)/
.PHONY: build-unit-tests unit-tests
build-unit-tests: $(UNIT_TEST_PROGS)
-unit-tests: $(UNIT_TEST_PROGS)
+unit-tests: $(UNIT_TEST_PROGS) t/helper/test-tool$X
$(MAKE) -C t/ unit-tests
diff --git a/t/Makefile b/t/Makefile
index 1283c90c10..6e6316c29b 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -45,6 +45,7 @@ CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.tes
CHAINLINT = '$(PERL_PATH_SQ)' chainlint.pl
UNIT_TEST_SOURCES = $(wildcard unit-tests/t-*.c)
UNIT_TESTS = $(patsubst unit-tests/%.c,unit-tests/bin/%$(X),$(UNIT_TEST_SOURCES))
+UNIT_TESTS_NO_DIR = $(notdir $(UNIT_TESTS))
# `test-chainlint` (which is a dependency of `test-lint`, `test` and `prove`)
# checks all tests in all scripts via a single invocation, so tell individual
@@ -72,7 +73,7 @@ $(T):
$(UNIT_TESTS):
@echo "*** $@ ***"; $@
-.PHONY: unit-tests unit-tests-raw unit-tests-prove
+.PHONY: unit-tests unit-tests-raw unit-tests-prove unit-tests-test-tool
unit-tests: $(DEFAULT_UNIT_TEST_TARGET)
unit-tests-raw: $(UNIT_TESTS)
@@ -80,6 +81,13 @@ unit-tests-raw: $(UNIT_TESTS)
unit-tests-prove:
@echo "*** prove - unit tests ***"; $(PROVE) $(GIT_PROVE_OPTS) $(UNIT_TESTS)
+unit-tests-test-tool:
+ @echo "*** test-tool - unit tests **"
+ ( \
+ cd unit-tests/bin && \
+ ../../helper/test-tool$X run-command testsuite $(UNIT_TESTS_NO_DIR)\
+ )
+
pre-clean:
$(RM) -r '$(TEST_RESULTS_DIRECTORY_SQ)'
--
2.43.0.594.gd9cf4e227d-goog
^ permalink raw reply related
* [RFC PATCH v2 4/6] test-tool run-command testsuite: support unit tests
From: Josh Steadmon @ 2024-02-03 0:50 UTC (permalink / raw)
To: git; +Cc: johannes.schindelin, peff, phillip.wood, gitster
In-Reply-To: <cover.1706921262.git.steadmon@google.com>
Teach the testsuite runner in `test-tool run-command testsuite` how to
run unit tests: if TEST_SHELL_PATH is not set, assume that we're running
the programs directly from CWD, rather than defaulting to "sh" as an
interpreter.
With this change, you can now use test-tool to run the unit tests:
$ make
$ cd t/unit-tests/bin
$ ../../helper/test-tool run-command testsuite
This should be helpful on Windows to allow running tests without
requiring Perl (for `prove`), as discussed in [1] and [2].
This again breaks backwards compatibility, as it is now required to set
TEST_SHELL_PATH properly for executing shell scripts, but again, as
noted in [2], there are no longer any such invocations in our codebase.
[1] https://lore.kernel.org/git/nycvar.QRO.7.76.6.2109091323150.59@tvgsbejvaqbjf.bet/
[2] https://lore.kernel.org/git/850ea42c-f103-68d5-896b-9120e2628686@gmx.de/
Signed-off-by: Josh Steadmon <steadmon@google.com>
---
t/helper/test-run-command.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c
index e6bd792274..a0b8dc6fd7 100644
--- a/t/helper/test-run-command.c
+++ b/t/helper/test-run-command.c
@@ -158,6 +158,8 @@ static int testsuite(int argc, const char **argv)
.task_finished = test_finished,
.data = &suite,
};
+ struct strbuf progpath = STRBUF_INIT;
+ size_t path_prefix_len;
argc = parse_options(argc, argv, NULL, options,
testsuite_usage, PARSE_OPT_STOP_AT_NON_OPTION);
@@ -165,9 +167,14 @@ static int testsuite(int argc, const char **argv)
if (max_jobs <= 0)
max_jobs = online_cpus();
+ /*
+ * If we run without a shell, we have to provide the relative path to
+ * the executables.
+ */
suite.shell_path = getenv("TEST_SHELL_PATH");
if (!suite.shell_path)
- suite.shell_path = "sh";
+ strbuf_addstr(&progpath, "./");
+ path_prefix_len = progpath.len;
dir = opendir(".");
if (!dir)
@@ -180,13 +187,17 @@ static int testsuite(int argc, const char **argv)
/* No pattern: match all */
if (!argc) {
- string_list_append(&suite.tests, p);
+ strbuf_setlen(&progpath, path_prefix_len);
+ strbuf_addstr(&progpath, p);
+ string_list_append(&suite.tests, progpath.buf);
continue;
}
for (i = 0; i < argc; i++)
if (!wildmatch(argv[i], p, 0)) {
- string_list_append(&suite.tests, p);
+ strbuf_setlen(&progpath, path_prefix_len);
+ strbuf_addstr(&progpath, p);
+ string_list_append(&suite.tests, progpath.buf);
break;
}
}
@@ -213,6 +224,7 @@ static int testsuite(int argc, const char **argv)
string_list_clear(&suite.tests, 0);
string_list_clear(&suite.failed, 0);
+ strbuf_release(&progpath);
return ret;
}
--
2.43.0.594.gd9cf4e227d-goog
^ 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