Git development
 help / color / mirror / Atom feed
* Re: [PATCH] rebase -i: stop setting GIT_CHERRY_PICK_HELP
From: Junio C Hamano @ 2024-02-27 18:38 UTC (permalink / raw)
  To: Phillip Wood via GitGitGadget; +Cc: git, Johannes Schindelin, Phillip Wood
In-Reply-To: <pull.1678.git.1709042783847.gitgitgadget@gmail.com>

"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Note that we retain the changes in e4301f73fff (sequencer: unset
> GIT_CHERRY_PICK_HELP for 'exec' commands, 2024-02-02) just in case
> GIT_CHERRY_PICK_HELP is set in the environment when "git rebase" is
> run.

Is this comment about this part of the code?

> +	const char *msg;
> +
> +	if (is_rebase_i(opts))
> +		msg = rebase_resolvemsg;
> +	else
> +		msg = getenv("GIT_CHERRY_PICK_HELP");

Testing is_rebase_i() first means we ignore the environment
unconditionally and use our own message always in "rebase -i", no?

Not that I think we should honor the environment variable and let it
override our message.  I just found the description a bit confusing.

> diff --git a/sequencer.h b/sequencer.h
> index dcef7bb99c0..437eabd38af 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -14,6 +14,8 @@ const char *rebase_path_todo(void);
>  const char *rebase_path_todo_backup(void);
>  const char *rebase_path_dropped(void);
>  
> +extern const char *rebase_resolvemsg;

This is more library-ish part of the system than a random file in
the builtin/ directory.  This place as the final location for the
string makes sense to me.

Thanks.

^ permalink raw reply

* Re: [PATCH v5 0/2] Implement `git log --merge` also for rebase/cherry-pick/revert
From: Junio C Hamano @ 2024-02-27 18:30 UTC (permalink / raw)
  To: Phillip Wood
  Cc: Philippe Blain, git, Johannes Sixt, Elijah Newren,
	Michael Lohmann, Phillip Wood, Patrick Steinhardt,
	Michael Lohmann
In-Reply-To: <ff5a3954-4e7a-42b7-988e-f306b45918bf@gmail.com>

Phillip Wood <phillip.wood123@gmail.com> writes:

> On 25/02/2024 21:56, Philippe Blain wrote:
>> Changes in v5:
>> - Marked error messages for translation and tweaked them as suggested by Phillip
>> - Reworded the message of 2/2 as suggested by Phillip
>> - Removed the change to gitk's doc in 2/2 as pointed out by Johannes
>> - Fixed the trailers in 2/2
>> - Improved the doc in 2/2 as suggested by Phillip and Jean-Noël
>
> These changes look good, thanks for making them. I agree with the
> other reviewers that it would be nice to improve the wording of the
> error message when we find a symbolic ref. Everything else looks good
> to me.
>
> Thanks
>
> Phillip

Thanks for a review.  Queued.

^ permalink raw reply

* Re: [PATCH v3 06/11] merge_bases_many(): pass on errors from `paint_down_to_common()`
From: Junio C Hamano @ 2024-02-27 18:29 UTC (permalink / raw)
  To: Johannes Schindelin via GitGitGadget
  Cc: git, Patrick Steinhardt, Johannes Schindelin
In-Reply-To: <2ae6a54dd596c3192b5be32f0e4e2605f4aac6bd.1709040499.git.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The `paint_down_to_common()` function was just taught to indicate
> parsing errors, and now the `merge_bases_many()` function is aware of
> that, too.
>
> One tricky aspect is that `merge_bases_many()` parses commits of its
> own, but wants to gracefully handle the scenario where NULL is passed as
> a merge head, returning the empty list of merge bases. The way this was
> handled involved calling `repo_parse_commit(NULL)` and relying on it to
> return an error. This has to be done differently now so that we can
> handle missing commits correctly by producing a fatal error.
>
> Next step: adjust the caller of `merge_bases_many()`:
> `get_merge_bases_many_0()`.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  commit-reach.c | 35 ++++++++++++++++++++++-------------
>  1 file changed, 22 insertions(+), 13 deletions(-)

This is the true gem of the series.  The next steps may have to be
noisy due to many callers adjusting to the new function signatures,
but split into these logical steps in a bottom-up way makes them
easier to follow.  Nice.

