* [PATCH v7 7/9] gc: add `gc.repackFilter` config option
From: Christian Couder @ 2023-09-25 15:25 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, John Cai, Jonathan Tan, Jonathan Nieder,
Taylor Blau, Derrick Stolee, Patrick Steinhardt, Christian Couder,
Christian Couder
In-Reply-To: <20230925152517.803579-1-christian.couder@gmail.com>
A previous commit has implemented `git repack --filter=<filter-spec>` to
allow users to filter out some objects from the main pack and move them
into a new different pack.
Users might want to perform such a cleanup regularly at the same time as
they perform other repacks and cleanups, so as part of `git gc`.
Let's allow them to configure a <filter-spec> for that purpose using a
new gc.repackFilter config option.
Now when `git gc` will perform a repack with a <filter-spec> configured
through this option and not empty, the repack process will be passed a
corresponding `--filter=<filter-spec>` argument.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/config/gc.txt | 5 +++++
builtin/gc.c | 6 ++++++
t/t6500-gc.sh | 13 +++++++++++++
3 files changed, 24 insertions(+)
diff --git a/Documentation/config/gc.txt b/Documentation/config/gc.txt
index ca47eb2008..2153bde7ac 100644
--- a/Documentation/config/gc.txt
+++ b/Documentation/config/gc.txt
@@ -145,6 +145,11 @@ Multiple hooks are supported, but all must exit successfully, else the
operation (either generating a cruft pack or unpacking unreachable
objects) will be halted.
+gc.repackFilter::
+ When repacking, use the specified filter to move certain
+ objects into a separate packfile. See the
+ `--filter=<filter-spec>` option of linkgit:git-repack[1].
+
gc.rerereResolved::
Records of conflicted merge you resolved earlier are
kept for this many days when 'git rerere gc' is run.
diff --git a/builtin/gc.c b/builtin/gc.c
index 00192ae5d3..98148e98fe 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -61,6 +61,7 @@ static timestamp_t gc_log_expire_time;
static const char *gc_log_expire = "1.day.ago";
static const char *prune_expire = "2.weeks.ago";
static const char *prune_worktrees_expire = "3.months.ago";
+static char *repack_filter;
static unsigned long big_pack_threshold;
static unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;
@@ -170,6 +171,8 @@ static void gc_config(void)
git_config_get_ulong("gc.bigpackthreshold", &big_pack_threshold);
git_config_get_ulong("pack.deltacachesize", &max_delta_cache_size);
+ git_config_get_string("gc.repackfilter", &repack_filter);
+
git_config(git_default_config, NULL);
}
@@ -355,6 +358,9 @@ static void add_repack_all_option(struct string_list *keep_pack)
if (keep_pack)
for_each_string_list(keep_pack, keep_one_pack, NULL);
+
+ if (repack_filter && *repack_filter)
+ strvec_pushf(&repack, "--filter=%s", repack_filter);
}
static void add_repack_incremental_option(void)
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 69509d0c11..232e403b66 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -202,6 +202,19 @@ test_expect_success 'one of gc.reflogExpire{Unreachable,}=never does not skip "e
grep -E "^trace: (built-in|exec|run_command): git reflog expire --" trace.out
'
+test_expect_success 'gc.repackFilter launches repack with a filter' '
+ test_when_finished "rm -rf bare.git" &&
+ git clone --no-local --bare . bare.git &&
+
+ git -C bare.git -c gc.cruftPacks=false gc &&
+ test_stdout_line_count = 1 ls bare.git/objects/pack/*.pack &&
+
+ GIT_TRACE=$(pwd)/trace.out git -C bare.git -c gc.repackFilter=blob:none \
+ -c repack.writeBitmaps=false -c gc.cruftPacks=false gc &&
+ test_stdout_line_count = 2 ls bare.git/objects/pack/*.pack &&
+ grep -E "^trace: (built-in|exec|run_command): git repack .* --filter=blob:none ?.*" trace.out
+'
+
prepare_cruft_history () {
test_commit base &&
--
2.42.0.279.g57b2ba444c
^ permalink raw reply related
* [PATCH v7 6/9] repack: add `--filter=<filter-spec>` option
From: Christian Couder @ 2023-09-25 15:25 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, John Cai, Jonathan Tan, Jonathan Nieder,
Taylor Blau, Derrick Stolee, Patrick Steinhardt, Christian Couder,
Christian Couder
In-Reply-To: <20230925152517.803579-1-christian.couder@gmail.com>
This new option puts the objects specified by `<filter-spec>` into a
separate packfile.
This could be useful if, for example, some blobs take up a lot of
precious space on fast storage while they are rarely accessed. It could
make sense to move them into a separate cheaper, though slower, storage.
It's possible to find which new packfile contains the filtered out
objects using one of the following:
- `git verify-pack -v ...`,
- `test-tool find-pack ...`, which a previous commit added,
- `--filter-to=<dir>`, which a following commit will add to specify
where the pack containing the filtered out objects will be.
This feature is implemented by running `git pack-objects` twice in a
row. The first command is run with `--filter=<filter-spec>`, using the
specified filter. It packs objects while omitting the objects specified
by the filter. Then another `git pack-objects` command is launched using
`--stdin-packs`. We pass it all the previously existing packs into its
stdin, so that it will pack all the objects in the previously existing
packs. But we also pass into its stdin, the pack created by the previous
`git pack-objects --filter=<filter-spec>` command as well as the kept
packs, all prefixed with '^', so that the objects in these packs will be
omitted from the resulting pack. The result is that only the objects
filtered out by the first `git pack-objects` command are in the pack
resulting from the second `git pack-objects` command.
As the interactions with kept packs are a bit tricky, a few related
tests are added.
Helped-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: John Cai <johncai86@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-repack.txt | 12 ++++
builtin/repack.c | 70 ++++++++++++++++++
t/t7700-repack.sh | 135 +++++++++++++++++++++++++++++++++++
3 files changed, 217 insertions(+)
diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt
index 4017157949..6d5bec7716 100644
--- a/Documentation/git-repack.txt
+++ b/Documentation/git-repack.txt
@@ -143,6 +143,18 @@ depth is 4095.
a larger and slower repository; see the discussion in
`pack.packSizeLimit`.
+--filter=<filter-spec>::
+ Remove objects matching the filter specification from the
+ resulting packfile and put them into a separate packfile. Note
+ that objects used in the working directory are not filtered
+ out. So for the split to fully work, it's best to perform it
+ in a bare repo and to use the `-a` and `-d` options along with
+ this option. Also `--no-write-bitmap-index` (or the
+ `repack.writebitmaps` config option set to `false`) should be
+ used otherwise writing bitmap index will fail, as it supposes
+ a single packfile containing all the objects. See
+ linkgit:git-rev-list[1] for valid `<filter-spec>` forms.
+
-b::
--write-bitmap-index::
Write a reachability bitmap index as part of the repack. This
diff --git a/builtin/repack.c b/builtin/repack.c
index 9ef0044384..c7b564192f 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -21,6 +21,7 @@
#include "pack.h"
#include "pack-bitmap.h"
#include "refs.h"
+#include "list-objects-filter-options.h"
#define ALL_INTO_ONE 1
#define LOOSEN_UNREACHABLE 2
@@ -56,6 +57,7 @@ struct pack_objects_args {
int no_reuse_object;
int quiet;
int local;
+ struct list_objects_filter_options filter_options;
};
static int repack_config(const char *var, const char *value,
@@ -836,6 +838,56 @@ static int finish_pack_objects_cmd(struct child_process *cmd,
return finish_command(cmd);
}
+static int write_filtered_pack(const struct pack_objects_args *args,
+ const char *destination,
+ const char *pack_prefix,
+ struct existing_packs *existing,
+ struct string_list *names)
+{
+ struct child_process cmd = CHILD_PROCESS_INIT;
+ struct string_list_item *item;
+ FILE *in;
+ int ret;
+ const char *caret;
+ const char *scratch;
+ int local = skip_prefix(destination, packdir, &scratch);
+
+ prepare_pack_objects(&cmd, args, destination);
+
+ strvec_push(&cmd.args, "--stdin-packs");
+
+ if (!pack_kept_objects)
+ strvec_push(&cmd.args, "--honor-pack-keep");
+ for_each_string_list_item(item, &existing->kept_packs)
+ strvec_pushf(&cmd.args, "--keep-pack=%s", item->string);
+
+ cmd.in = -1;
+
+ ret = start_command(&cmd);
+ if (ret)
+ return ret;
+
+ /*
+ * Here 'names' contains only the pack(s) that were just
+ * written, which is exactly the packs we want to keep. Also
+ * 'existing_kept_packs' already contains the packs in
+ * 'keep_pack_list'.
+ */
+ in = xfdopen(cmd.in, "w");
+ for_each_string_list_item(item, names)
+ fprintf(in, "^%s-%s.pack\n", pack_prefix, item->string);
+ for_each_string_list_item(item, &existing->non_kept_packs)
+ fprintf(in, "%s.pack\n", item->string);
+ for_each_string_list_item(item, &existing->cruft_packs)
+ fprintf(in, "%s.pack\n", item->string);
+ caret = pack_kept_objects ? "" : "^";
+ for_each_string_list_item(item, &existing->kept_packs)
+ fprintf(in, "%s%s.pack\n", caret, item->string);
+ fclose(in);
+
+ return finish_pack_objects_cmd(&cmd, names, local);
+}
+
static int write_cruft_pack(const struct pack_objects_args *args,
const char *destination,
const char *pack_prefix,
@@ -966,6 +1018,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
N_("limits the maximum number of threads")),
OPT_STRING(0, "max-pack-size", &po_args.max_pack_size, N_("bytes"),
N_("maximum size of each packfile")),
+ OPT_PARSE_LIST_OBJECTS_FILTER(&po_args.filter_options),
OPT_BOOL(0, "pack-kept-objects", &pack_kept_objects,
N_("repack objects in packs marked with .keep")),
OPT_STRING_LIST(0, "keep-pack", &keep_pack_list, N_("name"),
@@ -979,6 +1032,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
OPT_END()
};
+ list_objects_filter_init(&po_args.filter_options);
+
git_config(repack_config, &cruft_po_args);
argc = parse_options(argc, argv, prefix, builtin_repack_options,
@@ -1119,6 +1174,10 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
strvec_push(&cmd.args, "--incremental");
}
+ if (po_args.filter_options.choice)
+ strvec_pushf(&cmd.args, "--filter=%s",
+ expand_list_objects_filter_spec(&po_args.filter_options));
+
if (geometry.split_factor)
cmd.in = -1;
else
@@ -1205,6 +1264,16 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
}
}
+ if (po_args.filter_options.choice) {
+ ret = write_filtered_pack(&po_args,
+ packtmp,
+ find_pack_prefix(packdir, packtmp),
+ &existing,
+ &names);
+ if (ret)
+ goto cleanup;
+ }
+
string_list_sort(&names);
close_object_store(the_repository->objects);
@@ -1297,6 +1366,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
string_list_clear(&names, 1);
existing_packs_release(&existing);
free_pack_geometry(&geometry);
+ list_objects_filter_release(&po_args.filter_options);
return ret;
}
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 27b66807cd..39e89445fd 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -327,6 +327,141 @@ test_expect_success 'auto-bitmaps do not complain if unavailable' '
test_must_be_empty actual
'
+test_expect_success 'repacking with a filter works' '
+ git -C bare.git repack -a -d &&
+ test_stdout_line_count = 1 ls bare.git/objects/pack/*.pack &&
+ git -C bare.git -c repack.writebitmaps=false repack -a -d --filter=blob:none &&
+ test_stdout_line_count = 2 ls bare.git/objects/pack/*.pack &&
+ commit_pack=$(test-tool -C bare.git find-pack -c 1 HEAD) &&
+ blob_pack=$(test-tool -C bare.git find-pack -c 1 HEAD:file1) &&
+ test "$commit_pack" != "$blob_pack" &&
+ tree_pack=$(test-tool -C bare.git find-pack -c 1 HEAD^{tree}) &&
+ test "$tree_pack" = "$commit_pack" &&
+ blob_pack2=$(test-tool -C bare.git find-pack -c 1 HEAD:file2) &&
+ test "$blob_pack2" = "$blob_pack"
+'
+
+test_expect_success '--filter fails with --write-bitmap-index' '
+ test_must_fail \
+ env GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=0 \
+ git -C bare.git repack -a -d --write-bitmap-index --filter=blob:none
+'
+
+test_expect_success 'repacking with two filters works' '
+ git init two-filters &&
+ (
+ cd two-filters &&
+ mkdir subdir &&
+ test_commit foo &&
+ test_commit subdir_bar subdir/bar &&
+ test_commit subdir_baz subdir/baz
+ ) &&
+ git clone --no-local --bare two-filters two-filters.git &&
+ (
+ cd two-filters.git &&
+ test_stdout_line_count = 1 ls objects/pack/*.pack &&
+ git -c repack.writebitmaps=false repack -a -d \
+ --filter=blob:none --filter=tree:1 &&
+ test_stdout_line_count = 2 ls objects/pack/*.pack &&
+ commit_pack=$(test-tool find-pack -c 1 HEAD) &&
+ blob_pack=$(test-tool find-pack -c 1 HEAD:foo.t) &&
+ root_tree_pack=$(test-tool find-pack -c 1 HEAD^{tree}) &&
+ subdir_tree_hash=$(git ls-tree --object-only HEAD -- subdir) &&
+ subdir_tree_pack=$(test-tool find-pack -c 1 "$subdir_tree_hash") &&
+
+ # Root tree and subdir tree are not in the same packfiles
+ test "$commit_pack" != "$blob_pack" &&
+ test "$commit_pack" = "$root_tree_pack" &&
+ test "$blob_pack" = "$subdir_tree_pack"
+ )
+'
+
+prepare_for_keep_packs () {
+ git init keep-packs &&
+ (
+ cd keep-packs &&
+ test_commit foo &&
+ test_commit bar
+ ) &&
+ git clone --no-local --bare keep-packs keep-packs.git &&
+ (
+ cd keep-packs.git &&
+
+ # Create two packs
+ # The first pack will contain all of the objects except one blob
+ git rev-list --objects --all >objs &&
+ grep -v "bar.t" objs | git pack-objects pack &&
+ # The second pack will contain the excluded object and be kept
+ packid=$(grep "bar.t" objs | git pack-objects pack) &&
+ >pack-$packid.keep &&
+
+ # Replace the existing pack with the 2 new ones
+ rm -f objects/pack/pack* &&
+ mv pack-* objects/pack/
+ )
+}
+
+test_expect_success '--filter works with .keep packs' '
+ prepare_for_keep_packs &&
+ (
+ cd keep-packs.git &&
+
+ foo_pack=$(test-tool find-pack -c 1 HEAD:foo.t) &&
+ bar_pack=$(test-tool find-pack -c 1 HEAD:bar.t) &&
+ head_pack=$(test-tool find-pack -c 1 HEAD) &&
+
+ test "$foo_pack" != "$bar_pack" &&
+ test "$foo_pack" = "$head_pack" &&
+
+ git -c repack.writebitmaps=false repack -a -d --filter=blob:none &&
+
+ foo_pack_1=$(test-tool find-pack -c 1 HEAD:foo.t) &&
+ bar_pack_1=$(test-tool find-pack -c 1 HEAD:bar.t) &&
+ head_pack_1=$(test-tool find-pack -c 1 HEAD) &&
+
+ # Object bar is still only in the old .keep pack
+ test "$foo_pack_1" != "$foo_pack" &&
+ test "$bar_pack_1" = "$bar_pack" &&
+ test "$head_pack_1" != "$head_pack" &&
+
+ test "$foo_pack_1" != "$bar_pack_1" &&
+ test "$foo_pack_1" != "$head_pack_1" &&
+ test "$bar_pack_1" != "$head_pack_1"
+ )
+'
+
+test_expect_success '--filter works with --pack-kept-objects and .keep packs' '
+ rm -rf keep-packs keep-packs.git &&
+ prepare_for_keep_packs &&
+ (
+ cd keep-packs.git &&
+
+ foo_pack=$(test-tool find-pack -c 1 HEAD:foo.t) &&
+ bar_pack=$(test-tool find-pack -c 1 HEAD:bar.t) &&
+ head_pack=$(test-tool find-pack -c 1 HEAD) &&
+
+ test "$foo_pack" != "$bar_pack" &&
+ test "$foo_pack" = "$head_pack" &&
+
+ git -c repack.writebitmaps=false repack -a -d --filter=blob:none \
+ --pack-kept-objects &&
+
+ foo_pack_1=$(test-tool find-pack -c 1 HEAD:foo.t) &&
+ test-tool find-pack -c 2 HEAD:bar.t >bar_pack_1 &&
+ head_pack_1=$(test-tool find-pack -c 1 HEAD) &&
+
+ test "$foo_pack_1" != "$foo_pack" &&
+ test "$foo_pack_1" != "$bar_pack" &&
+ test "$head_pack_1" != "$head_pack" &&
+
+ # Object bar is in both the old .keep pack and the new
+ # pack that contained the filtered out objects
+ grep "$bar_pack" bar_pack_1 &&
+ grep "$foo_pack_1" bar_pack_1 &&
+ test "$foo_pack_1" != "$head_pack_1"
+ )
+'
+
objdir=.git/objects
midx=$objdir/pack/multi-pack-index
--
2.42.0.279.g57b2ba444c
^ permalink raw reply related
* [PATCH v7 8/9] repack: implement `--filter-to` for storing filtered out objects
From: Christian Couder @ 2023-09-25 15:25 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, John Cai, Jonathan Tan, Jonathan Nieder,
Taylor Blau, Derrick Stolee, Patrick Steinhardt, Christian Couder,
Christian Couder
In-Reply-To: <20230925152517.803579-1-christian.couder@gmail.com>
A previous commit has implemented `git repack --filter=<filter-spec>` to
allow users to filter out some objects from the main pack and move them
into a new different pack.
It would be nice if this new different pack could be created in a
different directory than the regular pack. This would make it possible
to move large blobs into a pack on a different kind of storage, for
example cheaper storage.
Even in a different directory, this pack can be accessible if, for
example, the Git alternates mechanism is used to point to it. In fact
not using the Git alternates mechanism can corrupt a repo as the
generated pack containing the filtered objects might not be accessible
from the repo any more. So setting up the Git alternates mechanism
should be done before using this feature if the user wants the repo to
be fully usable while this feature is used.
In some cases, like when a repo has just been cloned or when there is no
other activity in the repo, it's Ok to setup the Git alternates
mechanism afterwards though. It's also Ok to just inspect the generated
packfile containing the filtered objects and then just move it into the
'.git/objects/pack/' directory manually. That's why it's not necessary
for this command to check that the Git alternates mechanism has been
already setup.
While at it, as an example to show that `--filter` and `--filter-to`
work well with other options, let's also add a test to check that these
options work well with `--max-pack-size`.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-repack.txt | 11 +++++++
builtin/repack.c | 10 +++++-
t/t7700-repack.sh | 62 ++++++++++++++++++++++++++++++++++++
3 files changed, 82 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt
index 6d5bec7716..8545a32667 100644
--- a/Documentation/git-repack.txt
+++ b/Documentation/git-repack.txt
@@ -155,6 +155,17 @@ depth is 4095.
a single packfile containing all the objects. See
linkgit:git-rev-list[1] for valid `<filter-spec>` forms.
+--filter-to=<dir>::
+ Write the pack containing filtered out objects to the
+ directory `<dir>`. Only useful with `--filter`. This can be
+ used for putting the pack on a separate object directory that
+ is accessed through the Git alternates mechanism. **WARNING:**
+ If the packfile containing the filtered out objects is not
+ accessible, the repo can become corrupt as it might not be
+ possible to access the objects in that packfile. See the
+ `objects` and `objects/info/alternates` sections of
+ linkgit:gitrepository-layout[5].
+
-b::
--write-bitmap-index::
Write a reachability bitmap index as part of the repack. This
diff --git a/builtin/repack.c b/builtin/repack.c
index c7b564192f..db9277081d 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -977,6 +977,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
int write_midx = 0;
const char *cruft_expiration = NULL;
const char *expire_to = NULL;
+ const char *filter_to = NULL;
struct option builtin_repack_options[] = {
OPT_BIT('a', NULL, &pack_everything,
@@ -1029,6 +1030,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
N_("write a multi-pack index of the resulting packs")),
OPT_STRING(0, "expire-to", &expire_to, N_("dir"),
N_("pack prefix to store a pack containing pruned objects")),
+ OPT_STRING(0, "filter-to", &filter_to, N_("dir"),
+ N_("pack prefix to store a pack containing filtered out objects")),
OPT_END()
};
@@ -1177,6 +1180,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
if (po_args.filter_options.choice)
strvec_pushf(&cmd.args, "--filter=%s",
expand_list_objects_filter_spec(&po_args.filter_options));
+ else if (filter_to)
+ die(_("option '%s' can only be used along with '%s'"), "--filter-to", "--filter");
if (geometry.split_factor)
cmd.in = -1;
@@ -1265,8 +1270,11 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
}
if (po_args.filter_options.choice) {
+ if (!filter_to)
+ filter_to = packtmp;
+
ret = write_filtered_pack(&po_args,
- packtmp,
+ filter_to,
find_pack_prefix(packdir, packtmp),
&existing,
&names);
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 39e89445fd..48e92aa6f7 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -462,6 +462,68 @@ test_expect_success '--filter works with --pack-kept-objects and .keep packs' '
)
'
+test_expect_success '--filter-to stores filtered out objects' '
+ git -C bare.git repack -a -d &&
+ test_stdout_line_count = 1 ls bare.git/objects/pack/*.pack &&
+
+ git init --bare filtered.git &&
+ git -C bare.git -c repack.writebitmaps=false repack -a -d \
+ --filter=blob:none \
+ --filter-to=../filtered.git/objects/pack/pack &&
+ test_stdout_line_count = 1 ls bare.git/objects/pack/pack-*.pack &&
+ test_stdout_line_count = 1 ls filtered.git/objects/pack/pack-*.pack &&
+
+ commit_pack=$(test-tool -C bare.git find-pack -c 1 HEAD) &&
+ blob_pack=$(test-tool -C bare.git find-pack -c 0 HEAD:file1) &&
+ blob_hash=$(git -C bare.git rev-parse HEAD:file1) &&
+ test -n "$blob_hash" &&
+ blob_pack=$(test-tool -C filtered.git find-pack -c 1 $blob_hash) &&
+
+ echo $(pwd)/filtered.git/objects >bare.git/objects/info/alternates &&
+ blob_pack=$(test-tool -C bare.git find-pack -c 1 HEAD:file1) &&
+ blob_content=$(git -C bare.git show $blob_hash) &&
+ test "$blob_content" = "content1"
+'
+
+test_expect_success '--filter works with --max-pack-size' '
+ rm -rf filtered.git &&
+ git init --bare filtered.git &&
+ git init max-pack-size &&
+ (
+ cd max-pack-size &&
+ test_commit base &&
+ # two blobs which exceed the maximum pack size
+ test-tool genrandom foo 1048576 >foo &&
+ git hash-object -w foo &&
+ test-tool genrandom bar 1048576 >bar &&
+ git hash-object -w bar &&
+ git add foo bar &&
+ git commit -m "adding foo and bar"
+ ) &&
+ git clone --no-local --bare max-pack-size max-pack-size.git &&
+ (
+ cd max-pack-size.git &&
+ git -c repack.writebitmaps=false repack -a -d --filter=blob:none \
+ --max-pack-size=1M \
+ --filter-to=../filtered.git/objects/pack/pack &&
+ echo $(cd .. && pwd)/filtered.git/objects >objects/info/alternates &&
+
+ # Check that the 3 blobs are in different packfiles in filtered.git
+ test_stdout_line_count = 3 ls ../filtered.git/objects/pack/pack-*.pack &&
+ test_stdout_line_count = 1 ls objects/pack/pack-*.pack &&
+ foo_pack=$(test-tool find-pack -c 1 HEAD:foo) &&
+ bar_pack=$(test-tool find-pack -c 1 HEAD:bar) &&
+ base_pack=$(test-tool find-pack -c 1 HEAD:base.t) &&
+ test "$foo_pack" != "$bar_pack" &&
+ test "$foo_pack" != "$base_pack" &&
+ test "$bar_pack" != "$base_pack" &&
+ for pack in "$foo_pack" "$bar_pack" "$base_pack"
+ do
+ case "$foo_pack" in */filtered.git/objects/pack/*) true ;; *) return 1 ;; esac
+ done
+ )
+'
+
objdir=.git/objects
midx=$objdir/pack/multi-pack-index
--
2.42.0.279.g57b2ba444c
^ permalink raw reply related
* [PATCH v7 9/9] gc: add `gc.repackFilterTo` config option
From: Christian Couder @ 2023-09-25 15:25 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, John Cai, Jonathan Tan, Jonathan Nieder,
Taylor Blau, Derrick Stolee, Patrick Steinhardt, Christian Couder,
Christian Couder
In-Reply-To: <20230925152517.803579-1-christian.couder@gmail.com>
A previous commit implemented the `gc.repackFilter` config option
to specify a filter that should be used by `git gc` when
performing repacks.
Another previous commit has implemented
`git repack --filter-to=<dir>` to specify the location of the
packfile containing filtered out objects when using a filter.
Let's implement the `gc.repackFilterTo` config option to specify
that location in the config when `gc.repackFilter` is used.
Now when `git gc` will perform a repack with a <dir> configured
through this option and not empty, the repack process will be
passed a corresponding `--filter-to=<dir>` argument.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/config/gc.txt | 11 +++++++++++
builtin/gc.c | 4 ++++
t/t6500-gc.sh | 13 ++++++++++++-
3 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/Documentation/config/gc.txt b/Documentation/config/gc.txt
index 2153bde7ac..466466d6cc 100644
--- a/Documentation/config/gc.txt
+++ b/Documentation/config/gc.txt
@@ -150,6 +150,17 @@ gc.repackFilter::
objects into a separate packfile. See the
`--filter=<filter-spec>` option of linkgit:git-repack[1].
+gc.repackFilterTo::
+ When repacking and using a filter, see `gc.repackFilter`, the
+ specified location will be used to create the packfile
+ containing the filtered out objects. **WARNING:** The
+ specified location should be accessible, using for example the
+ Git alternates mechanism, otherwise the repo could be
+ considered corrupt by Git as it migh not be able to access the
+ objects in that packfile. See the `--filter-to=<dir>` option
+ of linkgit:git-repack[1] and the `objects/info/alternates`
+ section of linkgit:gitrepository-layout[5].
+
gc.rerereResolved::
Records of conflicted merge you resolved earlier are
kept for this many days when 'git rerere gc' is run.
diff --git a/builtin/gc.c b/builtin/gc.c
index 98148e98fe..68ca8d45bf 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -62,6 +62,7 @@ static const char *gc_log_expire = "1.day.ago";
static const char *prune_expire = "2.weeks.ago";
static const char *prune_worktrees_expire = "3.months.ago";
static char *repack_filter;
+static char *repack_filter_to;
static unsigned long big_pack_threshold;
static unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;
@@ -172,6 +173,7 @@ static void gc_config(void)
git_config_get_ulong("pack.deltacachesize", &max_delta_cache_size);
git_config_get_string("gc.repackfilter", &repack_filter);
+ git_config_get_string("gc.repackfilterto", &repack_filter_to);
git_config(git_default_config, NULL);
}
@@ -361,6 +363,8 @@ static void add_repack_all_option(struct string_list *keep_pack)
if (repack_filter && *repack_filter)
strvec_pushf(&repack, "--filter=%s", repack_filter);
+ if (repack_filter_to && *repack_filter_to)
+ strvec_pushf(&repack, "--filter-to=%s", repack_filter_to);
}
static void add_repack_incremental_option(void)
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 232e403b66..e412cf8daf 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -203,7 +203,6 @@ test_expect_success 'one of gc.reflogExpire{Unreachable,}=never does not skip "e
'
test_expect_success 'gc.repackFilter launches repack with a filter' '
- test_when_finished "rm -rf bare.git" &&
git clone --no-local --bare . bare.git &&
git -C bare.git -c gc.cruftPacks=false gc &&
@@ -215,6 +214,18 @@ test_expect_success 'gc.repackFilter launches repack with a filter' '
grep -E "^trace: (built-in|exec|run_command): git repack .* --filter=blob:none ?.*" trace.out
'
+test_expect_success 'gc.repackFilterTo store filtered out objects' '
+ test_when_finished "rm -rf bare.git filtered.git" &&
+
+ git init --bare filtered.git &&
+ git -C bare.git -c gc.repackFilter=blob:none \
+ -c gc.repackFilterTo=../filtered.git/objects/pack/pack \
+ -c repack.writeBitmaps=false -c gc.cruftPacks=false gc &&
+
+ test_stdout_line_count = 1 ls bare.git/objects/pack/*.pack &&
+ test_stdout_line_count = 1 ls filtered.git/objects/pack/*.pack
+'
+
prepare_cruft_history () {
test_commit base &&
--
2.42.0.279.g57b2ba444c
^ permalink raw reply related
* Request for Curl.exe update included in Git binaries
From: Robert Smith @ 2023-09-25 15:37 UTC (permalink / raw)
To: git@vger.kernel.org
Hello,
Regarding this CVE:
https://curl.se/docs/CVE-2023-38039.html
Is there any plan to update Git for Windows to include the updated 8.3.0 Curl binaries?
Thanks,
Robert S.
^ permalink raw reply
* [PATCH v2 1/3] test-pkt-line: add option parser for unpack-sideband
From: Jiang Xin @ 2023-09-25 15:41 UTC (permalink / raw)
To: Git List, Junio C Hamano, Jonathan Tan; +Cc: Jiang Xin
In-Reply-To: <CANYiYbF+Xmk4rCNLMJe+i_CFafg8=QU5vbXWNUZbOVsDLTe5QQ@mail.gmail.com>
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
We can use the test helper program "test-tool pkt-line" to test pkt-line
related functions. E.g.:
* Use "test-tool pkt-line send-split-sideband" to generate sideband
messages.
* We can pipe these generated sideband messages to command "test-tool
pkt-line unpack-sideband" to test packet_reader_read() function.
In order to make a complete test of the packet_reader_read() function,
add option parser for command "test-tool pkt-line unpack-sideband".
To remove newlines in sideband messages, we can use:
$ test-tool pkt-line unpack-sideband --chomp-newline
To preserve newlines in sideband messages, we can use:
$ test-tool pkt-line unpack-sideband --no-chomp-newline
To parse sideband messages using "demultiplex_sideband()" inside the
function "packet_reader_read()", we can use:
$ test-tool pkt-line unpack-sideband --reader-use-sideband
Add several new test cases in t0070. Among these test cases, we pipe
output of the "send-split-sideband" subcommand to the "unpack-sideband"
subcommand. We found two issues:
1. The two splitted sideband messages "Hello," and " world!\n" should
be concatenated together. But when we enabled the function
"demultiplex_sideband()" to parse sideband messages, the first part
of the splitted message ("Hello,") is lost.
2. The newline characters in sideband 2 (progress info) and sideband 3
(error message) should be preserved, but they are also trimmed.
Will fix the above two issues in subsequent commits.
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
t/helper/test-pkt-line.c | 58 ++++++++++++++++++++++++++++++++++++----
t/t0070-fundamental.sh | 58 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 111 insertions(+), 5 deletions(-)
diff --git a/t/helper/test-pkt-line.c b/t/helper/test-pkt-line.c
index f4d134a145..9aa35f7861 100644
--- a/t/helper/test-pkt-line.c
+++ b/t/helper/test-pkt-line.c
@@ -2,6 +2,7 @@
#include "test-tool.h"
#include "pkt-line.h"
#include "write-or-die.h"
+#include "parse-options.h"
static void pack_line(const char *line)
{
@@ -64,12 +65,33 @@ static void unpack(void)
}
}
-static void unpack_sideband(void)
+static void unpack_sideband(int argc, const char **argv)
{
struct packet_reader reader;
- packet_reader_init(&reader, 0, NULL, 0,
- PACKET_READ_GENTLE_ON_EOF |
- PACKET_READ_CHOMP_NEWLINE);
+ int options = PACKET_READ_GENTLE_ON_EOF;
+ int chomp_newline = 1;
+ int reader_use_sideband = 0;
+ const char *const unpack_sideband_usage[] = {
+ "test_tool unpack_sideband [options...]", NULL
+ };
+ struct option cmd_options[] = {
+ OPT_BOOL(0, "reader-use-sideband", &reader_use_sideband,
+ "set use_sideband bit for packet reader (Default: off)"),
+ OPT_BOOL(0, "chomp-newline", &chomp_newline,
+ "chomp newline in packet (Default: on)"),
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, "", cmd_options, unpack_sideband_usage,
+ 0);
+ if (argc > 0)
+ usage_msg_opt(_("too many arguments"), unpack_sideband_usage,
+ cmd_options);
+
+ if (chomp_newline)
+ options |= PACKET_READ_CHOMP_NEWLINE;
+ packet_reader_init(&reader, 0, NULL, 0, options);
+ reader.use_sideband = reader_use_sideband;
while (packet_reader_read(&reader) != PACKET_READ_EOF) {
int band;
@@ -79,6 +101,16 @@ static void unpack_sideband(void)
case PACKET_READ_EOF:
break;
case PACKET_READ_NORMAL:
+ /*
+ * When the "use_sideband" field of the reader is turned
+ * on, sideband packets other than the payload have been
+ * parsed and consumed.
+ */
+ if (reader.use_sideband) {
+ write_or_die(1, reader.line, reader.pktlen - 1);
+ break;
+ }
+
band = reader.line[0] & 0xff;
if (band < 1 || band > 2)
continue; /* skip non-sideband packets */
@@ -97,15 +129,31 @@ static void unpack_sideband(void)
static int send_split_sideband(void)
{
+ const char *foo = "Foo.\n";
+ const char *bar = "Bar.\n";
const char *part1 = "Hello,";
const char *primary = "\001primary: regular output\n";
const char *part2 = " world!\n";
+ /* Each sideband message has a trailing newline character. */
+ send_sideband(1, 2, foo, strlen(foo), LARGE_PACKET_MAX);
+ send_sideband(1, 2, bar, strlen(bar), LARGE_PACKET_MAX);
+
+ /*
+ * One sideband message is divided into part1 and part2
+ * by the primary message.
+ */
send_sideband(1, 2, part1, strlen(part1), LARGE_PACKET_MAX);
packet_write(1, primary, strlen(primary));
send_sideband(1, 2, part2, strlen(part2), LARGE_PACKET_MAX);
packet_response_end(1);
+ /*
+ * The unpack_sideband() function above requires a flush
+ * packet to end parsing.
+ */
+ packet_flush(1);
+
return 0;
}
@@ -126,7 +174,7 @@ int cmd__pkt_line(int argc, const char **argv)
else if (!strcmp(argv[1], "unpack"))
unpack();
else if (!strcmp(argv[1], "unpack-sideband"))
- unpack_sideband();
+ unpack_sideband(argc - 1, argv + 1);
else if (!strcmp(argv[1], "send-split-sideband"))
send_split_sideband();
else if (!strcmp(argv[1], "receive-sideband"))
diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh
index 574de34198..1053913d2d 100755
--- a/t/t0070-fundamental.sh
+++ b/t/t0070-fundamental.sh
@@ -53,4 +53,62 @@ test_expect_success 'missing sideband designator is reported' '
test_i18ngrep "missing sideband" err
'
+test_expect_success 'unpack-sideband: --no-chomp-newline' '
+ test_when_finished "rm -f expect-out expect-err" &&
+ test-tool pkt-line send-split-sideband >split-sideband &&
+ test-tool pkt-line unpack-sideband \
+ --no-chomp-newline <split-sideband >out 2>err &&
+ cat >expect-out <<-EOF &&
+ primary: regular output
+ EOF
+ cat >expect-err <<-EOF &&
+ Foo.
+ Bar.
+ Hello, world!
+ EOF
+ test_cmp expect-out out &&
+ test_cmp expect-err err
+'
+
+test_expect_success 'unpack-sideband: --chomp-newline (default)' '
+ test_when_finished "rm -f expect-out expect-err" &&
+ test-tool pkt-line send-split-sideband >split-sideband &&
+ test-tool pkt-line unpack-sideband \
+ --chomp-newline <split-sideband >out 2>err &&
+ printf "primary: regular output" >expect-out &&
+ printf "Foo.Bar.Hello, world!" >expect-err &&
+ test_cmp expect-out out &&
+ test_cmp expect-err err
+'
+
+test_expect_failure 'unpack-sideband with demultiplex_sideband(), no chomp newline' '
+ test_when_finished "rm -f expect-out expect-err" &&
+ test-tool pkt-line send-split-sideband >split-sideband &&
+ test-tool pkt-line unpack-sideband \
+ --reader-use-sideband \
+ --no-chomp-newline <split-sideband >out 2>err &&
+ cat >expect-out <<-EOF &&
+ primary: regular output
+ EOF
+ printf "remote: Foo. \n" >expect-err &&
+ printf "remote: Bar. \n" >>expect-err &&
+ printf "remote: Hello, world! \n" >>expect-err &&
+ test_cmp expect-out out &&
+ test_cmp expect-err err
+'
+
+test_expect_failure 'unpack-sideband with demultiplex_sideband(), chomp newline' '
+ test_when_finished "rm -f expect-out expect-err" &&
+ test-tool pkt-line send-split-sideband >split-sideband &&
+ test-tool pkt-line unpack-sideband \
+ --reader-use-sideband \
+ --chomp-newline <split-sideband >out 2>err &&
+ printf "primary: regular output" >expect-out &&
+ printf "remote: Foo. \n" >expect-err &&
+ printf "remote: Bar. \n" >>expect-err &&
+ printf "remote: Hello, world! \n" >>expect-err &&
+ test_cmp expect-out out &&
+ test_cmp expect-err err
+'
+
test_done
--
2.40.1.50.gf560bcc116.dirty
^ permalink raw reply related
* [PATCH v2 2/3] pkt-line: memorize sideband fragment in reader
From: Jiang Xin @ 2023-09-25 15:41 UTC (permalink / raw)
To: Git List, Junio C Hamano, Jonathan Tan; +Cc: Jiang Xin
In-Reply-To: <CANYiYbF+Xmk4rCNLMJe+i_CFafg8=QU5vbXWNUZbOVsDLTe5QQ@mail.gmail.com>
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
When we turn on the "use_sideband" field of the packet_reader,
"packet_reader_read()" will call the function "demultiplex_sideband()"
to parse and consume sideband messages. Sideband fragment which does not
end with "\r" or "\n" will be saved in the sixth parameter "scratch"
and it can be reused and be concatenated when parsing another sideband
message.
In "packet_reader_read()" function, the local variable "scratch" can
only be reused by subsequent sideband messages. But if there is a
payload message between two sideband fragments, the first fragment
which is saved in the local variable "scratch" will be lost.
To solve this problem, we can add a new field "scratch" in
packet_reader to memorize the sideband fragment across different calls
of "packet_reader_read()".
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
pkt-line.c | 5 ++---
pkt-line.h | 3 +++
t/t0070-fundamental.sh | 2 +-
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/pkt-line.c b/pkt-line.c
index af83a19f4d..5943777a17 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -592,12 +592,11 @@ void packet_reader_init(struct packet_reader *reader, int fd,
reader->options = options;
reader->me = "git";
reader->hash_algo = &hash_algos[GIT_HASH_SHA1];
+ strbuf_init(&reader->scratch, 0);
}
enum packet_read_status packet_reader_read(struct packet_reader *reader)
{
- struct strbuf scratch = STRBUF_INIT;
-
if (reader->line_peeked) {
reader->line_peeked = 0;
return reader->status;
@@ -620,7 +619,7 @@ enum packet_read_status packet_reader_read(struct packet_reader *reader)
break;
if (demultiplex_sideband(reader->me, reader->status,
reader->buffer, reader->pktlen, 1,
- &scratch, &sideband_type))
+ &reader->scratch, &sideband_type))
break;
}
diff --git a/pkt-line.h b/pkt-line.h
index 954eec8719..be1010d34e 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -194,6 +194,9 @@ struct packet_reader {
/* hash algorithm in use */
const struct git_hash_algo *hash_algo;
+
+ /* hold temporary sideband message */
+ struct strbuf scratch;
};
/*
diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh
index 1053913d2d..a927c665d6 100755
--- a/t/t0070-fundamental.sh
+++ b/t/t0070-fundamental.sh
@@ -81,7 +81,7 @@ test_expect_success 'unpack-sideband: --chomp-newline (default)' '
test_cmp expect-err err
'
-test_expect_failure 'unpack-sideband with demultiplex_sideband(), no chomp newline' '
+test_expect_success 'unpack-sideband with demultiplex_sideband(), no chomp newline' '
test_when_finished "rm -f expect-out expect-err" &&
test-tool pkt-line send-split-sideband >split-sideband &&
test-tool pkt-line unpack-sideband \
--
2.40.1.50.gf560bcc116.dirty
^ permalink raw reply related
* [PATCH v2 3/3] pkt-line: do not chomp newlines for sideband messages
From: Jiang Xin @ 2023-09-25 15:41 UTC (permalink / raw)
To: Git List, Junio C Hamano, Jonathan Tan; +Cc: Jiang Xin
In-Reply-To: <CANYiYbF+Xmk4rCNLMJe+i_CFafg8=QU5vbXWNUZbOVsDLTe5QQ@mail.gmail.com>
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
When calling "packet_read_with_status()" to parse pkt-line encoded
packets, we can turn on the flag "PACKET_READ_CHOMP_NEWLINE" to chomp
newline character for each packet for better line matching. But when
receiving data and progress information using sideband, we should turn
off the flag "PACKET_READ_CHOMP_NEWLINE" to prevent mangling newline
characters from data and progress information.
When both the server and the client support "sideband-all" capability,
we have a dilemma that newline characters in negotiation packets should
be removed, but the newline characters in the progress information
should be left intact.
Add new flag "PACKET_READ_USE_SIDEBAND" for "packet_read_with_status()"
to prevent mangling newline characters in sideband messages.
Helped-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
pkt-line.c | 32 ++++++++++++++++++++++++++++++--
pkt-line.h | 1 +
t/t0070-fundamental.sh | 2 +-
3 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/pkt-line.c b/pkt-line.c
index 5943777a17..865ad19484 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -462,8 +462,33 @@ enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
}
if ((options & PACKET_READ_CHOMP_NEWLINE) &&
- len && buffer[len-1] == '\n')
- len--;
+ len && buffer[len-1] == '\n') {
+ if (options & PACKET_READ_USE_SIDEBAND) {
+ int band = *buffer & 0xff;
+ switch (band) {
+ case 1:
+ /* Chomp newline for payload */
+ len--;
+ break;
+ case 2:
+ /* fallthrough */
+ case 3:
+ /*
+ * Do not chomp newline for progress and error
+ * message.
+ */
+ break;
+ default:
+ /*
+ * Bad sideband, let's leave it to
+ * demultiplex_sideband() to catch this error.
+ */
+ break;
+ }
+ } else {
+ len--;
+ }
+ }
buffer[len] = 0;
if (options & PACKET_READ_REDACT_URI_PATH &&
@@ -602,6 +627,9 @@ enum packet_read_status packet_reader_read(struct packet_reader *reader)
return reader->status;
}
+ if (reader->use_sideband)
+ reader->options |= PACKET_READ_USE_SIDEBAND;
+
/*
* Consume all progress packets until a primary payload packet is
* received
diff --git a/pkt-line.h b/pkt-line.h
index be1010d34e..a7ff2e2f18 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -85,6 +85,7 @@ void packet_fflush(FILE *f);
#define PACKET_READ_DIE_ON_ERR_PACKET (1u<<2)
#define PACKET_READ_GENTLE_ON_READ_ERROR (1u<<3)
#define PACKET_READ_REDACT_URI_PATH (1u<<4)
+#define PACKET_READ_USE_SIDEBAND (1u<<5)
int packet_read(int fd, char *buffer, unsigned size, int options);
/*
diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh
index a927c665d6..138c2becc1 100755
--- a/t/t0070-fundamental.sh
+++ b/t/t0070-fundamental.sh
@@ -97,7 +97,7 @@ test_expect_success 'unpack-sideband with demultiplex_sideband(), no chomp newli
test_cmp expect-err err
'
-test_expect_failure 'unpack-sideband with demultiplex_sideband(), chomp newline' '
+test_expect_success 'unpack-sideband with demultiplex_sideband(), chomp newline' '
test_when_finished "rm -f expect-out expect-err" &&
test-tool pkt-line send-split-sideband >split-sideband &&
test-tool pkt-line unpack-sideband \
--
2.40.1.50.gf560bcc116.dirty
^ permalink raw reply related
* Re: Please explain avoiding history simplifications when diffing merges
From: Magnus Holmgren @ 2023-09-25 16:11 UTC (permalink / raw)
To: git, Bagas Sanjaya
Cc: Santi Béjar, Junio C Hamano, Sergey Organov,
Christian Couder, Jeff King, René Scharfe
In-Reply-To: <ZQbNtgd82iARQ39D@debian.me>
söndag 17 september 2023 11:58:14 CEST skrev Bagas Sanjaya:
> On Fri, Sep 15, 2023 at 05:10:28PM +0200, Magnus Holmgren wrote:
> > Friday, 8 September 2023 11:09:20 CEST, I wrote
> >
> > > QGit was bitten by
> > > https://github.com/git/git/commit/0dec322d31db3920872f43bdd2a7ddd282a5be
> > > 67
> >
> > Maybe I should link to the QGit issue:
> > https://github.com/tibirna/qgit/issues/129
> >
> > > It looks like passing --simplify-merges to override the default solves
> > > the
> > > problem, but I still want to ask here because I'm not sure I fully
> > > understand
> > >
> > > the reasoning:
> > > > the default history simplification would remove merge commits from
> > > > consideration if the file "path" matched the second parent.
> >
> > As I wrote at the above URL, I realized that the old git log output
> > without -- simplify-merges and the output with --simplify-merges aren't
> > quite the same. The old output indeed omits some interesting merge
> > commits, which may explain why the change was made, but git log
> > --simplify-merges does include them, so it seems a reasonable default to
> > me.
>
> Can you provide examples?
git init git-test; cd git-test
echo foo > test; git add test; git commit -m Initial
git branch testbranch
git branch irrelevantbranch
echo foobar > test; git commit -am Change
git switch testbranch
echo barfoo > test; git commit -am Diverge
git switch irrelevantbranch
echo whatever > otherfile; git add otherfile
git commit -m "Add other file"
git switch master
git merge -m "Merge branch 'testbranch'" -s ours testbranch
git merge -m "Merge branch 'irrelevantbranch' irrelevantbranch
git log test # old git doesn't show either merge commit, new git shows both
git log --simplify-merges test # both versions show the first merge commit
git log --diff-merges=separate -p --simplify-merges test # only in new git
# note that only one diff is generated because the other would be empty,
# although the man page says "Separate log entry and diff is generated for
# each parent."
> > However, QGit has a problem: git log --diff-merges=separate includes a
> > separate diff for each parent, but only for each parent with differences
> > compared to the merge commit, *and* there's no custom format placeholder
> > for the current parent, only for the list of parents (%P/%p). How should
> > one go about adding that? I figure the format_commit_context struct in
> > pretty.c needs another field.
>
> What are you trying to accomplish with your proposed formatting verbs?
QGit builds annotated file histories by parsing git log output. To accomplish
that, it needs to use a custom, more machine-readable format, and to handle
merge commits correctly, it either needs one diff per parent in the order the
parents are listed, or it needs to know which parent each diff is relative to.
Because old git log (without --simplify-merges) only included merge commits
where the file in question is different to all parents, it didn't get confused
by missing diffs - it was all or nothing - but some merge commits were missing
in the file history view.
QGit should be rewritten to use libgit2, but regardless, I think all the
information you can get with a standard format should be available to custom
formats as well.
--
Magnus Holmgren
./¯\_/¯\. Milient
^ permalink raw reply
* Re: [REGRESSION] uninitialized value $address in git send-email when given multiple recipients separated by commas
From: Jeff King @ 2023-09-25 16:17 UTC (permalink / raw)
To: Todd Zullinger
Cc: Bagas Sanjaya, Michael Strawbridge, Junio C Hamano, Luben Tuikov,
Ævar Arnfjörð Bjarmason, Taylor Blau,
Git Mailing List
In-Reply-To: <ZRGdvRQuj4zllGnm@pobox.com>
On Mon, Sep 25, 2023 at 10:48:29AM -0400, Todd Zullinger wrote:
> From the peanut gallery... could the presence or lack of the
> Email::Valid perl module be a factor?
Ah, thanks! The thought of differing modules even occurred to me, since
I know we have a few optimistic dependencies, but when I looked I didn't
manage to find that one (somehow I thought Mail::Address was the
interesting one here; I think I might be getting senile).
With Email::Valid installed, I can reproduce with just (in git.git, but
I think it would work in any repo):
$ echo "exit 0" >.git/hooks/sendemail-validate
$ chmod +x .git/hooks/sendemail-validate
$ git send-email --dry-run -1 --to=foo@example.com,bar@example.com
error: unable to extract a valid address from: foo@example.com,bar@example.com
Disabling the hook with "chmod -x" makes the problem go away (and this
is with current "master", hence the more readable error message).
I think the issue is that a8022c5f7b ends up in extract_valid_address()
via this call stack:
$ = main::extract_valid_address('foo@example.com,bar@example.com') called from file '/home/peff/compile/git/git-send-email' line 1161
$ = main::extract_valid_address_or_die('foo@example.com,bar@example.com') called from file '/home/peff/compile/git/git-send-email' line 2087
@ = main::unique_email_list('foo@example.com,bar@example.com') called from file '/home/peff/compile/git/git-send-email' line 1507
@ = main::gen_header() called from file '/home/peff/compile/git/git-send-email' line 2113
. = main::validate_patch('/tmp/WfoPQSKCUa/0001-The-twelfth-batch.patch', 'auto') called from file '/home/peff/compile/git/git-send-email' line 815
whereas prior to that commit, we hit it later:
$ = main::extract_valid_address('foo@example.com') called from file '/home/peff/compile/git/git-send-email' line 1166
@ = main::validate_address('foo@example.com') called from file '/home/peff/compile/git/git-send-email' line 1189
@ = main::validate_address_list('foo@example.com', 'bar@example.com') called from file '/home/peff/compile/git/git-send-email' line 1348
@ = main::process_address_list('foo@example.com,bar@example.com') called from file '/home/peff/compile/git/git-send-email' line 1091
So the issue is the call to gen_header() added in validate_patch(). We
won't yet have processed the address lists by that point. We can move
those calls up, but it requires moving a bit of extra code, too (like
the parts prompting for the "to" list if it isn't filled in).
Possibly the validation checks need to be moved down, if they want to
see a more complete view of the emails. But now we're doing more work
(like asking the user to write the cover letter!) before we do
validation, which is probably bad.
So I dunno. Maybe gen_header() should be lazily doing this
process_address_list() stuff? I'm not very familiar with the send-email
code, so I'm not sure what secondary effects that could have.
-Peff
^ permalink raw reply
* Re: [PATCH v4] revision: add `--ignore-missing-links` user option
From: Junio C Hamano @ 2023-09-25 16:57 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, me
In-Reply-To: <CAOLa=ZThB0DrRKg98tr7JLu8yPRyDXe_ngkpS3ZfesHQ-f1DLg@mail.gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
> Let me prefix with saying that I was partly wrong. `--missing` does work for
> trees, only that it's ineffective when used along with the
> `ignore_missing_links` bit.
>
> But for commits, `--missing` was never configured to work with. I
> did a quick look at the code, we can do something like this for
> commits too, i.e. add support for the `--missing` option. We'll
> have to add a new flag (maybe MISSING) so it can be set during
> within `repo_parse_commit_gently` so we can parse this as a
> missing object in rev-list.c and act accordingly.
Do you mean that process_parents() would now throw such a commit to
the resulting list successfully instead of omitting when "--missing"
is requested? That sounds like a right thing to do but at the same
time is a fix with major impact. I do not offhand know what the
ramifications are, for example, when bitmap traversal is in use (I
assume such a missing commit would not be catalogued in the bitmap?).
Taylor, what do you think?
^ permalink raw reply
* Re: [PATCH v2 0/6] Add a GitHub workflow to submit builds to Coverity Scan
From: Junio C Hamano @ 2023-09-25 17:20 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Jeff King, Johannes Schindelin
In-Reply-To: <pull.1588.v2.git.1695642662.gitgitgadget@gmail.com>
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> Coverity [https://scan.coverity.com/] is a powerful static analysis tool
> that helps prevent vulnerabilities. It is free to use by open source
> projects, and Git benefits from this, as well as Git for Windows. As is the
> case with many powerful tools, using Coverity comes with its own set of
> challenges, one of which being that submitting a build is quite laborious.
> ...
One thing that caught my eye was the asterisks around "22" that look
as if they were designed to confuse readers and cause them wonder if
there are other codes like 122 and 224 that we would also want to
catch there. Unless they know what the case statement replaced,
that is---the old code to match http_code that was scraped from a
text file may not have the code alone and may contain other cruft,
so it is entirely understandable, but the new one checks $? and
there is no reason other than to catch 122 and 224 to use *22*.
> -+ http_code="$(sed -n 1p <"$RUNNER_TEMP"/headers.txt)"
> -+ case "$http_code" in
> -+ *200*) ;; # okay
> -+ *401*) # access denied
> -+ echo "::error::incorrect token or project? ($http_code)" >&2
> + --fail \
> + --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \
> + --form project="$COVERITY_PROJECT" \
> +- --form md5=1) &&
> ++ --form md5=1)
> ++ case $? in
> ++ 0) ;; # okay
> ++ *22*) # 40x, i.e. access denied
> ++ echo "::error::incorrect token or project?" >&2
> + exit 1
> + ;;
> + *) # other error
> -+ echo "::error::HTTP error $http_code" >&2
> ++ echo "::error::Failed to retrieve MD5" >&2
> + exit 1
> + ;;
> + esac
Other than that, while I was watching from the sideline, I am very
happy to see that you, with Peff's constructive input, came up with
a new iteration that looks simpler and more consistent in its use of
curl.
Will replace but I may be tempted to edit those asterisks out myself
while queueing.
Thanks.
^ permalink raw reply
* Re: [PATCH] pretty-formats.txt: fix whitespace
From: Junio C Hamano @ 2023-09-25 17:24 UTC (permalink / raw)
To: Josh Soref; +Cc: Josh Soref via GitGitGadget, git
In-Reply-To: <CACZqfqCVsv-ZaSRWt_ejMn5f_U_1E2h7wsCgUg_50A+KHzOgkA@mail.gmail.com>
Josh Soref <jsoref@gmail.com> writes:
>> > * comma after `etc.` when not ending a sentence
>>
>> There is one instance that says "A, B, C, D, etc., are all accepted."
>> without the comma after 'etc.' and the patch corrects it. OK.
>
> It seems like this is the only change that's of interest.
>
> Do I just make a distinct gitgitgadget PR with that change, or do I
> ask it to mark that single change as a V2 to this? (given the branch
> name assumed whitespace and the only change would be a comma, it'd be
> kinda wrong...)
;-)
Up to you. If I were doing this patch, I would wait and see if
others chime in (to support other changes in v1 that I was on the
negative side) and then make v2 with the changes you still believe
in when that happens.
Thanks.
^ permalink raw reply
* [PATCH v2 0/3] Add mailmap support to ref-filter
From: Kousik Sanagavarapu @ 2023-09-25 17:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Kousik Sanagavarapu
In-Reply-To: <20230920191654.6133-1-five231003@gmail.com>
Thanks Junio for review on the previous round.
PATCH 1/3 - Cleanup test_atom to be less error-prone
PATCH 2/3 - Fix hidden breakage
PATCH 3/3 - Unchanged
Kousik Sanagavarapu (3):
t/t6300: cleanup test_atom
t/t6300: introduce test_bad_atom
ref-filter: add mailmap support
Documentation/git-for-each-ref.txt | 6 +-
ref-filter.c | 152 ++++++++++++++++++++++-------
t/t6300-for-each-ref.sh | 123 +++++++++++++++++++++--
3 files changed, 239 insertions(+), 42 deletions(-)
Range-diff against v1:
-: ---------- > 1: b28e858e35 t/t6300: cleanup test_atom
1: 0a90b67889 ! 2: ee90d017d5 t/t6300: introduce test_bad_atom()
@@ Metadata
Author: Kousik Sanagavarapu <five231003@gmail.com>
## Commit message ##
- t/t6300: introduce test_bad_atom()
+ t/t6300: introduce test_bad_atom
- Introduce a new function "test_bad_atom()", which is similar to
+ Introduce a new function "test_bad_atom", which is similar to
"test_atom()" but should be used to check whether the correct error
message is shown on stderr.
- Like "test_atom()", the new function takes three arguments. The three
+ Like "test_atom", the new function takes three arguments. The three
arguments specify the ref, the format and the expected error message
respectively, with an optional fourth argument for tweaking
"test_expect_*" (which is by default "success").
@@ t/t6300-for-each-ref.sh: test_expect_success 'arguments to %(objectname:short=)
test_must_fail git for-each-ref --format="%(objectname:short=foo)"
'
-+test_bad_atom() {
++test_bad_atom () {
+ case "$1" in
+ head) ref=refs/heads/main ;;
+ tag) ref=refs/tags/testtag ;;
+ sym) ref=refs/heads/sym ;;
+ *) ref=$1 ;;
+ esac
-+ printf '%s\n' "$3">expect
-+ test_expect_${4:-success} $PREREQ "err basic atom: $1 $2" "
-+ test_must_fail git for-each-ref --format='%($2)' $ref 2>actual &&
-+ test_cmp expect actual
-+ "
++ format=$2
++ test_do=test_expect_${4:-success}
++
++ printf '%s\n' "$3" >expect
++ $test_do $PREREQ "err basic atom: $ref $format" '
++ test_must_fail git for-each-ref \
++ --format="%($format)" "$ref" 2>error &&
++ test_cmp expect error
++ '
+}
+
+test_bad_atom head 'authoremail:foo' \
+ 'fatal: unrecognized %(authoremail) argument: foo'
+
+test_bad_atom tag 'taggeremail:localpart trim' \
-+ 'fatal: unrecognized %(taggeremail) argument: trim'
++ 'fatal: unrecognized %(taggeremail) argument: localpart trim'
+
test_date () {
f=$1 &&
2: 63fc69f4dc ! 3: fdc14fe80b ref-filter: add mailmap support
@@ t/t6300-for-each-ref.sh: test_atom tag '*objectname' $(git rev-parse refs/tags/t
test_atom tag taggerdate 'Tue Jul 4 01:18:45 2006 +0200'
test_atom tag creator 'C O Mitter <committer@example.com> 1151968725 +0200'
test_atom tag creatordate 'Tue Jul 4 01:18:45 2006 +0200'
-@@ t/t6300-for-each-ref.sh: test_bad_atom() {
+@@ t/t6300-for-each-ref.sh: test_bad_atom () {
test_bad_atom head 'authoremail:foo' \
'fatal: unrecognized %(authoremail) argument: foo'
@@ t/t6300-for-each-ref.sh: test_bad_atom() {
+ 'fatal: unrecognized %(taggeremail) argument: ;localpart trim'
+
test_bad_atom tag 'taggeremail:localpart trim' \
- 'fatal: unrecognized %(taggeremail) argument: trim'
-
+- 'fatal: unrecognized %(taggeremail) argument: localpart trim'
++ 'fatal: unrecognized %(taggeremail) argument: trim'
++
+test_bad_atom tag 'taggeremail:mailmap,mailmap,trim,qux,localpart,trim' \
+ 'fatal: unrecognized %(taggeremail) argument: qux,localpart,trim'
-+
+
test_date () {
f=$1 &&
- committer_date=$2 &&
--
2.42.0.273.ge948a9aaf4
^ permalink raw reply
* [PATCH v2 1/3] t/t6300: cleanup test_atom
From: Kousik Sanagavarapu @ 2023-09-25 17:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Kousik Sanagavarapu, Christian Couder,
Hariom Verma
In-Reply-To: <20230925175050.3498-1-five231003@gmail.com>
Previously, when the executable part of "test_expect_{success,failure}"
(inside "test_atom") got "eval"ed, it would have been syntactically
incorrect if the second argument ($2, which is the format) to "test_atom"
were enclosed in single quotes because the $variables would get
interpolated even before the arguments to "test_expect_{success,failure}"
are formed.
So fix this and also some style issues along the way.
Helped-by: Junio C Hamano <gitster@pobox.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
t/t6300-for-each-ref.sh | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 7b943fd34c..7ba9949376 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -41,25 +41,29 @@ test_expect_success setup '
git config push.default current
'
-test_atom() {
+test_atom () {
case "$1" in
head) ref=refs/heads/main ;;
tag) ref=refs/tags/testtag ;;
sym) ref=refs/heads/sym ;;
*) ref=$1 ;;
esac
+ format=$2
+ test_do=test_expect_${4:-success}
+
printf '%s\n' "$3" >expected
- test_expect_${4:-success} $PREREQ "basic atom: $1 $2" "
- git for-each-ref --format='%($2)' $ref >actual &&
+ $test_do $PREREQ "basic atom: $ref $format" '
+ git for-each-ref --format="%($format)" "$ref" >actual &&
sanitize_pgp <actual >actual.clean &&
test_cmp expected actual.clean
- "
+ '
+
# Automatically test "contents:size" atom after testing "contents"
- if test "$2" = "contents"
+ if test "$format" = "contents"
then
# for commit leg, $3 is changed there
expect=$(printf '%s' "$3" | wc -c)
- test_expect_${4:-success} $PREREQ "basic atom: $1 contents:size" '
+ $test_do $PREREQ "basic atom: $ref contents:size" '
type=$(git cat-file -t "$ref") &&
case $type in
tag)
--
2.42.0.273.ge948a9aaf4
^ permalink raw reply related
* [PATCH v2 2/3] t/t6300: introduce test_bad_atom
From: Kousik Sanagavarapu @ 2023-09-25 17:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Kousik Sanagavarapu, Christian Couder,
Hariom Verma
In-Reply-To: <20230925175050.3498-1-five231003@gmail.com>
Introduce a new function "test_bad_atom", which is similar to
"test_atom()" but should be used to check whether the correct error
message is shown on stderr.
Like "test_atom", the new function takes three arguments. The three
arguments specify the ref, the format and the expected error message
respectively, with an optional fourth argument for tweaking
"test_expect_*" (which is by default "success").
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
t/t6300-for-each-ref.sh | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 7ba9949376..e4ec2926d6 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -271,6 +271,30 @@ test_expect_success 'arguments to %(objectname:short=) must be positive integers
test_must_fail git for-each-ref --format="%(objectname:short=foo)"
'
+test_bad_atom () {
+ case "$1" in
+ head) ref=refs/heads/main ;;
+ tag) ref=refs/tags/testtag ;;
+ sym) ref=refs/heads/sym ;;
+ *) ref=$1 ;;
+ esac
+ format=$2
+ test_do=test_expect_${4:-success}
+
+ printf '%s\n' "$3" >expect
+ $test_do $PREREQ "err basic atom: $ref $format" '
+ test_must_fail git for-each-ref \
+ --format="%($format)" "$ref" 2>error &&
+ test_cmp expect error
+ '
+}
+
+test_bad_atom head 'authoremail:foo' \
+ 'fatal: unrecognized %(authoremail) argument: foo'
+
+test_bad_atom tag 'taggeremail:localpart trim' \
+ 'fatal: unrecognized %(taggeremail) argument: localpart trim'
+
test_date () {
f=$1 &&
committer_date=$2 &&
--
2.42.0.273.ge948a9aaf4
^ permalink raw reply related
* [PATCH v2 3/3] ref-filter: add mailmap support
From: Kousik Sanagavarapu @ 2023-09-25 17:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Kousik Sanagavarapu, Christian Couder,
Hariom Verma
In-Reply-To: <20230925175050.3498-1-five231003@gmail.com>
Add mailmap support to ref-filter formats which are similar in
pretty. This support is such that the following pretty placeholders are
equivalent to the new ref-filter atoms:
%aN = authorname:mailmap
%cN = committername:mailmap
%aE = authoremail:mailmap
%aL = authoremail:mailmap,localpart
%cE = committeremail:mailmap
%cL = committeremail:mailmap,localpart
Additionally, mailmap can also be used with ":trim" option for email by
doing something like "authoremail:mailmap,trim".
The above also applies for the "tagger" atom, that is,
"taggername:mailmap", "taggeremail:mailmap", "taggeremail:mailmap,trim"
and "taggername:mailmap,localpart".
The functionality of ":trim" and ":localpart" remains the same. That is,
":trim" gives the email, but without the angle brackets and ":localpart"
gives the part of the email before the '@' character (if such a
character is not found then we directly grab everything between the
angle brackets).
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
Documentation/git-for-each-ref.txt | 6 +-
ref-filter.c | 152 ++++++++++++++++++++++-------
t/t6300-for-each-ref.sh | 85 +++++++++++++++-
3 files changed, 206 insertions(+), 37 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 11b2bc3121..e86d5700dd 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -303,7 +303,11 @@ Fields that have name-email-date tuple as its value (`author`,
and `date` to extract the named component. For email fields (`authoremail`,
`committeremail` and `taggeremail`), `:trim` can be appended to get the email
without angle brackets, and `:localpart` to get the part before the `@` symbol
-out of the trimmed email.
+out of the trimmed email. In addition to these, the `:mailmap` option and the
+corresponding `:mailmap,trim` and `:mailmap,localpart` can be used (order does
+not matter) to get values of the name and email according to the .mailmap file
+or according to the file set in the mailmap.file or mailmap.blob configuration
+variable (see linkgit:gitmailmap[5]).
The raw data in an object is `raw`.
diff --git a/ref-filter.c b/ref-filter.c
index fae9f4b8ed..e4d3510e28 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -13,6 +13,8 @@
#include "oid-array.h"
#include "repository.h"
#include "commit.h"
+#include "mailmap.h"
+#include "ident.h"
#include "remote.h"
#include "color.h"
#include "tag.h"
@@ -215,8 +217,16 @@ static struct used_atom {
struct {
enum { O_SIZE, O_SIZE_DISK } option;
} objectsize;
- struct email_option {
- enum { EO_RAW, EO_TRIM, EO_LOCALPART } option;
+ struct {
+ enum { N_RAW, N_MAILMAP } option;
+ } name_option;
+ struct {
+ enum {
+ EO_RAW = 0,
+ EO_TRIM = 1<<0,
+ EO_LOCALPART = 1<<1,
+ EO_MAILMAP = 1<<2,
+ } option;
} email_option;
struct {
enum { S_BARE, S_GRADE, S_SIGNER, S_KEY,
@@ -720,21 +730,55 @@ static int oid_atom_parser(struct ref_format *format UNUSED,
return 0;
}
-static int person_email_atom_parser(struct ref_format *format UNUSED,
- struct used_atom *atom,
- const char *arg, struct strbuf *err)
+static int person_name_atom_parser(struct ref_format *format UNUSED,
+ struct used_atom *atom,
+ const char *arg, struct strbuf *err)
{
if (!arg)
- atom->u.email_option.option = EO_RAW;
- else if (!strcmp(arg, "trim"))
- atom->u.email_option.option = EO_TRIM;
- else if (!strcmp(arg, "localpart"))
- atom->u.email_option.option = EO_LOCALPART;
+ atom->u.name_option.option = N_RAW;
+ else if (!strcmp(arg, "mailmap"))
+ atom->u.name_option.option = N_MAILMAP;
else
return err_bad_arg(err, atom->name, arg);
return 0;
}
+static int email_atom_option_parser(struct used_atom *atom,
+ const char **arg, struct strbuf *err)
+{
+ if (!*arg)
+ return EO_RAW;
+ if (skip_prefix(*arg, "trim", arg))
+ return EO_TRIM;
+ if (skip_prefix(*arg, "localpart", arg))
+ return EO_LOCALPART;
+ if (skip_prefix(*arg, "mailmap", arg))
+ return EO_MAILMAP;
+ return -1;
+}
+
+static int person_email_atom_parser(struct ref_format *format UNUSED,
+ struct used_atom *atom,
+ const char *arg, struct strbuf *err)
+{
+ for (;;) {
+ int opt = email_atom_option_parser(atom, &arg, err);
+ const char *bad_arg = arg;
+
+ if (opt < 0)
+ return err_bad_arg(err, atom->name, bad_arg);
+ atom->u.email_option.option |= opt;
+
+ if (!arg || !*arg)
+ break;
+ if (*arg == ',')
+ arg++;
+ else
+ return err_bad_arg(err, atom->name, bad_arg);
+ }
+ return 0;
+}
+
static int refname_atom_parser(struct ref_format *format UNUSED,
struct used_atom *atom,
const char *arg, struct strbuf *err)
@@ -877,15 +921,15 @@ static struct {
[ATOM_TYPE] = { "type", SOURCE_OBJ },
[ATOM_TAG] = { "tag", SOURCE_OBJ },
[ATOM_AUTHOR] = { "author", SOURCE_OBJ },
- [ATOM_AUTHORNAME] = { "authorname", SOURCE_OBJ },
+ [ATOM_AUTHORNAME] = { "authorname", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },
[ATOM_AUTHOREMAIL] = { "authoremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },
[ATOM_AUTHORDATE] = { "authordate", SOURCE_OBJ, FIELD_TIME },
[ATOM_COMMITTER] = { "committer", SOURCE_OBJ },
- [ATOM_COMMITTERNAME] = { "committername", SOURCE_OBJ },
+ [ATOM_COMMITTERNAME] = { "committername", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },
[ATOM_COMMITTEREMAIL] = { "committeremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },
[ATOM_COMMITTERDATE] = { "committerdate", SOURCE_OBJ, FIELD_TIME },
[ATOM_TAGGER] = { "tagger", SOURCE_OBJ },
- [ATOM_TAGGERNAME] = { "taggername", SOURCE_OBJ },
+ [ATOM_TAGGERNAME] = { "taggername", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },
[ATOM_TAGGEREMAIL] = { "taggeremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },
[ATOM_TAGGERDATE] = { "taggerdate", SOURCE_OBJ, FIELD_TIME },
[ATOM_CREATOR] = { "creator", SOURCE_OBJ },
@@ -1486,32 +1530,49 @@ static const char *copy_name(const char *buf)
return xstrdup("");
}
+static const char *find_end_of_email(const char *email, int opt)
+{
+ const char *eoemail;
+
+ if (opt & EO_LOCALPART) {
+ eoemail = strchr(email, '@');
+ if (eoemail)
+ return eoemail;
+ return strchr(email, '>');
+ }
+
+ if (opt & EO_TRIM)
+ return strchr(email, '>');
+
+ /*
+ * The option here is either the raw email option or the raw
+ * mailmap option (that is EO_RAW or EO_MAILMAP). In such cases,
+ * we directly grab the whole email including the closing
+ * angle brackets.
+ *
+ * If EO_MAILMAP was set with any other option (that is either
+ * EO_TRIM or EO_LOCALPART), we already grab the end of email
+ * above.
+ */
+ eoemail = strchr(email, '>');
+ if (eoemail)
+ eoemail++;
+ return eoemail;
+}
+
static const char *copy_email(const char *buf, struct used_atom *atom)
{
const char *email = strchr(buf, '<');
const char *eoemail;
+ int opt = atom->u.email_option.option;
+
if (!email)
return xstrdup("");
- switch (atom->u.email_option.option) {
- case EO_RAW:
- eoemail = strchr(email, '>');
- if (eoemail)
- eoemail++;
- break;
- case EO_TRIM:
- email++;
- eoemail = strchr(email, '>');
- break;
- case EO_LOCALPART:
+
+ if (opt & (EO_LOCALPART | EO_TRIM))
email++;
- eoemail = strchr(email, '@');
- if (!eoemail)
- eoemail = strchr(email, '>');
- break;
- default:
- BUG("unknown email option");
- }
+ eoemail = find_end_of_email(email, opt);
if (!eoemail)
return xstrdup("");
return xmemdupz(email, eoemail - email);
@@ -1572,16 +1633,23 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam
v->value = 0;
}
+static struct string_list mailmap = STRING_LIST_INIT_NODUP;
+
/* See grab_values */
static void grab_person(const char *who, struct atom_value *val, int deref, void *buf)
{
int i;
int wholen = strlen(who);
const char *wholine = NULL;
+ const char *headers[] = { "author ", "committer ",
+ "tagger ", NULL };
for (i = 0; i < used_atom_cnt; i++) {
- const char *name = used_atom[i].name;
+ struct used_atom *atom = &used_atom[i];
+ const char *name = atom->name;
struct atom_value *v = &val[i];
+ struct strbuf mailmap_buf = STRBUF_INIT;
+
if (!!deref != (*name == '*'))
continue;
if (deref)
@@ -1589,22 +1657,36 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void
if (strncmp(who, name, wholen))
continue;
if (name[wholen] != 0 &&
- strcmp(name + wholen, "name") &&
+ !starts_with(name + wholen, "name") &&
!starts_with(name + wholen, "email") &&
!starts_with(name + wholen, "date"))
continue;
- if (!wholine)
+
+ if ((starts_with(name + wholen, "name") &&
+ (atom->u.name_option.option == N_MAILMAP)) ||
+ (starts_with(name + wholen, "email") &&
+ (atom->u.email_option.option & EO_MAILMAP))) {
+ if (!mailmap.items)
+ read_mailmap(&mailmap);
+ strbuf_addstr(&mailmap_buf, buf);
+ apply_mailmap_to_header(&mailmap_buf, headers, &mailmap);
+ wholine = find_wholine(who, wholen, mailmap_buf.buf);
+ } else {
wholine = find_wholine(who, wholen, buf);
+ }
+
if (!wholine)
return; /* no point looking for it */
if (name[wholen] == 0)
v->s = copy_line(wholine);
- else if (!strcmp(name + wholen, "name"))
+ else if (starts_with(name + wholen, "name"))
v->s = copy_name(wholine);
else if (starts_with(name + wholen, "email"))
v->s = copy_email(wholine, &used_atom[i]);
else if (starts_with(name + wholen, "date"))
grab_date(wholine, v, name);
+
+ strbuf_release(&mailmap_buf);
}
/*
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index e4ec2926d6..00a060df0b 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -25,6 +25,13 @@ test_expect_success setup '
disklen sha1:138
disklen sha256:154
EOF
+
+ # setup .mailmap
+ cat >.mailmap <<-EOF &&
+ A Thor <athor@example.com> A U Thor <author@example.com>
+ C Mitter <cmitter@example.com> C O Mitter <committer@example.com>
+ EOF
+
setdate_and_increment &&
echo "Using $datestamp" > one &&
git add one &&
@@ -145,15 +152,31 @@ test_atom head '*objectname' ''
test_atom head '*objecttype' ''
test_atom head author 'A U Thor <author@example.com> 1151968724 +0200'
test_atom head authorname 'A U Thor'
+test_atom head authorname:mailmap 'A Thor'
test_atom head authoremail '<author@example.com>'
test_atom head authoremail:trim 'author@example.com'
test_atom head authoremail:localpart 'author'
+test_atom head authoremail:trim,localpart 'author'
+test_atom head authoremail:mailmap '<athor@example.com>'
+test_atom head authoremail:mailmap,trim 'athor@example.com'
+test_atom head authoremail:trim,mailmap 'athor@example.com'
+test_atom head authoremail:mailmap,localpart 'athor'
+test_atom head authoremail:localpart,mailmap 'athor'
+test_atom head authoremail:mailmap,trim,localpart,mailmap,trim 'athor'
test_atom head authordate 'Tue Jul 4 01:18:44 2006 +0200'
test_atom head committer 'C O Mitter <committer@example.com> 1151968723 +0200'
test_atom head committername 'C O Mitter'
+test_atom head committername:mailmap 'C Mitter'
test_atom head committeremail '<committer@example.com>'
test_atom head committeremail:trim 'committer@example.com'
test_atom head committeremail:localpart 'committer'
+test_atom head committeremail:localpart,trim 'committer'
+test_atom head committeremail:mailmap '<cmitter@example.com>'
+test_atom head committeremail:mailmap,trim 'cmitter@example.com'
+test_atom head committeremail:trim,mailmap 'cmitter@example.com'
+test_atom head committeremail:mailmap,localpart 'cmitter'
+test_atom head committeremail:localpart,mailmap 'cmitter'
+test_atom head committeremail:trim,mailmap,trim,trim,localpart 'cmitter'
test_atom head committerdate 'Tue Jul 4 01:18:43 2006 +0200'
test_atom head tag ''
test_atom head tagger ''
@@ -203,22 +226,46 @@ test_atom tag '*objectname' $(git rev-parse refs/tags/testtag^{})
test_atom tag '*objecttype' 'commit'
test_atom tag author ''
test_atom tag authorname ''
+test_atom tag authorname:mailmap ''
test_atom tag authoremail ''
test_atom tag authoremail:trim ''
test_atom tag authoremail:localpart ''
+test_atom tag authoremail:trim,localpart ''
+test_atom tag authoremail:mailmap ''
+test_atom tag authoremail:mailmap,trim ''
+test_atom tag authoremail:trim,mailmap ''
+test_atom tag authoremail:mailmap,localpart ''
+test_atom tag authoremail:localpart,mailmap ''
+test_atom tag authoremail:mailmap,trim,localpart,mailmap,trim ''
test_atom tag authordate ''
test_atom tag committer ''
test_atom tag committername ''
+test_atom tag committername:mailmap ''
test_atom tag committeremail ''
test_atom tag committeremail:trim ''
test_atom tag committeremail:localpart ''
+test_atom tag committeremail:localpart,trim ''
+test_atom tag committeremail:mailmap ''
+test_atom tag committeremail:mailmap,trim ''
+test_atom tag committeremail:trim,mailmap ''
+test_atom tag committeremail:mailmap,localpart ''
+test_atom tag committeremail:localpart,mailmap ''
+test_atom tag committeremail:trim,mailmap,trim,trim,localpart ''
test_atom tag committerdate ''
test_atom tag tag 'testtag'
test_atom tag tagger 'C O Mitter <committer@example.com> 1151968725 +0200'
test_atom tag taggername 'C O Mitter'
+test_atom tag taggername:mailmap 'C Mitter'
test_atom tag taggeremail '<committer@example.com>'
test_atom tag taggeremail:trim 'committer@example.com'
test_atom tag taggeremail:localpart 'committer'
+test_atom tag taggeremail:trim,localpart 'committer'
+test_atom tag taggeremail:mailmap '<cmitter@example.com>'
+test_atom tag taggeremail:mailmap,trim 'cmitter@example.com'
+test_atom tag taggeremail:trim,mailmap 'cmitter@example.com'
+test_atom tag taggeremail:mailmap,localpart 'cmitter'
+test_atom tag taggeremail:localpart,mailmap 'cmitter'
+test_atom tag taggeremail:trim,mailmap,trim,localpart,localpart 'cmitter'
test_atom tag taggerdate 'Tue Jul 4 01:18:45 2006 +0200'
test_atom tag creator 'C O Mitter <committer@example.com> 1151968725 +0200'
test_atom tag creatordate 'Tue Jul 4 01:18:45 2006 +0200'
@@ -292,8 +339,44 @@ test_bad_atom () {
test_bad_atom head 'authoremail:foo' \
'fatal: unrecognized %(authoremail) argument: foo'
+test_bad_atom head 'authoremail:mailmap,trim,bar' \
+ 'fatal: unrecognized %(authoremail) argument: bar'
+
+test_bad_atom head 'authoremail:trim,' \
+ 'fatal: unrecognized %(authoremail) argument: '
+
+test_bad_atom head 'authoremail:mailmaptrim' \
+ 'fatal: unrecognized %(authoremail) argument: trim'
+
+test_bad_atom head 'committeremail: ' \
+ 'fatal: unrecognized %(committeremail) argument: '
+
+test_bad_atom head 'committeremail: trim,foo' \
+ 'fatal: unrecognized %(committeremail) argument: trim,foo'
+
+test_bad_atom head 'committeremail:mailmap,localpart ' \
+ 'fatal: unrecognized %(committeremail) argument: '
+
+test_bad_atom head 'committeremail:trim_localpart' \
+ 'fatal: unrecognized %(committeremail) argument: _localpart'
+
+test_bad_atom head 'committeremail:localpart,,,trim' \
+ 'fatal: unrecognized %(committeremail) argument: ,,trim'
+
+test_bad_atom tag 'taggeremail:mailmap,trim, foo ' \
+ 'fatal: unrecognized %(taggeremail) argument: foo '
+
+test_bad_atom tag 'taggeremail:trim,localpart,' \
+ 'fatal: unrecognized %(taggeremail) argument: '
+
+test_bad_atom tag 'taggeremail:mailmap;localpart trim' \
+ 'fatal: unrecognized %(taggeremail) argument: ;localpart trim'
+
test_bad_atom tag 'taggeremail:localpart trim' \
- 'fatal: unrecognized %(taggeremail) argument: localpart trim'
+ 'fatal: unrecognized %(taggeremail) argument: trim'
+
+test_bad_atom tag 'taggeremail:mailmap,mailmap,trim,qux,localpart,trim' \
+ 'fatal: unrecognized %(taggeremail) argument: qux,localpart,trim'
test_date () {
f=$1 &&
--
2.42.0.273.ge948a9aaf4
^ permalink raw reply related
* Re: [PATCH 2/2] builtin/repack.c: implement support for `--cruft-max-size`
From: Taylor Blau @ 2023-09-25 18:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Jonathan Tan
In-Reply-To: <xmqqtts5wnwn.fsf@gitster.g>
On Thu, Sep 07, 2023 at 04:42:48PM -0700, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> > This all works, but can be costly from an I/O-perspective when a
> > repository has either (a) many unreachable objects, (b) prunes objects
> > relatively infrequently/never, or (c) both.
>
> I can certainly understand (a). If we need to write a lot of
> objects into the craft pack, among which many of them are already in
> the previous generation of craft pack, we would be copying bits from
> the old to the new craft pack, and having to do so for many objects
> would involve expensive I/O. But not (b). Whether we prune objects
> infrequently or we do so very often, as long as the number and
> on-disk size of objects that has to go into craft packs are the
> same, wouldn't the cost of I/O pretty much the same? IOW, (b) does
> not have much to do with how repacking is costly I/O wise, except
> that it is a contributing factor to make (a) worse, which is the
> real cause of the I/O cost.
Yeah, (b) on its own isn't enough to cost us a significant amount of I/O
overhead. (b) definitely exacerbates (a) if we are repacking relatively
frequently. I'll clarify in the patch notes.
> > This limits the I/O churn up to a quadratic function of the value
> > specified by the `--cruft-max-size` option, instead of behaving
> > quadratically in the number of total unreachable objects.
>
> ... I do not quite see how you would limit the I/O churn.
In the above (elided here for brevity) response, you are correct: the
idea is that once a cruft pack has grown to whatever threshold you set
via `--cruft-max-size`, it is effectively frozen, preventing you from
incurring the same I/O churn from it in the future.
> > When pruning unreachable objects, we bypass the new paths which combine
>
> "paths" here refers to...? code paths, I guess?
Yep.
> > +gc.cruftMaxSize::
> > + Limit the size of new cruft packs when repacking. When
> > + specified in addition to `--cruft-max-size`, the command line
> > + option takes priority. See the `--cruft-max-size` option of
> > + linkgit:git-repack[1].
>
> Hmph.
>
> I am reasonably sure that I will mix the name up and call it
> gc.maxCruftSize in my configuration file and scratch my head
> wondering why it is not working.
I have no strong preference for either "gc.cruftMaxSize" or
"gc.maxCruftSize" (or anything else, really), so long as we are
consistent with the command-line option.
> > diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
> > index 90806fd26a..8a90d684a7 100644
> > --- a/Documentation/git-gc.txt
> > +++ b/Documentation/git-gc.txt
> > @@ -59,6 +59,13 @@ be performed as well.
> > cruft pack instead of storing them as loose objects. `--cruft`
> > is on by default.
> >
> > +--cruft-max-size=<n>::
> > + When packing unreachable objects into a cruft pack, limit the
> > + size of new cruft packs to be at most `<n>`. Overrides any
> > + value specified via the `gc.cruftMaxSize` configuration. See
> > + the `--cruft-max-size` option of linkgit:git-repack[1] for
> > + more.
>
> At least this side giving --max-cruft-size=<n> (which I think is a
> lot more natural word order) would cause parse-options to give an
> error, so it won't risk mistakes go silently unnoticed.
Yeah, that's compelling. I'm convinced ;-).
> > diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt
> > index 4017157949..23fd203d79 100644
> > --- a/Documentation/git-repack.txt
> > +++ b/Documentation/git-repack.txt
> > @@ -74,6 +74,15 @@ to the new separate pack will be written.
> > immediately instead of waiting for the next `git gc` invocation.
> > Only useful with `--cruft -d`.
> >
> > +--cruft-max-size=<n>::
> > + Repack cruft objects into packs as large as `<n>` before
> > + creating new packs. As long as there are enough cruft packs
> > + smaller than `<n>`, repacking will cause a new cruft pack to
> > + be created containing objects from any combined cruft packs,
> > + along with any new unreachable objects. Cruft packs larger
> > + than `<n>` will not be modified. Only useful with `--cruft
> > + -d`.
>
> Here, the missing fourth point I pointed out above is mentioned,
> which is good.
>
> Describe the unit for <n> (I am assuming that is counted in bytes,
> honoring the human-friendly suffix, like 100M).
Yep, thanks.
> There may be some "interesting" behaviour around the size boundary,
> no? If you pack too many objects, your resulting size may slightly
> bust <n> and you will get a complaint, but by fixing that "bug", you
> will always stop short of filling the whole <n> bytes in the
> produced packfiles, and they will not be excempt from rewriting
> (becuase they are not "larger than <n>"), which defeats the point of
> this patch.
Yeah, the boundary conditions are definitely the most interesting part
of this patch IMHO. When packing with `--max-cruft-size`, we still do
cap the pack size of the resulting pack, so any excess spills over into
the next pack.
> Describe that <n> is a threshold that we stop soon after passing to
> explicitly allow us to go beyond it would solve the above problem, I
> would presume.
I am not sure I understand what you're getting at here.
> > diff --git a/builtin/gc.c b/builtin/gc.c
> > index 1f53b66c7b..b6640abd35 100644
> > --- a/builtin/gc.c
> > +++ b/builtin/gc.c
> > @@ -52,6 +52,7 @@ static const char * const builtin_gc_usage[] = {
> > static int pack_refs = 1;
> > static int prune_reflogs = 1;
> > static int cruft_packs = 1;
> > +static char *cruft_max_size;
>
> I do not think this type is a good idea.
Yeah, I marked this as a string because we don't ourselves do anything
with it in 'gc', and instead just immediately pass it down to 'repack'.
We could parse it ourselves and catch any malformed arguments earlier,
though, which sounds worthwhile to me.
> > diff --git a/builtin/repack.c b/builtin/repack.c
> > index 44cb261371..56e7f5f43d 100644
> > --- a/builtin/repack.c
> > +++ b/builtin/repack.c
> > @@ -26,6 +26,9 @@
> > #define LOOSEN_UNREACHABLE 2
> > #define PACK_CRUFT 4
> >
> > +#define DELETE_PACK ((void*)(uintptr_t)1)
> > +#define RETAIN_PACK ((uintptr_t)(1<<1))
>
> Shouldn't these look more similar? That is
>
> ((void *)(uintptr_t)(1<<0))
> ((void *)(uintptr_t)(1<<1))
Yeah, these have been shored up from some helpful review on the repack
internals cleanup series I posted earlier.
> > + if (existing_cruft_nr >= existing->cruft_packs.nr)
> > + BUG("too many cruft packs (found %"PRIuMAX", but knew "
> > + "of %"PRIuMAX")",
>
> Is that a BUG() that somehow miscounted the packs, or can it be a
> runtime error that may happen when a "git push" is pushing new
> objects into the repository, creating a new pack we did not know
> about? Something like the latter should not be marked a BUG(), but
This would be the former. We load the set of packs at the beginning of a
repack operation from collect_pack_filenames() via a call to
get_all_packs(). So the set won't change between our view of it in
collect_pack_filenames() and collapse_small_cruft_packs(). IOW, we
should expect to see as many cruft packs as we saw in
collect_pack_filenames(), and any difference there would indeed be a
BUG().
> > + (uintmax_t)existing_cruft_nr + 1,
> > + (uintmax_t)existing->cruft_packs.nr);
> > + existing_cruft[existing_cruft_nr++] = p;
> > + }
> > +
> > + QSORT(existing_cruft, existing_cruft_nr, existing_cruft_pack_cmp);
>
> We use the simplest "from smaller ones to larger ones, combine one
> by one together until the result gets large enough", which would not
> give us the best packing, but it is OK because it is not our goal to
> solve knapsack problem here, I presume?
Exactly. Whatever spill-over we generate (if any) will be the seed of
the next "generation" of cruft packs, which are allowed to grow and
accumulate until they themselves reach the size threshold, at which
point the process starts itself over again.
> > + for (i = 0; i < existing_cruft_nr; i++) {
> > + off_t proposed;
> > +
> > + p = existing_cruft[i];
> > + proposed = st_add(total_size, p->pack_size);
> > +
> > + if (proposed <= max_size) {
> > + total_size = proposed;
> > + fprintf(in, "-%s\n", pack_basename(p));
> > + } else {
> > + retain_cruft_pack(existing, p);
> > + fprintf(in, "%s\n", pack_basename(p));
> > + }
> > + }
>
> This is exactly what I talked about the possibly funny behaviour
> around the boundary earlier, but it may be even worse. This time,
> we may decide that a pack with size <n-epsilon> is to be retained,
> only because the pack that came next in the existing list happened
> to be larger than epsilon, but next time around, it may not be the
> case (i.e. our pack may be smaller than epsilon, the next one in the
> current round may be larger than epsilon, but before we repack the
> next time, a new pack that is slightly smaller than epsilon that is
> larger than our pack may have been created---now our pack will be
> combined with it), so the algorithm to choose which ones are kept
> does not depend on the pack itself alone but also depends on its
> surroundings.
If I understand your comment correctly, that behavior is as-designed. We
try to grow cruft packs by combining other cruft packs that are not yet
frozen, and I think that is going to be dependent on the entire set of
packs rather than the characteristic of any one pack.
An alternative approach might be to combine *all* cruft packs smaller
than some threshold, and let any spill-over get handled by pack-objects
with its --max-pack-size option. I do not have a strong preference
between the two.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v2 0/4] Switch links to https
From: Junio C Hamano @ 2023-09-25 18:33 UTC (permalink / raw)
To: Josh Soref via GitGitGadget; +Cc: git, Josh Soref
In-Reply-To: <pull.1589.v2.git.1695553041.gitgitgadget@gmail.com>
"Josh Soref via GitGitGadget" <gitgitgadget@gmail.com> writes:
> There are a couple of categories of http links
The same comment applies to the previous round, but it is not clear
how the categories of links that are HTTP in the current code (the
first enumeration) are related to how HTTP links are modified in the
series (the second enumeration). It is not like how the first one
in the first list (i.e. must be HTTP) is handled is described by the
first one in the second list (i.e. replace HTTP to HTTPS without
doing anything else), as the number of bullet points are different.
> * links that are required to be http: because they're copied from something
> that mandates it (the apache license, xml namespaces, xsl docbook
> things?)
So these are in "must stay as-is" category?
> * pages which exist at both http: and https: and can be safely switched
"Can be switched" does not mean we should switch, does it?
> * pages that have jittered a bit but are now available as https:
... but not available over HTTP:? If so, this falls into "must
switch to avoid a dangling link" category?
> * pages that have jittered a bit and are not available over https:
Again, "must stay as-is"?
> * pages that are gone and for which the best source is
> https://web.archive.org
Unfortunate, but definitely improves things.
> * urls that were imaginary
What is done to them? Just leave them as they are, or replace with
another imaginary link over HTTPS?
Hopefully we will see in the proposed commit log message of
individual patches what kind of http:// links the patch deals with
and how (e.g. "For some URLs in the documentation and the code, the
original http:// links are not reachable and dangling but the same
contents are still available at https://; for these URLs, replace
http:// with https:// without doing anything else."). Let's read
on.
> In order:
>
> * doc: switch links to https -- the simplest
> * doc: update links to current pages -- I found the current pages for
> these, it should be easy enough to verify these / reject them
> * doc: update links for andre-simon.de -- I've split this out, I don't like
> the idea of having to download binaries over http. If this were my
> project, I'd be tempted to remove the feature or self-host w/ https...
> * doc: refer to internet archive -- the original urls are dead, I've found
> internet archive date links for them. (There are some in git already, so
> this seemed like a very reasonable choice.)
> cc: Eric Sunshine sunshine@sunshineco.com cc: Josh Soref jsoref@gmail.com
(administrivia) I am not sure what effect, if any, this line has.
^ permalink raw reply
* Git / Software Freedom Conservancy status report (2023)
From: Taylor Blau @ 2023-09-25 18:37 UTC (permalink / raw)
To: git
Cc: Ævar Arnfjörð Bjarmason, Christian Couder,
Junio C Hamano
In last year's "Git / SFC" status report, I promised that I'd try and
share updates more regularly about our project financials, things that
SFC have shared with us, etc. I'm happy to report that I have kept that
promise ;-).
This email will serve as a report on the project's activities at
Conservancy for the year 2023.
The previous report (from last year, whose format I'll try to stick to
here) can be found at:
https://lore.kernel.org/git/YyELnLai0jXsnt3W@nand.local/
# Background
Git is a member project of the Software Freedom Conservancy. The Git
project joined Conservancy in 2010 so Conservancy could help us manage
our money and other assets, and provide legal representation for
trademark matters. Conservancy doesn't hold any copyright on any of the
project's code. Similarly, being a member project at Conservancy does
not grant Conservancy any influence in the project's development. The
technical direction that Git takes is up to us.
Interested readers can take a look at a more full picture of what
Conservancy does for the Git project at:
https://sfconservancy.org/projects/services/
A "Project Leadership Committee" (PLC) represents the Git project at
Conservancy. The PLC currently consists of Junio C Hamano, Christian
Couder, Ævar Arnfjörð Bjarmason, and myself.
# Financials
The most recent data I have this year is from 2023-09-18, so these
numbers should be more or less current.
We have ~$89k USD in our account, up ~$19k USD from where we were in
March, 2022. This was a relatively good year for us in that regard,
since the average year over year growth we have seen over the past is
closer to ~$10k USD/yr.
Here are some top-level ledger numbers gathered since the end of
March, 2022 (the date of the last report). Note that this is all
double-entry, so negative numbers are good.
$-22,058.48 Income:Git
$-21,566.67 Donations
$-491.14 Royalties
$3,098.57 Expenses:Git
$217.14 Banking Fees
$452.22 Conferences:Travel
$0.00 Filing Fees
$2,236.03 Hosting
$193.18 Tax:Sales
$-27.40 Assets:Receivable:Accounts
$-131.58 Liabilities:Payable:Accounts
--------------------
$-19,118.89
Like last time, most of our money comes from donations. This year just
2% of it comes from royalties (last year this number was closer to 5%),
all from Amazon affiliate links. 10% of all incoming money goes to
Conservancy's general fund (the above numbers are after that 10% has
been deducted).
Last year we spent money ($452.22 USD) sponsoring one GSoC student to
attend the Contributor's Summit and Git Merge events in Chicago, IL.
One notable change from last time is that our hosting fees have gone up
significantly. These are entirely from Heroku's change in policy to no
longer grant the Git project hosting credits for the git-scm.com
project. Our costs in the meantime have been supported by a generous
donation from Dan Moore at FusionAuth. The below email has some more
details:
https://lore.kernel.org/git/YkcmtqcFaO7v1jW5@nand.local/
It appears that since the above was written, Heroku has a new (?)
program for giving credits to open-source projects. The details are
below:
https://www.heroku.com/open-source-credit-program
I applied on behalf of the Git project on 2023-09-25, and will follow-up
on the list if/when we hear back from them.
# Trademark
We hold a trademark on the term "Git" and its logo in the space of
software and version control. The report from 2017 has a good overview
of the details there:
https://public-inbox.org/git/20170202024501.57hrw4657tsqerqq@sigill.intra.peff.net/
Last year, in response to some discussions we had with folks at
Conservancy, Christian brought a discussion to the list about rethinking
the way we treat our trademark in the future, particularly with respect
to enforcement.
https://lore.kernel.org/git/CAP8UFD3WQ64FuXarugF+CJ_-5sFNBCnqPE0AEBK-Ka78ituKTg@mail.gmail.com/
Conservancy has brought up this topic with the PLC a few times in our
meetings throughout the year. My impression is that they would
appreciate us taking another look at that discussion and (possibly)
rethinking our trademark strategy as a result. I encourage folks to dig
up that thread and share any thoughts they might have.
# Travel Budget Allocation
Like I mentioned above, we sponsored travel for one GSoC student to
attend Git Merge and the Contributor's Summit last year year. The PLC
formalized this process a little bit more rigidly in 2018. The report
from that year has a good overview of the details.
Similarly, that whole procedure is (still) open for comments and
suggestions. Our main focus is to make it possible for new contributors
(particularly ones who have participated in programs like GSoC and
Outreachy) to attend Git Merge (and, specifically, the Contributor's
Summit) where it wouldn't otherwise be possible.
# Conclusion
That's all for this year. I'm happy to answer any questions on the list,
and I'll propose a session on it at the Contributor's Summit tomorrow,
in case folks want to discuss this further in person.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v2 0/4] Switch links to https
From: Eric Sunshine @ 2023-09-25 18:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Josh Soref via GitGitGadget, git, Josh Soref
In-Reply-To: <xmqq34z2ump4.fsf@gitster.g>
On Mon, Sep 25, 2023 at 2:35 PM Junio C Hamano <gitster@pobox.com> wrote:
> "Josh Soref via GitGitGadget" <gitgitgadget@gmail.com> writes:
> > cc: Eric Sunshine sunshine@sunshineco.com cc: Josh Soref jsoref@gmail.com
>
> (administrivia) I am not sure what effect, if any, this line has.
GitGitGadget specially recognizes Cc: trailers in the pull-request's
description[1] to ensure that the named individuals receive a copy of
the emailed patch series, so this is a good way to Cc: people who
commented upon previous versions of the series. Unfortunately, in this
case, it appears that GitGitGadget ignored the two "cc:" lines because
they weren't proper trailers; there are additional non-trailer lines
following them in the pull-request's description (and they were taken
as just more Markdown body content).
[1]: https://github.com/gitgitgadget/git/pull/1589#issue-1908988061
^ permalink raw reply
* Re: [PATCH v7 2/3] unit tests: add TAP unit test framework
From: Junio C Hamano @ 2023-09-25 18:57 UTC (permalink / raw)
To: phillip.wood123; +Cc: Josh Steadmon, git, linusa, calvinwan, rsbecker
In-Reply-To: <0b6de919-8dbf-454f-807b-5abb64388cb7@gmail.com>
phillip.wood123@gmail.com writes:
> When I was writing this I was torn between whether to follow our usual
> convention of returning zero for success and minus one for failure or
> to return one for success and zero for failure. In the end I decided
> to go with the former but I tend to agree with you that the latter
> would be easier to understand.
An understandable contention.
>>>> @@ -0,0 +1,2 @@
>>>> +/t-basic
>>>> +/t-strbuf
>>>
>>> Also, can we come up with some naming convention so that we do not
>>> have to keep adding to this file every time we add a new test
>>> script?
>
> Perhaps we should put the unit test binaries in a separate directory
> so we can just add that directory to .gitignore.
Yeah, if we can do that, that would help organizing these tests.
Thanks for working on this.
^ permalink raw reply
* Re: [PATCH v3] format-patch: add --description-file option
From: Junio C Hamano @ 2023-09-25 19:01 UTC (permalink / raw)
To: Kristoffer Haugsbakk
Cc: Oswald Buddenhagen, Jeff King, Taylor Blau, Derrick Stolee, git
In-Reply-To: <a1920050-bedc-49d4-840d-350b8fd3c003@app.fastmail.com>
"Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:
> On Mon, Aug 21, 2023, at 19:07, Oswald Buddenhagen wrote:
>> This patch makes it possible to directly feed a branch description to
>> derive the cover letter from. The use case is formatting dynamically
>> created temporary commits which are not referenced anywhere.
>
> Thanks for implementing this. I've just written cover letter text in
> separate files and copied them into the generated files every time. (I
> don't use branch descriptions.) I've wanted some convenient way to feed
> these messages in, and if I end up writing a cover letter again I'll most
> probably be using this new option.
Thanks for a positive feedback. The changes is already in 'master'
since the beginning of this month or so and its way to be part of
the next release, I believe.
^ permalink raw reply
* Re: [PATCH v3 0/7] CMake(Visual C) support for js/doc-unit-tests
From: Junio C Hamano @ 2023-09-25 19:09 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget
Cc: git, Phillip Wood, Johannes Schindelin
In-Reply-To: <pull.1579.v3.git.1695640836.gitgitgadget@gmail.com>
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> The recent patch series that adds proper unit testing to Git requires a
> couple of add-on patches to make it work with the CMake build on Windows
> (Visual C). This patch series aims to provide that support.
>
> This patch series is based on js/doc-unit-tests.
>
> Changes since v2:
>
> * Thanks to Phillip Wood's prodding, I managed to avoid the "Empty
> Namespace" problem in Visual Studio's Test Explorer.
Thanks. Will replace.
> + ◢ ◈ git
> + ◢ ◈ t
> + ◢ ◈ suite
> + ◈ t0000-basic
> + ◈ t0001-init
> + ◈ t0002-gitfile
> + [...]
I somehow liked this part of the log message very much ;-)
^ permalink raw reply
* [PATCH] unicode: update the width tables to Unicode 15.1
From: Beat Bolli @ 2023-09-25 19:07 UTC (permalink / raw)
To: git; +Cc: Beat Bolli
Unicode 15.1 has been announced on 2023-09-12 [0], so update the
character width tables to the new version.
[0] http://blog.unicode.org/2023/09/announcing-unicode-standard-version-151.html
Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
unicode-width.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/unicode-width.h b/unicode-width.h
index e15fb0455b..be5bf8c4f2 100644
--- a/unicode-width.h
+++ b/unicode-width.h
@@ -396,14 +396,13 @@ static const struct interval double_width[] = {
{ 0x2E80, 0x2E99 },
{ 0x2E9B, 0x2EF3 },
{ 0x2F00, 0x2FD5 },
-{ 0x2FF0, 0x2FFB },
-{ 0x3000, 0x303E },
+{ 0x2FF0, 0x303E },
{ 0x3041, 0x3096 },
{ 0x3099, 0x30FF },
{ 0x3105, 0x312F },
{ 0x3131, 0x318E },
{ 0x3190, 0x31E3 },
-{ 0x31F0, 0x321E },
+{ 0x31EF, 0x321E },
{ 0x3220, 0x3247 },
{ 0x3250, 0x4DBF },
{ 0x4E00, 0xA48C },
--
2.40.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox