* [PATCH v4 1/6] merge-tree: fail with a non-zero exit code on missing tree objects
From: Johannes Schindelin via GitGitGadget @ 2024-02-23 8:34 UTC (permalink / raw)
To: git
Cc: Patrick Steinhardt, Eric Sunshine, Johannes Schindelin,
Johannes Schindelin
In-Reply-To: <pull.1651.v4.git.1708677266.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
When `git merge-tree` encounters a missing tree object, it should error
out and not continue quietly as if nothing had happened.
However, as of time of writing, `git merge-tree` _does_ continue, and
then offers the empty tree as result.
Let's fix this.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
merge-ort.c | 7 ++++---
t/t4301-merge-tree-write-tree.sh | 11 +++++++++++
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/merge-ort.c b/merge-ort.c
index 6491070d965..c37fc035f13 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -1659,9 +1659,10 @@ static int collect_merge_info(struct merge_options *opt,
info.data = opt;
info.show_all_errors = 1;
- parse_tree(merge_base);
- parse_tree(side1);
- parse_tree(side2);
+ if (parse_tree(merge_base) < 0 ||
+ parse_tree(side1) < 0 ||
+ parse_tree(side2) < 0)
+ return -1;
init_tree_desc(t + 0, merge_base->buffer, merge_base->size);
init_tree_desc(t + 1, side1->buffer, side1->size);
init_tree_desc(t + 2, side2->buffer, side2->size);
diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
index 7d0fa74da74..908c9b540c8 100755
--- a/t/t4301-merge-tree-write-tree.sh
+++ b/t/t4301-merge-tree-write-tree.sh
@@ -951,4 +951,15 @@ test_expect_success '--merge-base with tree OIDs' '
test_cmp with-commits with-trees
'
+test_expect_success 'error out on missing tree objects' '
+ git init --bare missing-tree.git &&
+ git rev-list side3 >list &&
+ git rev-parse side3^: >>list &&
+ git pack-objects missing-tree.git/objects/pack/side3-tree-is-missing <list &&
+ side3=$(git rev-parse side3) &&
+ test_must_fail git --git-dir=missing-tree.git merge-tree $side3^ $side3 >actual 2>err &&
+ test_grep "Could not read $(git rev-parse $side3:)" err &&
+ test_must_be_empty actual
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 0/6] merge-tree: handle missing objects correctly
From: Johannes Schindelin via GitGitGadget @ 2024-02-23 8:34 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Eric Sunshine, Johannes Schindelin
In-Reply-To: <pull.1651.v3.git.1708612605.gitgitgadget@gmail.com>
I recently looked into issues where git merge-tree calls returned bogus data
(in one instance returning an empty tree for non-empty merge parents). By
the time I had a look at the corresponding repository, the issue was no
longer reproducible, but a closer look at the code combined with some manual
experimenting turned up the fact that missing tree objects aren't handled as
errors by git merge-tree.
While at it, I added a commit on top that tries to catch all remaining
unchecked parse_tree() calls.
This patch series is based on 5f43cf5b2e4 (merge-tree: accept 3 trees as
arguments, 2024-01-28) (the original tip commit of js/merge-tree-3-trees)
because I introduced three unchecked parse_tree() calls in that topic.
Changes since v3:
* Aligned the translated error messages with pre-existing ones (sorry, I
forgot to make that change in v2!).
* Added a new commit at the end to mark the one error message for
translation which I had imitated, after making it consistent with the
remaining "unable to read tree" error messages.
Changes since v2:
* Fixed the new "missing tree object" test case in t4301 that succeeded for
the wrong reason.
* Adjusted the new "missing blob object" test case to avoid succeeding for
the wrong reason.
* Simplified the "missing blob object" test case.
Changes since v1:
* Simplified the test case, avoiding a subshell and a pipe in the process.
* Added a patch to remove a superfluous subtree->object.parsed guard around
a parse_tree(subtree) call.
Johannes Schindelin (6):
merge-tree: fail with a non-zero exit code on missing tree objects
merge-ort: do check `parse_tree()`'s return value
t4301: verify that merge-tree fails on missing blob objects
Always check `parse_tree*()`'s return value
cache-tree: avoid an unnecessary check
fill_tree_descriptor(): mark error message for translation
builtin/checkout.c | 19 ++++++++++++++++---
builtin/clone.c | 3 ++-
builtin/commit.c | 3 ++-
builtin/merge-tree.c | 6 ++++++
builtin/read-tree.c | 3 ++-
builtin/reset.c | 4 ++++
cache-tree.c | 4 ++--
merge-ort.c | 16 +++++++++++-----
merge-recursive.c | 3 ++-
merge.c | 5 ++++-
reset.c | 5 +++++
sequencer.c | 4 ++++
t/t4301-merge-tree-write-tree.sh | 27 +++++++++++++++++++++++++++
t/t6030-bisect-porcelain.sh | 2 +-
tree-walk.c | 2 +-
15 files changed, 89 insertions(+), 17 deletions(-)
base-commit: 5f43cf5b2e4b68386d3774bce880b0f74d801635
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1651%2Fdscho%2Fmerge-tree-and-missing-objects-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1651/dscho/merge-tree-and-missing-objects-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1651
Range-diff vs v3:
1: 11b9cd8c5da = 1: 11b9cd8c5da merge-tree: fail with a non-zero exit code on missing tree objects
2: f01f4eb011b = 2: f01f4eb011b merge-ort: do check `parse_tree()`'s return value
3: e82fdf7fbcb = 3: e82fdf7fbcb t4301: verify that merge-tree fails on missing blob objects
4: 9e4dc94ef03 ! 4: 5942c27f439 Always check `parse_tree*()`'s return value
@@ builtin/checkout.c: static int merge_working_tree(const struct checkout_opts *op
new_tree = repo_get_commit_tree(the_repository,
new_branch_info->commit);
+ if (!new_tree)
-+ return error(_("unable to read tree %s"),
++ return error(_("unable to read tree (%s)"),
+ oid_to_hex(&new_branch_info->commit->object.oid));
+ }
if (opts->discard_changes) {
@@ builtin/checkout.c: static void setup_new_branch_info_and_source_tree(
/* not a commit */
*source_tree = parse_tree_indirect(rev);
+ if (!*source_tree)
-+ die(_("unable to read tree %s"), oid_to_hex(rev));
++ die(_("unable to read tree (%s)"), oid_to_hex(rev));
} else {
parse_commit_or_die(new_branch_info->commit);
*source_tree = repo_get_commit_tree(the_repository,
new_branch_info->commit);
+ if (!*source_tree)
-+ die(_("unable to read tree %s"),
++ die(_("unable to read tree (%s)"),
+ oid_to_hex(&new_branch_info->commit->object.oid));
}
}
@@ builtin/merge-tree.c: static int real_merge(struct merge_tree_options *o,
die(_("could not parse as tree '%s'"), merge_base);
base_tree = parse_tree_indirect(&base_oid);
+ if (!base_tree)
-+ die(_("unable to read tree %s"), oid_to_hex(&base_oid));
++ die(_("unable to read tree (%s)"), oid_to_hex(&base_oid));
if (repo_get_oid_treeish(the_repository, branch1, &head_oid))
die(_("could not parse as tree '%s'"), branch1);
parent1_tree = parse_tree_indirect(&head_oid);
+ if (!parent1_tree)
-+ die(_("unable to read tree %s"), oid_to_hex(&head_oid));
++ die(_("unable to read tree (%s)"), oid_to_hex(&head_oid));
if (repo_get_oid_treeish(the_repository, branch2, &merge_oid))
die(_("could not parse as tree '%s'"), branch2);
parent2_tree = parse_tree_indirect(&merge_oid);
+ if (!parent2_tree)
-+ die(_("unable to read tree %s"), oid_to_hex(&merge_oid));
++ die(_("unable to read tree (%s)"), oid_to_hex(&merge_oid));
opt.ancestor = merge_base;
merge_incore_nonrecursive(&opt, base_tree, parent1_tree, parent2_tree, &result);
@@ builtin/reset.c: static int reset_index(const char *ref, const struct object_id
if (reset_type == MIXED || reset_type == HARD) {
tree = parse_tree_indirect(oid);
+ if (!tree) {
-+ error(_("unable to read tree %s"), oid_to_hex(oid));
++ error(_("unable to read tree (%s)"), oid_to_hex(oid));
+ goto out;
+ }
prime_cache_tree(the_repository, the_repository->index, tree);
@@ merge-ort.c: static void merge_ort_nonrecursive_internal(struct merge_options *o
if (result->clean >= 0) {
result->tree = parse_tree_indirect(&working_tree_oid);
+ if (!result->tree)
-+ die(_("unable to read tree %s"),
++ die(_("unable to read tree (%s)"),
+ oid_to_hex(&working_tree_oid));
/* existence of conflicted entries implies unclean */
result->clean &= strmap_empty(&opt->priv->conflicted);
@@ reset.c: int reset_head(struct repository *r, const struct reset_head_opts *opts
tree = parse_tree_indirect(oid);
+ if (!tree) {
-+ ret = error(_("unable to read tree %s"), oid_to_hex(oid));
++ ret = error(_("unable to read tree (%s)"), oid_to_hex(oid));
+ goto leave_reset_head;
+ }
+
@@ sequencer.c: static int do_recursive_merge(struct repository *r,
head_tree = parse_tree_indirect(head);
+ if (!head_tree)
-+ return error(_("unable to read tree %s"), oid_to_hex(head));
++ return error(_("unable to read tree (%s)"), oid_to_hex(head));
next_tree = next ? repo_get_commit_tree(r, next) : empty_tree(r);
base_tree = base ? repo_get_commit_tree(r, base) : empty_tree(r);
@@ sequencer.c: static int do_reset(struct repository *r,
tree = parse_tree_indirect(&oid);
+ if (!tree)
-+ return error(_("unable to read tree %s"), oid_to_hex(&oid));
++ return error(_("unable to read tree (%s)"), oid_to_hex(&oid));
prime_cache_tree(r, r->index, tree);
if (write_locked_index(r->index, &lock, COMMIT_LOCK) < 0)
5: 91dc4ccd04e = 5: 7e5e84a4e7c cache-tree: avoid an unnecessary check
-: ----------- > 6: ee2fcee5a10 fill_tree_descriptor(): mark error message for translation
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH v3 4/5] Always check `parse_tree*()`'s return value
From: Johannes Schindelin @ 2024-02-23 8:33 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin via GitGitGadget, git, Patrick Steinhardt,
Eric Sunshine
In-Reply-To: <xmqqplwoe5yv.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 1679 bytes --]
Hi Junio,
thank you so much for reviewing!
On Thu, 22 Feb 2024, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > + } else {
> > new_tree = repo_get_commit_tree(the_repository,
> > new_branch_info->commit);
> > + if (!new_tree)
> > + return error(_("unable to read tree %s"),
> > + oid_to_hex(&new_branch_info->commit->object.oid));
>
> We can help translators by enclosing %s inside a pair of parentheses.
>
> $ git grep -h 'msgid .*unable to read tree' po | sort | uniq -c
> 18 msgid "unable to read tree (%s)"
Right. I had used
$ git grep 'unable to read tree .*%s' | sed -n 's/.*_("\([^"]*\).*/\1/p' | sort | uniq -c
11 unable to read tree %s
3 unable to read tree (%s)
only to realize that the 11 were the ones I added.🤦 Re-running the same
command on v2.43.0 reports only the 3 parenthesized ones.
I've fixed those error messages in v4, and also added a patch to adjust
the one error message that I imitated (and to mark it for translation).
This patch might be slightly controversial because, in what is probably
only a hypothetical scenario, users might have scripted around `git
bisect` to parse error messages _and_ special-cased the "unable to read
tree" message to be able to deal with the missing tree in a programmatical
manner. I might be misjudging the likelihood for something like that, but
I vividly remember the, ahem, "exciting times" I had after 7560f547e61
(treewide: correct several "up-to-date" to "up to date", 2017-08-23). If
that patch is too concerning, I am open to dropping it.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH v2 04/16] fsmonitor: refactor refresh callback on directory events
From: Junio C Hamano @ 2024-02-23 8:18 UTC (permalink / raw)
To: Jeff Hostetler via GitGitGadget
Cc: git, Patrick Steinhardt, Jeff Hostetler, Jeff Hostetler
In-Reply-To: <5516670e30e26c5b50c67b69e48e3e8a5e0d8990.1708658300.git.gitgitgadget@gmail.com>
"Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Jeff Hostetler <jeffhostetler@github.com>
>
> Move the code to handle directory FSEvents (containing pathnames with
> a trailing slash) into a helper function.
>
> Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
> ---
> fsmonitor.c | 52 ++++++++++++++++++++++++++++++----------------------
> 1 file changed, 30 insertions(+), 22 deletions(-)
Nothing unexpected to see here. Looking good.
> diff --git a/fsmonitor.c b/fsmonitor.c
> index f670c509378..6fecae9aeb2 100644
> --- a/fsmonitor.c
> +++ b/fsmonitor.c
> @@ -183,6 +183,35 @@ static int query_fsmonitor_hook(struct repository *r,
> return result;
> }
>
> +static void handle_path_with_trailing_slash(
> + struct index_state *istate, const char *name, int pos)
> +{
> + int i;
> +
> + /*
> + * The daemon can decorate directory events, such as
> + * moves or renames, with a trailing slash if the OS
> + * FS Event contains sufficient information, such as
> + * MacOS.
> + *
> + * Use this to invalidate the entire cone under that
> + * directory.
> + *
> + * We do not expect an exact match because the index
> + * does not normally contain directory entries, so we
> + * start at the insertion point and scan.
> + */
> + if (pos < 0)
> + pos = -pos - 1;
> +
> + /* Mark all entries for the folder invalid */
> + for (i = pos; i < istate->cache_nr; i++) {
> + if (!starts_with(istate->cache[i]->name, name))
> + break;
> + istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID;
> + }
> +}
> +
> static void fsmonitor_refresh_callback(struct index_state *istate, char *name)
> {
> int i, len = strlen(name);
> @@ -193,28 +222,7 @@ static void fsmonitor_refresh_callback(struct index_state *istate, char *name)
> name, pos);
>
> if (name[len - 1] == '/') {
> - /*
> - * The daemon can decorate directory events, such as
> - * moves or renames, with a trailing slash if the OS
> - * FS Event contains sufficient information, such as
> - * MacOS.
> - *
> - * Use this to invalidate the entire cone under that
> - * directory.
> - *
> - * We do not expect an exact match because the index
> - * does not normally contain directory entries, so we
> - * start at the insertion point and scan.
> - */
> - if (pos < 0)
> - pos = -pos - 1;
> -
> - /* Mark all entries for the folder invalid */
> - for (i = pos; i < istate->cache_nr; i++) {
> - if (!starts_with(istate->cache[i]->name, name))
> - break;
> - istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID;
> - }
> + handle_path_with_trailing_slash(istate, name, pos);
>
> /*
> * We need to remove the traling "/" from the path
^ permalink raw reply
* Re: [PATCH v2 06/16] fsmonitor: refactor refresh callback for non-directory events
From: Junio C Hamano @ 2024-02-23 8:18 UTC (permalink / raw)
To: Jeff Hostetler via GitGitGadget
Cc: git, Patrick Steinhardt, Jeff Hostetler, Jeff Hostetler
In-Reply-To: <7ee6ca1aefd34a37d749300e317df10d80ef2b29.1708658300.git.gitgitgadget@gmail.com>
"Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Jeff Hostetler <jeffhostetler@github.com>
>
> Move the code handle unqualified FSEvents (without a trailing slash)
> into a helper function.
-ECANNOTPARSE. "code handle" -> "code that handles"?
In the patch text itself, there is nothing unexpected. Looking
good.
Thanks.
> Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
> ---
> fsmonitor.c | 67 +++++++++++++++++++++++++++++++----------------------
> 1 file changed, 39 insertions(+), 28 deletions(-)
>
> diff --git a/fsmonitor.c b/fsmonitor.c
> index 29cce32d81c..364198d258f 100644
> --- a/fsmonitor.c
> +++ b/fsmonitor.c
> @@ -183,6 +183,43 @@ static int query_fsmonitor_hook(struct repository *r,
> return result;
> }
>
> +static void handle_path_without_trailing_slash(
> + struct index_state *istate, const char *name, int pos)
> +{
> + int i;
> +
> + if (pos >= 0) {
> + /*
> + * We have an exact match for this path and can just
> + * invalidate it.
> + */
> + istate->cache[pos]->ce_flags &= ~CE_FSMONITOR_VALID;
> + } else {
> + /*
> + * The path is not a tracked file -or- it is a
> + * directory event on a platform that cannot
> + * distinguish between file and directory events in
> + * the event handler, such as Windows.
> + *
> + * Scan as if it is a directory and invalidate the
> + * cone under it. (But remember to ignore items
> + * between "name" and "name/", such as "name-" and
> + * "name.".
> + */
> + int len = strlen(name);
> + pos = -pos - 1;
> +
> + for (i = pos; i < istate->cache_nr; i++) {
> + if (!starts_with(istate->cache[i]->name, name))
> + break;
> + if ((unsigned char)istate->cache[i]->name[len] > '/')
> + break;
> + if (istate->cache[i]->name[len] == '/')
> + istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID;
> + }
> + }
> +}
> +
> /*
> * The daemon can decorate directory events, such as a move or rename,
> * by adding a trailing slash to the observed name. Use this to
> @@ -225,7 +262,7 @@ static void handle_path_with_trailing_slash(
>
> static void fsmonitor_refresh_callback(struct index_state *istate, char *name)
> {
> - int i, len = strlen(name);
> + int len = strlen(name);
> int pos = index_name_pos(istate, name, len);
>
> trace_printf_key(&trace_fsmonitor,
> @@ -240,34 +277,8 @@ static void fsmonitor_refresh_callback(struct index_state *istate, char *name)
> * for the untracked cache.
> */
> name[len - 1] = '\0';
> - } else if (pos >= 0) {
> - /*
> - * We have an exact match for this path and can just
> - * invalidate it.
> - */
> - istate->cache[pos]->ce_flags &= ~CE_FSMONITOR_VALID;
> } else {
> - /*
> - * The path is not a tracked file -or- it is a
> - * directory event on a platform that cannot
> - * distinguish between file and directory events in
> - * the event handler, such as Windows.
> - *
> - * Scan as if it is a directory and invalidate the
> - * cone under it. (But remember to ignore items
> - * between "name" and "name/", such as "name-" and
> - * "name.".
> - */
> - pos = -pos - 1;
> -
> - for (i = pos; i < istate->cache_nr; i++) {
> - if (!starts_with(istate->cache[i]->name, name))
> - break;
> - if ((unsigned char)istate->cache[i]->name[len] > '/')
> - break;
> - if (istate->cache[i]->name[len] == '/')
> - istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID;
> - }
> + handle_path_without_trailing_slash(istate, name, pos);
> }
>
> /*
^ permalink raw reply
* Re: [PATCH v2 03/16] t7527: temporarily disable case-insensitive tests
From: Junio C Hamano @ 2024-02-23 8:17 UTC (permalink / raw)
To: Jeff Hostetler via GitGitGadget
Cc: git, Patrick Steinhardt, Jeff Hostetler, Jeff Hostetler
In-Reply-To: <dad079ade7f8817f235d59c5339b82aecf4a17db.1708658300.git.gitgitgadget@gmail.com>
"Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Jeff Hostetler <jeffhostetler@github.com>
>
> Add non-existent "SKIPME" prereq to the case-insensitive tests.
>
> The previous commit added test cases to demonstrate an error where
> FSMonitor can get confused on a case-insensitive file system when the
> on-disk spelling of a file or directory is wrong. Let's disable those
> tests before we incrementally teach Git to properly recognize and
> handle those types of problems (so that a bisect between here and the
> final commit in this patch series won't throw a false alarm).
You talk about bisection, but hasn't the previous step already
broken bisection without these SKIPME prerequisites? IOW, shouldn't
this step squashed into the previous?
Also, it is much more common to replace "test_expect_success" with
"test_expect_failure" to indicate that the steps are broken. Was
there a reason why we choose to do it differently?
> Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
> ---
> t/t7527-builtin-fsmonitor.sh | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
> index 3d21295f789..4acb547819c 100755
> --- a/t/t7527-builtin-fsmonitor.sh
> +++ b/t/t7527-builtin-fsmonitor.sh
> @@ -1051,7 +1051,7 @@ test_expect_success 'split-index and FSMonitor work well together' '
> #
> # The setup is a little contrived.
> #
> -test_expect_success CASE_INSENSITIVE_FS 'fsmonitor subdir case wrong on disk' '
> +test_expect_success SKIPME,CASE_INSENSITIVE_FS 'fsmonitor subdir case wrong on disk' '
> test_when_finished "stop_daemon_delete_repo subdir_case_wrong" &&
>
> git init subdir_case_wrong &&
> @@ -1128,7 +1128,7 @@ test_expect_success CASE_INSENSITIVE_FS 'fsmonitor subdir case wrong on disk' '
> ! grep -q " M dir1/dir2/dir3/file3" "$PWD/subdir_case_wrong.out"
> '
>
> -test_expect_success CASE_INSENSITIVE_FS 'fsmonitor file case wrong on disk' '
> +test_expect_success SKIPME,CASE_INSENSITIVE_FS 'fsmonitor file case wrong on disk' '
> test_when_finished "stop_daemon_delete_repo file_case_wrong" &&
>
> git init file_case_wrong &&
^ permalink raw reply
* Re: [PATCH v2 2/2] an improvement: removed configure.ac changes
From: Junio C Hamano @ 2024-02-23 7:38 UTC (permalink / raw)
To: Haritha D via GitGitGadget; +Cc: git, Kristoffer Haugsbakk, Haritha
In-Reply-To: <05df5d7e2d50cba77f53273c781f97d24144efc6.1708660111.git.gitgitgadget@gmail.com>
"Haritha D via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Haritha D <harithamma.d@ibm.com>
>
> Hello Reviewers, as an improvement, I have
> removed the CC_LD_DYNPATH changes because
> I was able to achieve the desired outcome
> by passing it to the make step.
>
> Signed-off-by: Haritha D <harithamma.d@ibm.com>
> ---
> configure.ac | 3 ---
> 1 file changed, 3 deletions(-)
We frown upon a patch series that makes mistakes in an earlier step,
only to fix them in a later step. The "git rebase -i" command helps
us pretend to be more perfect developers than we actually are,
whipping your patch series into a shape that builds one small step
on top of another in a logical succession. Such a patch series is
easier to understand than a history that faithfully records all the
stumbles the developer made until they reached the final solution.
In this case, if you know before you sent these two patches that you
do not need to touch configure.ac for CC_LD_DYNPATH at all, you can
"git rebase -i" to whip the first patch into the desired shape, i.e.
no changes to configure.ac, and drop this second patch, I think.
> diff --git a/configure.ac b/configure.ac
> index 64569a80d53..d1a96da14eb 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -463,9 +463,6 @@ else
> CC_LD_DYNPATH=-Wl,+b,
> else
> CC_LD_DYNPATH=
> - if test "$(uname -s)" = "OS/390"; then
> - CC_LD_DYNPATH=-L
> - fi
> AC_MSG_WARN([linker does not support runtime path to dynamic libraries])
> fi
> fi
^ permalink raw reply
* Re: [PATCH v2 0/2] This PR enables a successful git build on z/OS.
From: Junio C Hamano @ 2024-02-23 7:37 UTC (permalink / raw)
To: Haritha via GitGitGadget; +Cc: git, Kristoffer Haugsbakk, Haritha
In-Reply-To: <pull.1663.v2.git.git.1708660111.gitgitgadget@gmail.com>
"Haritha via GitGitGadget" <gitgitgadget@gmail.com> writes:
> Since the z/OS linker does not support searching dynamic libraries, and the
> current setting of CC_LD_DYNPATH results in a directory to be supplied to
> the link step with no option as the suffix, it causes a linker error because
> the z/OS LD linker does not accept directories as input. Therefore, we
> workaround this by adding the -L option.
Hmph, do we still need that CC_LD_DYNPATH hack, with 07bbe4ca (Merge
branch 'jc/make-libpath-template', 2024-02-06) already in 'master'?
> And, Introduced z/OS (OS/390) as a
> platform in config.mak.uname
Add support for z/OS (OS/390) in config.mak.uname.
or something (cf. Documentation/SubmittingPatches)?
> Thanks for taking the time to contribute to Git! Please be advised that the
> Git community does not use github.com for their contributions. Instead, we
> use a mailing list (git@vger.kernel.org) for code submissions, code reviews,
> and bug reports. Nevertheless, you can use GitGitGadget
> (https://gitgitgadget.github.io/) to conveniently send your Pull Requests
> commits to our mailing list.
>
> Please read the "guidelines for contributing" linked above!
For whom is the above message addressed? Who is expected to read
that guidelines?
^ permalink raw reply
* Re: [PATCH v2 8/8] cherry-pick: add `--empty` for more robust redundant commit handling
From: Brian Lyles @ 2024-02-23 6:58 UTC (permalink / raw)
To: phillip.wood; +Cc: git, newren, me, gitster
In-Reply-To: <bb23026e-72c5-40a0-bd5f-356a03efa5aa@gmail.com>
On Thu, Feb 22, 2024 at 10:36 AM <phillip.wood123@gmail.com> wrote:
> I'm leaning towards leaving `--keep-redundant-commits` alone. That
> introduces an inconsistency between `--keep-redundant-commits` and
> `--empty=keep` as the latter does not imply `--allow-empty` but it does
> avoid breaking existing users. We could document
> `--keep-redundant-commits` as predating `--empty` and behaving like
> `--empty=keep --allow-empty`. The documentation and implementation of
> the new option look good modulo the typo that has already been pointed
> out and a couple of small comments below.
I think I'm on board with leaving `--keep-redundant-commits` alone. I'm
on the fence about having `--empty=keep` imply `--allow-empty` after
seeing Junio's concerns. I laid out the options that I see in a reply to
patch 6/8[1] and would appreciate input there. I'll adjust the details
of this commit in v3 based on what we decide there.
[1]: https://lore.kernel.org/git/17b666ca6c4b7561.70b1dd9aae081c6e.203dcd72f6563036@zivdesk/
>
>> +enum empty_action {
>> + EMPTY_COMMIT_UNSPECIFIED = 0,
>
> We tend to use -1 for unspecified options
Thanks, I'll update this in v3.
>> +test_expect_success 'cherry-pick persists --empty=stop correctly' '
>> + pristine_detach initial &&
>> + # to make sure that the session to cherry-pick a sequence
>> + # gets interrupted, use a high-enough number that is larger
>> + # than the number of parents of any commit we have created
>> + mainline=4 &&
>> + test_expect_code 128 git cherry-pick -s -m $mainline --empty=stop initial..anotherpick &&
>> + test_path_is_file .git/sequencer/opts &&
>> + test_must_fail git config --file=.git/sequencer/opts --get-all options.keep-redundant-commits &&
>> + test_must_fail git config --file=.git/sequencer/opts --get-all options.drop-redundant-commits
>> +'
>
> Thanks for adding these tests to check that the --empty option persists.
> Usually for tests like this we prefer to check the user visible behavior
> rather than the implementation details (I suspect we have some older
> tests that do the latter). To check the behavior we usually arrange for
> a merge conflict but using -m is a creative alternative, then we'd run
> "git cherry-pick --continue" and check that the commits that become
> empty have been preserved or dropped or that the cherry-pick stops.
Indeed, I was modelling these new tests after other existing tests in
this file. While I agree with you in theory, I am hesitant to make these
new tests drastically different from the existing tests that are testing
the same mechanisms (and appear to be very intentionally testing that
the options are persisted in that config file). I'm also hesitant to
update the existing tests as part of this series (primarily due to a
lack of familiarity, and partially to avoid scope creep of the series).
How concerned are you about the current implementation? Does it make
sense to you to defer this suggestion to a future series that cleans up
the tests to do more user-oriented checks?
--
Thank you,
Brian Lyles
^ permalink raw reply
* Re: [PATCH v2 01/16] name-hash: add index_dir_find()
From: Junio C Hamano @ 2024-02-23 6:37 UTC (permalink / raw)
To: Jeff Hostetler via GitGitGadget
Cc: git, Patrick Steinhardt, Jeff Hostetler, Jeff Hostetler
In-Reply-To: <03b07d9c25edb951db4de518212f8a3e9e184e49.1708658300.git.gitgitgadget@gmail.com>
"Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Jeff Hostetler <jeffhostetler@github.com>
>
> Replace the index_dir_exists() function with index_dir_find() and
> change the API to take an optional strbuf to return the canonical
> spelling of the matched directory prefix.
>
> Create an index_dir_exists() wrapper macro for existing callers.
>
> The existing index_dir_exists() returns a boolean to indicate if
> there is a case-insensitive match in the directory name-hash, but
> it doesn't tell the caller the exact spelling of that match.
>
> The new version also copies the matched spelling to a provided strbuf.
> This lets the caller, for example, then call index_name_pos() with the
> correct case to search the cache-entry array for the real insertion
> position.
The usual way to compose a log message of this project is to
- Give an observation on how the current system work in the present
tense (so no need to say "Currently X is Y", just "X is Y"), and
discuss what you perceive as a problem in it.
- Propose a solution (optional---often, problem description
trivially leads to an obvious solution in reader's minds).
- Give commands to the codebase to "become like so".
in this order.
I think the third paragraph you wrote should come at the beginning,
then the first (now second) paragraph should describe more clearly
that index_dir_find() is a new function and what it does (perhaps by
reusing what is in the "The new version also..." paragraph),
without mentioning index_dir_exists().
The second (now third) paragraph then can talk about reimplementing
index_dir_exists() in terms of index_dir_find().
The patch text looks good.
Thanks.
> -int index_dir_exists(struct index_state *istate, const char *name, int namelen)
> +int index_dir_find(struct index_state *istate, const char *name, int namelen,
> + struct strbuf *canonical_path)
> {
> struct dir_entry *dir;
>
> lazy_init_name_hash(istate);
> expand_to_path(istate, name, namelen, 0);
> dir = find_dir_entry(istate, name, namelen);
> +
> + if (canonical_path && dir && dir->nr) {
> + strbuf_reset(canonical_path);
> + strbuf_add(canonical_path, dir->name, dir->namelen);
> + }
> +
> return dir && dir->nr;
> }
>
> diff --git a/name-hash.h b/name-hash.h
> index b1b4b0fb337..0cbfc428631 100644
> --- a/name-hash.h
> +++ b/name-hash.h
> @@ -4,7 +4,12 @@
> struct cache_entry;
> struct index_state;
>
> -int index_dir_exists(struct index_state *istate, const char *name, int namelen);
> +
> +int index_dir_find(struct index_state *istate, const char *name, int namelen,
> + struct strbuf *canonical_path);
> +
> +#define index_dir_exists(i, n, l) index_dir_find((i), (n), (l), NULL)
> +
> void adjust_dirname_case(struct index_state *istate, char *name);
> struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int igncase);
^ permalink raw reply
* Re: [PATCH v2 7/8] cherry-pick: enforce `--keep-redundant-commits` incompatibility
From: Brian Lyles @ 2024-02-23 6:23 UTC (permalink / raw)
To: phillip.wood; +Cc: git, newren, me, gitster
In-Reply-To: <8c2eec1b-9a59-4739-a903-b2e8955f3ff5@gmail.com>
On Thu, Feb 22, 2024 at 10:35 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
> Well spotted, do we really need a new test file just for this though? I
> wonder if the new test would be better off living in
> t3505-cherry-pick-empty.sh or t3507-cherry-pick-conflict.sh
I was modelling this off of 't3422-rebase-incompatible-options.sh'.
Additionally, I do feel like these tests are only tangentially related
to the tests that actually exercise the features themselves. Notably,
the setup requirements are drastically different (simpler) since the
test should fail long before any setup actually matters. For that
reason, I think a separate file where other future tests for
incompatible options can also live does make sense.
Is there any particular downside to the new file that I am unaware of?
--
Thank you,
Brian Lyles
^ permalink raw reply
* Re: [PATCH v2 4/8] sequencer: treat error reading HEAD as unborn branch
From: Brian Lyles @ 2024-02-23 5:28 UTC (permalink / raw)
To: phillip.wood; +Cc: git, newren, me, gitster
In-Reply-To: <08073d69-18f4-40c9-90a5-23db914c163e@gmail.com>
On Thu, Feb 22, 2024 at 10:34 AM <phillip.wood123@gmail.com> wrote:
>> static int is_index_unchanged(struct repository *r)
>> {
>> - struct object_id head_oid, *cache_tree_oid;
>> + struct object_id head_oid, *cache_tree_oid, head_tree_oid;
>
> I think we can make `head_tree_oid` a pointer like cache_tree_oid and
> avoid de-referencing `the_hash_algo->empty_tree` and the return value of
> `get_commit_tree_oid()`. I think the only reason to copy it would be if
> the underlying object had a shorter lifetime than `head_tree_oid` but I
> don't think that's the case.
Makes sense -- I'll update this in v3.
>> +test_expect_success 'cherry-pick on unborn branch with --allow-empty' '
>> + git checkout main &&
>
> I'm a bit confused by this - are we already on the branch "unborn" and
> so need to move away from it to delete it?
Yes, the previous test leaves us on that branch. In v3, I will update
this to instead just use `git checkout --detach`, as that does seem a
little less confusing than switching to some other branch that is only
relevant because it's not `unborn`. If there is a cleaner way to do
this, I'd be happy to switch to it.
>> + git branch -D unborn &&
>> + git checkout --orphan unborn &&
>> + git rm --cached -r . &&
>> + rm -rf * &&
>
> "git switch --orphan" leaves us with an empty index and working copy
> without having to remove the files ourselves.
Thanks for pointing this out. Using git-switch(1) here instead of
git-checkout(1) allows us to drop the `rm -rf *` call form both the
existing 'cherry-pick on unborn branch' test as well as my new test. It
appears that the `git rm --cached -r .` call is still necessary in the
existing test.
>> + git cherry-pick initial --allow-empty &&
>> + git diff --quiet initial &&
>
> I'd drop "--quiet" here as it makes debugging easier if we can see the
> diff if the test fails.
This makes sense. In v3, I will update this new test as well as the
existing test to not use `--quiet`.
Combining the above suggestions, here's the version of the existing and
new tests that I intend to use in v3. Let me know if this isn't what you
had in mind!
test_expect_success 'cherry-pick on unborn branch' '
git switch --orphan unborn &&
git rm --cached -r . &&
git cherry-pick initial &&
git diff initial &&
test_cmp_rev ! initial HEAD
'
test_expect_success 'cherry-pick on unborn branch with --allow-empty' '
git checkout --detach &&
git branch -D unborn &&
git switch --orphan unborn &&
git cherry-pick initial --allow-empty &&
git diff initial &&
test_cmp_rev ! initial HEAD
'
--
Thank you,
Brian Lyles
^ permalink raw reply
* [PATCH v2 2/2] an improvement: removed configure.ac changes
From: Haritha D via GitGitGadget @ 2024-02-23 3:48 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Haritha, Haritha D
In-Reply-To: <pull.1663.v2.git.git.1708660111.gitgitgadget@gmail.com>
From: Haritha D <harithamma.d@ibm.com>
Hello Reviewers, as an improvement, I have
removed the CC_LD_DYNPATH changes because
I was able to achieve the desired outcome
by passing it to the make step.
Signed-off-by: Haritha D <harithamma.d@ibm.com>
---
configure.ac | 3 ---
1 file changed, 3 deletions(-)
diff --git a/configure.ac b/configure.ac
index 64569a80d53..d1a96da14eb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -463,9 +463,6 @@ else
CC_LD_DYNPATH=-Wl,+b,
else
CC_LD_DYNPATH=
- if test "$(uname -s)" = "OS/390"; then
- CC_LD_DYNPATH=-L
- fi
AC_MSG_WARN([linker does not support runtime path to dynamic libraries])
fi
fi
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 1/2] build: support z/OS (OS/390).
From: Haritha D via GitGitGadget @ 2024-02-23 3:48 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Haritha, Haritha D
In-Reply-To: <pull.1663.v2.git.git.1708660111.gitgitgadget@gmail.com>
From: Haritha D <harithamma.d@ibm.com>
Since the z/OS linker does not support searching dynamic libraries,
and the current setting of CC_LD_DYNPATH results in a directory
to be supplied to the link step with no option as the suffix,
it causes a linker error because the z/OS LD linker
does not accept directories as input.
Therefore, -L option is added.
Also introduced z/OS (OS/390) as a platform in config.mak.uname
Signed-off-by: Haritha D <harithamma.d@ibm.com>
---
config.mak.uname | 12 ++++++++++++
configure.ac | 3 +++
2 files changed, 15 insertions(+)
diff --git a/config.mak.uname b/config.mak.uname
index dacc95172dc..c8006f854e5 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -638,6 +638,18 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
SANE_TOOL_PATH = /usr/coreutils/bin:/usr/local/bin
SHELL_PATH = /usr/coreutils/bin/bash
endif
+ifeq ($(uname_S),OS/390)
+ NO_SYS_POLL_H = YesPlease
+ NO_STRCASESTR = YesPlease
+ NO_REGEX = YesPlease
+ NO_MMAP = YesPlease
+ NO_NSEC = YesPlease
+ NO_STRLCPY = YesPlease
+ NO_MEMMEM = YesPlease
+ NO_GECOS_IN_PWENT = YesPlease
+ HAVE_STRINGS_H = YesPlease
+ NEEDS_MODE_TRANSLATION = YesPlease
+endif
ifeq ($(uname_S),MINGW)
ifeq ($(shell expr "$(uname_R)" : '1\.'),2)
$(error "Building with MSys is no longer supported")
diff --git a/configure.ac b/configure.ac
index d1a96da14eb..64569a80d53 100644
--- a/configure.ac
+++ b/configure.ac
@@ -463,6 +463,9 @@ else
CC_LD_DYNPATH=-Wl,+b,
else
CC_LD_DYNPATH=
+ if test "$(uname -s)" = "OS/390"; then
+ CC_LD_DYNPATH=-L
+ fi
AC_MSG_WARN([linker does not support runtime path to dynamic libraries])
fi
fi
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 0/2] This PR enables a successful git build on z/OS.
From: Haritha via GitGitGadget @ 2024-02-23 3:48 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Haritha
In-Reply-To: <pull.1663.git.git.1706710861778.gitgitgadget@gmail.com>
Since the z/OS linker does not support searching dynamic libraries, and the
current setting of CC_LD_DYNPATH results in a directory to be supplied to
the link step with no option as the suffix, it causes a linker error because
the z/OS LD linker does not accept directories as input. Therefore, we
workaround this by adding the -L option. And, Introduced z/OS (OS/390) as a
platform in config.mak.uname
Thanks for taking the time to contribute to Git! Please be advised that the
Git community does not use github.com for their contributions. Instead, we
use a mailing list (git@vger.kernel.org) for code submissions, code reviews,
and bug reports. Nevertheless, you can use GitGitGadget
(https://gitgitgadget.github.io/) to conveniently send your Pull Requests
commits to our mailing list.
Please read the "guidelines for contributing" linked above!
Haritha D (2):
build: support z/OS (OS/390).
an improvement: removed configure.ac changes
config.mak.uname | 12 ++++++++++++
1 file changed, 12 insertions(+)
base-commit: f41f85c9ec8d4d46de0fd5fded88db94d3ec8c11
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1663%2FHarithaIBM%2Fzos-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1663/HarithaIBM/zos-v2
Pull-Request: https://github.com/git/git/pull/1663
Range-diff vs v1:
1: 2eddc8f8860 ! 1: 53e211d7a65 This PR enables a successful git build on z/OS.
@@ Metadata
Author: Haritha D <harithamma.d@ibm.com>
## Commit message ##
- This PR enables a successful git build on z/OS.
+ build: support z/OS (OS/390).
Since the z/OS linker does not support searching dynamic libraries,
and the current setting of CC_LD_DYNPATH results in a directory
to be supplied to the link step with no option as the suffix,
it causes a linker error because the z/OS LD linker
does not accept directories as input.
- Therefore, we workaround this by adding the -L option.
- And, Introduced z/OS (OS/390) as a platform in config.mak.uname
+ Therefore, -L option is added.
+ Also introduced z/OS (OS/390) as a platform in config.mak.uname
Signed-off-by: Haritha D <harithamma.d@ibm.com>
-: ----------- > 2: 05df5d7e2d5 an improvement: removed configure.ac changes
--
gitgitgadget
^ permalink raw reply
* [PATCH v2 16/16] t7527: update case-insenstive fsmonitor test
From: Jeff Hostetler via GitGitGadget @ 2024-02-23 3:18 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Jeff Hostetler, Jeff Hostetler,
Jeff Hostetler
In-Reply-To: <pull.1662.v2.git.1708658300.gitgitgadget@gmail.com>
From: Jeff Hostetler <jeffhostetler@github.com>
Now that the FSMonitor client has been updated to better
handle events on case-insenstive file systems, update the
two tests that demonstrated the bug and remove the temporary
SKIPME prereq.
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
---
t/t7527-builtin-fsmonitor.sh | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
index 4acb547819c..939521a0fac 100755
--- a/t/t7527-builtin-fsmonitor.sh
+++ b/t/t7527-builtin-fsmonitor.sh
@@ -1051,7 +1051,7 @@ test_expect_success 'split-index and FSMonitor work well together' '
#
# The setup is a little contrived.
#
-test_expect_success SKIPME,CASE_INSENSITIVE_FS 'fsmonitor subdir case wrong on disk' '
+test_expect_success CASE_INSENSITIVE_FS 'fsmonitor subdir case wrong on disk' '
test_when_finished "stop_daemon_delete_repo subdir_case_wrong" &&
git init subdir_case_wrong &&
@@ -1116,19 +1116,20 @@ test_expect_success SKIPME,CASE_INSENSITIVE_FS 'fsmonitor subdir case wrong on d
grep -q "dir1/DIR2/dir3/file3.*pos -3" "$PWD/subdir_case_wrong.log1" &&
+ # Also verify that we get a mapping event to correct the case.
+ grep -q "MAP:.*dir1/DIR2/dir3/file3.*dir1/dir2/dir3/file3" \
+ "$PWD/subdir_case_wrong.log1" &&
+
# The refresh-callbacks should have caused "git status" to clear
# the CE_FSMONITOR_VALID bit on each of those files and caused
# the worktree scan to visit them and mark them as modified.
grep -q " M AAA" "$PWD/subdir_case_wrong.out" &&
grep -q " M zzz" "$PWD/subdir_case_wrong.out" &&
- # However, with the fsmonitor client bug, the "(pos -3)" causes
- # the client to not update the bit and never rescan the file
- # and therefore not report it as dirty.
- ! grep -q " M dir1/dir2/dir3/file3" "$PWD/subdir_case_wrong.out"
+ grep -q " M dir1/dir2/dir3/file3" "$PWD/subdir_case_wrong.out"
'
-test_expect_success SKIPME,CASE_INSENSITIVE_FS 'fsmonitor file case wrong on disk' '
+test_expect_success CASE_INSENSITIVE_FS 'fsmonitor file case wrong on disk' '
test_when_finished "stop_daemon_delete_repo file_case_wrong" &&
git init file_case_wrong &&
@@ -1246,12 +1247,14 @@ test_expect_success SKIPME,CASE_INSENSITIVE_FS 'fsmonitor file case wrong on dis
grep -q "fsmonitor_refresh_callback.*FILE-3-A.*pos -3" "$PWD/file_case_wrong-try3.log" &&
grep -q "fsmonitor_refresh_callback.*file-4-a.*pos -9" "$PWD/file_case_wrong-try3.log" &&
- # Status should say these files are modified, but with the case
- # bug, the "pos -3" cause the client to not update the FSM bit
- # and never cause the file to be rescanned and therefore to not
- # report it dirty.
- ! grep -q " M dir1/dir2/dir3/file-3-a" "$PWD/file_case_wrong-try3.out" &&
- ! grep -q " M dir1/dir2/dir4/FILE-4-A" "$PWD/file_case_wrong-try3.out"
+ # Also verify that we get a mapping event to correct the case.
+ grep -q "fsmonitor_refresh_callback MAP:.*dir1/dir2/dir3/FILE-3-A.*dir1/dir2/dir3/file-3-a" \
+ "$PWD/file_case_wrong-try3.log" &&
+ grep -q "fsmonitor_refresh_callback MAP:.*dir1/dir2/dir4/file-4-a.*dir1/dir2/dir4/FILE-4-A" \
+ "$PWD/file_case_wrong-try3.log" &&
+
+ grep -q " M dir1/dir2/dir3/file-3-a" "$PWD/file_case_wrong-try3.out" &&
+ grep -q " M dir1/dir2/dir4/FILE-4-A" "$PWD/file_case_wrong-try3.out"
'
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 15/16] fsmonitor: refactor bit invalidation in refresh callback
From: Jeff Hostetler via GitGitGadget @ 2024-02-23 3:18 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Jeff Hostetler, Jeff Hostetler,
Jeff Hostetler
In-Reply-To: <pull.1662.v2.git.1708658300.gitgitgadget@gmail.com>
From: Jeff Hostetler <jeffhostetler@github.com>
Refactor code in the fsmonitor_refresh_callback() call chain dealing
with invalidating the CE_FSMONITOR_VALID bit and add a trace message.
During the refresh, we clear the CE_FSMONITOR_VALID bit in response to
data from the FSMonitor daemon (so that a later phase will lstat() and
verify the true state of the file).
Create a new function to clear the bit and add some unique tracing for
it to help debug edge cases.
This is similar to the existing `mark_fsmonitor_invalid()` function,
but we don't need the extra stuff that it does.
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
---
fsmonitor.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/fsmonitor.c b/fsmonitor.c
index ac638a61c00..0667a8c297c 100644
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -187,6 +187,20 @@ static int query_fsmonitor_hook(struct repository *r,
static size_t handle_path_with_trailing_slash(
struct index_state *istate, const char *name, int pos);
+/*
+ * Invalidate the FSM bit on this CE. This is like mark_fsmonitor_invalid()
+ * but we've already handled the untracked-cache and I want a different
+ * trace message.
+ */
+static void invalidate_ce_fsm(struct cache_entry *ce)
+{
+ if (ce->ce_flags & CE_FSMONITOR_VALID)
+ trace_printf_key(&trace_fsmonitor,
+ "fsmonitor_refresh_callback INV: '%s'",
+ ce->name);
+ ce->ce_flags &= ~CE_FSMONITOR_VALID;
+}
+
/*
* Use the name-hash to do a case-insensitive cache-entry lookup with
* the pathname and invalidate the cache-entry.
@@ -224,7 +238,7 @@ static size_t handle_using_name_hash_icase(
untracked_cache_invalidate_trimmed_path(istate, ce->name, 0);
- ce->ce_flags &= ~CE_FSMONITOR_VALID;
+ invalidate_ce_fsm(ce);
return 1;
}
@@ -316,7 +330,7 @@ static size_t handle_path_without_trailing_slash(
* cache-entry with the same pathname, nor for a cone
* at that directory. (That is, assume no D/F conflicts.)
*/
- istate->cache[pos]->ce_flags &= ~CE_FSMONITOR_VALID;
+ invalidate_ce_fsm(istate->cache[pos]);
return 1;
} else {
size_t nr_in_cone;
@@ -394,7 +408,7 @@ static size_t handle_path_with_trailing_slash(
for (i = pos; i < istate->cache_nr; i++) {
if (!starts_with(istate->cache[i]->name, name))
break;
- istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID;
+ invalidate_ce_fsm(istate->cache[i]);
nr_in_cone++;
}
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 14/16] fsmonitor: support case-insensitive events
From: Jeff Hostetler via GitGitGadget @ 2024-02-23 3:18 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Jeff Hostetler, Jeff Hostetler,
Jeff Hostetler
In-Reply-To: <pull.1662.v2.git.1708658300.gitgitgadget@gmail.com>
From: Jeff Hostetler <jeffhostetler@github.com>
Teach fsmonitor_refresh_callback() to handle case-insensitive
lookups if case-sensitive lookups fail on case-insensitive systems.
This can cause 'git status' to report stale status for files if there
are case issues/errors in the worktree.
The FSMonitor daemon sends FSEvents using the observed spelling
of each pathname. On case-insensitive file systems this may be
different than the expected case spelling.
The existing code uses index_name_pos() to find the cache-entry for
the pathname in the FSEvent and clear the CE_FSMONITOR_VALID bit so
that the worktree scan/index refresh will revisit and revalidate the
path.
On a case-insensitive file system, the exact match lookup may fail
to find the associated cache-entry. This causes status to think that
the cached CE flags are correct and skip over the file.
Update event handling to optionally use the name-hash and dir-name-hash
if necessary.
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
---
fsmonitor.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 110 insertions(+)
diff --git a/fsmonitor.c b/fsmonitor.c
index 739ddbf7aca..ac638a61c00 100644
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -5,6 +5,7 @@
#include "ewah/ewok.h"
#include "fsmonitor.h"
#include "fsmonitor-ipc.h"
+#include "name-hash.h"
#include "run-command.h"
#include "strbuf.h"
#include "trace2.h"
@@ -186,6 +187,102 @@ static int query_fsmonitor_hook(struct repository *r,
static size_t handle_path_with_trailing_slash(
struct index_state *istate, const char *name, int pos);
+/*
+ * Use the name-hash to do a case-insensitive cache-entry lookup with
+ * the pathname and invalidate the cache-entry.
+ *
+ * Returns the number of cache-entries that we invalidated.
+ */
+static size_t handle_using_name_hash_icase(
+ struct index_state *istate, const char *name)
+{
+ struct cache_entry *ce = NULL;
+
+ ce = index_file_exists(istate, name, strlen(name), 1);
+ if (!ce)
+ return 0;
+
+ /*
+ * A case-insensitive search in the name-hash using the
+ * observed pathname found a cache-entry, so the observed path
+ * is case-incorrect. Invalidate the cache-entry and use the
+ * correct spelling from the cache-entry to invalidate the
+ * untracked-cache. Since we now have sparse-directories in
+ * the index, the observed pathname may represent a regular
+ * file or a sparse-index directory.
+ *
+ * Note that we should not have seen FSEvents for a
+ * sparse-index directory, but we handle it just in case.
+ *
+ * Either way, we know that there are not any cache-entries for
+ * children inside the cone of the directory, so we don't need to
+ * do the usual scan.
+ */
+ trace_printf_key(&trace_fsmonitor,
+ "fsmonitor_refresh_callback MAP: '%s' '%s'",
+ name, ce->name);
+
+ untracked_cache_invalidate_trimmed_path(istate, ce->name, 0);
+
+ ce->ce_flags &= ~CE_FSMONITOR_VALID;
+ return 1;
+}
+
+/*
+ * Use the dir-name-hash to find the correct-case spelling of the
+ * directory. Use the canonical spelling to invalidate all of the
+ * cache-entries within the matching cone.
+ *
+ * Returns the number of cache-entries that we invalidated.
+ */
+static size_t handle_using_dir_name_hash_icase(
+ struct index_state *istate, const char *name)
+{
+ struct strbuf canonical_path = STRBUF_INIT;
+ int pos;
+ size_t len = strlen(name);
+ size_t nr_in_cone;
+
+ if (name[len - 1] == '/')
+ len--;
+
+ if (!index_dir_find(istate, name, len, &canonical_path))
+ return 0; /* name is untracked */
+
+ if (!memcmp(name, canonical_path.buf, canonical_path.len)) {
+ strbuf_release(&canonical_path);
+ /*
+ * NEEDSWORK: Our caller already tried an exact match
+ * and failed to find one. They called us to do an
+ * ICASE match, so we should never get an exact match,
+ * so we could promote this to a BUG() here if we
+ * wanted to. It doesn't hurt anything to just return
+ * 0 and go on becaus we should never get here. Or we
+ * could just get rid of the memcmp() and this "if"
+ * clause completely.
+ */
+ return 0; /* should not happen */
+ }
+
+ trace_printf_key(&trace_fsmonitor,
+ "fsmonitor_refresh_callback MAP: '%s' '%s'",
+ name, canonical_path.buf);
+
+ /*
+ * The dir-name-hash only tells us the corrected spelling of
+ * the prefix. We have to use this canonical path to do a
+ * lookup in the cache-entry array so that we repeat the
+ * original search using the case-corrected spelling.
+ */
+ strbuf_addch(&canonical_path, '/');
+ pos = index_name_pos(istate, canonical_path.buf,
+ canonical_path.len);
+ nr_in_cone = handle_path_with_trailing_slash(
+ istate, canonical_path.buf, pos);
+ strbuf_release(&canonical_path);
+ return nr_in_cone;
+}
+
/*
* The daemon sent an observed pathname without a trailing slash.
* (This is the normal case.) We do not know if it is a tracked or
@@ -319,6 +416,19 @@ static void fsmonitor_refresh_callback(struct index_state *istate, char *name)
else
nr_in_cone = handle_path_without_trailing_slash(istate, name, pos);
+ /*
+ * If we did not find an exact match for this pathname or any
+ * cache-entries with this directory prefix and we're on a
+ * case-insensitive file system, try again using the name-hash
+ * and dir-name-hash.
+ */
+ if (!nr_in_cone && ignore_case) {
+ nr_in_cone = handle_using_name_hash_icase(istate, name);
+ if (!nr_in_cone)
+ nr_in_cone = handle_using_dir_name_hash_icase(
+ istate, name);
+ }
+
if (nr_in_cone)
trace_printf_key(&trace_fsmonitor,
"fsmonitor_refresh_callback CNT: %d",
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 12/16] fsmonitor: return invalided cache-entry count on non-directory event
From: Jeff Hostetler via GitGitGadget @ 2024-02-23 3:18 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Jeff Hostetler, Jeff Hostetler,
Jeff Hostetler
In-Reply-To: <pull.1662.v2.git.1708658300.gitgitgadget@gmail.com>
From: Jeff Hostetler <jeffhostetler@github.com>
Teah the refresh callback helper function for unqualified FSEvents
(pathnames without a trailing slash) to return the number of
cache-entries that were invalided in response to the event.
This will be used in a later commit to help determine if the observed
pathname was (possibly) case-incorrect when (on a case-insensitive
file system).
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
---
fsmonitor.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/fsmonitor.c b/fsmonitor.c
index a51c17cda70..c16ed5d8758 100644
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -196,8 +196,10 @@ static size_t handle_path_with_trailing_slash(
* do not know it is case-correct or -incorrect.
*
* Assume it is case-correct and try an exact match.
+ *
+ * Return the number of cache-entries that we invalidated.
*/
-static void handle_path_without_trailing_slash(
+static size_t handle_path_without_trailing_slash(
struct index_state *istate, const char *name, int pos)
{
/*
@@ -218,7 +220,9 @@ static void handle_path_without_trailing_slash(
* at that directory. (That is, assume no D/F conflicts.)
*/
istate->cache[pos]->ce_flags &= ~CE_FSMONITOR_VALID;
+ return 1;
} else {
+ size_t nr_in_cone;
struct strbuf work_path = STRBUF_INIT;
/*
@@ -232,8 +236,10 @@ static void handle_path_without_trailing_slash(
strbuf_add(&work_path, name, strlen(name));
strbuf_addch(&work_path, '/');
pos = index_name_pos(istate, work_path.buf, work_path.len);
- handle_path_with_trailing_slash(istate, work_path.buf, pos);
+ nr_in_cone = handle_path_with_trailing_slash(
+ istate, work_path.buf, pos);
strbuf_release(&work_path);
+ return nr_in_cone;
}
}
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 13/16] fsmonitor: trace the new invalidated cache-entry count
From: Jeff Hostetler via GitGitGadget @ 2024-02-23 3:18 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Jeff Hostetler, Jeff Hostetler,
Jeff Hostetler
In-Reply-To: <pull.1662.v2.git.1708658300.gitgitgadget@gmail.com>
From: Jeff Hostetler <jeffhostetler@github.com>
Consolidate the directory/non-directory calls to the refresh handler
code. Log the resulting count of invalidated cache-entries.
The nr_in_cone value will be used in a later commit to decide if
we also need to try to do case-insensitive lookups.
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
---
fsmonitor.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/fsmonitor.c b/fsmonitor.c
index c16ed5d8758..739ddbf7aca 100644
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -308,16 +308,21 @@ static void fsmonitor_refresh_callback(struct index_state *istate, char *name)
{
int len = strlen(name);
int pos = index_name_pos(istate, name, len);
+ size_t nr_in_cone;
trace_printf_key(&trace_fsmonitor,
"fsmonitor_refresh_callback '%s' (pos %d)",
name, pos);
- if (name[len - 1] == '/') {
- handle_path_with_trailing_slash(istate, name, pos);
- } else {
- handle_path_without_trailing_slash(istate, name, pos);
- }
+ if (name[len - 1] == '/')
+ nr_in_cone = handle_path_with_trailing_slash(istate, name, pos);
+ else
+ nr_in_cone = handle_path_without_trailing_slash(istate, name, pos);
+
+ if (nr_in_cone)
+ trace_printf_key(&trace_fsmonitor,
+ "fsmonitor_refresh_callback CNT: %d",
+ (int)nr_in_cone);
}
/*
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 11/16] fsmonitor: remove custom loop from non-directory path handler
From: Jeff Hostetler via GitGitGadget @ 2024-02-23 3:18 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Jeff Hostetler, Jeff Hostetler,
Jeff Hostetler
In-Reply-To: <pull.1662.v2.git.1708658300.gitgitgadget@gmail.com>
From: Jeff Hostetler <jeffhostetler@github.com>
Refactor the code that handles refresh events for pathnames that do
not contain a trailing slash. Instead of using a custom loop to try
to scan the index and detect if the FSEvent named a file or might be a
directory prefix, use the recently created helper function to do that.
Also update the comments to describe what and why we are doing this.
On platforms that DO NOT annotate FS events with a trailing
slash, if we fail to find an exact match for the pathname
in the index, we do not know if the pathname represents a
directory or simply an untracked file. Pretend that the pathname
is a directory and try again before assuming it is an untracked
file.
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
---
fsmonitor.c | 55 ++++++++++++++++++++++++++++++-----------------------
1 file changed, 31 insertions(+), 24 deletions(-)
diff --git a/fsmonitor.c b/fsmonitor.c
index 9424bd17230..a51c17cda70 100644
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -183,11 +183,23 @@ static int query_fsmonitor_hook(struct repository *r,
return result;
}
+static size_t handle_path_with_trailing_slash(
+ struct index_state *istate, const char *name, int pos);
+
+/*
+ * The daemon sent an observed pathname without a trailing slash.
+ * (This is the normal case.) We do not know if it is a tracked or
+ * untracked file, a sparse-directory, or a populated directory (on a
+ * platform such as Windows where FSEvents are not qualified).
+ *
+ * The pathname contains the observed case reported by the FS. We
+ * do not know it is case-correct or -incorrect.
+ *
+ * Assume it is case-correct and try an exact match.
+ */
static void handle_path_without_trailing_slash(
struct index_state *istate, const char *name, int pos)
{
- int i;
-
/*
* Mark the untracked cache dirty for this path (regardless of
* whether or not we find an exact match for it in the index).
@@ -200,33 +212,28 @@ static void handle_path_without_trailing_slash(
if (pos >= 0) {
/*
- * We have an exact match for this path and can just
- * invalidate it.
+ * An exact match on a tracked file. We assume that we
+ * do not need to scan forward for a sparse-directory
+ * cache-entry with the same pathname, nor for a cone
+ * at that directory. (That is, assume no D/F conflicts.)
*/
istate->cache[pos]->ce_flags &= ~CE_FSMONITOR_VALID;
} else {
+ struct strbuf work_path = STRBUF_INIT;
+
/*
- * The path is not a tracked file -or- it is a
- * directory event on a platform that cannot
- * distinguish between file and directory events in
- * the event handler, such as Windows.
- *
- * Scan as if it is a directory and invalidate the
- * cone under it. (But remember to ignore items
- * between "name" and "name/", such as "name-" and
- * "name.".
+ * The negative "pos" gives us the suggested insertion
+ * point for the pathname (without the trailing slash).
+ * We need to see if there is a directory with that
+ * prefix, but there can be lots of pathnames between
+ * "foo" and "foo/" like "foo-" or "foo-bar", so we
+ * don't want to do our own scan.
*/
- int len = strlen(name);
- pos = -pos - 1;
-
- for (i = pos; i < istate->cache_nr; i++) {
- if (!starts_with(istate->cache[i]->name, name))
- break;
- if ((unsigned char)istate->cache[i]->name[len] > '/')
- break;
- if (istate->cache[i]->name[len] == '/')
- istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID;
- }
+ strbuf_add(&work_path, name, strlen(name));
+ strbuf_addch(&work_path, '/');
+ pos = index_name_pos(istate, work_path.buf, work_path.len);
+ handle_path_with_trailing_slash(istate, work_path.buf, pos);
+ strbuf_release(&work_path);
}
}
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 10/16] fsmonitor: return invalidated cache-entry count on directory event
From: Jeff Hostetler via GitGitGadget @ 2024-02-23 3:18 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Jeff Hostetler, Jeff Hostetler,
Jeff Hostetler
In-Reply-To: <pull.1662.v2.git.1708658300.gitgitgadget@gmail.com>
From: Jeff Hostetler <jeffhostetler@github.com>
Teach the refresh callback helper function for directory FSEvents to
return the number of cache-entries that were invalidated in response
to a directory event.
This will be used in a later commit to help determine if the observed
pathname in the FSEvent was a (possibly) case-incorrect directory
prefix (on a case-insensitive filesystem) of one or more actual
cache-entries.
If there exists at least one case-insensitive prefix match, then we
can assume that the directory is a (case-incorrect) prefix of at least
one tracked item rather than a completely unknown/untracked file or
directory.
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
---
fsmonitor.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/fsmonitor.c b/fsmonitor.c
index 2f58ee2fe5a..9424bd17230 100644
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -253,11 +253,20 @@ static void handle_path_without_trailing_slash(
* same way and just invalidate the cache-entry and the untracked
* cache (and in this case, the forward cache-entry scan won't find
* anything and it doesn't hurt to let it run).
+ *
+ * Return the number of cache-entries that we invalidated. We will
+ * use this later to determine if we need to attempt a second
+ * case-insensitive search on case-insensitive file systems. That is,
+ * if the search using the observed-case in the FSEvent yields any
+ * results, we assume the prefix is case-correct. If there are no
+ * matches, we still don't know if the observed path is simply
+ * untracked or case-incorrect.
*/
-static void handle_path_with_trailing_slash(
+static size_t handle_path_with_trailing_slash(
struct index_state *istate, const char *name, int pos)
{
int i;
+ size_t nr_in_cone = 0;
/*
* Mark the untracked cache dirty for this directory path
@@ -276,7 +285,10 @@ static void handle_path_with_trailing_slash(
if (!starts_with(istate->cache[i]->name, name))
break;
istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID;
+ nr_in_cone++;
}
+
+ return nr_in_cone;
}
static void fsmonitor_refresh_callback(struct index_state *istate, char *name)
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 09/16] fsmonitor: move untracked invalidation into helper functions
From: Jeff Hostetler via GitGitGadget @ 2024-02-23 3:18 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Jeff Hostetler, Jeff Hostetler,
Jeff Hostetler
In-Reply-To: <pull.1662.v2.git.1708658300.gitgitgadget@gmail.com>
From: Jeff Hostetler <jeffhostetler@github.com>
Move the call to invalidate the untracked cache for the FSEvent
pathname into the two helper functions.
In a later commit in this series, we will call these helpers
from other contexts and it safer to include the UC invalidation
in the helper than to remember to also add it to each helper
call-site.
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
---
fsmonitor.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/fsmonitor.c b/fsmonitor.c
index 2787f7ca5d1..2f58ee2fe5a 100644
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -188,6 +188,16 @@ static void handle_path_without_trailing_slash(
{
int i;
+ /*
+ * Mark the untracked cache dirty for this path (regardless of
+ * whether or not we find an exact match for it in the index).
+ * Since the path is unqualified (no trailing slash hint in the
+ * FSEvent), it may refer to a file or directory. So we should
+ * not assume one or the other and should always let the untracked
+ * cache decide what needs to invalidated.
+ */
+ untracked_cache_invalidate_trimmed_path(istate, name, 0);
+
if (pos >= 0) {
/*
* We have an exact match for this path and can just
@@ -249,6 +259,15 @@ static void handle_path_with_trailing_slash(
{
int i;
+ /*
+ * Mark the untracked cache dirty for this directory path
+ * (regardless of whether or not we find an exact match for it
+ * in the index or find it to be proper prefix of one or more
+ * files in the index), since the FSEvent is hinting that
+ * there may be changes on or within the directory.
+ */
+ untracked_cache_invalidate_trimmed_path(istate, name, 0);
+
if (pos < 0)
pos = -pos - 1;
@@ -274,13 +293,6 @@ static void fsmonitor_refresh_callback(struct index_state *istate, char *name)
} else {
handle_path_without_trailing_slash(istate, name, pos);
}
-
- /*
- * Mark the untracked cache dirty even if it wasn't found in the index
- * as it could be a new untracked file. (Let the untracked cache
- * layer silently deal with any trailing slash.)
- */
- untracked_cache_invalidate_trimmed_path(istate, name, 0);
}
/*
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 08/16] fsmonitor: refactor untracked-cache invalidation
From: Jeff Hostetler via GitGitGadget @ 2024-02-23 3:18 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Jeff Hostetler, Jeff Hostetler,
Jeff Hostetler
In-Reply-To: <pull.1662.v2.git.1708658300.gitgitgadget@gmail.com>
From: Jeff Hostetler <jeffhostetler@github.com>
Update fsmonitor_refresh_callback() to use the new
untracked_cache_invalidate_trimmed_path() to invalidate
the cache using the observed pathname without needing to
modify the caller's buffer.
Previously, we modified the caller's buffer when the observed pathname
contained a trailing slash (and did not restore it). This wasn't a
problem for the single use-case caller, but felt dirty nontheless. In
a later commit we will want to invalidate case-corrected versions of
the pathname (using possibly borrowed pathnames from the name-hash or
dir-name-hash) and we may not want to keep the tradition of altering
the passed-in pathname.
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
---
fsmonitor.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/fsmonitor.c b/fsmonitor.c
index 364198d258f..2787f7ca5d1 100644
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -271,21 +271,16 @@ static void fsmonitor_refresh_callback(struct index_state *istate, char *name)
if (name[len - 1] == '/') {
handle_path_with_trailing_slash(istate, name, pos);
-
- /*
- * We need to remove the traling "/" from the path
- * for the untracked cache.
- */
- name[len - 1] = '\0';
} else {
handle_path_without_trailing_slash(istate, name, pos);
}
/*
* Mark the untracked cache dirty even if it wasn't found in the index
- * as it could be a new untracked file.
+ * as it could be a new untracked file. (Let the untracked cache
+ * layer silently deal with any trailing slash.)
*/
- untracked_cache_invalidate_path(istate, name, 0);
+ untracked_cache_invalidate_trimmed_path(istate, name, 0);
}
/*
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 07/16] dir: create untracked_cache_invalidate_trimmed_path()
From: Jeff Hostetler via GitGitGadget @ 2024-02-23 3:18 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Jeff Hostetler, Jeff Hostetler,
Jeff Hostetler
In-Reply-To: <pull.1662.v2.git.1708658300.gitgitgadget@gmail.com>
From: Jeff Hostetler <jeffhostetler@github.com>
Create a wrapper function for untracked_cache_invalidate_path()
that silently trims a trailing slash, if present, before calling
the wrapped function.
The untracked cache expects to be called with a pathname that
does not contain a trailing slash. This can make it inconvenient
for callers that have a directory path. Lets hide this complexity.
This will be used by a later commit in the FSMonitor code which
may receive directory pathnames from an FSEvent.
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
---
dir.c | 20 ++++++++++++++++++++
dir.h | 7 +++++++
2 files changed, 27 insertions(+)
diff --git a/dir.c b/dir.c
index ac699542302..1157f3e43fa 100644
--- a/dir.c
+++ b/dir.c
@@ -3918,6 +3918,26 @@ void untracked_cache_invalidate_path(struct index_state *istate,
path, strlen(path));
}
+void untracked_cache_invalidate_trimmed_path(struct index_state *istate,
+ const char *path,
+ int safe_path)
+{
+ size_t len = strlen(path);
+
+ if (!len)
+ return; /* should not happen */
+
+ if (path[len - 1] != '/') {
+ untracked_cache_invalidate_path(istate, path, safe_path);
+ } else {
+ struct strbuf tmp = STRBUF_INIT;
+
+ strbuf_add(&tmp, path, len - 1);
+ untracked_cache_invalidate_path(istate, tmp.buf, safe_path);
+ strbuf_release(&tmp);
+ }
+}
+
void untracked_cache_remove_from_index(struct index_state *istate,
const char *path)
{
diff --git a/dir.h b/dir.h
index 98aa85fcc0e..45a7b9ec5f2 100644
--- a/dir.h
+++ b/dir.h
@@ -576,6 +576,13 @@ int cmp_dir_entry(const void *p1, const void *p2);
int check_dir_entry_contains(const struct dir_entry *out, const struct dir_entry *in);
void untracked_cache_invalidate_path(struct index_state *, const char *, int safe_path);
+/*
+ * Invalidate the untracked-cache for this path, but first strip
+ * off a trailing slash, if present.
+ */
+void untracked_cache_invalidate_trimmed_path(struct index_state *,
+ const char *path,
+ int safe_path);
void untracked_cache_remove_from_index(struct index_state *, const char *);
void untracked_cache_add_to_index(struct index_state *, const char *);
--
gitgitgadget
^ 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