* Orphan branch not well-defined?
From: Craig H Maynard @ 2023-11-22 0:28 UTC (permalink / raw)
To: Git Community
Team,
Recently I tried creating an orphan branch in an existing repo using git-checkout and git-switch.
Both commands have an --orphan option.
The results were different:
git checkout --orphan <newbranch> retained the entire working tree, including subdirectories and their contents.
git switch --orphan <newbranch> retained subdirectories but NOT their content.
Leaving aside the question of whether or not this is a bug, there doesn't appear to be any formal definition of the term "orphan branch" in the git documentation. Am I missing something?
Thanks,
Craig
^ permalink raw reply
* [PATCH v2 6/6] t7420: test that we correctly handle renamed submodules
From: Jan Alexander Steffens (heftig) @ 2023-11-21 20:32 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Shourya Shukla,
Ævar Arnfjörð Bjarmason, Denton Liu,
Jan Alexander Steffens (heftig)
In-Reply-To: <20231121203413.176414-1-heftig@archlinux.org>
Create a second submodule with a name that differs from its path. Test
that calling set-url modifies the correct .gitmodules entries. Make sure
we don't create a section named after the path instead of the name.
Signed-off-by: Jan Alexander Steffens (heftig) <heftig@archlinux.org>
---
t/t7420-submodule-set-url.sh | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/t/t7420-submodule-set-url.sh b/t/t7420-submodule-set-url.sh
index aa63d806fe..bf7f15ee79 100755
--- a/t/t7420-submodule-set-url.sh
+++ b/t/t7420-submodule-set-url.sh
@@ -25,34 +25,56 @@ test_expect_success 'submodule config cache setup' '
git add file &&
git commit -ma
) &&
+ mkdir namedsubmodule &&
+ (
+ cd namedsubmodule &&
+ git init &&
+ echo 1 >file &&
+ git add file &&
+ git commit -m1
+ ) &&
mkdir super &&
(
cd super &&
git init &&
git submodule add ../submodule &&
- git commit -m "add submodule"
+ git submodule add --name thename ../namedsubmodule thepath &&
+ git commit -m "add submodules"
)
'
test_expect_success 'test submodule set-url' '
- # add a commit and move the submodule (change the url)
+ # add commits and move the submodules (change the urls)
(
cd submodule &&
echo b >>file &&
git add file &&
git commit -mb
) &&
mv submodule newsubmodule &&
+ (
+ cd namedsubmodule &&
+ echo 2 >>file &&
+ git add file &&
+ git commit -m2
+ ) &&
+ mv namedsubmodule newnamedsubmodule &&
+
git -C newsubmodule show >expect &&
+ git -C newnamedsubmodule show >>expect &&
(
cd super &&
test_must_fail git submodule update --remote &&
git submodule set-url submodule ../newsubmodule &&
test_cmp_config ../newsubmodule -f .gitmodules submodule.submodule.url &&
+ git submodule set-url thepath ../newnamedsubmodule &&
+ test_cmp_config ../newnamedsubmodule -f .gitmodules submodule.thename.url &&
+ test_cmp_config "" -f .gitmodules --default "" submodule.thepath.url &&
git submodule update --remote
) &&
git -C super/submodule show >actual &&
+ git -C super/thepath show >>actual &&
test_cmp expect actual
'
--
2.43.0
^ permalink raw reply related
* [PATCH v2 5/6] t7419: test that we correctly handle renamed submodules
From: Jan Alexander Steffens (heftig) @ 2023-11-21 20:32 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Shourya Shukla,
Ævar Arnfjörð Bjarmason, Denton Liu,
Jan Alexander Steffens (heftig)
In-Reply-To: <20231121203413.176414-1-heftig@archlinux.org>
Add the submodule again with an explicitly different name and path. Test
that calling set-branch modifies the correct .gitmodules entries. Make
sure we don't create a section named after the path instead of the name.
Signed-off-by: Jan Alexander Steffens (heftig) <heftig@archlinux.org>
---
t/t7419-submodule-set-branch.sh | 30 +++++++++++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/t/t7419-submodule-set-branch.sh b/t/t7419-submodule-set-branch.sh
index 3cd30865a7..a5d1bc5c54 100755
--- a/t/t7419-submodule-set-branch.sh
+++ b/t/t7419-submodule-set-branch.sh
@@ -38,7 +38,8 @@ test_expect_success 'submodule config cache setup' '
(cd super &&
git init &&
git submodule add ../submodule &&
- git commit -m "add submodule"
+ git submodule add --name thename ../submodule thepath &&
+ git commit -m "add submodules"
)
'
@@ -100,4 +101,31 @@ test_expect_success 'test submodule set-branch -d' '
)
'
+test_expect_success 'test submodule set-branch --branch with named submodule' '
+ (cd super &&
+ git submodule set-branch --branch topic thepath &&
+ test_cmp_config topic -f .gitmodules submodule.thename.branch &&
+ test_cmp_config "" -f .gitmodules --default "" submodule.thepath.branch &&
+ git submodule update --remote &&
+ cat <<-\EOF >expect &&
+ b
+ EOF
+ git -C thepath show -s --pretty=%s >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'test submodule set-branch --default with named submodule' '
+ (cd super &&
+ git submodule set-branch --default thepath &&
+ test_cmp_config "" -f .gitmodules --default "" submodule.thename.branch &&
+ git submodule update --remote &&
+ cat <<-\EOF >expect &&
+ a
+ EOF
+ git -C thepath show -s --pretty=%s >actual &&
+ test_cmp expect actual
+ )
+'
+
test_done
--
2.43.0
^ permalink raw reply related
* [PATCH v2 3/6] t7419: actually test the branch switching
From: Jan Alexander Steffens (heftig) @ 2023-11-21 20:32 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Shourya Shukla,
Ævar Arnfjörð Bjarmason, Denton Liu,
Jan Alexander Steffens (heftig)
In-Reply-To: <20231121203413.176414-1-heftig@archlinux.org>
The submodule repo the test set up had the 'topic' branch checked out,
meaning the repo's default branch (HEAD) is the 'topic' branch.
The following tests then pretended to switch between the default branch
and the 'topic' branch. This was papered over by continually adding
commits to the 'topic' branch and checking if the submodule gets updated
to this new commit.
Return the submodule repo to the 'main' branch after setup so we can
actually test the switching behavior.
Signed-off-by: Jan Alexander Steffens (heftig) <heftig@archlinux.org>
---
Notes:
v2 changes:
- fixed subject
t/t7419-submodule-set-branch.sh | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/t/t7419-submodule-set-branch.sh b/t/t7419-submodule-set-branch.sh
index 232065504c..5ac16d0eb7 100755
--- a/t/t7419-submodule-set-branch.sh
+++ b/t/t7419-submodule-set-branch.sh
@@ -11,23 +11,28 @@ as expected.
TEST_PASSES_SANITIZE_LEAK=true
TEST_NO_CREATE_REPO=1
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
. ./test-lib.sh
test_expect_success 'setup' '
git config --global protocol.file.allow always
'
test_expect_success 'submodule config cache setup' '
mkdir submodule &&
(cd submodule &&
git init &&
echo a >a &&
git add . &&
git commit -ma &&
git checkout -b topic &&
echo b >a &&
git add . &&
- git commit -mb
+ git commit -mb &&
+ git checkout main
) &&
mkdir super &&
(cd super &&
@@ -57,41 +62,38 @@ test_expect_success 'test submodule set-branch --branch' '
'
test_expect_success 'test submodule set-branch --default' '
- test_commit -C submodule c &&
(cd super &&
git submodule set-branch --default submodule &&
! grep branch .gitmodules &&
git submodule update --remote &&
cat <<-\EOF >expect &&
- c
+ a
EOF
git -C submodule show -s --pretty=%s >actual &&
test_cmp expect actual
)
'
test_expect_success 'test submodule set-branch -b' '
- test_commit -C submodule b &&
(cd super &&
git submodule set-branch -b topic submodule &&
grep "branch = topic" .gitmodules &&
git submodule update --remote &&
cat <<-\EOF >expect &&
b
EOF
git -C submodule show -s --pretty=%s >actual &&
test_cmp expect actual
)
'
test_expect_success 'test submodule set-branch -d' '
- test_commit -C submodule d &&
(cd super &&
git submodule set-branch -d submodule &&
! grep branch .gitmodules &&
git submodule update --remote &&
cat <<-\EOF >expect &&
- d
+ a
EOF
git -C submodule show -s --pretty=%s >actual &&
test_cmp expect actual
--
2.43.0
^ permalink raw reply related
* [PATCH v2 4/6] t7419, t7420: use test_cmp_config instead of grepping .gitmodules
From: Jan Alexander Steffens (heftig) @ 2023-11-21 20:32 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Shourya Shukla,
Ævar Arnfjörð Bjarmason, Denton Liu,
Jan Alexander Steffens (heftig)
In-Reply-To: <20231121203413.176414-1-heftig@archlinux.org>
We have a test function to verify config files. Use it as it's more
precise.
Signed-off-by: Jan Alexander Steffens (heftig) <heftig@archlinux.org>
---
t/t7419-submodule-set-branch.sh | 10 +++++-----
t/t7420-submodule-set-url.sh | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/t/t7419-submodule-set-branch.sh b/t/t7419-submodule-set-branch.sh
index 5ac16d0eb7..3cd30865a7 100755
--- a/t/t7419-submodule-set-branch.sh
+++ b/t/t7419-submodule-set-branch.sh
@@ -44,53 +44,53 @@ test_expect_success 'submodule config cache setup' '
test_expect_success 'ensure submodule branch is unset' '
(cd super &&
- ! grep branch .gitmodules
+ test_cmp_config "" -f .gitmodules --default "" submodule.submodule.branch
)
'
test_expect_success 'test submodule set-branch --branch' '
(cd super &&
git submodule set-branch --branch topic submodule &&
- grep "branch = topic" .gitmodules &&
+ test_cmp_config topic -f .gitmodules submodule.submodule.branch &&
git submodule update --remote &&
cat <<-\EOF >expect &&
b
EOF
git -C submodule show -s --pretty=%s >actual &&
test_cmp expect actual
)
'
test_expect_success 'test submodule set-branch --default' '
(cd super &&
git submodule set-branch --default submodule &&
- ! grep branch .gitmodules &&
+ test_cmp_config "" -f .gitmodules --default "" submodule.submodule.branch &&
git submodule update --remote &&
cat <<-\EOF >expect &&
a
EOF
git -C submodule show -s --pretty=%s >actual &&
test_cmp expect actual
)
'
test_expect_success 'test submodule set-branch -b' '
(cd super &&
git submodule set-branch -b topic submodule &&
- grep "branch = topic" .gitmodules &&
+ test_cmp_config topic -f .gitmodules submodule.submodule.branch &&
git submodule update --remote &&
cat <<-\EOF >expect &&
b
EOF
git -C submodule show -s --pretty=%s >actual &&
test_cmp expect actual
)
'
test_expect_success 'test submodule set-branch -d' '
(cd super &&
git submodule set-branch -d submodule &&
- ! grep branch .gitmodules &&
+ test_cmp_config "" -f .gitmodules --default "" submodule.submodule.branch &&
git submodule update --remote &&
cat <<-\EOF >expect &&
a
diff --git a/t/t7420-submodule-set-url.sh b/t/t7420-submodule-set-url.sh
index d6bf62b3ac..aa63d806fe 100755
--- a/t/t7420-submodule-set-url.sh
+++ b/t/t7420-submodule-set-url.sh
@@ -49,7 +49,7 @@ test_expect_success 'test submodule set-url' '
cd super &&
test_must_fail git submodule update --remote &&
git submodule set-url submodule ../newsubmodule &&
- grep -F "url = ../newsubmodule" .gitmodules &&
+ test_cmp_config ../newsubmodule -f .gitmodules submodule.submodule.url &&
git submodule update --remote
) &&
git -C super/submodule show >actual &&
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/6] submodule--helper: return error from set-url when modifying failed
From: Jan Alexander Steffens (heftig) @ 2023-11-21 20:32 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Shourya Shukla,
Ævar Arnfjörð Bjarmason, Denton Liu,
Jan Alexander Steffens (heftig)
In-Reply-To: <20231121203413.176414-1-heftig@archlinux.org>
set-branch will return an error when setting the config fails so I don't
see why set-url shouldn't. Also skip the sync in this case.
Signed-off-by: Jan Alexander Steffens (heftig) <heftig@archlinux.org>
---
builtin/submodule--helper.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index af461ada8b..0013ea1ab0 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2889,39 +2889,41 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
static int module_set_url(int argc, const char **argv, const char *prefix)
{
- int quiet = 0;
+ int quiet = 0, ret;
const char *newurl;
const char *path;
char *config_name;
struct option options[] = {
OPT__QUIET(&quiet, N_("suppress output for setting url of a submodule")),
OPT_END()
};
const char *const usage[] = {
N_("git submodule set-url [--quiet] <path> <newurl>"),
NULL
};
const struct submodule *sub;
argc = parse_options(argc, argv, prefix, options, usage, 0);
if (argc != 2 || !(path = argv[0]) || !(newurl = argv[1]))
usage_with_options(usage, options);
sub = submodule_from_path(the_repository, null_oid(), path);
if (!sub)
die(_("no submodule mapping found in .gitmodules for path '%s'"),
path);
config_name = xstrfmt("submodule.%s.url", sub->name);
- config_set_in_gitmodules_file_gently(config_name, newurl);
+ ret = config_set_in_gitmodules_file_gently(config_name, newurl);
- repo_read_gitmodules(the_repository, 0);
- sync_submodule(path, prefix, NULL, quiet ? OPT_QUIET : 0);
+ if (!ret) {
+ repo_read_gitmodules(the_repository, 0);
+ sync_submodule(path, prefix, NULL, quiet ? OPT_QUIET : 0);
+ }
free(config_name);
- return 0;
+ return !!ret;
}
static int module_set_branch(int argc, const char **argv, const char *prefix)
--
2.43.0
^ permalink raw reply related
* [PATCH v2 1/6] submodule--helper: use submodule_from_path in set-{url,branch}
From: Jan Alexander Steffens (heftig) @ 2023-11-21 20:32 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Shourya Shukla,
Ævar Arnfjörð Bjarmason, Denton Liu,
Jan Alexander Steffens (heftig)
In-Reply-To: <20231003185047.2697995-1-heftig@archlinux.org>
The commands need a path to a submodule but treated it as the name when
modifying the .gitmodules file, leading to confusion when a submodule's
name does not match its path.
Because calling submodule_from_path initializes the submodule cache, we
need to manually trigger a reread before syncing, as the cache is
missing the config change we just made.
Signed-off-by: Jan Alexander Steffens (heftig) <heftig@archlinux.org>
---
Notes:
v2 changes:
- fixed code style
- replaced potentially unsafe use of `sub->path` with `path`
builtin/submodule--helper.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 6f3bf33e61..af461ada8b 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2901,19 +2901,26 @@ static int module_set_url(int argc, const char **argv, const char *prefix)
N_("git submodule set-url [--quiet] <path> <newurl>"),
NULL
};
+ const struct submodule *sub;
argc = parse_options(argc, argv, prefix, options, usage, 0);
if (argc != 2 || !(path = argv[0]) || !(newurl = argv[1]))
usage_with_options(usage, options);
- config_name = xstrfmt("submodule.%s.url", path);
+ sub = submodule_from_path(the_repository, null_oid(), path);
+ if (!sub)
+ die(_("no submodule mapping found in .gitmodules for path '%s'"),
+ path);
+
+ config_name = xstrfmt("submodule.%s.url", sub->name);
config_set_in_gitmodules_file_gently(config_name, newurl);
+
+ repo_read_gitmodules(the_repository, 0);
sync_submodule(path, prefix, NULL, quiet ? OPT_QUIET : 0);
free(config_name);
-
return 0;
}
@@ -2941,19 +2948,26 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)
N_("git submodule set-branch [-q|--quiet] (-b|--branch) <branch> <path>"),
NULL
};
+ const struct submodule *sub;
argc = parse_options(argc, argv, prefix, options, usage, 0);
if (!opt_branch && !opt_default)
die(_("--branch or --default required"));
if (opt_branch && opt_default)
die(_("options '%s' and '%s' cannot be used together"), "--branch", "--default");
if (argc != 1 || !(path = argv[0]))
usage_with_options(usage, options);
- config_name = xstrfmt("submodule.%s.branch", path);
+ sub = submodule_from_path(the_repository, null_oid(), path);
+
+ if (!sub)
+ die(_("no submodule mapping found in .gitmodules for path '%s'"),
+ path);
+
+ config_name = xstrfmt("submodule.%s.branch", sub->name);
ret = config_set_in_gitmodules_file_gently(config_name, opt_branch);
free(config_name);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2] merge-file: add --diff-algorithm option
From: Phillip Wood @ 2023-11-21 14:58 UTC (permalink / raw)
To: Antonin Delpeuch via GitGitGadget, git; +Cc: Antonin Delpeuch
In-Reply-To: <pull.1606.v2.git.git.1700507932937.gitgitgadget@gmail.com>
Hi Antonin
On 20/11/2023 19:18, Antonin Delpeuch via GitGitGadget wrote:
> From: Antonin Delpeuch <antonin@delpeuch.eu>
>
> This makes it possible to use other diff algorithms than the 'myers'
> default algorithm, when using the 'git merge-file' command. This helps
> avoid spurious conflicts by selecting a more recent algorithm such as
> 'histogram', for instance when using 'git merge-file' as part of a custom
> merge driver.
>
> Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu>
> Reviewed-by: Phillip Wood <phillip.wood@dunelm.org.uk>
This version looks good to me. Thanks for adding the tests and well done
for finding a test case that shows the benefits of changing the diff
algorithm so clearly.
For future reference note that the custom on this list is not to add
"Reviewed-by:" unless the reviewer explicitly suggests it. In this case
I'm happy for it to be left as is.
Best Wishes
Phillip
> ---
> merge-file: add --diff-algorithm option
>
> Changes since v1:
>
> * improve commit message to mention the use case of custom merge
> drivers
> * improve documentation to show available options and recommend
> switching to "histogram"
> * add tests
>
> I have left out:
>
> * switching the default to "histogram", because it should only be done
> in a subsequent release
> * adding a configuration variable to control this option, because I was
> not sure how to call it. Perhaps "merge-file.diffAlgorithm"?
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1606%2Fwetneb%2Fmerge_file_configurable_diff_algorithm-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1606/wetneb/merge_file_configurable_diff_algorithm-v2
> Pull-Request: https://github.com/git/git/pull/1606
>
> Range-diff vs v1:
>
> 1: 4aa453e30be ! 1: 842b5abf33c merge-file: add --diff-algorithm option
> @@ Commit message
> merge-file: add --diff-algorithm option
>
> This makes it possible to use other diff algorithms than the 'myers'
> - default algorithm, when using the 'git merge-file' command.
> + default algorithm, when using the 'git merge-file' command. This helps
> + avoid spurious conflicts by selecting a more recent algorithm such as
> + 'histogram', for instance when using 'git merge-file' as part of a custom
> + merge driver.
>
> Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu>
> + Reviewed-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> ## Documentation/git-merge-file.txt ##
> @@ Documentation/git-merge-file.txt: object store and the object ID of its blob is written to standard output.
> Instead of leaving conflicts in the file, resolve conflicts
> favouring our (or their or both) side of the lines.
>
> -+--diff-algorithm <algorithm>::
> -+ Use a different diff algorithm while merging, which can help
> ++--diff-algorithm={patience|minimal|histogram|myers}::
> ++ Use a different diff algorithm while merging. The current default is "myers",
> ++ but selecting more recent algorithm such as "histogram" can help
> + avoid mismerges that occur due to unimportant matching lines
> -+ (such as braces from distinct functions). See also
> ++ (such as braces from distinct functions). See also
> + linkgit:git-diff[1] `--diff-algorithm`.
>
> EXAMPLES
> @@ builtin/merge-file.c: int cmd_merge_file(int argc, const char **argv, const char
> OPT_INTEGER(0, "marker-size", &xmp.marker_size,
> N_("for conflicts, use this marker size")),
> OPT__QUIET(&quiet, N_("do not warn about conflicts")),
> +
> + ## t/t6403-merge-file.sh ##
> +@@ t/t6403-merge-file.sh: test_expect_success 'setup' '
> + deduxit me super semitas jusitiae,
> + EOF
> +
> +- printf "propter nomen suum." >>new4.txt
> ++ printf "propter nomen suum." >>new4.txt &&
> ++
> ++ cat >base.c <<-\EOF &&
> ++ int f(int x, int y)
> ++ {
> ++ if (x == 0)
> ++ {
> ++ return y;
> ++ }
> ++ return x;
> ++ }
> ++
> ++ int g(size_t u)
> ++ {
> ++ while (u < 30)
> ++ {
> ++ u++;
> ++ }
> ++ return u;
> ++ }
> ++ EOF
> ++
> ++ cat >ours.c <<-\EOF &&
> ++ int g(size_t u)
> ++ {
> ++ while (u < 30)
> ++ {
> ++ u++;
> ++ }
> ++ return u;
> ++ }
> ++
> ++ int h(int x, int y, int z)
> ++ {
> ++ if (z == 0)
> ++ {
> ++ return x;
> ++ }
> ++ return y;
> ++ }
> ++ EOF
> ++
> ++ cat >theirs.c <<-\EOF
> ++ int f(int x, int y)
> ++ {
> ++ if (x == 0)
> ++ {
> ++ return y;
> ++ }
> ++ return x;
> ++ }
> ++
> ++ int g(size_t u)
> ++ {
> ++ while (u > 34)
> ++ {
> ++ u--;
> ++ }
> ++ return u;
> ++ }
> ++ EOF
> + '
> +
> + test_expect_success 'merge with no changes' '
> +@@ t/t6403-merge-file.sh: test_expect_success '--object-id fails without repository' '
> + grep "not a git repository" err
> + '
> +
> ++test_expect_success 'merging C files with "myers" diff algorithm creates some spurious conflicts' '
> ++ cat >expect.c <<-\EOF &&
> ++ int g(size_t u)
> ++ {
> ++ while (u < 30)
> ++ {
> ++ u++;
> ++ }
> ++ return u;
> ++ }
> ++
> ++ int h(int x, int y, int z)
> ++ {
> ++ <<<<<<< ours.c
> ++ if (z == 0)
> ++ ||||||| base.c
> ++ while (u < 30)
> ++ =======
> ++ while (u > 34)
> ++ >>>>>>> theirs.c
> ++ {
> ++ <<<<<<< ours.c
> ++ return x;
> ++ ||||||| base.c
> ++ u++;
> ++ =======
> ++ u--;
> ++ >>>>>>> theirs.c
> ++ }
> ++ return y;
> ++ }
> ++ EOF
> ++
> ++ test_must_fail git merge-file -p --diff3 --diff-algorithm myers ours.c base.c theirs.c >myers_output.c &&
> ++ test_cmp expect.c myers_output.c
> ++'
> ++
> ++test_expect_success 'merging C files with "histogram" diff algorithm avoids some spurious conflicts' '
> ++ cat >expect.c <<-\EOF &&
> ++ int g(size_t u)
> ++ {
> ++ while (u > 34)
> ++ {
> ++ u--;
> ++ }
> ++ return u;
> ++ }
> ++
> ++ int h(int x, int y, int z)
> ++ {
> ++ if (z == 0)
> ++ {
> ++ return x;
> ++ }
> ++ return y;
> ++ }
> ++ EOF
> ++
> ++ git merge-file -p --diff3 --diff-algorithm histogram ours.c base.c theirs.c >histogram_output.c &&
> ++ test_cmp expect.c histogram_output.c
> ++'
> ++
> + test_done
>
>
> Documentation/git-merge-file.txt | 6 ++
> builtin/merge-file.c | 28 +++++++
> t/t6403-merge-file.sh | 124 ++++++++++++++++++++++++++++++-
> 3 files changed, 157 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-merge-file.txt b/Documentation/git-merge-file.txt
> index 6a081eacb72..71915a00fa4 100644
> --- a/Documentation/git-merge-file.txt
> +++ b/Documentation/git-merge-file.txt
> @@ -92,6 +92,12 @@ object store and the object ID of its blob is written to standard output.
> Instead of leaving conflicts in the file, resolve conflicts
> favouring our (or their or both) side of the lines.
>
> +--diff-algorithm={patience|minimal|histogram|myers}::
> + Use a different diff algorithm while merging. The current default is "myers",
> + but selecting more recent algorithm such as "histogram" can help
> + avoid mismerges that occur due to unimportant matching lines
> + (such as braces from distinct functions). See also
> + linkgit:git-diff[1] `--diff-algorithm`.
>
> EXAMPLES
> --------
> diff --git a/builtin/merge-file.c b/builtin/merge-file.c
> index 832c93d8d54..1f987334a31 100644
> --- a/builtin/merge-file.c
> +++ b/builtin/merge-file.c
> @@ -1,5 +1,6 @@
> #include "builtin.h"
> #include "abspath.h"
> +#include "diff.h"
> #include "hex.h"
> #include "object-name.h"
> #include "object-store.h"
> @@ -28,6 +29,30 @@ static int label_cb(const struct option *opt, const char *arg, int unset)
> return 0;
> }
>
> +static int set_diff_algorithm(xpparam_t *xpp,
> + const char *alg)
> +{
> + long diff_algorithm = parse_algorithm_value(alg);
> + if (diff_algorithm < 0)
> + return -1;
> + xpp->flags = (xpp->flags & ~XDF_DIFF_ALGORITHM_MASK) | diff_algorithm;
> + return 0;
> +}
> +
> +static int diff_algorithm_cb(const struct option *opt,
> + const char *arg, int unset)
> +{
> + xpparam_t *xpp = opt->value;
> +
> + BUG_ON_OPT_NEG(unset);
> +
> + if (set_diff_algorithm(xpp, arg))
> + return error(_("option diff-algorithm accepts \"myers\", "
> + "\"minimal\", \"patience\" and \"histogram\""));
> +
> + return 0;
> +}
> +
> int cmd_merge_file(int argc, const char **argv, const char *prefix)
> {
> const char *names[3] = { 0 };
> @@ -48,6 +73,9 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
> XDL_MERGE_FAVOR_THEIRS),
> OPT_SET_INT(0, "union", &xmp.favor, N_("for conflicts, use a union version"),
> XDL_MERGE_FAVOR_UNION),
> + OPT_CALLBACK_F(0, "diff-algorithm", &xmp.xpp, N_("<algorithm>"),
> + N_("choose a diff algorithm"),
> + PARSE_OPT_NONEG, diff_algorithm_cb),
> OPT_INTEGER(0, "marker-size", &xmp.marker_size,
> N_("for conflicts, use this marker size")),
> OPT__QUIET(&quiet, N_("do not warn about conflicts")),
> diff --git a/t/t6403-merge-file.sh b/t/t6403-merge-file.sh
> index 2c92209ecab..fb872c5a113 100755
> --- a/t/t6403-merge-file.sh
> +++ b/t/t6403-merge-file.sh
> @@ -56,7 +56,67 @@ test_expect_success 'setup' '
> deduxit me super semitas jusitiae,
> EOF
>
> - printf "propter nomen suum." >>new4.txt
> + printf "propter nomen suum." >>new4.txt &&
> +
> + cat >base.c <<-\EOF &&
> + int f(int x, int y)
> + {
> + if (x == 0)
> + {
> + return y;
> + }
> + return x;
> + }
> +
> + int g(size_t u)
> + {
> + while (u < 30)
> + {
> + u++;
> + }
> + return u;
> + }
> + EOF
> +
> + cat >ours.c <<-\EOF &&
> + int g(size_t u)
> + {
> + while (u < 30)
> + {
> + u++;
> + }
> + return u;
> + }
> +
> + int h(int x, int y, int z)
> + {
> + if (z == 0)
> + {
> + return x;
> + }
> + return y;
> + }
> + EOF
> +
> + cat >theirs.c <<-\EOF
> + int f(int x, int y)
> + {
> + if (x == 0)
> + {
> + return y;
> + }
> + return x;
> + }
> +
> + int g(size_t u)
> + {
> + while (u > 34)
> + {
> + u--;
> + }
> + return u;
> + }
> + EOF
> '
>
> test_expect_success 'merge with no changes' '
> @@ -447,4 +507,66 @@ test_expect_success '--object-id fails without repository' '
> grep "not a git repository" err
> '
>
> +test_expect_success 'merging C files with "myers" diff algorithm creates some spurious conflicts' '
> + cat >expect.c <<-\EOF &&
> + int g(size_t u)
> + {
> + while (u < 30)
> + {
> + u++;
> + }
> + return u;
> + }
> +
> + int h(int x, int y, int z)
> + {
> + <<<<<<< ours.c
> + if (z == 0)
> + ||||||| base.c
> + while (u < 30)
> + =======
> + while (u > 34)
> + >>>>>>> theirs.c
> + {
> + <<<<<<< ours.c
> + return x;
> + ||||||| base.c
> + u++;
> + =======
> + u--;
> + >>>>>>> theirs.c
> + }
> + return y;
> + }
> + EOF
> +
> + test_must_fail git merge-file -p --diff3 --diff-algorithm myers ours.c base.c theirs.c >myers_output.c &&
> + test_cmp expect.c myers_output.c
> +'
> +
> +test_expect_success 'merging C files with "histogram" diff algorithm avoids some spurious conflicts' '
> + cat >expect.c <<-\EOF &&
> + int g(size_t u)
> + {
> + while (u > 34)
> + {
> + u--;
> + }
> + return u;
> + }
> +
> + int h(int x, int y, int z)
> + {
> + if (z == 0)
> + {
> + return x;
> + }
> + return y;
> + }
> + EOF
> +
> + git merge-file -p --diff3 --diff-algorithm histogram ours.c base.c theirs.c >histogram_output.c &&
> + test_cmp expect.c histogram_output.c
> +'
> +
> test_done
>
> base-commit: 98009afd24e2304bf923a64750340423473809ff
^ permalink raw reply
* Re: Migration of git-scm.com to a static web site: ready for review/testing
From: Johannes Schindelin @ 2023-11-21 14:25 UTC (permalink / raw)
To: Todd Zullinger; +Cc: git, Matt Burke, Victoria Dye, Matthias Aßhauer
In-Reply-To: <ZVgoKPAg6jKZk_M6@pobox.com>
[-- Attachment #1: Type: text/plain, Size: 4881 bytes --]
Hi Todd,
On Fri, 17 Nov 2023, Todd Zullinger wrote:
> Johannes Schindelin wrote:
> >> For checking links, a tool like linkcheker[1] is very handy.
> >> This is run against the local docs in the Fedora package
> >> builds to catch broken links.
> >
> > Hmm, `linkchecker` is really slow for me, even locally.
>
> Yeah, it took an hour and a half to run for me, both on an
> old laptop and a fast server with plenty of threads,
> bandwidth, and memory.
>
> Checking the git HTML documentation takes under 30 seconds,
> which is largely the only place I've used it. It has been
> very helpful in catching broken links in the docs during the
> build and the time is short enough that I never minded.
I found https://lychee.cli.rs/#/ in the meantime and figured out how to
use it in a local setup:
First, I run:
HUGO_TIMEOUT=777 HUGO_BASEURL= HUGO_UGLYURLS=false time hugo
The first `HUGO_*` setting is to make sure that even though I sometimes
use all of the cores of my laptop's CPU it should not fail. The other two
are to override settings from `hugo.yml` so that `lychee` can handle the
output (`lychee` will not auto-append `.html`, unlike GitHub Pages, and
would therefore mis-detect tons of broken links, without
`HUGO_UGLYURLS=false`).
In my setup, this command typically runs for something like half a minute,
but sometimes takes for as long as 1 minute. (I noticed that it is much
slower when I open the directory in VS Code because I'm running this in
WSL and the filesystem watcher kind of eats all resources.)
After that, I run:
time lychee --offline --exclude-mail \
--exclude file:///path/to/repo.git/ \
--exclude file:///caminho/para/o/reposit%C3%B3rio.git/ \
--exclude file:///ruta/a/repositorio.git/ \
--exclude file:///sl%C3%B3%C3%B0/til/hirsla.git/ \
--exclude file:///Pfad/zum/Repo.git/ \
--exclude file:///chemin/du/d%C3%A9p%C3%B4t.git/ \
--exclude file:///srv/git/project.git \
--exclude "file://$PWD/public/pagefind/pagefind-ui.css" \
--format markdown -o lychee-local.md public/
Without `--offline`, there would be a couple of broken links (the
http://git.or.cz/gitwiki/InterfacesFrontendsAndTools link leads to
"Forbidden", it needs to be changed to https://).
The `file:///` URLs are all examples that are not expected to be valid.
And we do not want to check the emails (tons of `xyz@example.com` would be
"broken").
This command typically takes another half minute, sometimes a bit longer.
Given those times and the configurability (and the lure of a GitHub
Action that could be easily integrated into a GitHub workflow:
https://github.com/marketplace/actions/lychee-broken-link-checker), I have
up on linkchecker and focused exclusively on lychee.
Now, when I started working on this on Friday, lychee reported about
12,000 broken links.
There were a couple of legitimate mistakes I made (when feeding paths to
Hugo's `relURL` function, the path must not have a leading slash or it
will remain unchanged, for example). These are fixed.
But there were also many other issues such as some manual page translation
being incomplete yet linking to not-yet-existing pages. In those cases, I
changed he code to generate redirects to the English version. For example,
https://git.github.io/git-scm.com/docs/git-clone/fr#_git has a link to
`git[1]` that _should_ lead to the French version of the `git` manual
page. However, that does not exist. So both the Rails App as well as the
static website redirect to the English variant of that page.
My most recent lychee run results in 0 broken links.
As a bonus, some of the links that are currently broken on
https://git-scm.com/ are fixed in https://git.github.io/git-scm.com/.
For example, following the `Pull Request Referləri` link at the top of
https://git-scm.com/book/az/v2/Appendix-C:-Git-%C6%8Fmrl%C9%99ri-Plumbing-%C6%8Fmrl%C9%99ri/
leads to a 404. But following it in
https://git.github.io/git-scm.com/book/az/v2/Appendix-C:-Git-%C6%8Fmrl%C9%99ri-Plumbing-%C6%8Fmrl%C9%99ri/
directs the browser to the correct URL:
https://git.github.io/git-scm.com/book/az/v2/GitHub-Bir-Layih%C9%99nin-Saxlan%C4%B1lmas%C4%B1/#_pr_refs
Another thing that is broken on https://git-scm.com/ are the footnotes in
the Czech translation of the ProGit book. These were broken in the Hugo
version, too, but now they are fixed. See e.g.
https://dscho.github.io/git-scm.com/book/cs/v2/Z%C3%A1klady-pr%C3%A1ce-se-syst%C3%A9mem-Git-Zobrazen%C3%AD-historie-reviz%C3%AD/#_footnotedef_7
and note that the Rails App redirects to
https://git-scm.com/book/cs/v2/Z%C3%A1klady-pr%C3%A1ce-se-syst%C3%A9mem-Git-Zobrazen%C3%AD-historie-reviz%C3%AD/ch00/_footnotedef_7
when clicking on the `[7]`, which 404s.
Could you double-check that the links in the current version?
Thank you,
Johannes
^ permalink raw reply
* reftable: How to represent deleted referees of symrefs in the reflog?
From: Patrick Steinhardt @ 2023-11-21 13:46 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Jonathan Nieder, Terry Parker
[-- Attachment #1: Type: text/plain, Size: 5004 bytes --]
Hi,
I'm currently trying to fully understand how exactly reflogs and reflog
entries are supposed to be deleted in the reftable backend. For ref
records it's easy enough, as explained in the technical documentation
for reftables that is part of our tree:
> Deletion of any reference can be explicitly stored by setting the type
> to 0x0 and omitting the value field of the ref_record. This serves as
> a tombstone, overriding any assertions about the existence of the
> reference from earlier files in the stack.
So you simply create a new ref record with type 0x0 and are done.
For log records it seems to be a bit of a different story though.
Again, quoting the technical documentation:
> The log_type = 0x0 is mostly useful for git stash drop, removing an
> entry from the reflog of refs/stash in a transaction file (below),
> without needing to rewrite larger files. Readers reading a stack of
> reflogs must treat this as a deletion.
To me it seems like deletions in this case only delete a particular log
entry instead of the complete log for a particular reference. And some
older discussion [1] seems to confirm my hunch that a complete reflog is
deleted not with `log_type = 0x0`, but instead by writing the null
object ID as new ID.
So: a single entry is deleted with `log_type = 0x0`, the complete reflog
entry is deleted with the null object ID as new ID. Fair enough, even
though the documentation could be updated to make this easier to
understand. I'll happily do so if my understanding is correct here.
In any case though, this proposed behaviour is not sufficient to cover
all cases that the files-based reflog supports. The following case may
be weird, but we do have tests for it in t1400:
```
$ git init repo
Initialized empty Git repository in /tmp/repo/.git/
$ cd repo/
$ git commit --allow-empty --message initial-commit
[main (root-commit) 9f10b3f] initial-commit
# Everything looks as expected up to this point.
$ git reflog show HEAD
9f10b3f (HEAD -> main) HEAD@{0}: commit (initial): initial-commit
# This behaviour is a bit more on the weird side. We delete the
# referee, and that causes the files backend to claim that the reflog
# for HEAD is gone, as well. The reflog still exists though, as
# demonstrated in the next command.
$ git update-ref -m delete-main -d refs/heads/main
$ git reflog show HEAD
fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
# We now re-create the referee, which revives the reflog _including_
# the old entry.
$ git update-ref -m recreate-main refs/heads/main 9f10b3f
$ git reflog show HEAD
9f10b3f (HEAD -> main) HEAD@{0}: recreate-main
9f10b3f (HEAD -> main) HEAD@{2}: commit (initial): initial-commit
$ cat .git/logs/HEAD
0000000000000000000000000000000000000000 9f10b3f9b20962690fdeff76cd592722fbf57deb Patrick Steinhardt <ps@pks.im> 1700573003 +0100 commit (initial): initial-commit
9f10b3f9b20962690fdeff76cd592722fbf57deb 0000000000000000000000000000000000000000 Patrick Steinhardt <ps@pks.im> 1700573060 +0100 delete-main
0000000000000000000000000000000000000000 9f10b3f9b20962690fdeff76cd592722fbf57deb Patrick Steinhardt <ps@pks.im> 1700573078 +0100 recreate-main
```
It kind of feels like the second step in the files backend where the
reflog is claimed to not exist is buggy -- I'd have expected to still
see the reflog, as the HEAD reference exists quite alright and has never
stopped to exist. And in the third step, I'd have expected to see three
reflog entries, including the deletion that is indeed still present in
the on-disk logfile.
But with the reftable backend the problem becomes worse: we cannot even
represent the fact that the reflog still exists, but that the deletion
of the referee has caused the HEAD to point to the null OID, because the
null OID indicates complete deletion of the reflog. Consequentially, if
we wrote the null OID, we'd only be able to see the last log entry here.
It may totally be that I'm missing something obvious here. But if not,
it leaves us with a couple of options for how to fix it:
1. Disregard this edge case and accept that the reftable backend
does not do exactly the same thing as the files backend in very
specific situations like this.
2. Change the reftable format so that it can discern these cases,
e.g. by indicating deletion via a new log type.
3. Change the files backend to adapt.
None of these alternatives feel particularly great to me. In my opinion
(2) is likely the best option, but would require us to change the format
format that's already in use by Google in the context of multiple
projects. So I'm not quite sure how thrilled you folks at Google and
other users of the reftable library are with this prospect.
Anyway, happy to hear about alternative takes or corrections.
Patrick
[1]: <CAFQ2z_P-cf38yR-VyvfDSgPUO_d38mgsi32UkRSPWMZKJOmjZg@mail.gmail.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH 8/8] reftable/stack: fix stale lock when dying
From: Patrick Steinhardt @ 2023-11-21 7:04 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Jonathan Nieder
In-Reply-To: <cover.1700549493.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 4515 bytes --]
When starting a transaction via `reftable_stack_init_addition()`, we
create a lockfile for the reftable stack itself which we'll write the
new list of tables to. But if we terminate abnormally e.g. via a call to
`die()`, then we do not remove the lockfile. Subsequent executions of
Git which try to modify references will thus fail with an out-of-date
error.
Fix this bug by registering the lock as a `struct tempfile`, which
ensures automatic cleanup for us.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reftable/stack.c | 47 +++++++++++++++--------------------------------
1 file changed, 15 insertions(+), 32 deletions(-)
diff --git a/reftable/stack.c b/reftable/stack.c
index 2dd2373360..2f1494aef2 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -17,6 +17,8 @@ license that can be found in the LICENSE file or at
#include "reftable-merged.h"
#include "writer.h"
+#include "tempfile.h"
+
static int stack_try_add(struct reftable_stack *st,
int (*write_table)(struct reftable_writer *wr,
void *arg),
@@ -440,8 +442,7 @@ static void format_name(struct strbuf *dest, uint64_t min, uint64_t max)
}
struct reftable_addition {
- int lock_file_fd;
- struct strbuf lock_file_name;
+ struct tempfile *lock_file;
struct reftable_stack *stack;
char **new_tables;
@@ -449,24 +450,19 @@ struct reftable_addition {
uint64_t next_update_index;
};
-#define REFTABLE_ADDITION_INIT \
- { \
- .lock_file_name = STRBUF_INIT \
- }
+#define REFTABLE_ADDITION_INIT {0}
static int reftable_stack_init_addition(struct reftable_addition *add,
struct reftable_stack *st)
{
+ struct strbuf lock_file_name = STRBUF_INIT;
int err = 0;
add->stack = st;
- strbuf_reset(&add->lock_file_name);
- strbuf_addstr(&add->lock_file_name, st->list_file);
- strbuf_addstr(&add->lock_file_name, ".lock");
+ strbuf_addf(&lock_file_name, "%s.lock", st->list_file);
- add->lock_file_fd = open(add->lock_file_name.buf,
- O_EXCL | O_CREAT | O_WRONLY, 0666);
- if (add->lock_file_fd < 0) {
+ add->lock_file = create_tempfile(lock_file_name.buf);
+ if (!add->lock_file) {
if (errno == EEXIST) {
err = REFTABLE_LOCK_ERROR;
} else {
@@ -475,7 +471,7 @@ static int reftable_stack_init_addition(struct reftable_addition *add,
goto done;
}
if (st->config.default_permissions) {
- if (chmod(add->lock_file_name.buf, st->config.default_permissions) < 0) {
+ if (chmod(lock_file_name.buf, st->config.default_permissions) < 0) {
err = REFTABLE_IO_ERROR;
goto done;
}
@@ -495,6 +491,7 @@ static int reftable_stack_init_addition(struct reftable_addition *add,
if (err) {
reftable_addition_close(add);
}
+ strbuf_release(&lock_file_name);
return err;
}
@@ -512,15 +509,7 @@ static void reftable_addition_close(struct reftable_addition *add)
add->new_tables = NULL;
add->new_tables_len = 0;
- if (add->lock_file_fd > 0) {
- close(add->lock_file_fd);
- add->lock_file_fd = 0;
- }
- if (add->lock_file_name.len > 0) {
- unlink(add->lock_file_name.buf);
- strbuf_release(&add->lock_file_name);
- }
-
+ delete_tempfile(&add->lock_file);
strbuf_release(&nm);
}
@@ -536,8 +525,10 @@ void reftable_addition_destroy(struct reftable_addition *add)
int reftable_addition_commit(struct reftable_addition *add)
{
struct strbuf table_list = STRBUF_INIT;
+ int lock_file_fd = get_tempfile_fd(add->lock_file);
int i = 0;
int err = 0;
+
if (add->new_tables_len == 0)
goto done;
@@ -550,28 +541,20 @@ int reftable_addition_commit(struct reftable_addition *add)
strbuf_addstr(&table_list, "\n");
}
- err = write_in_full(add->lock_file_fd, table_list.buf, table_list.len);
+ err = write_in_full(lock_file_fd, table_list.buf, table_list.len);
strbuf_release(&table_list);
if (err < 0) {
err = REFTABLE_IO_ERROR;
goto done;
}
- err = close(add->lock_file_fd);
- add->lock_file_fd = 0;
- if (err < 0) {
- err = REFTABLE_IO_ERROR;
- goto done;
- }
-
- err = rename(add->lock_file_name.buf, add->stack->list_file);
+ err = rename_tempfile(&add->lock_file, add->stack->list_file);
if (err < 0) {
err = REFTABLE_IO_ERROR;
goto done;
}
/* success, no more state to clean up. */
- strbuf_release(&add->lock_file_name);
for (i = 0; i < add->new_tables_len; i++) {
reftable_free(add->new_tables[i]);
}
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 7/8] reftable/merged: reuse buffer to compute record keys
From: Patrick Steinhardt @ 2023-11-21 7:04 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Jonathan Nieder
In-Reply-To: <cover.1700549493.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2100 bytes --]
When iterating over entries in the merged iterator's queue, we compute
the key of each of the entries and write it into a buffer. We do not
reuse the buffer though and thus re-allocate it on every iteration,
which is wasteful given that we never transfer ownership of the
allocated bytes outside of the loop.
Refactor the code to reuse the buffer. This also fixes a potential
memory leak when `merged_iter_advance_subiter()` returns an error.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reftable/merged.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/reftable/merged.c b/reftable/merged.c
index 5ded470c08..cd03f6da13 100644
--- a/reftable/merged.c
+++ b/reftable/merged.c
@@ -85,7 +85,7 @@ static int merged_iter_advance_subiter(struct merged_iter *mi, size_t idx)
static int merged_iter_next_entry(struct merged_iter *mi,
struct reftable_record *rec)
{
- struct strbuf entry_key = STRBUF_INIT;
+ struct strbuf entry_key = STRBUF_INIT, k = STRBUF_INIT;
struct pq_entry entry = { 0 };
int err = 0;
@@ -108,30 +108,28 @@ static int merged_iter_next_entry(struct merged_iter *mi,
reftable_record_key(&entry.rec, &entry_key);
while (!merged_iter_pqueue_is_empty(mi->pq)) {
struct pq_entry top = merged_iter_pqueue_top(mi->pq);
- struct strbuf k = STRBUF_INIT;
- int err = 0, cmp = 0;
+ int cmp = 0;
reftable_record_key(&top.rec, &k);
cmp = strbuf_cmp(&k, &entry_key);
- strbuf_release(&k);
-
- if (cmp > 0) {
+ if (cmp > 0)
break;
- }
merged_iter_pqueue_remove(&mi->pq);
err = merged_iter_advance_subiter(mi, top.index);
- if (err < 0) {
- return err;
- }
+ if (err < 0)
+ goto done;
reftable_record_release(&top.rec);
}
reftable_record_copy_from(rec, &entry.rec, hash_size(mi->hash_id));
+
+done:
reftable_record_release(&entry.rec);
strbuf_release(&entry_key);
- return 0;
+ strbuf_release(&k);
+ return err;
}
static int merged_iter_next(struct merged_iter *mi, struct reftable_record *rec)
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 6/8] reftable/stack: reuse buffers when reloading stack
From: Patrick Steinhardt @ 2023-11-21 7:04 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Jonathan Nieder
In-Reply-To: <cover.1700549493.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2328 bytes --]
In `reftable_stack_reload_once()` we iterate over all the tables added
to the stack in order to figure out whether any of the tables needs to
be reloaded. We use a set of buffers in this context to compute the
paths of these tables, but discard those buffers on every iteration.
This is quite wasteful given that we do not need to transfer ownership
of the allocated buffer outside of the loop.
Refactor the code to instead reuse the buffers to reduce the number of
allocations we need to do.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reftable/stack.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/reftable/stack.c b/reftable/stack.c
index f5d18a842a..2dd2373360 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -204,6 +204,7 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names,
reftable_calloc(sizeof(struct reftable_table) * names_len);
int new_readers_len = 0;
struct reftable_merged_table *new_merged = NULL;
+ struct strbuf table_path = STRBUF_INIT;
int i;
while (*names) {
@@ -223,13 +224,10 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names,
if (!rd) {
struct reftable_block_source src = { NULL };
- struct strbuf table_path = STRBUF_INIT;
stack_filename(&table_path, st, name);
err = reftable_block_source_from_file(&src,
table_path.buf);
- strbuf_release(&table_path);
-
if (err < 0)
goto done;
@@ -267,16 +265,13 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names,
for (i = 0; i < cur_len; i++) {
if (cur[i]) {
const char *name = reader_name(cur[i]);
- struct strbuf filename = STRBUF_INIT;
- stack_filename(&filename, st, name);
+ stack_filename(&table_path, st, name);
reader_close(cur[i]);
reftable_reader_free(cur[i]);
/* On Windows, can only unlink after closing. */
- unlink(filename.buf);
-
- strbuf_release(&filename);
+ unlink(table_path.buf);
}
}
@@ -288,6 +283,7 @@ static int reftable_stack_reload_once(struct reftable_stack *st, char **names,
reftable_free(new_readers);
reftable_free(new_tables);
reftable_free(cur);
+ strbuf_release(&table_path);
return err;
}
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 5/8] reftable/stack: perform auto-compaction with transactional interface
From: Patrick Steinhardt @ 2023-11-21 7:04 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Jonathan Nieder
In-Reply-To: <cover.1700549493.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 4295 bytes --]
Whenever updating references or reflog entries in the reftable stack, we
need to add a new table to the stack, thus growing the stack's length by
one. It can thus happen quite fast that the stack grows very long, which
results in performance issues when trying to read records. But besides
performance issues, this can also lead to exhaustion of file descriptors
very rapidly as every single table requires a separate descriptor when
opening the stack.
While git-pack-refs(1) fixes this issue for us by merging the tables, it
runs too irregularly to keep the length of the stack within reasonable
limits. This is why the reftable stack has an auto-compaction mechanism:
`reftable_stack_add()` will call `reftable_stack_auto_compact()` after
its added the new table, which will auto-compact the stack as required.
But while this logic works alright for `reftable_stack_add()`, we do not
do the same in `reftable_addition_commit()`, which is the transactional
equivalent to the former function that allows us to write multiple
updates to the stack atomically. Consequentially, we will easily run
into file descriptor exhaustion in code paths that use many separate
transactions like e.g. non-atomic fetches.
Fix this issue by calling `reftable_stack_auto_compact()`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reftable/stack.c | 6 +++++
reftable/stack_test.c | 56 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 62 insertions(+)
diff --git a/reftable/stack.c b/reftable/stack.c
index f0cadad490..f5d18a842a 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -584,6 +584,12 @@ int reftable_addition_commit(struct reftable_addition *add)
add->new_tables_len = 0;
err = reftable_stack_reload(add->stack);
+ if (err)
+ goto done;
+
+ if (!add->stack->disable_auto_compact)
+ err = reftable_stack_auto_compact(add->stack);
+
done:
reftable_addition_close(add);
return err;
diff --git a/reftable/stack_test.c b/reftable/stack_test.c
index c979d177c2..4c2f794c49 100644
--- a/reftable/stack_test.c
+++ b/reftable/stack_test.c
@@ -289,6 +289,61 @@ static void test_reftable_stack_transaction_api(void)
clear_dir(dir);
}
+static void test_reftable_stack_transaction_api_performs_auto_compaction(void)
+{
+ char *dir = get_tmp_dir(__LINE__);
+ struct reftable_write_options cfg = {0};
+ struct reftable_addition *add = NULL;
+ struct reftable_stack *st = NULL;
+ int i, n = 20, err;
+
+ err = reftable_new_stack(&st, dir, cfg);
+ EXPECT_ERR(err);
+
+ for (i = 0; i <= n; i++) {
+ struct reftable_ref_record ref = {
+ .update_index = reftable_stack_next_update_index(st),
+ .value_type = REFTABLE_REF_SYMREF,
+ .value.symref = "master",
+ };
+ char name[100];
+
+ snprintf(name, sizeof(name), "branch%04d", i);
+ ref.refname = name;
+
+ /*
+ * Disable auto-compaction for all but the last runs. Like this
+ * we can ensure that we indeed honor this setting and have
+ * better control over when exactly auto compaction runs.
+ */
+ st->disable_auto_compact = i != n;
+
+ err = reftable_stack_new_addition(&add, st);
+ EXPECT_ERR(err);
+
+ err = reftable_addition_add(add, &write_test_ref, &ref);
+ EXPECT_ERR(err);
+
+ err = reftable_addition_commit(add);
+ EXPECT_ERR(err);
+
+ reftable_addition_destroy(add);
+
+ /*
+ * The stack length should grow continuously for all runs where
+ * auto compaction is disabled. When enabled, we should merge
+ * all tables in the stack.
+ */
+ if (i != n)
+ EXPECT(st->merged->stack_len == i + 1);
+ else
+ EXPECT(st->merged->stack_len == 1);
+ }
+
+ reftable_stack_destroy(st);
+ clear_dir(dir);
+}
+
static void test_reftable_stack_validate_refname(void)
{
struct reftable_write_options cfg = { 0 };
@@ -1014,6 +1069,7 @@ int stack_test_main(int argc, const char *argv[])
RUN_TEST(test_reftable_stack_log_normalize);
RUN_TEST(test_reftable_stack_tombstone);
RUN_TEST(test_reftable_stack_transaction_api);
+ RUN_TEST(test_reftable_stack_transaction_api_performs_auto_compaction);
RUN_TEST(test_reftable_stack_update_index_check);
RUN_TEST(test_reftable_stack_uptodate);
RUN_TEST(test_reftable_stack_validate_refname);
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 4/8] reftable/stack: verify that `reftable_stack_add()` uses auto-compaction
From: Patrick Steinhardt @ 2023-11-21 7:04 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Jonathan Nieder
In-Reply-To: <cover.1700549493.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2503 bytes --]
While we have several tests that check whether we correctly perform
auto-compaction when manually calling `reftable_stack_auto_compact()`,
we don't have any tests that verify whether `reftable_stack_add()` does
call it automatically. Add one.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reftable/stack_test.c | 47 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/reftable/stack_test.c b/reftable/stack_test.c
index 0644c8ad2e..c979d177c2 100644
--- a/reftable/stack_test.c
+++ b/reftable/stack_test.c
@@ -850,6 +850,52 @@ static void test_reftable_stack_auto_compaction(void)
clear_dir(dir);
}
+static void test_reftable_stack_add_performs_auto_compaction(void)
+{
+ struct reftable_write_options cfg = { 0 };
+ struct reftable_stack *st = NULL;
+ char *dir = get_tmp_dir(__LINE__);
+ int err, i, n = 20;
+
+ err = reftable_new_stack(&st, dir, cfg);
+ EXPECT_ERR(err);
+
+ for (i = 0; i <= n; i++) {
+ struct reftable_ref_record ref = {
+ .update_index = reftable_stack_next_update_index(st),
+ .value_type = REFTABLE_REF_SYMREF,
+ .value.symref = "master",
+ };
+ char name[100];
+
+ /*
+ * Disable auto-compaction for all but the last runs. Like this
+ * we can ensure that we indeed honor this setting and have
+ * better control over when exactly auto compaction runs.
+ */
+ st->disable_auto_compact = i != n;
+
+ snprintf(name, sizeof(name), "branch%04d", i);
+ ref.refname = name;
+
+ err = reftable_stack_add(st, &write_test_ref, &ref);
+ EXPECT_ERR(err);
+
+ /*
+ * The stack length should grow continuously for all runs where
+ * auto compaction is disabled. When enabled, we should merge
+ * all tables in the stack.
+ */
+ if (i != n)
+ EXPECT(st->merged->stack_len == i + 1);
+ else
+ EXPECT(st->merged->stack_len == 1);
+ }
+
+ reftable_stack_destroy(st);
+ clear_dir(dir);
+}
+
static void test_reftable_stack_compaction_concurrent(void)
{
struct reftable_write_options cfg = { 0 };
@@ -960,6 +1006,7 @@ int stack_test_main(int argc, const char *argv[])
RUN_TEST(test_reftable_stack_add);
RUN_TEST(test_reftable_stack_add_one);
RUN_TEST(test_reftable_stack_auto_compaction);
+ RUN_TEST(test_reftable_stack_add_performs_auto_compaction);
RUN_TEST(test_reftable_stack_compaction_concurrent);
RUN_TEST(test_reftable_stack_compaction_concurrent_clean);
RUN_TEST(test_reftable_stack_hash_id);
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 3/8] reftable: handle interrupted writes
From: Patrick Steinhardt @ 2023-11-21 7:04 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Jonathan Nieder
In-Reply-To: <cover.1700549493.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1923 bytes --]
There are calls to write(3P) where we don't properly handle interrupts.
Convert them to use `write_in_full()`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reftable/stack.c | 6 +++---
reftable/stack_test.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/reftable/stack.c b/reftable/stack.c
index ed108a929b..f0cadad490 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -42,7 +42,7 @@ static void stack_filename(struct strbuf *dest, struct reftable_stack *st,
static ssize_t reftable_fd_write(void *arg, const void *data, size_t sz)
{
int *fdp = (int *)arg;
- return write(*fdp, data, sz);
+ return write_in_full(*fdp, data, sz);
}
int reftable_new_stack(struct reftable_stack **dest, const char *dir,
@@ -554,7 +554,7 @@ int reftable_addition_commit(struct reftable_addition *add)
strbuf_addstr(&table_list, "\n");
}
- err = write(add->lock_file_fd, table_list.buf, table_list.len);
+ err = write_in_full(add->lock_file_fd, table_list.buf, table_list.len);
strbuf_release(&table_list);
if (err < 0) {
err = REFTABLE_IO_ERROR;
@@ -1024,7 +1024,7 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last,
strbuf_addstr(&ref_list_contents, "\n");
}
- err = write(lock_file_fd, ref_list_contents.buf, ref_list_contents.len);
+ err = write_in_full(lock_file_fd, ref_list_contents.buf, ref_list_contents.len);
if (err < 0) {
err = REFTABLE_IO_ERROR;
unlink(new_table_path.buf);
diff --git a/reftable/stack_test.c b/reftable/stack_test.c
index d0b717510f..0644c8ad2e 100644
--- a/reftable/stack_test.c
+++ b/reftable/stack_test.c
@@ -78,7 +78,7 @@ static void test_read_file(void)
int i = 0;
EXPECT(fd > 0);
- n = write(fd, out, strlen(out));
+ n = write_in_full(fd, out, strlen(out));
EXPECT(n == strlen(out));
err = close(fd);
EXPECT(err >= 0);
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 2/8] reftable: handle interrupted reads
From: Patrick Steinhardt @ 2023-11-21 7:04 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Jonathan Nieder
In-Reply-To: <cover.1700549493.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1260 bytes --]
There are calls to pread(3P) and read(3P) where we don't properly handle
interrupts. Convert them to use `pread_in_full()` and `read_in_full()`,
respectively.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reftable/blocksource.c | 2 +-
reftable/stack.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/reftable/blocksource.c b/reftable/blocksource.c
index 8331b34e82..a1ea304429 100644
--- a/reftable/blocksource.c
+++ b/reftable/blocksource.c
@@ -109,7 +109,7 @@ static int file_read_block(void *v, struct reftable_block *dest, uint64_t off,
struct file_block_source *b = v;
assert(off + size <= b->size);
dest->data = reftable_malloc(size);
- if (pread(b->fd, dest->data, size, off) != size)
+ if (pread_in_full(b->fd, dest->data, size, off) != size)
return -1;
dest->len = size;
return size;
diff --git a/reftable/stack.c b/reftable/stack.c
index ddbdf1b9c8..ed108a929b 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -92,7 +92,7 @@ static int fd_read_lines(int fd, char ***namesp)
}
buf = reftable_malloc(size + 1);
- if (read(fd, buf, size) != size) {
+ if (read_in_full(fd, buf, size) != size) {
err = REFTABLE_IO_ERROR;
goto done;
}
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 0/8] reftable: small set of fixes
From: Patrick Steinhardt @ 2023-11-21 7:04 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Jonathan Nieder
[-- Attachment #1: Type: text/plain, Size: 2095 bytes --]
Hi,
while working on the reftable backend I've hit several smaller issues in
the reftable library, which this patch series addresses.
We probably want to refactor t0032-reftable-unittest.sh to plug into the
new unit test architecture eventually, but for now I refrained from
doing so. I care more about getting things to a working state right now,
but I or somebody else from the Gitaly team will probably pick this
topic up later in this release cycle.
One issue I had was that this patch series starts to use more of the Git
infrastructure. Back when the library was introduced that there was some
discussion around whether it should work standalone or not, but if I
remember correctly the outcome was that it's okay to use internals like
e.g. `strbuf`. And while things like `read_in_full()` and related are
trivial wrappers, this patch series start to hook into the tempfiles
interface which I really didn't want to reimplement.
It's a bit unfortunate that we don't yet have good test coverage as
there are no end-to-end tests, and most of the changes I did are not
easily testable in unit tests. So until the reftable backend gets
submitted you'll have to trust my reasoning as layed out in the commit
messages that the changes actually improve things.
Patrick
Patrick Steinhardt (8):
reftable: wrap EXPECT macros in do/while
reftable: handle interrupted reads
reftable: handle interrupted writes
reftable/stack: verify that `reftable_stack_add()` uses
auto-compaction
reftable/stack: perform auto-compaction with transactional interface
reftable/stack: reuse buffers when reloading stack
reftable/merged: reuse buffer to compute record keys
reftable/stack: fix stale lock when dying
reftable/blocksource.c | 2 +-
reftable/merged.c | 20 ++++----
reftable/stack.c | 71 ++++++++++----------------
reftable/stack_test.c | 105 +++++++++++++++++++++++++++++++++++++-
reftable/test_framework.h | 58 +++++++++++----------
5 files changed, 174 insertions(+), 82 deletions(-)
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH 1/8] reftable: wrap EXPECT macros in do/while
From: Patrick Steinhardt @ 2023-11-21 7:04 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Jonathan Nieder
In-Reply-To: <cover.1700549493.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 4427 bytes --]
The `EXPECT` macros used by the reftable test framework are all using a
single `if` statement with the actual condition. This results in weird
syntax when using them in if/else statements like the following:
```
if (foo)
EXPECT(foo == 2)
else
EXPECT(bar == 2)
```
Note that there need not be a trailing semicolon. Furthermore, it is not
immediately obvious whether the else now belongs to the `if (foo)` or
whether it belongs to the expanded `if (foo == 2)` from the macro.
Fix this by wrapping the macros in a do/while loop.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reftable/test_framework.h | 58 +++++++++++++++++++++------------------
1 file changed, 32 insertions(+), 26 deletions(-)
diff --git a/reftable/test_framework.h b/reftable/test_framework.h
index 774cb275bf..ee44f735ae 100644
--- a/reftable/test_framework.h
+++ b/reftable/test_framework.h
@@ -12,32 +12,38 @@ license that can be found in the LICENSE file or at
#include "system.h"
#include "reftable-error.h"
-#define EXPECT_ERR(c) \
- if (c != 0) { \
- fflush(stderr); \
- fflush(stdout); \
- fprintf(stderr, "%s: %d: error == %d (%s), want 0\n", \
- __FILE__, __LINE__, c, reftable_error_str(c)); \
- abort(); \
- }
-
-#define EXPECT_STREQ(a, b) \
- if (strcmp(a, b)) { \
- fflush(stderr); \
- fflush(stdout); \
- fprintf(stderr, "%s:%d: %s (%s) != %s (%s)\n", __FILE__, \
- __LINE__, #a, a, #b, b); \
- abort(); \
- }
-
-#define EXPECT(c) \
- if (!(c)) { \
- fflush(stderr); \
- fflush(stdout); \
- fprintf(stderr, "%s: %d: failed assertion %s\n", __FILE__, \
- __LINE__, #c); \
- abort(); \
- }
+#define EXPECT_ERR(c) \
+ do { \
+ if (c != 0) { \
+ fflush(stderr); \
+ fflush(stdout); \
+ fprintf(stderr, "%s: %d: error == %d (%s), want 0\n", \
+ __FILE__, __LINE__, c, reftable_error_str(c)); \
+ abort(); \
+ } \
+ } while (0)
+
+#define EXPECT_STREQ(a, b) \
+ do { \
+ if (strcmp(a, b)) { \
+ fflush(stderr); \
+ fflush(stdout); \
+ fprintf(stderr, "%s:%d: %s (%s) != %s (%s)\n", __FILE__, \
+ __LINE__, #a, a, #b, b); \
+ abort(); \
+ } \
+ } while (0)
+
+#define EXPECT(c) \
+ do { \
+ if (!(c)) { \
+ fflush(stderr); \
+ fflush(stdout); \
+ fprintf(stderr, "%s: %d: failed assertion %s\n", __FILE__, \
+ __LINE__, #c); \
+ abort(); \
+ } \
+ } while (0)
#define RUN_TEST(f) \
fprintf(stderr, "running %s\n", #f); \
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* KONFIDENCIALITĀTE
From: tele Silvana Tenreyro @ 2023-11-20 21:06 UTC (permalink / raw)
To: git
FYI: uz jūsu vārda ir nepieprasīti līdzekļi. Lūdzu, sazinieties
ar S Tenreyro, rakstot uz e-pastu Silvanatenreyro@instaddr.uk,
norādot savu pilnu vārdu un uzvārdu pretenzijai.
^ permalink raw reply
* Re: [PATCH 2/2] pretty: add '%aA' to show domain-part of email addresses
From: Junio C Hamano @ 2023-11-20 20:21 UTC (permalink / raw)
To: Jeff King; +Cc: Kousik Sanagavarapu, Liam Beguin, git
In-Reply-To: <xmqqv8apgf4y.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Another line of thought is perhaps it is potentially useful to teach
> the --format= machinery to be a bit more programmable, e.g. allowing
> to compute a substring of an existing field %{%aE#*@} without having
> to waste a letter each for the local part and domain part. But as I
> already said, we are now talking about "postprocessing", and adding
> complexity to our codebase only to have incomplete flexibility may
> not be worth it. A more specific %(authoremail:localpart) and its
> domain counterpart may be easier to explain and understand.
>
> In any case, it is a bit too late to say "let's not waste the
> precious single letter namespace to add useless features", as we
> have come way too far, so I do not mind too much using a currently
> unused letter $X for yet another author and committer trait.
When I wrote the above, I somehow forgot the existing work in the
ref-filter (aka "for-each-ref") placeholders, where we have support
to a lot more flexible way to customize these things.
For example, "%(authoremail:mailmap,localpart)" can be used to say,
instead of wasting two letters 'l' and 'L' out of precious 52, that
we want e-mail address honoring the mailmap, and take only the local
part. And the support for the host part of the address that this
topic discussed should be implementable fairly easily (just adding
EO_HOSTPART bit to the email_option structure would be sufficient)
on the ref-filter side.
We saw efforts from time to time to give "log --pretty=format:" more
of the good things from the "for-each-ref --format=" placeholders
(and vice versa), and it may give us a good way forward.
^ permalink raw reply
* [PATCH v2] merge-file: add --diff-algorithm option
From: Antonin Delpeuch via GitGitGadget @ 2023-11-20 19:18 UTC (permalink / raw)
To: git; +Cc: Phillip Wood, Antonin Delpeuch, Antonin Delpeuch
In-Reply-To: <pull.1606.git.git.1699480494355.gitgitgadget@gmail.com>
From: Antonin Delpeuch <antonin@delpeuch.eu>
This makes it possible to use other diff algorithms than the 'myers'
default algorithm, when using the 'git merge-file' command. This helps
avoid spurious conflicts by selecting a more recent algorithm such as
'histogram', for instance when using 'git merge-file' as part of a custom
merge driver.
Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu>
Reviewed-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
merge-file: add --diff-algorithm option
Changes since v1:
* improve commit message to mention the use case of custom merge
drivers
* improve documentation to show available options and recommend
switching to "histogram"
* add tests
I have left out:
* switching the default to "histogram", because it should only be done
in a subsequent release
* adding a configuration variable to control this option, because I was
not sure how to call it. Perhaps "merge-file.diffAlgorithm"?
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1606%2Fwetneb%2Fmerge_file_configurable_diff_algorithm-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1606/wetneb/merge_file_configurable_diff_algorithm-v2
Pull-Request: https://github.com/git/git/pull/1606
Range-diff vs v1:
1: 4aa453e30be ! 1: 842b5abf33c merge-file: add --diff-algorithm option
@@ Commit message
merge-file: add --diff-algorithm option
This makes it possible to use other diff algorithms than the 'myers'
- default algorithm, when using the 'git merge-file' command.
+ default algorithm, when using the 'git merge-file' command. This helps
+ avoid spurious conflicts by selecting a more recent algorithm such as
+ 'histogram', for instance when using 'git merge-file' as part of a custom
+ merge driver.
Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu>
+ Reviewed-by: Phillip Wood <phillip.wood@dunelm.org.uk>
## Documentation/git-merge-file.txt ##
@@ Documentation/git-merge-file.txt: object store and the object ID of its blob is written to standard output.
Instead of leaving conflicts in the file, resolve conflicts
favouring our (or their or both) side of the lines.
-+--diff-algorithm <algorithm>::
-+ Use a different diff algorithm while merging, which can help
++--diff-algorithm={patience|minimal|histogram|myers}::
++ Use a different diff algorithm while merging. The current default is "myers",
++ but selecting more recent algorithm such as "histogram" can help
+ avoid mismerges that occur due to unimportant matching lines
-+ (such as braces from distinct functions). See also
++ (such as braces from distinct functions). See also
+ linkgit:git-diff[1] `--diff-algorithm`.
EXAMPLES
@@ builtin/merge-file.c: int cmd_merge_file(int argc, const char **argv, const char
OPT_INTEGER(0, "marker-size", &xmp.marker_size,
N_("for conflicts, use this marker size")),
OPT__QUIET(&quiet, N_("do not warn about conflicts")),
+
+ ## t/t6403-merge-file.sh ##
+@@ t/t6403-merge-file.sh: test_expect_success 'setup' '
+ deduxit me super semitas jusitiae,
+ EOF
+
+- printf "propter nomen suum." >>new4.txt
++ printf "propter nomen suum." >>new4.txt &&
++
++ cat >base.c <<-\EOF &&
++ int f(int x, int y)
++ {
++ if (x == 0)
++ {
++ return y;
++ }
++ return x;
++ }
++
++ int g(size_t u)
++ {
++ while (u < 30)
++ {
++ u++;
++ }
++ return u;
++ }
++ EOF
++
++ cat >ours.c <<-\EOF &&
++ int g(size_t u)
++ {
++ while (u < 30)
++ {
++ u++;
++ }
++ return u;
++ }
++
++ int h(int x, int y, int z)
++ {
++ if (z == 0)
++ {
++ return x;
++ }
++ return y;
++ }
++ EOF
++
++ cat >theirs.c <<-\EOF
++ int f(int x, int y)
++ {
++ if (x == 0)
++ {
++ return y;
++ }
++ return x;
++ }
++
++ int g(size_t u)
++ {
++ while (u > 34)
++ {
++ u--;
++ }
++ return u;
++ }
++ EOF
+ '
+
+ test_expect_success 'merge with no changes' '
+@@ t/t6403-merge-file.sh: test_expect_success '--object-id fails without repository' '
+ grep "not a git repository" err
+ '
+
++test_expect_success 'merging C files with "myers" diff algorithm creates some spurious conflicts' '
++ cat >expect.c <<-\EOF &&
++ int g(size_t u)
++ {
++ while (u < 30)
++ {
++ u++;
++ }
++ return u;
++ }
++
++ int h(int x, int y, int z)
++ {
++ <<<<<<< ours.c
++ if (z == 0)
++ ||||||| base.c
++ while (u < 30)
++ =======
++ while (u > 34)
++ >>>>>>> theirs.c
++ {
++ <<<<<<< ours.c
++ return x;
++ ||||||| base.c
++ u++;
++ =======
++ u--;
++ >>>>>>> theirs.c
++ }
++ return y;
++ }
++ EOF
++
++ test_must_fail git merge-file -p --diff3 --diff-algorithm myers ours.c base.c theirs.c >myers_output.c &&
++ test_cmp expect.c myers_output.c
++'
++
++test_expect_success 'merging C files with "histogram" diff algorithm avoids some spurious conflicts' '
++ cat >expect.c <<-\EOF &&
++ int g(size_t u)
++ {
++ while (u > 34)
++ {
++ u--;
++ }
++ return u;
++ }
++
++ int h(int x, int y, int z)
++ {
++ if (z == 0)
++ {
++ return x;
++ }
++ return y;
++ }
++ EOF
++
++ git merge-file -p --diff3 --diff-algorithm histogram ours.c base.c theirs.c >histogram_output.c &&
++ test_cmp expect.c histogram_output.c
++'
++
+ test_done
Documentation/git-merge-file.txt | 6 ++
builtin/merge-file.c | 28 +++++++
t/t6403-merge-file.sh | 124 ++++++++++++++++++++++++++++++-
3 files changed, 157 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-merge-file.txt b/Documentation/git-merge-file.txt
index 6a081eacb72..71915a00fa4 100644
--- a/Documentation/git-merge-file.txt
+++ b/Documentation/git-merge-file.txt
@@ -92,6 +92,12 @@ object store and the object ID of its blob is written to standard output.
Instead of leaving conflicts in the file, resolve conflicts
favouring our (or their or both) side of the lines.
+--diff-algorithm={patience|minimal|histogram|myers}::
+ Use a different diff algorithm while merging. The current default is "myers",
+ but selecting more recent algorithm such as "histogram" can help
+ avoid mismerges that occur due to unimportant matching lines
+ (such as braces from distinct functions). See also
+ linkgit:git-diff[1] `--diff-algorithm`.
EXAMPLES
--------
diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index 832c93d8d54..1f987334a31 100644
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c
@@ -1,5 +1,6 @@
#include "builtin.h"
#include "abspath.h"
+#include "diff.h"
#include "hex.h"
#include "object-name.h"
#include "object-store.h"
@@ -28,6 +29,30 @@ static int label_cb(const struct option *opt, const char *arg, int unset)
return 0;
}
+static int set_diff_algorithm(xpparam_t *xpp,
+ const char *alg)
+{
+ long diff_algorithm = parse_algorithm_value(alg);
+ if (diff_algorithm < 0)
+ return -1;
+ xpp->flags = (xpp->flags & ~XDF_DIFF_ALGORITHM_MASK) | diff_algorithm;
+ return 0;
+}
+
+static int diff_algorithm_cb(const struct option *opt,
+ const char *arg, int unset)
+{
+ xpparam_t *xpp = opt->value;
+
+ BUG_ON_OPT_NEG(unset);
+
+ if (set_diff_algorithm(xpp, arg))
+ return error(_("option diff-algorithm accepts \"myers\", "
+ "\"minimal\", \"patience\" and \"histogram\""));
+
+ return 0;
+}
+
int cmd_merge_file(int argc, const char **argv, const char *prefix)
{
const char *names[3] = { 0 };
@@ -48,6 +73,9 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
XDL_MERGE_FAVOR_THEIRS),
OPT_SET_INT(0, "union", &xmp.favor, N_("for conflicts, use a union version"),
XDL_MERGE_FAVOR_UNION),
+ OPT_CALLBACK_F(0, "diff-algorithm", &xmp.xpp, N_("<algorithm>"),
+ N_("choose a diff algorithm"),
+ PARSE_OPT_NONEG, diff_algorithm_cb),
OPT_INTEGER(0, "marker-size", &xmp.marker_size,
N_("for conflicts, use this marker size")),
OPT__QUIET(&quiet, N_("do not warn about conflicts")),
diff --git a/t/t6403-merge-file.sh b/t/t6403-merge-file.sh
index 2c92209ecab..fb872c5a113 100755
--- a/t/t6403-merge-file.sh
+++ b/t/t6403-merge-file.sh
@@ -56,7 +56,67 @@ test_expect_success 'setup' '
deduxit me super semitas jusitiae,
EOF
- printf "propter nomen suum." >>new4.txt
+ printf "propter nomen suum." >>new4.txt &&
+
+ cat >base.c <<-\EOF &&
+ int f(int x, int y)
+ {
+ if (x == 0)
+ {
+ return y;
+ }
+ return x;
+ }
+
+ int g(size_t u)
+ {
+ while (u < 30)
+ {
+ u++;
+ }
+ return u;
+ }
+ EOF
+
+ cat >ours.c <<-\EOF &&
+ int g(size_t u)
+ {
+ while (u < 30)
+ {
+ u++;
+ }
+ return u;
+ }
+
+ int h(int x, int y, int z)
+ {
+ if (z == 0)
+ {
+ return x;
+ }
+ return y;
+ }
+ EOF
+
+ cat >theirs.c <<-\EOF
+ int f(int x, int y)
+ {
+ if (x == 0)
+ {
+ return y;
+ }
+ return x;
+ }
+
+ int g(size_t u)
+ {
+ while (u > 34)
+ {
+ u--;
+ }
+ return u;
+ }
+ EOF
'
test_expect_success 'merge with no changes' '
@@ -447,4 +507,66 @@ test_expect_success '--object-id fails without repository' '
grep "not a git repository" err
'
+test_expect_success 'merging C files with "myers" diff algorithm creates some spurious conflicts' '
+ cat >expect.c <<-\EOF &&
+ int g(size_t u)
+ {
+ while (u < 30)
+ {
+ u++;
+ }
+ return u;
+ }
+
+ int h(int x, int y, int z)
+ {
+ <<<<<<< ours.c
+ if (z == 0)
+ ||||||| base.c
+ while (u < 30)
+ =======
+ while (u > 34)
+ >>>>>>> theirs.c
+ {
+ <<<<<<< ours.c
+ return x;
+ ||||||| base.c
+ u++;
+ =======
+ u--;
+ >>>>>>> theirs.c
+ }
+ return y;
+ }
+ EOF
+
+ test_must_fail git merge-file -p --diff3 --diff-algorithm myers ours.c base.c theirs.c >myers_output.c &&
+ test_cmp expect.c myers_output.c
+'
+
+test_expect_success 'merging C files with "histogram" diff algorithm avoids some spurious conflicts' '
+ cat >expect.c <<-\EOF &&
+ int g(size_t u)
+ {
+ while (u > 34)
+ {
+ u--;
+ }
+ return u;
+ }
+
+ int h(int x, int y, int z)
+ {
+ if (z == 0)
+ {
+ return x;
+ }
+ return y;
+ }
+ EOF
+
+ git merge-file -p --diff3 --diff-algorithm histogram ours.c base.c theirs.c >histogram_output.c &&
+ test_cmp expect.c histogram_output.c
+'
+
test_done
base-commit: 98009afd24e2304bf923a64750340423473809ff
--
gitgitgadget
^ permalink raw reply related
* [ANNOUNCE] Git for Windows 2.43.0
From: Johannes Schindelin @ 2023-11-20 19:01 UTC (permalink / raw)
To: git-for-windows, git, git-packagers; +Cc: Johannes Schindelin
Dear Git users,
I hereby announce that Git for Windows 2.43.0 is available from:
https://gitforwindows.org/
Changes since Git for Windows v2.42.0(2) (August 30th 2023)
As announced previously, Git for Windows will drop support for Windows
7 and for Windows 8 in one of the next versions, following Cygwin's and
MSYS2's lead (Git for Windows relies on MSYS2 for components such as
Bash and Perl).
Following the footsteps of the MSYS2 and Cygwin projects on which Git
for Windows depends, the 32-bit variant of Git for Windows is being
phased out.
New Features
* Comes with Git v2.43.0.
* Comes with MSYS2 runtime v3.4.9.
* Comes with GNU TLS v3.8.1.
* When installing into a Windows setup with Mandatory Address Space
Layout Randomization (ASLR) enabled, which is incompatible with the
MSYS2 runtime powering Git Bash, SSH and some other programs
distributed with Git for Windows, the Git for Windows installer now
offers to add exceptions that will allow those programs to work as
expected.
* Comes with OpenSSH v9.5.P1.
* Comes with cURL v8.4.0.
* Comes with OpenSSL v3.1.4.
* Comes with Git Credential Manager v2.4.1.
* Comes with Bash v5.2.21.
* Comes with MinTTY v3.7.0.
Bug Fixes
* Symbolic links whose target is an absolute path without the drive
prefix accidentally had a drive prefix added when checked out,
rendering them "eternally modified". This bug has been fixed.
* Git for Windows's installer is no longer confused by global GIT_*
environment variables.
* The installer no longer claims that "fast-forward or merge" is the
default git pull behavior: The default behavior has changed in Git
a while ago, to "fast-forward only".
Git-2.43.0-64-bit.exe | a6058d7c4c16bfa5bcd6fde051a92de8c68535fd7ebade55fc0ab1c41be3c8d5
Git-2.43.0-32-bit.exe | aee1587a4004c6a57b614c81fdc2ae1fa33de0daaf6b650cf6467e4253e024a9
PortableGit-2.43.0-64-bit.7z.exe | c76216d032685fa972d129eca30f8c9fb957eb9f46ccbce954e70e07d6211961
PortableGit-2.43.0-32-bit.7z.exe | c33f9aa7bf9c59e24db71b65e9d75b1e8532562175afef380119aa1eee90afd1
MinGit-2.43.0-64-bit.zip | 1905d93068e986258fafc69517df8fddff829bb2a289c1fa4dcc6cdf720ddf36
MinGit-2.43.0-32-bit.zip | d46fac9c17b55627f714aefa36c3b00d81651d2bb4076a12b4455b5f841f1a9e
MinGit-2.43.0-busybox-64-bit.zip | 2bd705f2c378ccbbf25a9095432aada3ac9dd2d963eff51421944beaccdc3e0c
MinGit-2.43.0-busybox-32-bit.zip | 70799d1f5b9d2469f44299ff33461efd7814531dd9bfb7ae912d1cbf83478162
Git-2.43.0-64-bit.tar.bz2 | 4c19cc73003e55ec71d6f1ce4a961ab32ca22f9c57217d224982535161123f79
Git-2.43.0-32-bit.tar.bz2 | 192f58080247f1eea2845fb61e37e91c05a89b44260c7e045b936ca3e45ac7f6
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH] object-name: reject too-deep recursive ancestor queries
From: Junio C Hamano @ 2023-11-20 17:14 UTC (permalink / raw)
To: Taylor Blau
Cc: git, Jeff King, Patrick Steinhardt,
Carlos Andrés Ramírez Cataño
In-Reply-To: <57c0b30ddfe7c0ae78069682ff8454791e54469f.1700496801.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
> Since this is a local-only exploit, a user would have to be tricked into
> running such a query by an adversary. Even if they were successfully
> tricked into running the malicious query, the blast radius is limited to
> a local stack overflow, which does not have meaningful paths to remote
> code execution, arbitrary memory reads, or any more grave security
> concerns.
> ...
So the difference in practice is if we make a controlled call to
die() or just let it crash? It still does sound worthwhile thing to
do to make sure we make a controlled death. But ...
> +static int get_nth_ancestor_max_depth = 4096;
> +static int get_nth_ancestor_curr_depth;
... do we have a lock at a much higher level that prevents multiple
name-to-oid look-ups from running simultaneously, or something
similar, to make use of this static counter safe? I am not offhand
sure how safe it is to assume that we'd always be single-threaded.
This variable leaves a bad taste in my mouth.
I am not offhand sure how hard it is to count the depth per
callpath; get_oid_1() is the sole caller of get_nth_ancestor(), so
if you rename the former into a separate helper with a new
"recursion_depth" parameter, create a thin wrapper around it that
starts the recursion at depth 0 and have everybody else (i.e.,
peel_onion() and get_oid_with_context_1()) call it, and have
get_nth_ancestor increment (and die as needed) the counter, would
that be sufficient to ensure that we count the depth per call
invocation?
Thanks.
^ permalink raw reply
* What's cooking in git.git (Nov 2023, #08; Mon, 20)
From: Junio C Hamano @ 2023-11-20 17:01 UTC (permalink / raw)
To: git
Here are the topics that have been cooking in my tree. Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release). Commits prefixed with '-' are only in 'seen', and
aren't considered "accepted" at all and may be annotated with an URL
to a message that raises issues but they are no means exhaustive. A
topic without enough support may be discarded after a long period of
no activity (of course they can be resubmit when new interests
arise).
Git 2.43 has been tagged. With many folks are away from the
keyboard for vacation, I would expect it will be a very slow week.
I'll be taking a few weeks off, too, so please enjoy this release ;-).
Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors. Some
repositories have only a subset of branches.
With maint, master, next, seen, todo:
git://git.kernel.org/pub/scm/git/git.git/
git://repo.or.cz/alt-git.git/
https://kernel.googlesource.com/pub/scm/git/git/
https://github.com/git/git/
https://gitlab.com/git-vcs/git/
With all the integration branches and topics broken out:
https://github.com/gitster/git/
Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):
git://git.kernel.org/pub/scm/git/git-htmldocs.git/
https://github.com/gitster/git-htmldocs.git/
Release tarballs are available at:
https://www.kernel.org/pub/software/scm/git/
--------------------------------------------------
[Graduated to 'master']
* tz/send-email-helpfix (2023-11-16) 1 commit
(merged to 'next' on 2023-11-17 at 8422271795)
+ send-email: remove stray characters from usage
Typoes in "git send-email -h" have been corrected.
source: <20231115173952.339303-3-tmz@pobox.com>
* vd/glossary-dereference-peel (2023-11-14) 1 commit
(merged to 'next' on 2023-11-17 at bac3ab0c0b)
+ glossary: add definitions for dereference & peel
"To dereference" and "to peel" were sometimes used in in-code
comments and documentation but without description in the glossary.
source: <pull.1610.v2.git.1699917471769.gitgitgadget@gmail.com>
--------------------------------------------------
[New Topics]
* ac/fuzz-show-date (2023-11-20) 1 commit
- fuzz: add new oss-fuzz fuzzer for date.c / date.h
Subject approxidate() and show_date() macchinery to OSS-Fuzz.
Will merge to 'next'?
source: <pull.1612.v4.git.1700243267653.gitgitgadget@gmail.com>
* js/packfile-h-typofix (2023-11-20) 1 commit
- packfile.c: fix a typo in `each_file_in_pack_dir_fn()`'s declaration
Typofix.
Will merge to 'next'.
source: <pull.1614.git.1700226915859.gitgitgadget@gmail.com>
--------------------------------------------------
[Stalled]
* pw/rebase-sigint (2023-09-07) 1 commit
- rebase -i: ignore signals when forking subprocesses
If the commit log editor or other external programs (spawned via
"exec" insn in the todo list) receive internactive signal during
"git rebase -i", it caused not just the spawned program but the
"Git" process that spawned them, which is often not what the end
user intended. "git" learned to ignore SIGINT and SIGQUIT while
waiting for these subprocesses.
Expecting a reroll.
cf. <12c956ea-330d-4441-937f-7885ab519e26@gmail.com>
source: <pull.1581.git.1694080982621.gitgitgadget@gmail.com>
* tk/cherry-pick-sequence-requires-clean-worktree (2023-06-01) 1 commit
- cherry-pick: refuse cherry-pick sequence if index is dirty
"git cherry-pick A" that replays a single commit stopped before
clobbering local modification, but "git cherry-pick A..B" did not,
which has been corrected.
Expecting a reroll.
cf. <999f12b2-38d6-f446-e763-4985116ad37d@gmail.com>
source: <pull.1535.v2.git.1685264889088.gitgitgadget@gmail.com>
* jc/diff-cached-fsmonitor-fix (2023-09-15) 3 commits
- diff-lib: fix check_removed() when fsmonitor is active
- Merge branch 'jc/fake-lstat' into jc/diff-cached-fsmonitor-fix
- Merge branch 'js/diff-cached-fsmonitor-fix' into jc/diff-cached-fsmonitor-fix
(this branch uses jc/fake-lstat.)
The optimization based on fsmonitor in the "diff --cached"
codepath is resurrected with the "fake-lstat" introduced earlier.
It is unknown if the optimization is worth resurrecting, but in case...
source: <xmqqr0n0h0tw.fsf@gitster.g>
--------------------------------------------------
[Cooking]
* jw/builtin-objectmode-attr (2023-11-16) 2 commits
- SQUASH???
- attr: add builtin objectmode values support
The builtin_objectmode attribute is populated for each path
without adding anything in .gitattributes files, which would be
useful in magic pathspec, e.g., ":(attr:builtin_objectmode=100755)"
to limit to executables.
source: <20231116054437.2343549-1-jojwang@google.com>
* ps/ref-deletion-updates (2023-11-17) 4 commits
- refs: remove `delete_refs` callback from backends
- refs: deduplicate code to delete references
- refs/files: use transactions to delete references
- t5510: ensure that the packed-refs file needs locking
Simplify API implementation to delete references by eliminating
duplication.
Will merge to 'next'.
source: <cover.1699951815.git.ps@pks.im>
* tz/send-email-negatable-options (2023-11-17) 2 commits
(merged to 'next' on 2023-11-17 at f09e533e43)
+ send-email: avoid duplicate specification warnings
+ perl: bump the required Perl version to 5.8.1 from 5.8.0
Newer versions of Getopt::Long started giving warnings against our
(ab)use of it in "git send-email". Bump the minimum version
requirement for Perl to 5.8.1 (from September 2002) to allow
simplifying our implementation.
Will cook in 'next'.
source: <20231116193014.470420-1-tmz@pobox.com>
* js/ci-discard-prove-state (2023-11-14) 1 commit
(merged to 'next' on 2023-11-14 at fade3ba143)
+ ci: avoid running the test suite _twice_
(this branch uses ps/ci-gitlab.)
The way CI testing used "prove" could lead to running the test
suite twice needlessly, which has been corrected.
Will cook in 'next'.
source: <pull.1613.git.1699894837844.gitgitgadget@gmail.com>
* jk/chunk-bounds-more (2023-11-09) 9 commits
(merged to 'next' on 2023-11-13 at 3df4b18bea)
+ commit-graph: mark chunk error messages for translation
+ commit-graph: drop verify_commit_graph_lite()
+ commit-graph: check order while reading fanout chunk
+ commit-graph: use fanout value for graph size
+ commit-graph: abort as soon as we see a bogus chunk
+ commit-graph: clarify missing-chunk error messages
+ commit-graph: drop redundant call to "lite" verification
+ midx: check consistency of fanout table
+ commit-graph: handle overflow in chunk_size checks
(this branch is used by tb/pair-chunk-expect.)
Code clean-up for jk/chunk-bounds topic.
Will cook in 'next'.
source: <20231109070310.GA2697602@coredump.intra.peff.net>
* ps/httpd-tests-on-nixos (2023-11-11) 3 commits
(merged to 'next' on 2023-11-13 at 81bd6f5334)
+ t9164: fix inability to find basename(1) in Subversion hooks
+ t/lib-httpd: stop using legacy crypt(3) for authentication
+ t/lib-httpd: dynamically detect httpd and modules path
Portability tweak.
Will cook in 'next'.
source: <cover.1699596457.git.ps@pks.im>
* ss/format-patch-use-encode-headers-for-cover-letter (2023-11-10) 1 commit
(merged to 'next' on 2023-11-14 at 1a4bd59e15)
+ format-patch: fix ignored encode_email_headers for cover letter
"git format-patch --encode-email-headers" ignored the option when
preparing the cover letter, which has been corrected.
Will cook in 'next'.
source: <20231109111950.387219-1-contact@emersion.fr>
* ps/ban-a-or-o-operator-with-test (2023-11-11) 4 commits
(merged to 'next' on 2023-11-14 at d84471baab)
+ Makefile: stop using `test -o` when unlinking duplicate executables
+ contrib/subtree: convert subtree type check to use case statement
+ contrib/subtree: stop using `-o` to test for number of args
+ global: convert trivial usages of `test <expr> -a/-o <expr>`
Test and shell scripts clean-up.
Will cook in 'next'.
source: <cover.1699609940.git.ps@pks.im>
* ak/rebase-autosquash (2023-11-16) 3 commits
(merged to 'next' on 2023-11-17 at 3ed6e79445)
+ rebase: rewrite --(no-)autosquash documentation
+ rebase: support --autosquash without -i
+ rebase: fully ignore rebase.autoSquash without -i
"git rebase --autosquash" is now enabled for non-interactive rebase,
but it is still incompatible with the apply backend.
Will cook in 'next'.
source: <20231114214339.10925-1-andy.koppe@gmail.com>
* vd/for-each-ref-unsorted-optimization (2023-11-16) 10 commits
(merged to 'next' on 2023-11-17 at ff99420bf6)
+ t/perf: add perf tests for for-each-ref
+ ref-filter.c: use peeled tag for '*' format fields
+ for-each-ref: clean up documentation of --format
+ ref-filter.c: filter & format refs in the same callback
+ ref-filter.c: refactor to create common helper functions
+ ref-filter.c: rename 'ref_filter_handler()' to 'filter_one()'
+ ref-filter.h: add functions for filter/format & format-only
+ ref-filter.h: move contains caches into filter
+ ref-filter.h: add max_count and omit_empty to ref_format
+ ref-filter.c: really don't sort when using --no-sort
"git for-each-ref --no-sort" still sorted the refs alphabetically
which paid non-trivial cost. It has been redefined to show output
in an unspecified order, to allow certain optimizations to take
advantage of.
Will cook in 'next'.
source: <pull.1609.v2.git.1699991638.gitgitgadget@gmail.com>
* jw/git-add-attr-pathspec (2023-11-04) 1 commit
(merged to 'next' on 2023-11-13 at b61be94e4d)
+ attr: enable attr pathspec magic for git-add and git-stash
"git add" and "git stash" learned to support the ":(attr:...)"
magic pathspec.
Will cook in 'next'.
source: <20231103163449.1578841-1-jojwang@google.com>
* ps/ci-gitlab (2023-11-09) 8 commits
(merged to 'next' on 2023-11-10 at ea7ed67945)
+ ci: add support for GitLab CI
+ ci: install test dependencies for linux-musl
+ ci: squelch warnings when testing with unusable Git repo
+ ci: unify setup of some environment variables
+ ci: split out logic to set up failed test artifacts
+ ci: group installation of Docker dependencies
+ ci: make grouping setup more generic
+ ci: reorder definitions for grouping functions
(this branch is used by js/ci-discard-prove-state.)
Add support for GitLab CI.
Will cook in 'next'.
source: <cover.1699514143.git.ps@pks.im>
* ps/ref-tests-update (2023-11-03) 10 commits
(merged to 'next' on 2023-11-13 at dc26e55d6f)
+ t: mark several tests that assume the files backend with REFFILES
+ t7900: assert the absence of refs via git-for-each-ref(1)
+ t7300: assert exact states of repo
+ t4207: delete replace references via git-update-ref(1)
+ t1450: convert tests to remove worktrees via git-worktree(1)
+ t: convert tests to not access reflog via the filesystem
+ t: convert tests to not access symrefs via the filesystem
+ t: convert tests to not write references via the filesystem
+ t: allow skipping expected object ID in `ref-store update-ref`
+ Merge branch 'ps/show-ref' into ps/ref-tests-update
Update ref-related tests.
Will cook in 'next'.
source: <cover.1698914571.git.ps@pks.im>
* jx/fetch-atomic-error-message-fix (2023-10-19) 2 commits
- fetch: no redundant error message for atomic fetch
- t5574: test porcelain output of atomic fetch
"git fetch --atomic" issued an unnecessary empty error message,
which has been corrected.
Expecting an update.
cf. <ZTjQIrCgSANAT8wR@tanuki>
source: <ced46baeb1c18b416b4b4cc947f498bea2910b1b.1697725898.git.zhiyou.jx@alibaba-inc.com>
* js/bugreport-in-the-same-minute (2023-10-16) 1 commit
- bugreport: include +i in outfile suffix as needed
Instead of auto-generating a filename that is already in use for
output and fail the command, `git bugreport` learned to fuzz the
filename to avoid collisions with existing files.
Expecting a reroll.
cf. <ZTtZ5CbIGETy1ucV.jacob@initialcommit.io>
source: <20231016214045.146862-2-jacob@initialcommit.io>
* kh/t7900-cleanup (2023-10-17) 9 commits
- t7900: fix register dependency
- t7900: factor out packfile dependency
- t7900: fix `print-args` dependency
- t7900: fix `pfx` dependency
- t7900: factor out common schedule setup
- t7900: factor out inheritance test dependency
- t7900: create commit so that branch is born
- t7900: setup and tear down clones
- t7900: remove register dependency
Test clean-up.
Perhaps discard?
cf. <655ca147-c214-41be-919d-023c1b27b311@app.fastmail.com>
source: <cover.1697319294.git.code@khaugsbakk.name>
* tb/merge-tree-write-pack (2023-10-23) 5 commits
- builtin/merge-tree.c: implement support for `--write-pack`
- bulk-checkin: introduce `index_tree_bulk_checkin_incore()`
- bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
- bulk-checkin: generify `stream_blob_to_pack()` for arbitrary types
- bulk-checkin: extract abstract `bulk_checkin_source`
"git merge-tree" learned "--write-pack" to record its result
without creating loose objects.
Broken when an object created during a merge is needed to continue merge
cf. <CABPp-BEfy9VOvimP9==ry_rZXu=metOQ8s=_-XiG_Pdx9c06Ww@mail.gmail.com>
source: <cover.1698101088.git.me@ttaylorr.com>
* tb/pair-chunk-expect (2023-11-10) 8 commits
- midx: read `OOFF` chunk with `pair_chunk_expect()`
- midx: read `OIDL` chunk with `pair_chunk_expect()`
- commit-graph: read `BIDX` chunk with `pair_chunk_expect()`
- commit-graph: read `GDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `CDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `OIDL` chunk with `pair_chunk_expect()`
- chunk-format: introduce `pair_chunk_expect()` helper
- Merge branch 'jk/chunk-bounds-more' into HEAD
(this branch uses jk/chunk-bounds-more.)
Further code clean-up.
Needs review.
source: <cover.1699569246.git.me@ttaylorr.com>
* tb/path-filter-fix (2023-10-18) 17 commits
- bloom: introduce `deinit_bloom_filters()`
- commit-graph: reuse existing Bloom filters where possible
- object.h: fix mis-aligned flag bits table
- commit-graph: drop unnecessary `graph_read_bloom_data_context`
- commit-graph.c: unconditionally load Bloom filters
- bloom: prepare to discard incompatible Bloom filters
- bloom: annotate filters with hash version
- commit-graph: new filter ver. that fixes murmur3
- repo-settings: introduce commitgraph.changedPathsVersion
- t4216: test changed path filters with high bit paths
- t/helper/test-read-graph: implement `bloom-filters` mode
- bloom.h: make `load_bloom_filter_from_graph()` public
- t/helper/test-read-graph.c: extract `dump_graph_info()`
- gitformat-commit-graph: describe version 2 of BDAT
- commit-graph: ensure Bloom filters are read with consistent settings
- revision.c: consult Bloom filters for root commits
- t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
The Bloom filter used for path limited history traversal was broken
on systems whose "char" is unsigned; update the implementation and
bump the format version to 2.
Needs (hopefully final and quick) review.
source: <cover.1697653929.git.me@ttaylorr.com>
* cc/git-replay (2023-11-16) 14 commits
- replay: stop assuming replayed branches do not diverge
- replay: add --contained to rebase contained branches
- replay: add --advance or 'cherry-pick' mode
- replay: use standard revision ranges
- replay: make it a minimal server side command
- replay: remove HEAD related sanity check
- replay: remove progress and info output
- replay: add an important FIXME comment about gpg signing
- replay: change rev walking options
- replay: introduce pick_regular_commit()
- replay: die() instead of failing assert()
- replay: start using parse_options API
- replay: introduce new builtin
- t6429: remove switching aspects of fast-rebase
Introduce "git replay", a tool meant on the server side without
working tree to recreate a history.
source: <20231115143327.2441397-1-christian.couder@gmail.com>
* ak/color-decorate-symbols (2023-10-23) 7 commits
- log: add color.decorate.pseudoref config variable
- refs: exempt pseudorefs from pattern prefixing
- refs: add pseudorefs array and iteration functions
- log: add color.decorate.ref config variable
- log: add color.decorate.symbol config variable
- log: use designated inits for decoration_colors
- config: restructure color.decorate documentation
A new config for coloring.
Needs review.
source: <20231023221143.72489-1-andy.koppe@gmail.com>
* js/update-urls-in-doc-and-comment (2023-09-26) 4 commits
- doc: refer to internet archive
- doc: update links for andre-simon.de
- doc: update links to current pages
- doc: switch links to https
Stale URLs have been updated to their current counterparts (or
archive.org) and HTTP links are replaced with working HTTPS links.
Needs review.
source: <pull.1589.v2.git.1695553041.gitgitgadget@gmail.com>
* la/trailer-cleanups (2023-10-20) 3 commits
- trailer: use offsets for trailer_start/trailer_end
- trailer: find the end of the log message
- commit: ignore_non_trailer computes number of bytes to ignore
Code clean-up.
Comments?
source: <pull.1563.v5.git.1697828495.gitgitgadget@gmail.com>
* eb/hash-transition (2023-10-02) 30 commits
- t1016-compatObjectFormat: add tests to verify the conversion between objects
- t1006: test oid compatibility with cat-file
- t1006: rename sha1 to oid
- test-lib: compute the compatibility hash so tests may use it
- builtin/ls-tree: let the oid determine the output algorithm
- object-file: handle compat objects in check_object_signature
- tree-walk: init_tree_desc take an oid to get the hash algorithm
- builtin/cat-file: let the oid determine the output algorithm
- rev-parse: add an --output-object-format parameter
- repository: implement extensions.compatObjectFormat
- object-file: update object_info_extended to reencode objects
- object-file-convert: convert commits that embed signed tags
- object-file-convert: convert commit objects when writing
- object-file-convert: don't leak when converting tag objects
- object-file-convert: convert tag objects when writing
- object-file-convert: add a function to convert trees between algorithms
- object: factor out parse_mode out of fast-import and tree-walk into in object.h
- cache: add a function to read an OID of a specific algorithm
- tag: sign both hashes
- commit: export add_header_signature to support handling signatures on tags
- commit: convert mergetag before computing the signature of a commit
- commit: write commits for both hashes
- object-file: add a compat_oid_in parameter to write_object_file_flags
- object-file: update the loose object map when writing loose objects
- loose: compatibilty short name support
- loose: add a mapping between SHA-1 and SHA-256 for loose objects
- repository: add a compatibility hash algorithm
- object-names: support input of oids in any supported hash
- oid-array: teach oid-array to handle multiple kinds of oids
- object-file-convert: stubs for converting from one object format to another
Teach a repository to work with both SHA-1 and SHA-256 hash algorithms.
Needs review.
source: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
* jx/remote-archive-over-smart-http (2023-10-04) 4 commits
- archive: support remote archive from stateless transport
- transport-helper: call do_take_over() in connect_helper
- transport-helper: call do_take_over() in process_connect
- transport-helper: no connection restriction in connect_helper
"git archive --remote=<remote>" learned to talk over the smart
http (aka stateless) transport.
Needs review.
source: <cover.1696432593.git.zhiyou.jx@alibaba-inc.com>
* jx/sideband-chomp-newline-fix (2023-10-04) 3 commits
- pkt-line: do not chomp newlines for sideband messages
- pkt-line: memorize sideband fragment in reader
- test-pkt-line: add option parser for unpack-sideband
Sideband demultiplexer fixes.
Needs review.
source: <cover.1696425168.git.zhiyou.jx@alibaba-inc.com>
* js/config-parse (2023-09-21) 5 commits
- config-parse: split library out of config.[c|h]
- config.c: accept config_parse_options in git_config_from_stdin
- config: report config parse errors using cb
- config: split do_event() into start and flush operations
- config: split out config_parse_options
The parsing routines for the configuration files have been split
into a separate file.
Needs review.
source: <cover.1695330852.git.steadmon@google.com>
* jc/fake-lstat (2023-09-15) 1 commit
- cache: add fake_lstat()
(this branch is used by jc/diff-cached-fsmonitor-fix.)
A new helper to let us pretend that we called lstat() when we know
our cache_entry is up-to-date via fsmonitor.
Needs review.
source: <xmqqcyykig1l.fsf@gitster.g>
* js/doc-unit-tests (2023-11-10) 3 commits
(merged to 'next' on 2023-11-10 at 7d00ffd06b)
+ ci: run unit tests in CI
+ unit tests: add TAP unit test framework
+ unit tests: add a project plan document
(this branch is used by js/doc-unit-tests-with-cmake.)
Process to add some form of low-level unit tests has started.
Will cook in 'next'.
source: <cover.1699555664.git.steadmon@google.com>
* js/doc-unit-tests-with-cmake (2023-11-10) 7 commits
(merged to 'next' on 2023-11-10 at b4503c9c8c)
+ cmake: handle also unit tests
+ cmake: use test names instead of full paths
+ cmake: fix typo in variable name
+ artifacts-tar: when including `.dll` files, don't forget the unit-tests
+ unit-tests: do show relative file paths
+ unit-tests: do not mistake `.pdb` files for being executable
+ cmake: also build unit tests
(this branch uses js/doc-unit-tests.)
Update the base topic to work with CMake builds.
Will cook in 'next'.
source: <pull.1579.v3.git.1695640836.gitgitgadget@gmail.com>
* jc/rerere-cleanup (2023-08-25) 4 commits
- rerere: modernize use of empty strbuf
- rerere: try_merge() should use LL_MERGE_ERROR when it means an error
- rerere: fix comment on handle_file() helper
- rerere: simplify check_one_conflict() helper function
Code clean-up.
Not ready to be reviewed yet.
source: <20230824205456.1231371-1-gitster@pobox.com>
* rj/status-bisect-while-rebase (2023-10-16) 1 commit
- status: fix branch shown when not only bisecting
"git status" is taught to show both the branch being bisected and
being rebased when both are in effect at the same time.
Needs review.
source: <2e24ca9b-9c5f-f4df-b9f8-6574a714dfb2@gmail.com>
--------------------------------------------------
[Discarded]
* jc/strbuf-comment-line-char (2023-11-01) 4 commits
. strbuf: move env-using functions to environment.c
. strbuf: make add_lines() public
. strbuf_add_commented_lines(): drop the comment_line_char parameter
. strbuf_commented_addf(): drop the comment_line_char parameter
Code simplification that goes directly against a past libification
topic. It is hard to judge because the "libification" is done
piecewise without seemingly clear design principle.
Will discard.
source: <cover.1698791220.git.jonathantanmy@google.com>
^ permalink raw reply
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