>
> diff --git a/commit-reach.c b/commit-reach.c
> index 9148a7dcbc0..2c74583c8e0 100644
> --- a/commit-reach.c
> +++ b/commit-reach.c
> @@ -131,41 +131,49 @@ static int paint_down_to_common(struct repository *r,
>  	return 0;
>  }
>  
> -static struct commit_list *merge_bases_many(struct repository *r,
> -					    struct commit *one, int n,
> -					    struct commit **twos)
> +static int merge_bases_many(struct repository *r,
> +			    struct commit *one, int n,
> +			    struct commit **twos,
> +			    struct commit_list **result)
>  {
>  	struct commit_list *list = NULL;
> -	struct commit_list *result = NULL;
>  	int i;
>  
>  	for (i = 0; i < n; i++) {
> -		if (one == twos[i])
> +		if (one == twos[i]) {
>  			/*
>  			 * We do not mark this even with RESULT so we do not
>  			 * have to clean it up.
>  			 */
> -			return commit_list_insert(one, &result);
> +			*result = commit_list_insert(one, result);
> +			return 0;
> +		}
>  	}
>  
> +	if (!one)
> +		return 0;
>  	if (repo_parse_commit(r, one))
> -		return NULL;
> +		return error(_("could not parse commit %s"),
> +			     oid_to_hex(&one->object.oid));
>  	for (i = 0; i < n; i++) {
> +		if (!twos[i])
> +			return 0;
>  		if (repo_parse_commit(r, twos[i]))
> -			return NULL;
> +			return error(_("could not parse commit %s"),
> +				     oid_to_hex(&twos[i]->object.oid));
>  	}
>  
>  	if (paint_down_to_common(r, one, n, twos, 0, 0, &list) < 0) {
>  		free_commit_list(list);
> -		return NULL;
> +		return -1;
>  	}
>  
>  	while (list) {
>  		struct commit *commit = pop_commit(&list);
>  		if (!(commit->object.flags & STALE))
> -			commit_list_insert_by_date(commit, &result);
> +			commit_list_insert_by_date(commit, result);
>  	}
> -	return result;
> +	return 0;
>  }
>  
>  struct commit_list *get_octopus_merge_bases(struct commit_list *in)
> @@ -410,10 +418,11 @@ static struct commit_list *get_merge_bases_many_0(struct repository *r,
>  {
>  	struct commit_list *list;
>  	struct commit **rslt;
> -	struct commit_list *result;
> +	struct commit_list *result = NULL;
>  	int cnt, i;
>  
> -	result = merge_bases_many(r, one, n, twos);
> +	if (merge_bases_many(r, one, n, twos, &result) < 0)
> +		return NULL;
>  	for (i = 0; i < n; i++) {
>  		if (one == twos[i])
>  			return result;

^ permalink raw reply

* Re: [PATCH v3 05/11] commit-reach: start reporting errors in `paint_down_to_common()`
From: Junio C Hamano @ 2024-02-27 18:24 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Dirk Gouders, Johannes Schindelin via GitGitGadget, git,
	Patrick Steinhardt
In-Reply-To: <fa8ebbfd-e01f-fbde-c851-54c162b610ff@gmx.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Is it lazy to omit the `< 0` here? Not actually, the reason why I omitted
> it here was to stay under 80 columns per line.
>
> Good eyes, though. If `paint_down_to_common()` _did_ return values other
> than -1 and 0, in particular positive ones that would not indicate a fatal
> error, the hunk under discussion would have introduced a problematic bug.

The same patch does compare the returned value with '< 0' in another
function (that is far from this place), which probably made the hunk
stand out during a review, I suspect.

After having fixed a bug elsewhere about a codepath that mixed the
"non-zero is an error" and "negative is an error" conventions during
the last cycle, I would have to say that being consistent would be
nice.  I think we at the end decided to make the callee be more
strict (returning negative to signal an error) while allowing the
caller to be more lenient (taking non-zero as an error) in that
case.

Thanks.


^ permalink raw reply

* Re: [PATCH v3 04/11] Prepare `paint_down_to_common()` for handling shallow commits
From: Junio C Hamano @ 2024-02-27 18:10 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Dirk Gouders, Johannes Schindelin via GitGitGadget, git,
	Patrick Steinhardt
In-Reply-To: <xmqqcyshu6es.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

> I suspect that what made it harder to follow in the original
> construct is that we called the behaviour "incorrect" upfront and
> then come back with "that incorrectness is what we want".

Oh, I forgot to say that lack of "<area>: " on the title of the
earlier parts of the series were a bit uncomfortable to read.



^ permalink raw reply

* Re: [PATCH v2] completion: fix __git_complete_worktree_paths
From: Rubén Justo @ 2024-02-27 18:09 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: Git List
In-Reply-To: <Zd2bnJZtgW2tFMFn@tanuki>

On Tue, Feb 27, 2024 at 09:21:48AM +0100, Patrick Steinhardt wrote:

> > +test_expect_success '__git_complete_worktree_paths - not a git repository' '
> > +	(
> > +		cd non-repo &&
> > +		GIT_CEILING_DIRECTORIES="$ROOT" &&
> > +		export GIT_CEILING_DIRECTORIES &&
> > +		test_completion "git worktree remove " "" 2>err &&
> > +		test_must_be_empty err
> > +	)
> > +'
> 
> If I understand correctly, we assume that the repo isn't detected here,
> and thus we will fail to complete the command. We don't want an error
> message though, which we assert.

Correct.

> But do we also want to assert that
> there is no output on stdout?

To me, the check makes sense;  to notice if we leak a message in such a
circumstance, for instance.  I can drop it if you think it does not add
value.

The test for stderr is my main goal here.

> 
> > +
> > +test_expect_success '__git_complete_worktree_paths with -C' '
> > +	test_when_finished "rm -rf to_delete" &&
> 
> What does this delete? I don't see "to_delete" being created as part of
> this test.

Good eyes.  It's noise.  I'll drop this line.  Thanks.

> 
> > +	git -C otherrepo worktree add --orphan otherrepo_wt &&
> > +	run_completion "git -C otherrepo worktree remove " &&
> > +	grep otherrepo_wt out
> 
> And as far as I can see, we don't write to "out" in this test, either.
> So I think we're accidentally relying on state by the first test here.

The function run_completion leaves the result of the completion in the
file "out".  So we're checking here if "otherrepo_wt" is present in what
"git -C otherrepo worktree remove <TAB>" returns.

Maybe a new function: grep_completion, similar to test_completion, could
make this clearer?

> 
> Patrick

Thanks.

> 
> > +'
> > +
> >  test_expect_success 'git switch - with no options, complete local branches and unique remote branch names for DWIM logic' ' ow
> >  	test_completion "git switch " <<-\EOF
> >  	branch-in-other Z
> > -- 
> > 2.44.0.1.g0da3aa8f7f
> > 



^ permalink raw reply

* Re: [PATCH v3 04/11] Prepare `paint_down_to_common()` for handling shallow commits
From: Junio C Hamano @ 2024-02-27 18:08 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Dirk Gouders, Johannes Schindelin via GitGitGadget, git,
	Patrick Steinhardt
In-Reply-To: <79914d16-f58d-7ab0-5c25-f29870a73402@gmx.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> 	Currently, that logic pretends that a commit whose parent
> 	commit is missing is a root commit (and likewise merge commits
> 	with missing parent commits are handled incorrectly, too).
> 	However, for the purpose of `--update-shallow` that is exactly
> 	what we need to do (and only then).

I suspect that what made it harder to follow in the original
construct is that we called the behaviour "incorrect" upfront and
then come back with "that incorrectness is what we want".  I wonder
if it makes it easier to follow by flipping them around.

    For the purpose of `--update-shallow`, when some of the parent
    commits of a commit are missing from the repository, we need to
    treat as if the parents of the commit are only the ones that do
    exist in the repository and these missing commits have no
    ancestry relationship with it.  If all its parent commits are
    missing, the commit needs to be treated as if it were a root
    commit.

    Add a flag to optionally ask for such a behaviour, while
    detecting missing objects as a repository corruption error by
    default ...

or something?

> 	Therefore [...]
>
> Better?
>
> Ciao,
> Johannes

^ permalink raw reply

* Re: [PATCH 1/2] mem-pool: add mem_pool_strfmt()
From: René Scharfe @ 2024-02-27 17:58 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20240227075350.GE3263678@coredump.intra.peff.net>

Am 27.02.24 um 08:53 schrieb Jeff King:
> On Mon, Feb 26, 2024 at 07:17:00PM +0100, René Scharfe wrote:
>
>>> This is pulling heavily from strbuf_vaddf(). This might be a dumb idea,
>>> but... would it be reasonable to instead push a global flag that causes
>>> xmalloc() to use a memory pool instead of the regular heap?
>>>
>>> Then you could do something like:
>>>
>>>   push_mem_pool(pool);
>>>   str = xstrfmt("%.*s~%d^%d", ...etc...);
>>>   pop_mem_pool(pool);
>>>
>>> It's a little more involved at the caller, but it means that it now
>>> works for all allocations, not just this one string helper.
>>
>> That would allow to keep track of allocations that would otherwise leak.
>> We can achieve that more easily by pushing the pointer to a global array
>> and never freeing it.  Hmm.
>
> I see two uses for memory pools:
>
>   1. You want to optimize allocations (either locality or per-allocation
>      overhead).
>
>   2. You want to make a bunch of allocations with the same lifetime
>      without worrying about their lifetime otherwise. And then you can
>      free them all in one swoop at the end.
>
> And my impression is that you were interested in (2) here. If what
> you're saying is that another way to do that is:
>
>   str = xstrfmt(...whatever...);
>   attach_to_pool(pool, str);
>
>   ...much later...
>   free_pool(pool);
>
> then yeah, I'd agree. And that is a lot less tricky / invasive than what
> I suggested.
>
>> It would not allow the shortcut of using the vast pool as a scratch
>> space to format the string with a single vsnprintf call in most cases.
>> Or am I missing something?
>
> So here it sounds like you do care about some of the performance
> aspects. So no, it would not allow that. You'd be using the vast pool of
> heap memory provided by malloc(), and trusting that a call to malloc()
> is not that expensive in practice. I don't know how true that is, or how
> much it matters for this case.

In the specific use case we can look at three cases:

1. xstrfmt() [before 1c56fc2084]
2. size calculation + pre-size + strbuf_addf() [1c56fc2084]
3. mem_pool_strfmt() [this patch]

The performance of 2 and 3 is basically the same, 1 was worse.

xstrfmt() and strbuf_addf() both wrap strbuf_vaddf(), which uses
malloc() and vsnprintf().  My conclusion is that malloc() is fast
enough, but running vsnprintf() twice is slow (first time to determine
the allocation size, second time to actually build the string).  With a
memory pool we almost always only need to call it once per string, and
that's why I use it here.

The benefit of this patch (to me) is better code readability (no more
manual pre-sizing) without sacrificing performance.

The ability to clear (or UNLEAK) all these strings in one go is just a
bonus.

René

^ permalink raw reply

* Re: [PATCH v2 1/2] commit: Avoid redundant scissor line with --cleanup=scissors -v
From: Junio C Hamano @ 2024-02-27 17:43 UTC (permalink / raw)
  To: Josh Triplett; +Cc: git
In-Reply-To: <4f97933f173220544a5be2bf05c2bee2b044d2b1.1709024540.git.josh@joshtriplett.org>

Josh Triplett <josh@joshtriplett.org> writes:

> diff --git a/wt-status.c b/wt-status.c
> index ea13f5d8db..2d576f7a44 100644

I do not seem to have the preimage ea13f5d8db; as a bugfix patch, it
would be preferrable to make the patches not to depend on anything
in flight.  If feasible, it may even be nicer to base them on one of
the maintenance tracks.

I managed to wiggle the patch in (somehow a context line was
misindented), so there hopefully is no need to resend.

Thanks.

^ permalink raw reply

* Re: [PATCH v2 8/8] cherry-pick: add `--empty` for more robust redundant commit handling
From: Junio C Hamano @ 2024-02-27 17:33 UTC (permalink / raw)
  To: phillip.wood123; +Cc: Brian Lyles, phillip.wood, git, newren, me
In-Reply-To: <1004c565-ee6c-4aa4-8226-47d0ef7c8631@gmail.com>

phillip.wood123@gmail.com writes:

>>>> I do not quite see a good reason to do the opposite, dropping
>>>> commits that started out as empty but keeping the ones that have
>>>> become empty.  Such a behaviour has additional downside that after
>>>> such a cherry-pick, when you cherry-pick the resulting history onto
>>>> yet another base, your precious "were not empty but have become so
>>>> during the initial cherry-pick" commits will appear as commits that
>>>> were empty from the start.  So I do not see much reason to allow the
>>>> decoupling, even with the new "empty=keep" thing that does not imply
>>>> "allow-empty".
>>>
>>> Junio -- can you clarify this part?
>>>
>>>> So I do not see much reason to allow the decoupling, even with the new
>>>> "empty=keep" thing that does not imply "allow-empty"
>>>
>>> I'm not 100% sure if you are saying that you want `--empty=keep` to
>>> *also* imply `--allow-empty`, or that you simply want
>>> `--keep-redundant-commits` to continue implying `--allow-empty`
>>> *despite* the new `--empty=keep` no implying the same.
>
> FWIW I read it as the latter, but I can't claim to know what was in
> Junio's mind when he wrote it.

Given that "drop what was empty originally while keeping what became
empty" would "lose" what it wanted to keep (i.e. the one that has
become empty") when used to cherry-pick the result of doing such a
cherry-pick, I do not think allowing such combination makes as much
sense as the opposite "keep what was empty originally while dropping
what became empty", which does not have such a property.

And it does not matter if that is expressed via the combination of
existing --allow-empty and --keep-redundant-commits options, or the
newly proposed --empty=keep option.  If we start allowing the "drop
what was originally empty and keep what has become empty"
combination if we make empty=keep not to imply --allow-empty, I do
not think it is a good idea.

That was what was on my mind when I wrote it.  It may be that I was
not following the discussion correctly, and making "empty=keep" not
to imply "allow-empty" does *not* allow a request to "drop what was
originally empty, keep what has become empty".  If that is the case,
then I have no objection to making "empty=keep" not to imply
"allow-empty".

Thanks.

^ permalink raw reply

* Re: [PATCH] commit: Avoid redundant scissor line with --cleanup=scissors -v
From: Junio C Hamano @ 2024-02-27 17:10 UTC (permalink / raw)
  To: Josh Triplett; +Cc: git
In-Reply-To: <Zd2eLxPelxvP8FDk@localhost>

Josh Triplett <josh@joshtriplett.org> writes:

> I could add statefulness to wt_status_add_cut_line instead, on the
> assumption that it's the only thing that should be adding a cut line,
> and having it not add the line if previously added. For instance, it
> could accept a pointer to the full wt_status rather than just the fp,
> and keep a boolean state there.

Yeah, that approach also has to assume that wt_status structure is
used only once to create a single message buffer without being
reused, but I think that is a safe assumption, too.  The function
being the only thing that adds the scissors line should also be a
safe assumption in code hygiene standpoint---if somebody else tries
to manually write such a line, we'll shoot such a patch down and
tell them to call this function anyway ;-).

> I did run the testsuite, and it passed. I can add a simple test easily
> enough.

It would be prudent to do so.

Thanks.


^ permalink raw reply

* Bug: merge-driver not executed with ort and merge-tree
From: André Frimberger @ 2024-02-27 17:04 UTC (permalink / raw)
  To: git@vger.kernel.org; +Cc: Andreas.Borkenhagen@dwpbank.de

Hi Git community,

we’re using a merge driver to automatically solve predictable merge conflicts in pom.xml files.

In our use-case Git is executed by Bitbucket. With the update of Bitbucket from 7.x to 8.x and Git from 2.23 to 2.43.2 the merge driver
integration breaks.

Bitbucket internally calls “merge-tree” on a Git repo without working directory (dirty details below). While digging down the rabbit hole we found
the following two bugs / inconsistencies:

* The "new" default merge strategy ort implements its own lookup logic for the .gitattributes file and ignores HEAD:.gitattributes.
  This behaviour is inconsistent to the default behaviour "-s recursive" prior to 2.34.
* merge-tree ignores HEAD:.gitattributes.

*Expected behavior:* merge-tree and “ort” respect HEAD:.gitattributes


Steps to reproduce (aligned with the calls from Bitbucket, so please don’t blame me for the calls 😊):
You’ll find a demo repo and the instructions in https://github.com/afrimberger/mergedriver-test/blob/main/README.md

------------------------------------------------------------------------------------------------------------------------
```shell
$ git --version
git version 2.43.2
```

Clone this repo as bare repo to simulate Bitbucket's local repository storage

```shell
$ cd /tmp
$ git clone --bare https://github.com/afrimberger/mergedriver-test.git mergedriver-test-bare
```

Copy and activate the test merge driver
```shell
$ cd /tmp/mergedriver-test-bare
$ git show HEAD:custom-merge-driver.sh > /tmp/custom-merge-driver.sh
$ chmod 777 /tmp/custom-merge-driver.sh
$ /tmp/custom-merge-driver.sh
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
Merge Driver is active
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
```

Define the merge driver settings in one of the "global" Git configs
```shell
$ cat <<EOF >> ~/.gitconfig
[user]
  name = John Doe
  email = john@doe.com
[merge "keeplocalversion"]
  name = "Keep local pom version"
  driver = /tmp/custom-merge-driver.sh
EOF
```

Create temporary Git repo to prevent changes in bare repo

```shell
$ mkdir /tmp/mergedriver-test
$ cd /tmp/mergedriver-test
$ git init
Initialized empty Git repository in /tmp/mergedriver-test/.git/
```

Reference objects from bare repo
```shell
$ echo "/tmp/mergedriver-test-bare/objects" > ".git/objects/info/alternates"
```

The working directory is empty
```shell
$ ls -al
total 12
drwxr-xr-x 3 root root 4096 Feb 23 18:10 .
drwxrwxrwt 1 root root 4096 Feb 23 18:10 ..
drwxr-xr-x 8 root root 4096 Feb 23 18:10 .git
```

Prepare merge

```shell
$ git reset 4814fce142b95b910d0cbe87c3c304b15f279a0a –
Unstaged changes after reset:
D             .gitattributes
D             .gitconfig-example
D             custom-merge-driver.sh
D             pom.xml
```


working directory is empty, but `HEAD:.gitattributes` exists
```shell
$ git show HEAD:.gitattributes
* merge=keeplocalversion
```

Execute `git merge-tree`. The merge driver isn't executed even though `HEAD:.gitattributes` exists.
```shell
$ git merge-tree --write-tree --allow-unrelated-histories --messages 561da86fa3990aa712aa3ec6681dfafd50def3a0 4814fce142b95b910d0cbe87c3c304b15f279a0a
8c1c8d51a82b07712c8e64109717f615a5e4a81e
100644 ded65fd3b11aa6a0b82f6ab170eaee5411c5ce2e 1            pom.xml
100644 29418c82b95793ee6b688bf6761ee53d8f42c1c5 2            pom.xml
100644 9bc48d5ca4b62d32cebf09f678148c8ef35b0820 3             pom.xml

Auto-merging pom.xml
CONFLICT (content): Merge conflict in pom.xml
```

**Expected Behaviour**: merge driver is executed

Proof merge driver is really active (Note: _ort_ doesn't respect `HEAD:.gitattributes`, but _recursive_ does)

```shell
$ git merge -s recursive -m "Automatic merge" --no-ff --no-log --allow-unrelated-histories --no-verify 561da86fa3990aa712aa3ec6681dfafd50def3a0 4814fce142b95b910d0cbe87c3c304b15f279a0a
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
Merge Driver is active
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
Auto-merging pom.xml
Merge made by the 'recursive' strategy.
```

Use "new" default merge strategy _ort_. Changed with Git 2.34 (breaks backwards compatibility).
_ort_ **doesn't** execute the merge driver

```shell
$ git merge -m 'Automatic merge' --no-ff --no-log --allow-unrelated-histories --no-verify 561da86fa3990aa712aa3ec6681dfafd50def3a0 4814fce142b95b910d0cbe87c3c304b15f279a0a
Auto-merging pom.xml
CONFLICT (content): Merge conflict in pom.xml
Automatic merge failed; fix conflicts and then commit the result.
```
------------------------------------------------------------------------------------------------------------------------

Kind Regards, André

Diese E-Mail enthält vertrauliche und/oder rechtlich geschützte Informationen. Wenn Sie nicht der richtige Adressat sind oder diese E-Mail irrtümlich erhalten haben, informieren Sie bitte sofort den Absender und vernichten Sie diese E-Mail. Das unerlaubte Kopieren sowie die unbefugte Weitergabe dieser Mail ist nicht gestattet.
This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and destroy this e-mail. Any unauthorized copying, disclosure or distribution of the material in this e-mail is strictly forbidden.

^ permalink raw reply

* Re: [PATCH 3/3] read_ref_at(): special-case ref@{0} for an empty reflog
From: Junio C Hamano @ 2024-02-27 17:03 UTC (permalink / raw)
  To: Jeff King; +Cc: Patrick Steinhardt, Yasushi SHOJI, Denton Liu, Git Mailing List
In-Reply-To: <20240227080501.GF3263678@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Let me try to lay out my thinking. If you _do_ have a reflog and the
> request (whether time or count-based) goes too far back, read_ref_at()
> will give you the oldest entry and return "1". And then in
> get_oid_basic():
> ...
>   - what we _could_ do (but this series does not), and what would be the
>     true counterpart to the @{20.years.ago} case, is to allow @{9999}
>     for a reflog with only 20 entries, returning the old value from 20
>     (or the new value if it was a creation!?) and issuing a warning
>     saying "well, it only went back 20, but here you go".

Ah, I wasn't drawing *that* similarity.  My thinking was more like

 - When you have two entries in reflog, ref@{0} will use and find
   the latest entry whose value is the same as the ref itself.

 - When you have one entry, @{0} will use and find the latest entry
   whose value is the same as the ref itself.

 - When you have zero entry, @{0} can do the same by taking
   advantage of the fact that its value is supposed to be the same
   as the ref itself anyway.

that happens near the youngest end of a reflog, contrasting with the
@{20.years.ago} that happens near the oldest end.

> I'm not so sure about that last one. It is pretty subjective, but
> somehow asking for timestamps feels more "fuzzy" to me, and Git
> returning a fuzzy answer is OK. Whereas asking for item 9999 in a list
> with 20 items and getting back an answer feels more absolutely wrong. I
> could be persuaded if there were a concrete use case, but I can't really
> think of one. It seems more likely to confuse and hinder a user than to
> help them.

I do not think anybody misses @{9999} not giving the oldest
available, simply because "oldest" is a concept that fits better
with time-based queries than count-based queries.

^ permalink raw reply

* Re: [PATCH v5 0/5] for-each-ref: add '--include-root-refs' option
From: Junio C Hamano @ 2024-02-27 16:54 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: Karthik Nayak, git, phillip.wood123
In-Reply-To: <Zd2Ru7LWYyGprvcr@tanuki>

Patrick Steinhardt <ps@pks.im> writes:

>> Changes from v4:
>> 1. Fixed erratic whitespace
>> 2. Remove braces from single line block
>> 3. Starting the comments with a capital and also adding more context.
>> 4. Removed a duplicate check.
>> 
>> Thanks for the reviews.
>> 
>> Range diff against v4:
>
> The range-diff looks as expected, so this patch series looks good to me.
> Thanks!
>
> Patrick

Thanks, let's mark the topic for 'next'.

^ permalink raw reply

* Re: [PATCH] rebase: fix typo in autosquash documentation
From: Junio C Hamano @ 2024-02-27 16:50 UTC (permalink / raw)
  To: Richard Macklin via GitGitGadget; +Cc: git, Richard Macklin, Andy Koppe
In-Reply-To: <pull.1676.git.1709015578890.gitgitgadget@gmail.com>

"Richard Macklin via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Richard Macklin <code@rmacklin.dev>
>
> This is a minor follow-up to cb00f524df (rebase: rewrite
> --(no-)autosquash documentation, 2023-11-14) to fix a typo introduced in
> that commit.
>
> Signed-off-by: Richard Macklin <code@rmacklin.dev>
> ---

[jc: andy cc'ed as the original author to give him a chance to give
his Acked-by]

>  Documentation/git-rebase.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 06206521fc3..e7e725044db 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -607,7 +607,7 @@ The recommended way to create commits with squash markers is by using the
>  linkgit:git-commit[1], which take the target commit as an argument and
>  automatically fill in the subject line of the new commit from that.
>  +
> -Settting configuration variable `rebase.autoSquash` to true enables
> +Setting configuration variable `rebase.autoSquash` to true enables

Good eyes.  Thanks for fixing.

>  auto-squashing by default for interactive rebase.  The `--no-autosquash`
>  option can be used to override that setting.
>  +
>
> base-commit: 3c2a3fdc388747b9eaf4a4a4f2035c1c9ddb26d0

^ permalink raw reply

* Re: [PATCH 03/12] reftable/merged: advance subiter on subsequent iteration
From: Patrick Steinhardt @ 2024-02-27 16:50 UTC (permalink / raw)
  To: git
In-Reply-To: <ns2yw3icnl3udejbgsv4ojwgzbe7eg57bvsm53kciuemlvmgbr@mpjtogpzdavi>

[-- Attachment #1: Type: text/plain, Size: 5191 bytes --]

On Tue, Feb 20, 2024 at 12:25:10PM -0600, Justin Tobler wrote:
> On 24/02/14 08:45AM, Patrick Steinhardt wrote:
> > When advancing the merged iterator, we pop the top-most entry from its
> 
> s/top-most/topmost
> 
> > priority queue and then advance the sub-iterator that the entry belongs
> > to, adding the result as a new entry. This is quite sensible in the case
> > where the merged iterator is used to actual iterate through records. But
> 
> s/actual/actually
> 
> > the merged iterator is also used when we look up a single record, only,
> > so advancing the sub-iterator is wasted effort because we would never
> > even look at the result.
> > 
> > Instead of immediately advancing the sub-iterator, we can also defer
> > this to the next iteration of the merged iterator by storing the
> > intent-to-advance. This results in a small speedup when reading many
> > records. The following benchmark creates 10000 refs, which will also end
> > up with many ref lookups:
> > 
> >     Benchmark 1: update-ref: create many refs (revision = HEAD~)
> >       Time (mean ± σ):     337.2 ms ±   7.3 ms    [User: 200.1 ms, System: 136.9 ms]
> >       Range (min … max):   329.3 ms … 373.2 ms    100 runs
> > 
> >     Benchmark 2: update-ref: create many refs (revision = HEAD)
> >       Time (mean ± σ):     332.5 ms ±   5.9 ms    [User: 197.2 ms, System: 135.1 ms]
> >       Range (min … max):   327.6 ms … 359.8 ms    100 runs
> > 
> >     Summary
> >       update-ref: create many refs (revision = HEAD) ran
> >         1.01 ± 0.03 times faster than update-ref: create many refs (revision = HEAD~)
> > 
> > While this speedup alone isn't really worth it, this refactoring will
> > also allow two additional optimizations in subsequent patches. First, it
> > will allow us to special-case when there is only a single sub-iter left
> > to circumvent the priority queue altogether. And second, it makes it
> > easier to avoid copying records to the caller.
> > 
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> >  reftable/merged.c | 26 ++++++++++++--------------
> >  1 file changed, 12 insertions(+), 14 deletions(-)
> > 
> > diff --git a/reftable/merged.c b/reftable/merged.c
> > index 12ebd732e8..9b1ccfff00 100644
> > --- a/reftable/merged.c
> > +++ b/reftable/merged.c
> > @@ -19,11 +19,12 @@ license that can be found in the LICENSE file or at
> >  
> >  struct merged_iter {
> >  	struct reftable_iterator *stack;
> > +	struct merged_iter_pqueue pq;
> >  	uint32_t hash_id;
> >  	size_t stack_len;
> >  	uint8_t typ;
> >  	int suppress_deletions;
> > -	struct merged_iter_pqueue pq;
> > +	ssize_t advance_index;
> >  };
> >  
> >  static int merged_iter_init(struct merged_iter *mi)
> > @@ -96,13 +97,17 @@ static int merged_iter_next_entry(struct merged_iter *mi,
> >  	struct pq_entry entry = { 0 };
> >  	int err = 0;
> >  
> > +	if (mi->advance_index >= 0) {
> > +		err = merged_iter_advance_subiter(mi, mi->advance_index);
> > +		if (err < 0)
> > +			return err;
> > +		mi->advance_index = -1;
> > +	}
> > +
> 
> Without additional context, it isn't immediately clear to me why the
> sub-iterator is condionally advanced at the beginning. Maybe a comment
> could be added to explain as done in the commit message to help with
> clarity?

I tried to mention this in the commit message with the last paragraph.
Adding a comment doesn't make much sense at this point in the patch
seires because a later patch changes how this works.

> >  	if (merged_iter_pqueue_is_empty(mi->pq))
> >  		return 1;
> >  
> >  	entry = merged_iter_pqueue_remove(&mi->pq);
> > -	err = merged_iter_advance_subiter(mi, entry.index);
> > -	if (err < 0)
> > -		return err;
> >  
> >  	/*
> >  	  One can also use reftable as datacenter-local storage, where the ref
> > @@ -116,14 +121,6 @@ static int merged_iter_next_entry(struct merged_iter *mi,
> >  		struct pq_entry top = merged_iter_pqueue_top(mi->pq);
> >  		int cmp;
> >  
> > -		/*
> > -		 * When the next entry comes from the same queue as the current
> > -		 * entry then it must by definition be larger. This avoids a
> > -		 * comparison in the most common case.
> > -		 */
> > -		if (top.index == entry.index)
> > -			break;
> > -
> 
> I'm not quite sure I follow by the above check is removed as part of
> this change. Would you mind clarifying?

The loop that this comparison has been part of was popping all entries
from the priority queue that are being shadowed by the sub-iterator from
which we're about to return the entry. So e.g. in the case of a ref
record, we discard all records with the same refname which are shadowed
by a newer (higher update-index) table.

The removed condition was an optimization was a micro-optimization: when
the next entry in the pqueue is from the same index as the entry we are
about to return, then we know that it cannot have been shadowed. This
allowed us to avoid a key comparison.

But with the change in this commit we don't even add the next record of
the current sub-iter to the pqueue, and thus the condition cannot happen
anymore.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH 02/12] reftable/merged: make `merged_iter` structure private
From: Patrick Steinhardt @ 2024-02-27 16:49 UTC (permalink / raw)
  To: git
In-Reply-To: <qprofop624nevbicid4rplfqlanfrujyxiilfqwchrppnmw7u4@3dvw5udjtrah>

[-- Attachment #1: Type: text/plain, Size: 2036 bytes --]

On Tue, Feb 20, 2024 at 12:15:23PM -0600, Justin Tobler wrote:
> On 24/02/14 08:45AM, Patrick Steinhardt wrote:
> > The `merged_iter` structure is not used anywhere outside of "merged.c",
> > but is declared in its header. Move it into the code file so that it is
> > clear that its implementation details are never exposed to anything.
> > 
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> >  reftable/merged.c | 9 +++++++++
> >  reftable/merged.h | 9 ---------
> >  2 files changed, 9 insertions(+), 9 deletions(-)
> > 
> > diff --git a/reftable/merged.c b/reftable/merged.c
> > index 1aa6cd31b7..12ebd732e8 100644
> > --- a/reftable/merged.c
> > +++ b/reftable/merged.c
> > @@ -17,6 +17,15 @@ license that can be found in the LICENSE file or at
> >  #include "reftable-error.h"
> >  #include "system.h"
> >  
> 
> suggestion: I think it would be nice to document a little about the
> merge iterator here at a high-level. Maybe just to explain that this
> allows iteration over multiple tables as if it were a single table.

Agreed. I have planned to invest more time into documenting the reftable
library overall, but would rather want to push this out to another patch
series.

> > +struct merged_iter {
> > +	struct reftable_iterator *stack;
> > +	uint32_t hash_id;
> > +	size_t stack_len;
> > +	uint8_t typ;
> > +	int suppress_deletions;
> > +	struct merged_iter_pqueue pq;
> > +};
> > +
> >  static int merged_iter_init(struct merged_iter *mi)
> >  {
> >  	for (size_t i = 0; i < mi->stack_len; i++) {
> > diff --git a/reftable/merged.h b/reftable/merged.h
> > index 7d9f95d27e..288ad66656 100644
> > --- a/reftable/merged.h
> > +++ b/reftable/merged.h
> > @@ -24,15 +24,6 @@ struct reftable_merged_table {
> >  	uint64_t max;
> >  };
> >  
> 
> Since we are removing `merge_iter` from the header here, I think we can
> also remove the `#include "pg.h"`.

Good catch! We can replace it with "system.h", which makes sure to
include <git-compat-util.h>.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH] git: extend --no-lazy-fetch to work across subprocesses
From: Junio C Hamano @ 2024-02-27 16:48 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Christian Couder
In-Reply-To: <20240227074903.GD3263678@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> There's some prior art there, I think, in how GIT_CEILING_DIRECTORIES
> works, or even something like "git --literal-pathspecs", neither of
> which appear in local_repo_env.

Unlike GIT_CEILING_DIRECTORIES that is more or less permanent and a
part of configuring an everyday environment for real work, I view
this --no-lazy-fetch thing as solely a debugging aid.  A repository
with promisor wouldn't be able to "fill the gap" by forbidding
on-demand fetching of promised objects while running say "git fetch"
from elsewhere and it just will die with "some objects we are
supposed to have are missing from this repository".

So I do not have a strong opinion either way, if it is more
convenient to propagate the request out to other repositories when
we run processes in two or more repositories (e.g. "git clone
--local"), or if it is more convenient to make sure that the request
is limited to the target repository.  Here is a version without the
local_repo_env[] change.

----- >8 --------- >8 --------- >8 --------- >8 --------- >8 -----
Subject: [PATCH v3 3/3] git: extend --no-lazy-fetch to work across subprocesses

Modeling after how the `--no-replace-objects` option is made usable
across subprocess spawning (e.g., cURL based remote helpers are
spawned as a separate process while running "git fetch"), allow the
`--no-lazy-fetch` option to be passed across process boundaries.

Do not model how the value of GIT_NO_REPLACE_OBJECTS environment
variable is ignored, though.  Just use the usual git_env_bool() to
allow "export GIT_NO_LAZY_FETCH=0" and "unset GIT_NO_LAZY_FETCH"
to be equivalents.

Also do not model how the request is not propagated to subprocesses
we spawn (e.g. "git clone --local" that spawns a new process to work
in the origin repository, while the original one working in the
newly created one) by the "--no-replace-objects" option, as this "do
not lazily fetch from the promisor" is more about a per-request
debugging aid, not "this repository's promisor should not be relied
upon" property specific to a repository.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Range-diff:
1:  5fa7ddbf68 ! 1:  ea40bb9a1d git: extend --no-lazy-fetch to work across subprocesses
    @@ Commit message
         allow "export GIT_NO_LAZY_FETCH=0" and "unset GIT_NO_LAZY_FETCH"
         to be equivalents.
     
    +    Also do not model how the request is not propagated to subprocesses
    +    we spawn (e.g. "git clone --local" that spawns a new process to work
    +    in the origin repository, while the original one working in the
    +    newly created one) by the "--no-replace-objects" option, as this "do
    +    not lazily fetch from the promisor" is more about a per-request
    +    debugging aid, not "this repository's promisor should not be relied
    +    upon" property specific to a repository.
    +
         Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## Documentation/git.txt ##
    @@ Documentation/git.txt: for full details.
      	track of the reason why the ref was updated (which is
     
      ## environment.c ##
    -@@ environment.c: const char * const local_repo_env[] = {
    - 	GRAFT_ENVIRONMENT,
    - 	INDEX_ENVIRONMENT,
    - 	NO_REPLACE_OBJECTS_ENVIRONMENT,
    -+	NO_LAZY_FETCH_ENVIRONMENT,
    - 	GIT_REPLACE_REF_BASE_ENVIRONMENT,
    - 	GIT_PREFIX_ENVIRONMENT,
    - 	GIT_SHALLOW_FILE_ENVIRONMENT,
     @@ environment.c: void setup_git_env(const char *git_dir)
      	shallow_file = getenv(GIT_SHALLOW_FILE_ENVIRONMENT);
      	if (shallow_file)

 Documentation/git.txt    |  7 +++++++
 environment.c            |  3 +++
 environment.h            |  1 +
 git.c                    |  3 +++
 t/t0410-partial-clone.sh | 15 +++++++++++++++
 5 files changed, 29 insertions(+)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index b1f754e84b..a517d94a7a 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -183,6 +183,8 @@ If you just want to run git as if it was started in `<path>` then use
 	Do not fetch missing objects from the promisor remote on
 	demand.  Useful together with `git cat-file -e <object>` to
 	see if the object is locally available.
+	This is equivalent to setting the `GIT_NO_LAZY_FETCH`
+	environment variable to `1`.
 
 --literal-pathspecs::
 	Treat pathspecs literally (i.e. no globbing, no pathspec magic).
@@ -900,6 +902,11 @@ for full details.
 	Setting this Boolean environment variable to true will cause Git to treat all
 	pathspecs as case-insensitive.
 
+`GIT_NO_LAZY_FETCH`::
+	Setting this Boolean environment variable to true tells Git
+	not to lazily fetch missing objects from the promisor remote
+	on demand.
+
 `GIT_REFLOG_ACTION`::
 	When a ref is updated, reflog entries are created to keep
 	track of the reason why the ref was updated (which is
diff --git a/environment.c b/environment.c
index 9e37bf58c0..0ae5bbd4a1 100644
--- a/environment.c
+++ b/environment.c
@@ -207,6 +207,9 @@ void setup_git_env(const char *git_dir)
 	shallow_file = getenv(GIT_SHALLOW_FILE_ENVIRONMENT);
 	if (shallow_file)
 		set_alternate_shallow_file(the_repository, shallow_file, 0);
+
+	if (git_env_bool(NO_LAZY_FETCH_ENVIRONMENT, 0))
+		fetch_if_missing = 0;
 }
 
 int is_bare_repository(void)
diff --git a/environment.h b/environment.h
index e5351c9dd9..5cec19cecc 100644
--- a/environment.h
+++ b/environment.h
@@ -36,6 +36,7 @@ const char *getenv_safe(struct strvec *argv, const char *name);
 #define CEILING_DIRECTORIES_ENVIRONMENT "GIT_CEILING_DIRECTORIES"
 #define NO_REPLACE_OBJECTS_ENVIRONMENT "GIT_NO_REPLACE_OBJECTS"
 #define GIT_REPLACE_REF_BASE_ENVIRONMENT "GIT_REPLACE_REF_BASE"
+#define NO_LAZY_FETCH_ENVIRONMENT "GIT_NO_LAZY_FETCH"
 #define GITATTRIBUTES_FILE ".gitattributes"
 #define INFOATTRIBUTES_FILE "info/attributes"
 #define ATTRIBUTE_MACRO_PREFIX "[attr]"
diff --git a/git.c b/git.c
index 28e8bf7497..d11d4dc77b 100644
--- a/git.c
+++ b/git.c
@@ -189,6 +189,9 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 				*envchanged = 1;
 		} else if (!strcmp(cmd, "--no-lazy-fetch")) {
 			fetch_if_missing = 0;
+			setenv(NO_LAZY_FETCH_ENVIRONMENT, "1", 1);
+			if (envchanged)
+				*envchanged = 1;
 		} else if (!strcmp(cmd, "--no-replace-objects")) {
 			disable_replace_refs();
 			setenv(NO_REPLACE_OBJECTS_ENVIRONMENT, "1", 1);
diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index 5b7bee888d..c282851af7 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -665,6 +665,21 @@ test_expect_success 'lazy-fetch when accessing object not in the_repository' '
 	git -C partial.git rev-list --objects --missing=print HEAD >out &&
 	grep "[?]$FILE_HASH" out &&
 
+	# The no-lazy-fetch mechanism prevents Git from fetching
+	test_must_fail env GIT_NO_LAZY_FETCH=1 \
+		git -C partial.git cat-file -e "$FILE_HASH" &&
+
+	# The same with command line option to "git"
+	test_must_fail git --no-lazy-fetch -C partial.git cat-file -e "$FILE_HASH" &&
+
+	# The same, forcing a subprocess via an alias
+	test_must_fail git --no-lazy-fetch -C partial.git \
+		-c alias.foo="!git cat-file" foo -e "$FILE_HASH" &&
+
+	# Sanity check that the file is still missing
+	git -C partial.git rev-list --objects --missing=print HEAD >out &&
+	grep "[?]$FILE_HASH" out &&
+
 	git -C full cat-file -s "$FILE_HASH" >expect &&
 	test-tool partial-clone object-info partial.git "$FILE_HASH" >actual &&
 	test_cmp expect actual &&
-- 
2.44.0-35-ga2082dbdd3



^ permalink raw reply related

* [GSOC][PATCH v2 1/1] t7301: use test_path_is_(missing|file)
From: Vincenzo Mezzela @ 2024-02-27 16:17 UTC (permalink / raw)
  To: git; +Cc: Vincenzo Mezzela
In-Reply-To: <20240227161734.52830-1-vincenzo.mezzela@gmail.com>

Refactor test -(f|e) to utilize the corresponding helper functions from
test-lib-functions.sh. These functions perform indentical operations
while enhancing debugging capabilities in case of test failures.
  
In the context of this file, 'test ! -f' is meant to check if the file
has been correctly cleaned, thus its usage is replaced with
'test_path_is_missing' instead of '! test_path_is_file'.
 

Signed-off-by: Vincenzo Mezzela <vincenzo.mezzela@gmail.com>
---
 t/t7301-clean-interactive.sh | 490 +++++++++++++++++------------------
 1 file changed, 245 insertions(+), 245 deletions()

diff --git a/t/t7301-clean-interactive.sh b/t/t7301-clean-interactive.sh
index d82a3210a1..4afe53c66a 100755
--- a/t/t7301-clean-interactive.sh
+++ b/t/t7301-clean-interactive.sh
@@ -25,18 +25,18 @@ test_expect_success 'git clean -i (c: clean hotkey)' '
 	touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \
 	docs/manual.txt obj.o build/lib.so &&
 	echo c | git clean -i &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test ! -f a.out &&
-	test -f docs/manual.txt &&
-	test ! -f src/part3.c &&
-	test ! -f src/part3.h &&
-	test ! -f src/part4.c &&
-	test ! -f src/part4.h &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_missing a.out &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_missing src/part3.h &&
+	test_path_is_missing src/part4.c &&
+	test_path_is_missing src/part4.h &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
@@ -46,18 +46,18 @@ test_expect_success 'git clean -i (cl: clean prefix)' '
 	touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \
 	docs/manual.txt obj.o build/lib.so &&
 	echo cl | git clean -i &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test ! -f a.out &&
-	test -f docs/manual.txt &&
-	test ! -f src/part3.c &&
-	test ! -f src/part3.h &&
-	test ! -f src/part4.c &&
-	test ! -f src/part4.h &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_missing a.out &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_missing src/part3.h &&
+	test_path_is_missing src/part4.c &&
+	test_path_is_missing src/part4.h &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
@@ -67,18 +67,18 @@ test_expect_success 'git clean -i (quit)' '
 	touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \
 	docs/manual.txt obj.o build/lib.so &&
 	echo quit | git clean -i &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test -f a.out &&
-	test -f docs/manual.txt &&
-	test -f src/part3.c &&
-	test -f src/part3.h &&
-	test -f src/part4.c &&
-	test -f src/part4.h &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_file a.out &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_file src/part3.c &&
+	test_path_is_file src/part3.h &&
+	test_path_is_file src/part4.c &&
+	test_path_is_file src/part4.h &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
@@ -88,18 +88,18 @@ test_expect_success 'git clean -i (Ctrl+D)' '
 	touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \
 	docs/manual.txt obj.o build/lib.so &&
 	echo "\04" | git clean -i &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test -f a.out &&
-	test -f docs/manual.txt &&
-	test -f src/part3.c &&
-	test -f src/part3.h &&
-	test -f src/part4.c &&
-	test -f src/part4.h &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_file a.out &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_file src/part3.c &&
+	test_path_is_file src/part3.h &&
+	test_path_is_file src/part4.c &&
+	test_path_is_file src/part4.h &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
@@ -110,18 +110,18 @@ test_expect_success 'git clean -id (filter all)' '
 	docs/manual.txt obj.o build/lib.so &&
 	test_write_lines f "*" "" c |
 	git clean -id &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test -f a.out &&
-	test -f docs/manual.txt &&
-	test -f src/part3.c &&
-	test -f src/part3.h &&
-	test -f src/part4.c &&
-	test -f src/part4.h &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_file a.out &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_file src/part3.c &&
+	test_path_is_file src/part3.h &&
+	test_path_is_file src/part4.c &&
+	test_path_is_file src/part4.h &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
@@ -132,18 +132,18 @@ test_expect_success 'git clean -id (filter patterns)' '
 	docs/manual.txt obj.o build/lib.so &&
 	test_write_lines f "part3.* *.out" "" c |
 	git clean -id &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test -f a.out &&
-	test ! -f docs/manual.txt &&
-	test -f src/part3.c &&
-	test -f src/part3.h &&
-	test ! -f src/part4.c &&
-	test ! -f src/part4.h &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_file a.out &&
+	test_path_is_missing docs/manual.txt &&
+	test_path_is_file src/part3.c &&
+	test_path_is_file src/part3.h &&
+	test_path_is_missing src/part4.c &&
+	test_path_is_missing src/part4.h &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
@@ -154,18 +154,18 @@ test_expect_success 'git clean -id (filter patterns 2)' '
 	docs/manual.txt obj.o build/lib.so &&
 	test_write_lines f "* !*.out" "" c |
 	git clean -id &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test ! -f a.out &&
-	test -f docs/manual.txt &&
-	test -f src/part3.c &&
-	test -f src/part3.h &&
-	test -f src/part4.c &&
-	test -f src/part4.h &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_missing a.out &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_file src/part3.c &&
+	test_path_is_file src/part3.h &&
+	test_path_is_file src/part4.c &&
+	test_path_is_file src/part4.h &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
@@ -176,18 +176,18 @@ test_expect_success 'git clean -id (select - all)' '
 	docs/manual.txt obj.o build/lib.so &&
 	test_write_lines s "*" "" c |
 	git clean -id &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test ! -f a.out &&
-	test ! -f docs/manual.txt &&
-	test ! -f src/part3.c &&
-	test ! -f src/part3.h &&
-	test ! -f src/part4.c &&
-	test ! -f src/part4.h &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_missing a.out &&
+	test_path_is_missing docs/manual.txt &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_missing src/part3.h &&
+	test_path_is_missing src/part4.c &&
+	test_path_is_missing src/part4.h &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
@@ -198,18 +198,18 @@ test_expect_success 'git clean -id (select - none)' '
 	docs/manual.txt obj.o build/lib.so &&
 	test_write_lines s "" c |
 	git clean -id &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test -f a.out &&
-	test -f docs/manual.txt &&
-	test -f src/part3.c &&
-	test -f src/part3.h &&
-	test -f src/part4.c &&
-	test -f src/part4.h &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_file a.out &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_file src/part3.c &&
+	test_path_is_file src/part3.h &&
+	test_path_is_file src/part4.c &&
+	test_path_is_file src/part4.h &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
@@ -220,18 +220,18 @@ test_expect_success 'git clean -id (select - number)' '
 	docs/manual.txt obj.o build/lib.so &&
 	test_write_lines s 3 "" c |
 	git clean -id &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test -f a.out &&
-	test -f docs/manual.txt &&
-	test ! -f src/part3.c &&
-	test -f src/part3.h &&
-	test -f src/part4.c &&
-	test -f src/part4.h &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_file a.out &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_file src/part3.h &&
+	test_path_is_file src/part4.c &&
+	test_path_is_file src/part4.h &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
@@ -242,18 +242,18 @@ test_expect_success 'git clean -id (select - number 2)' '
 	docs/manual.txt obj.o build/lib.so &&
 	test_write_lines s "2 3" 5 "" c |
 	git clean -id &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test -f a.out &&
-	test ! -f docs/manual.txt &&
-	test ! -f src/part3.c &&
-	test -f src/part3.h &&
-	test ! -f src/part4.c &&
-	test -f src/part4.h &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_file a.out &&
+	test_path_is_missing docs/manual.txt &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_file src/part3.h &&
+	test_path_is_missing src/part4.c &&
+	test_path_is_file src/part4.h &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
@@ -264,18 +264,18 @@ test_expect_success 'git clean -id (select - number 3)' '
 	docs/manual.txt obj.o build/lib.so &&
 	test_write_lines s "3,4 5" "" c |
 	git clean -id &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test -f a.out &&
-	test -f docs/manual.txt &&
-	test ! -f src/part3.c &&
-	test ! -f src/part3.h &&
-	test ! -f src/part4.c &&
-	test -f src/part4.h &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_file a.out &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_missing src/part3.h &&
+	test_path_is_missing src/part4.c &&
+	test_path_is_file src/part4.h &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
@@ -285,11 +285,11 @@ test_expect_success 'git clean -id (select - filenames)' '
 	touch a.out foo.txt bar.txt baz.txt &&
 	test_write_lines s "a.out fo ba bar" "" c |
 	git clean -id &&
-	test -f Makefile &&
-	test ! -f a.out &&
-	test ! -f foo.txt &&
-	test ! -f bar.txt &&
-	test -f baz.txt &&
+	test_path_is_file Makefile &&
+	test_path_is_missing a.out &&
+	test_path_is_missing foo.txt &&
+	test_path_is_missing bar.txt &&
+	test_path_is_file baz.txt &&
 	rm baz.txt
 
 '
@@ -301,18 +301,18 @@ test_expect_success 'git clean -id (select - range)' '
 	docs/manual.txt obj.o build/lib.so &&
 	test_write_lines s "1,3-4" 2 "" c |
 	git clean -id &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test ! -f a.out &&
-	test ! -f src/part3.c &&
-	test ! -f src/part3.h &&
-	test -f src/part4.c &&
-	test -f src/part4.h &&
-	test ! -f docs/manual.txt &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_missing a.out &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_missing src/part3.h &&
+	test_path_is_file src/part4.c &&
+	test_path_is_file src/part4.h &&
+	test_path_is_missing docs/manual.txt &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
@@ -323,18 +323,18 @@ test_expect_success 'git clean -id (select - range 2)' '
 	docs/manual.txt obj.o build/lib.so &&
 	test_write_lines s "4- 1" "" c |
 	git clean -id &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test ! -f a.out &&
-	test -f docs/manual.txt &&
-	test -f src/part3.c &&
-	test ! -f src/part3.h &&
-	test ! -f src/part4.c &&
-	test ! -f src/part4.h &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_missing a.out &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_file src/part3.c &&
+	test_path_is_missing src/part3.h &&
+	test_path_is_missing src/part4.c &&
+	test_path_is_missing src/part4.h &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
@@ -345,18 +345,18 @@ test_expect_success 'git clean -id (inverse select)' '
 	docs/manual.txt obj.o build/lib.so &&
 	test_write_lines s "*" "-5- 1 -2" "" c |
 	git clean -id &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test ! -f a.out &&
-	test -f docs/manual.txt &&
-	test ! -f src/part3.c &&
-	test ! -f src/part3.h &&
-	test -f src/part4.c &&
-	test -f src/part4.h &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_missing a.out &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_missing src/part3.h &&
+	test_path_is_file src/part4.c &&
+	test_path_is_file src/part4.h &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
@@ -367,18 +367,18 @@ test_expect_success 'git clean -id (ask)' '
 	docs/manual.txt obj.o build/lib.so &&
 	test_write_lines a Y y no yes bad "" |
 	git clean -id &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test ! -f a.out &&
-	test ! -f docs/manual.txt &&
-	test -f src/part3.c &&
-	test ! -f src/part3.h &&
-	test -f src/part4.c &&
-	test -f src/part4.h &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_missing a.out &&
+	test_path_is_missing docs/manual.txt &&
+	test_path_is_file src/part3.c &&
+	test_path_is_missing src/part3.h &&
+	test_path_is_file src/part4.c &&
+	test_path_is_file src/part4.h &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
@@ -389,18 +389,18 @@ test_expect_success 'git clean -id (ask - Ctrl+D)' '
 	docs/manual.txt obj.o build/lib.so &&
 	test_write_lines a Y no yes "\04" |
 	git clean -id &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test ! -f a.out &&
-	test -f docs/manual.txt &&
-	test ! -f src/part3.c &&
-	test -f src/part3.h &&
-	test -f src/part4.c &&
-	test -f src/part4.h &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_missing a.out &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_file src/part3.h &&
+	test_path_is_file src/part4.c &&
+	test_path_is_file src/part4.h &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
@@ -412,18 +412,18 @@ test_expect_success 'git clean -id with prefix and path (filter)' '
 	(cd build/ &&
 	 test_write_lines f docs "*.h" "" c |
 	 git clean -id ..) &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test ! -f a.out &&
-	test -f docs/manual.txt &&
-	test ! -f src/part3.c &&
-	test -f src/part3.h &&
-	test ! -f src/part4.c &&
-	test -f src/part4.h &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_missing a.out &&
+	test_path_is_file docs/manual.txt &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_file src/part3.h &&
+	test_path_is_missing src/part4.c &&
+	test_path_is_file src/part4.h &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
@@ -435,18 +435,18 @@ test_expect_success 'git clean -id with prefix and path (select by name)' '
 	(cd build/ &&
 	 test_write_lines s ../docs/ ../src/part3.c ../src/part4.c "" c |
 	 git clean -id ..) &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test -f a.out &&
-	test ! -f docs/manual.txt &&
-	test ! -f src/part3.c &&
-	test -f src/part3.h &&
-	test ! -f src/part4.c &&
-	test -f src/part4.h &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_file a.out &&
+	test_path_is_missing docs/manual.txt &&
+	test_path_is_missing src/part3.c &&
+	test_path_is_file src/part3.h &&
+	test_path_is_missing src/part4.c &&
+	test_path_is_file src/part4.h &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
@@ -458,18 +458,18 @@ test_expect_success 'git clean -id with prefix and path (ask)' '
 	(cd build/ &&
 	 test_write_lines a Y y no yes bad "" |
 	 git clean -id ..) &&
-	test -f Makefile &&
-	test -f README &&
-	test -f src/part1.c &&
-	test -f src/part2.c &&
-	test ! -f a.out &&
-	test ! -f docs/manual.txt &&
-	test -f src/part3.c &&
-	test ! -f src/part3.h &&
-	test -f src/part4.c &&
-	test -f src/part4.h &&
-	test -f obj.o &&
-	test -f build/lib.so
+	test_path_is_file Makefile &&
+	test_path_is_file README &&
+	test_path_is_file src/part1.c &&
+	test_path_is_file src/part2.c &&
+	test_path_is_missing a.out &&
+	test_path_is_missing docs/manual.txt &&
+	test_path_is_file src/part3.c &&
+	test_path_is_missing src/part3.h &&
+	test_path_is_file src/part4.c &&
+	test_path_is_file src/part4.h &&
+	test_path_is_file obj.o &&
+	test_path_is_file build/lib.so
 
 '
 
-- 
2.34.1


^ permalink raw reply related

* [GSOC][PATCH v2 0/1] microproject: use test_path_is_* functions in test scripts
From: Vincenzo Mezzela @ 2024-02-27 16:17 UTC (permalink / raw)
  To: git; +Cc: Vincenzo Mezzela
In-Reply-To: <20240219172214.7644-1-vincenzo.mezzela@gmail.com>

Hi,
Following previous discussions[1][2], this patch is submitted as a microproject
for the application to the GSOC.

Thanks,
Vincenzo

Changes in V2:
* Fixed commit message[2].

[1] https://lore.kernel.org/git/xmqqy1bo5k5h.fsf@gitster.g/
[2] https://lore.kernel.org/git/20240219172214.7644-1-vincenzo.mezzela@gmail.com/

Vincenzo Mezzela (1):
  t: t7301-clean-interactive: Use test_path_is_(missing|file)

 t/t7301-clean-interactive.sh | 490 +++++++++++++++++------------------
 1 file changed, 245 insertions(+), 245 deletions(-)

-- 
2.34.1


^ permalink raw reply

* [PATCH 2/2] Subject:[GSOC] [RFC PATCH 2/2] Add test for JavaScript function detection in Git diffs
From: Sergius Nyah @ 2024-02-27 16:02 UTC (permalink / raw)
  To: christian.couder, pk; +Cc: git, Sergius Justus Chesami Nyah
In-Reply-To: <20240227160253.104011-1-sergiusnyah@gmail.com>

From: Sergius Justus Chesami Nyah <sergiusnyah@gmail.com>

This commit introduces a new test case in t4018-diff-funcname.sh to verify the enhanced JavaScript function detection in Git diffs. The test creates a JavaScript file with function declarations and expressions, modifies them, and then checks the output of git diff to ensure that the changes are correctly identified. This test validates the changes made to userdiff.c for improved JavaScript function detection.
---
 t/t4018-diff-funcname.sh | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index e026fac1f4..e88e63bd1f 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -11,7 +11,7 @@ test_expect_success 'setup' '
 	# a non-trivial custom pattern
 	git config diff.custom1.funcname "!static
 !String
-[^ 	].*s.*" &&
+[^ 	].*s.*" && 
 
 	# a custom pattern which matches to end of line
 	git config diff.custom2.funcname "......Beer\$" &&
@@ -119,4 +119,25 @@ do
 	"
 done
 
-test_done
+test_expect_success 'identify builtin patterns in Javascript' '
+    # setup
+    echo "function myFunction() { return true; }" > test.js &&
+    echo "var myVar = function() { return false; }" >> test.js &&
+    git add test.js &&
+    git commit -m "add test.js" &&
+
+    # modify the file
+    echo "function myFunction() { return false; }" > test.js &&
+    echo "var myVar = function() { return true; }" >> test.js &&
+
+    # command under test
+    git diff >output &&
+
+    # check results
+    test_i18ngrep "function myFunction() { return true; }" output &&
+    test_i18ngrep "function myFunction() { return false; }" output &&
+    test_i18ngrep "var myVar = function() { return false; }" output &&
+    test_i18ngrep "var myVar = function() { return true; }" output
+'
+
+test_done 
\ No newline at end of file
-- 
2.43.2



^ permalink raw reply related

* [PATCH 1/2] Subject: [GSOC][RFC PATCH 1/2] Add builtin patterns for JavaScript function detection in userdiff.
From: Sergius Nyah @ 2024-02-27 16:02 UTC (permalink / raw)
  To: christian.couder, pk; +Cc: git, Sergius Nyah
In-Reply-To: <20240227160253.104011-1-sergiusnyah@gmail.com>

This patch adds the regular expression for detecting JavaScript functions and expressions in Git diffs. The pattern accurately identifies function declerations, expressions, and definitions inside blocks.
---
 userdiff.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/userdiff.c b/userdiff.c
index e399543823..12e31ff14d 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -1,7 +1,7 @@
 #include "git-compat-util.h"
 #include "config.h"
 #include "userdiff.h"
-#include "attr.h"
+#include "attr.h" 
 #include "strbuf.h"
 
 static struct userdiff_driver *drivers;
@@ -183,6 +183,19 @@ PATTERNS("java",
 	 "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
 	 "|[-+*/<>%&^|=!]="
 	 "|--|\\+\\+|<<=?|>>>?=?|&&|\\|\\|"),
+PATTERNS("javascript",
+     /* 
+	  * Looks for lines that start with optional whitespace, followed 
+	  * by 'function'* and any characters (for function declarations), 
+      * or valid JavaScript identifiers, equals sign '=', 'function' keyword
+	  * and any characters (for function expressions).
+      * Also considers functions defined inside blocks with '{...}'.
+	  */ 
+	 "^[ \t]*(function[ \t]*.*|[a-zA-Z_$][0-9a-zA-Z_$]*[ \t]*=[ \t]*function[ \t]*.*|(\\{[ \t]*)?)\n",
+     /* This pattern matches JavaScript identifiers */
+     "[a-zA-Z_$][0-9a-zA-Z_$]*"
+     "|[-+0-9.eE]+|0[xX][0-9a-fA-F]+" 
+     "|[-+*/<>%&^|=!:]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\|"), 
 PATTERNS("kotlin",
 	 "^[ \t]*(([a-z]+[ \t]+)*(fun|class|interface)[ \t]+.*)$",
 	 /* -- */
@@ -192,7 +205,7 @@ PATTERNS("kotlin",
 	 /* integers and floats */
 	 "|[0-9][0-9_]*([.][0-9_]*)?([Ee][-+]?[0-9]+)?[fFlLuU]*"
 	 /* floating point numbers beginning with decimal point */
-	 "|[.][0-9][0-9_]*([Ee][-+]?[0-9]+)?[fFlLuU]?"
+	 "|[.][0-9][0-9_]*([Ee][-+]?[0-9]+ )?[fFlLuU]?"
 	 /* unary and binary operators */
 	 "|[-+*/<>%&^|=!]==?|--|\\+\\+|<<=|>>=|&&|\\|\\||->|\\.\\*|!!|[?:.][.:]"),
 PATTERNS("markdown",
-- 
2.43.2
:wq



^ permalink raw reply related

* [GSOC][PATCH 0/2] Add builtin patterns for userdiff in JavaScript, as Microproject.
From: Sergius Nyah @ 2024-02-27 16:02 UTC (permalink / raw)
  To: christian.couder, pk; +Cc: git, Sergius Nyah
In-Reply-To: <CANAnif-OganZLi0Cu_uq=nveC+u5n14c=o_DQHT-wFOqQ9Vs0Q@mail.gmail.com>

I would like to apologize for the numerous irrelevant emails that have been sent to this mailing list recently. 
They were the result of a configuration issue with my setup, which caused my patches to be sent from a `noreply` GitHub address. 
This led to a series of failed delivery attempts and unnecessary emails.

Firstly, I'm so sorry for the delay between selecting the Microproject and sending the patch series.
Learning about regular expressions took me a bit longer than I expected, but it was all worth it.
I'm very grateful for the opportunity to work on this project and I'm looking forward to contributing more to Git.

This patch series adds builtin patterns for JavaScript function detection in userdiff, as 
my Microproject for GSOC. The first patch adds a regular expression for detecting JavaScript
functions in Git diffs while the second adds a test for JavaScript function detection in Git diffs.
This new pattern looks for lines that start with optional whitespace, followed by 'function' and any 
characters (for function declarations), or valid JavaScript identifiers, equals sign '=', 'function'
keyword and any characters (for function expressions). It also considers functions defined inside blocks with '{...}'.


 t/t4018-diff-funcname.sh | 25 +++++++++++++++++++++++--
 userdiff.c               | 17 +++++++++++++++--
 2 files changed, 38 insertions(+), 4 deletions(-)
 

base-commit: c5b454771e6b086f60c7f1f139025f174bcedac9
-- 
2.43.2

I would greatly appreciate any feedback on the patch series.
Best, 
Sergius.


^ permalink raw reply

* [PATCH v2 3/3] builtin/unpack-objects.c: change xwrite to write_in_full avoid truncation.
From: Randall S. Becker @ 2024-02-27 15:09 UTC (permalink / raw)
  To: git; +Cc: Randall S. Becker
In-Reply-To: <20240227150934.7950-1-randall.becker@nexbridge.ca>

From: "Randall S. Becker" <rsbecker@nexbridge.com>

This change is required because some platforms do not support file writes of
arbitrary sizes (e.g, NonStop). xwrite ends up truncating the output to the
maximum single I/O size possible for the destination device if the supplied
len value exceeds the supported value. Replacing xwrite with write_in_full
corrects this problem. Future optimisations could remove the loop in favour
of just calling write_in_full.

Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
---
 builtin/unpack-objects.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index e0a701f2b3..6935c4574e 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -680,7 +680,7 @@ int cmd_unpack_objects(int argc, const char **argv, const char *prefix UNUSED)
 
 	/* Write the last part of the buffer to stdout */
 	while (len) {
-		int ret = xwrite(1, buffer + offset, len);
+		int ret = write_in_full(1, buffer + offset, len);
 		if (ret <= 0)
 			break;
 		len -= ret;
-- 
2.42.1


^ permalink raw reply related

* [PATCH v2 2/3] builtin/receive-pack.c: change xwrite to write_in_full.
From: Randall S. Becker @ 2024-02-27 15:09 UTC (permalink / raw)
  To: git; +Cc: Randall S. Becker
In-Reply-To: <20240227150934.7950-1-randall.becker@nexbridge.ca>

From: "Randall S. Becker" <rsbecker@nexbridge.com>

This change is required because some platforms do not support file writes of    arbitrary sizes (e.g, NonStop). xwrite ends up truncating the output to the
maximum single I/O size possible for the destination device.

Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
---
 builtin/receive-pack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index db65607485..4277c63d08 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -456,7 +456,7 @@ static void report_message(const char *prefix, const char *err, va_list params)
 	if (use_sideband)
 		send_sideband(1, 2, msg, sz, use_sideband);
 	else
-		xwrite(2, msg, sz);
+		write_in_full(2, msg, sz);
 }
 
 __attribute__((format (printf, 1, 2)))
-- 
2.42.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox