* Re: [PATCH 3/3] advice: allow disabling the automatic hint in advise_if_enabled()
From: Junio C Hamano @ 2024-01-18 20:19 UTC (permalink / raw)
To: Dragan Simic; +Cc: Jeff King, Rubén Justo, Git List
In-Reply-To: <bfded0571f133ffc1612fcbca1811a43@manjaro.org>
Dragan Simic <dsimic@manjaro.org> writes:
> On 2024-01-18 19:26, Junio C Hamano wrote:
>> Dragan Simic <dsimic@manjaro.org> writes:
>>
>>> Perhaps having both options available could be a good thing. Though,
>>> adding quite a few knobs may end up confusing the users, so we should
>>> make sure to document it well.
>> I agree there is a need for documentation, but we are not increasing
>> the number of knobs, are we?
>
> Perhaps there would be one more knob if we end up deciding to implement
> support for both approaches, as previously discussed.
But that would be just one knob (which I personally do not quite see
the need for), not "quite a few knobs" as you said, no?
^ permalink raw reply
* Re: [PATCH v2] merge-ll: expose revision names to custom drivers
From: Junio C Hamano @ 2024-01-18 20:16 UTC (permalink / raw)
To: Antonin Delpeuch via GitGitGadget
Cc: git, Kristoffer Haugsbakk, Antonin Delpeuch
In-Reply-To: <pull.1648.v2.git.git.1705592581272.gitgitgadget@gmail.com>
"Antonin Delpeuch via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Antonin Delpeuch <antonin@delpeuch.eu>
>
> Custom merge drivers need access to the names of the revisions they
> are working on, so that the merge conflict markers they introduce
> can refer to those revisions. The placeholders '%S', '%X' and '%Y'
> are introduced to this end.
>
> Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu>
> ---
> merge-ll: expose revision names to custom drivers
>
> Documentation/gitattributes.txt | 10 +++++++---
> merge-ll.c | 17 ++++++++++++++---
> t/t6406-merge-attr.sh | 16 +++++++++++-----
> 3 files changed, 32 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
> index 201bdf5edbd..108c2e23baa 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -1141,7 +1141,7 @@ command to run to merge ancestor's version (`%O`), current
> version (`%A`) and the other branches' version (`%B`). These
> three tokens are replaced with the names of temporary files that
> hold the contents of these versions when the command line is
> -built. Additionally, %L will be replaced with the conflict marker
> +built. Additionally, `%L` will be replaced with the conflict marker
> size (see below).
Good eyes. Nice to fix it while we are in the vicinity.
> @@ -1159,8 +1159,12 @@ When left unspecified, the driver itself is used for both
> internal merge and the final merge.
>
> The merge driver can learn the pathname in which the merged result
> -will be stored via placeholder `%P`.
> -
> +will be stored via placeholder `%P`. Additionally, the names of the
> +merge ancestor revision (`%S`), of the current revision (`%X`) and
The phrase already existsin the description of '%O', but the correct
word for that is "the common ancestor", not "the merge ancestor".
At least this one is consistent with the existing error, so we can
fix them together in a clean-up patch after the dust settles.
Or you could fix %O's description "while at it" and use the right
term from the get-go for %S.
> diff --git a/merge-ll.c b/merge-ll.c
> index 1df58ebaac0..ae9eb69be14 100644
> --- a/merge-ll.c
> +++ b/merge-ll.c
> @@ -185,9 +185,9 @@ static void create_temp(mmfile_t *src, char *path, size_t len)
> static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn,
> mmbuffer_t *result,
> const char *path,
> - mmfile_t *orig, const char *orig_name UNUSED,
> - mmfile_t *src1, const char *name1 UNUSED,
> - mmfile_t *src2, const char *name2 UNUSED,
> + mmfile_t *orig, const char *orig_name,
> + mmfile_t *src1, const char *name1,
> + mmfile_t *src2, const char *name2,
> const struct ll_merge_options *opts,
> int marker_size)
> {
> @@ -222,6 +222,12 @@ static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn,
> strbuf_addf(&cmd, "%d", marker_size);
> else if (skip_prefix(format, "P", &format))
> sq_quote_buf(&cmd, path);
> + else if (skip_prefix(format, "S", &format))
> + sq_quote_buf(&cmd, orig_name);
> + else if (skip_prefix(format, "X", &format))
> + sq_quote_buf(&cmd, name1);
> + else if (skip_prefix(format, "Y", &format))
> + sq_quote_buf(&cmd, name2);
> else
> strbuf_addch(&cmd, '%');
> }
I see some funny indentation for "S" here.
> diff --git a/t/t6406-merge-attr.sh b/t/t6406-merge-attr.sh
> index 72f8c1722ff..156a1efacfe 100755
> --- a/t/t6406-merge-attr.sh
> +++ b/t/t6406-merge-attr.sh
> @@ -42,11 +42,15 @@ test_expect_success setup '
> #!/bin/sh
>
> orig="$1" ours="$2" theirs="$3" exit="$4" path=$5
> + orig_name="$6" our_name="$7" their_name="$8"
> (
> echo "orig is $orig"
> echo "ours is $ours"
> echo "theirs is $theirs"
> echo "path is $path"
> + echo "orig_name is $orig_name"
> + echo "our_name is $our_name"
> + echo "their_name is $their_name"
> echo "=== orig ==="
> cat "$orig"
> echo "=== ours ==="
> @@ -121,7 +125,7 @@ test_expect_success 'custom merge backend' '
>
> git reset --hard anchor &&
> git config --replace-all \
> - merge.custom.driver "./custom-merge %O %A %B 0 %P" &&
> + merge.custom.driver "./custom-merge %O %A %B 0 %P %S %X %Y" &&
> git config --replace-all \
> merge.custom.name "custom merge driver for testing" &&
>
> @@ -132,7 +136,8 @@ test_expect_success 'custom merge backend' '
> o=$(git unpack-file main^:text) &&
> a=$(git unpack-file side^:text) &&
> b=$(git unpack-file main:text) &&
> - sh -c "./custom-merge $o $a $b 0 text" &&
> + base_revid=$(git rev-parse --short main^) &&
> + sh -c "./custom-merge $o $a $b 0 text $base_revid HEAD main" &&
> sed -e 1,3d $a >check-2 &&
> cmp check-1 check-2 &&
> rm -f $o $a $b
OK.
> @@ -142,7 +147,7 @@ test_expect_success 'custom merge backend' '
>
> git reset --hard anchor &&
> git config --replace-all \
> - merge.custom.driver "./custom-merge %O %A %B 1 %P" &&
> + merge.custom.driver "./custom-merge %O %A %B 1 %P %S %X %Y" &&
> git config --replace-all \
> merge.custom.name "custom merge driver for testing" &&
>
> @@ -159,7 +164,8 @@ test_expect_success 'custom merge backend' '
> o=$(git unpack-file main^:text) &&
> a=$(git unpack-file anchor:text) &&
> b=$(git unpack-file main:text) &&
> - sh -c "./custom-merge $o $a $b 0 text" &&
> + base_revid=$(git rev-parse --short main^) &&
> + sh -c "./custom-merge $o $a $b 0 text $base_revid HEAD main" &&
> sed -e 1,3d $a >check-2 &&
> cmp check-1 check-2 &&
> sed -e 1,3d -e 4q $a >check-3 &&
OK.
> @@ -173,7 +179,7 @@ test_expect_success !WINDOWS 'custom merge driver that is killed with a signal'
>
> git reset --hard anchor &&
> git config --replace-all \
> - merge.custom.driver "./custom-merge %O %A %B 0 %P" &&
> + merge.custom.driver "./custom-merge %O %A %B 0 %P %S %X %Y" &&
> git config --replace-all \
> merge.custom.name "custom merge driver for testing" &&
;-)
This one is expected to die and not produce meaningful output;
I was wondering why this does not need to make corresponding changes
to the expected output pattern like the earlier tests.
^ permalink raw reply
* Re: [PATCH 2/2] reftable/stack: fix race in up-to-date check
From: Junio C Hamano @ 2024-01-18 20:07 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <713e51a25c1c4cfa830db97f71cd7c39e85864d4.1705585037.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> This should address the race in a POSIX-compliant way. The only real
> downside is that this mechanism cannot be used on non-POSIX-compliant
> systems like Windows. But we at least have the second-level caching
> mechanism in place that compares contents of "files.list" with the
> currently loaded list of tables.
OK.
> + /*
> + * Cache stat information in case it provides a useful signal to us.
> + * According to POSIX, "The st_ino and st_dev fields taken together
> + * uniquely identify the file within the system." That being said,
> + * Windows is not POSIX compliant and we do not have these fields
> + * available. So the information we have there is insufficient to
> + * determine whether two file descriptors point to the same file.
> + *
> + * While we could fall back to using other signals like the file's
> + * mtime, those are not sufficient to avoid races. We thus refrain from
> + * using the stat cache on such systems and fall back to the secondary
> + * caching mechanism, which is to check whether contents of the file
> + * have changed.
OK.
> + *
> + * On other systems which are POSIX compliant we must keep the file
> + * descriptor open. This is to avoid a race condition where two
> + * processes access the reftable stack at the same point in time:
> + *
> + * 1. A reads the reftable stack and caches its stat info.
> + *
> + * 2. B updates the stack, appending a new table to "tables.list".
> + * This will both use a new inode and result in a different file
> + * size, thus invalidating A's cache in theory.
> + *
> + * 3. B decides to auto-compact the stack and merges two tables. The
> + * file size now matches what A has cached again. Furthermore, the
> + * filesystem may decide to recycle the inode number of the file
> + * we have replaced in (2) because it is not in use anymore.
> + *
> + * 4. A reloads the reftable stack. Neither the inode number nor the
> + * file size changed. If the timestamps did not change either then
> + * we think the cached copy of our stack is up-to-date.
> + *
> + * By keeping the file descriptor open the inode number cannot be
> + * recycled, mitigating the race.
> + */
This is nasty. Well diagnosed and fixed.
Will queue.
Thanks.
^ permalink raw reply
* Re: [PATCH 00/12] Group reffiles tests
From: Junio C Hamano @ 2024-01-18 20:00 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: John Cai via GitGitGadget, git, John Cai
In-Reply-To: <ZakNqm0zyw8IiIhB@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
> I think it depends. If we use the REFFILES prereq for the files-specific
> tests, then we should likely also use the REFTABLE prereq for the
> reftable-specific tests.
Correct. I've assumed that as a given; while introducing any new
implementation of a subsystem that has widespread impact, we would
test things with the original and new implementations. It happened
while we were moving "ort" to replace "recursive" as an internal
tree merge machinery, for example. linux-TEST-vars job that is
available both in GitHub and GitLab CI is an example of a separate
job that runs everything with non-default configurations, and "use
reftable as the default backend" GIT_TEST_REFTABLE knob may be an
appropriate thing to set there.
> But that raises the question of whether we want to add a CI job that
> exercises code with the reftable backend for every major platform
> (Linux, macOS, Windows). If so then your proposal would be fine with me
> as we make sure that things work alright on all of them. But if we think
> that this would be too expensive then I'd like to at least have very
> basic test coverage on all platforms by always running these
> backend-specific tests.
>
> Patrick
^ permalink raw reply
* Re: [PATCH 1/1] builtin/show-ref: treat directory directory as non-existing in --exists
From: Junio C Hamano @ 2024-01-18 19:19 UTC (permalink / raw)
To: Toon Claes; +Cc: Patrick Steinhardt, Eric Sunshine, git
In-Reply-To: <87le8nqc73.fsf@iotcl.com>
Toon Claes <toon@iotcl.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Yup. The patch looks good. Here is what I tentatively queued.
>>
>> ----- >8 -----
>> From: Toon Claes <toon@iotcl.com>
>> Date: Wed, 10 Jan 2024 15:15:59 +0100
>> Subject: [PATCH] builtin/show-ref: treat directory directory as non-existing
>> in --exists
>
> You have a duplicate "directory" here.
Yikes. I did notice a breakage in somebody's mailer that inserted a
SP after the first dash in "--exists" and fixed it, but did not
notice you had "directory" duplicated there. Thanks for noticing.
Amended. Let's merge it down to 'next'.
Thanks.
^ permalink raw reply
* Re: [PATCH] precious-files.txt: new document proposing new precious file type
From: Junio C Hamano @ 2024-01-18 19:14 UTC (permalink / raw)
To: Sebastian Thiel
Cc: Elijah Newren, Elijah Newren via GitGitGadget, git, Josh Triplett,
Phillip Wood
In-Reply-To: <298F7067-D572-433F-A7DD-5256B43B97D7@icloud.com>
Sebastian Thiel <sebastian.thiel@icloud.com> writes:
> #(keep)
> .config
>
> As a side-effect of the syntax, it's obvious this is an 'upgrade', with
> perfect backwards compatibility as old git does the same as always.
Yes but ...
The point Elijah makes is worth considering. To existing versions
of git, having this entry for ".config" means that it is ignored
(i.e. "git add ." will not include it), but expendable (i.e. "git
clean" considers ".config" as a candidate for removal; "git checkout
other", if the "other" branch has it as a tracked path, will clobber
it). Compared to the case where ".config" is not mentioned in
".gitignore", where it may be added by use of "git add .", it won't
be clobbered by "git clean".
So this syntax having "perfect backward compatibility" is not quite
true. It does have downsides when used by existing versions of Git.
If we use Elijah's syntax to say
$.config
then the entry to existing versions of git is a no-op wrt a file
named ".config". It simply does not match the pattern, so an
accidental "git add ." *will* add ".config" to the index, while "git
clean" may not touch it, simply because it is treated as "untracked
and precious". In other words, its downside is the same as not
marking the path ".config" in any way in ".gitignore", as far as
existing versions of Git are concerned.
We of course discount the possibility that people keep a file whose
name literally is dollar-dot followed by "config" and older versions
of Git would start treating them as ignored-and-expendable. While
it *is* an additional downside compared to Phillip's "#(keep)"
approach, I do not think that particular downside is worth worrying
about. Yet another downside compared to Phillip's is that it is
less extensible. Over the years, however, the ignored-but-precious
is the only one we heard from users that lack of which is hurting
them, so lack of extensibility may not be too huge a deal.
For projects that are currently listing these files in ".gitignore"
as "ignored-and-expendable" already and want to categorize them as
"ignored-and-precious" by changing ".config" to "$.config" (or
adding "#(keep)" comment before the existing entry), the
pros-and-cons equation may differ. Their current participants are
protected from accidentally adding them with "git add ." but risking
to lose them with "git clean -f". They may even be trained to be
careful to see "git clean -n" output before actually running the
command with "-f". Now, if their project ships a new version of
".gitignore" that marks these paths as "ignored-and-precious", both
approaches will have intended effect to participants who upgraded to
the version of Git.
To participants using the current version of Git:
* Phillip's approach to add "#(keep)" will not change anything.
They will be protected from accidental "git add ." as before, and
they will still have to be careful about "git clean -f".
* Elijah's approach to rewrite existing'.config' to '$.config',
however, will stop protecting them from "git add .", even though
it will start protecting them from "git clean -f".
The devil you already know may be the lessor of two evils in such a
situation.
So, all it boils down to is these two questions.
* Which one between "'git add .' adds '.config' that users did not
want to add" and "'git clean -f' removes '.config' together with
other files" a larger problem to the users, who participate in a
project that already decided to use the new .gitignore feature to
mark ".config" as "precious", of older versions of Git that
predate "precious"?
* What are projects doing to paths that they want to make
"precious" with the current system? Do they leave them out of
".gitignore" and have them subject to accidental "git add ." to
protect them from "git clean -f"? Or do they list them in
".gitignore" to prevent "git add ." from touching, but leave them
susceptible to accidental removal by "git clean -f"?
Thanks.
^ permalink raw reply
* Re: [PATCH 3/3] advice: allow disabling the automatic hint in advise_if_enabled()
From: Dragan Simic @ 2024-01-18 18:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Rubén Justo, Git List
In-Reply-To: <xmqqil3qsdkv.fsf@gitster.g>
On 2024-01-18 19:26, Junio C Hamano wrote:
> Dragan Simic <dsimic@manjaro.org> writes:
>
>> Perhaps having both options available could be a good thing. Though,
>> adding quite a few knobs may end up confusing the users, so we should
>> make sure to document it well.
>
> I agree there is a need for documentation, but we are not increasing
> the number of knobs, are we?
Perhaps there would be one more knob if we end up deciding to implement
support for both approaches, as previously discussed.
^ permalink raw reply
* Suggestion for Git docummntation Book, Chapter "7.7 Git Tools - Reset Demystified"
From: Peter Hunkeler @ 2024-01-18 18:42 UTC (permalink / raw)
To: git
Hi,
I appreciate all the work that has been done to give us Git users good
documentation.
As a rather-newbie I have to lookup the commands I need for rarely used,
specific tasks. <git reset ...> being one of them. If find chapter "7.7
Git Tools - Reset Demystified" very helpful. There is however one point
that newbies might not (yet) clearly understand: The git reset does
*not* do anything to new, untracked files that might exist in the
working tree. To cleanup, the advice from various places on the internet
is to issue a "git clean -f -d" after the reset.
Maybe it would be helpful if said chapter had a short discussion about
non tracked files, how to clean with "git clean -f -d"
Thanks
Peter
^ permalink raw reply
* Re: [PATCH 3/3] advice: allow disabling the automatic hint in advise_if_enabled()
From: Junio C Hamano @ 2024-01-18 18:26 UTC (permalink / raw)
To: Dragan Simic; +Cc: Jeff King, Rubén Justo, Git List
In-Reply-To: <5f322b3e6053c27dda58ef1c1f025d11@manjaro.org>
Dragan Simic <dsimic@manjaro.org> writes:
> Perhaps having both options available could be a good thing. Though,
> adding quite a few knobs may end up confusing the users, so we should
> make sure to document it well.
I agree there is a need for documentation, but we are not increasing
the number of knobs, are we?
^ permalink raw reply
* Re: [PATCH v2 0/4] Strengthen fsck checks for submodule URLs
From: Junio C Hamano @ 2024-01-18 18:24 UTC (permalink / raw)
To: Victoria Dye via GitGitGadget
Cc: git, Patrick Steinhardt, Jeff King, Victoria Dye
In-Reply-To: <pull.1635.v2.git.1705542918.gitgitgadget@gmail.com>
"Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com> writes:
> While testing 'git fsck' checks on .gitmodules URLs, I noticed that some
> invalid URLs were passing the checks. Digging into it a bit more, the issue
> turned out to be that 'credential_from_url_gently()' parses certain URLs
> (like "http://example.com:something/deeper/path") incorrectly, in a way that
> appeared to return a valid result.
>
> Fortunately, these URLs are rejected in fetches/clones/pushes anyway because
> 'url_normalize()' (called in 'validate_remote_url()') correctly identifies
> them as invalid. So, to bring 'git fsck' in line with other (stronger)
> validation done on remote URLs, this series replaces the
> 'credential_from_url_gently()' check with one that uses 'url_normalize()'.
>
> * Patch 1 moves 'check_submodule_url()' to a public location so that it can
> be used outside of 'fsck.c'.
> * Patch 2 removes the obsolete/never-used code in 'test-tool submodule
> check-name' handling names provided on the command line.
> * Patch 3 adds a 'check-url' mode to 'test-tool submodule', calling the
> now-public 'check_submodule_url()' method on a given URL, and adds new
> tests checking valid and invalid submodule URLs.
> * Patch 4 replaces the 'credential_from_url_gently()' check with
> 'url_normalize()' followed by 'url_decode()' and an explicit check for
> newlines (to preserve the newline handling added in 07259e74ec1 (fsck:
> detect gitmodules URLs with embedded newlines, 2020-03-11)).
Nicely done. I'll wait for a few days to see if anybody else has
reaction but after reading the patches myself, my inclination is to
suggest merging it to 'next'.
Thanks.
^ permalink raw reply
* Re: [PATCH 01/12] t3210: move to t0602
From: John Cai @ 2024-01-18 16:25 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Junio C Hamano, John Cai via GitGitGadget, git
In-Reply-To: <ZakMQP3r44eVc5Dh@tanuki>
Hi Patrick,
On 18 Jan 2024, at 6:32, Patrick Steinhardt wrote:
> On Wed, Jan 17, 2024 at 04:40:10PM -0800, Junio C Hamano wrote:
>> "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>>> From: John Cai <johncai86@gmail.com>
>>>
>>> Move t3210 to t0602, since these tests are reffiles specific in that
>>> they modify loose refs manually. This is part of the effort to
>>> categorize these tests together based on the ref backend they test. When
>>> we upstream the reftable backend, we can add more tests to t06xx. This
>>> way, all tests that test specific ref backend behavior will be grouped
>>> together.
>>
>> So, ... is the idea to have (1) majority of ref tests, against which
>> all backends ought to behave the same way, will be written in
>> backend agnostic way (e.g., we have seen some patches to stop
>> touching the filesystem .git/refs/ hierarchy manually), and (2) some
>> backend specific tests will be grouped in a small number of test
>> script files for each backend and they all will use t6xx numbrs?
>>
>> OK. Sounds like a good plan to me.
>
> Yes, that's the plan. The backend specific tests will be free to also
> exercise filesystem-level behaviour in order to pin down that things
> work as expected. But once their behaviour is nailed down all other
> generic tests should refrain from doing that to the best extent possible
> and instead use Git commands to do their thing.
>
>>> Signed-off-by: John Cai <johncai86@gmail.com>
>>> ---
>>> t/{t3210-pack-refs.sh => t0602-reffiles-pack-refs.sh} | 0
>>> 1 file changed, 0 insertions(+), 0 deletions(-)
>>> rename t/{t3210-pack-refs.sh => t0602-reffiles-pack-refs.sh} (100%)
>
> Is there a reason why you picked t0602 instead of the not-yet-taken
> t0601? If it's only because I use t0601 in my reftable integration
> branch then I'd like us to pick t0601 here instead to avoid a weird gap.
> I'll adapt accordingly and rename the reftable tests to have a t061x
> prefix in that case so that they are nicely grouped together.
Yes if I remember correctly, that's the reason. I can move this to t0601 then,
thanks.
>
> Patrick
^ permalink raw reply
* [PATCH v3 4/4] maintenance: use XDG config if it exists
From: Kristoffer Haugsbakk @ 2024-01-18 16:12 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, ps, stolee, Eric Sunshine, Taylor Blau,
Junio C Hamano
In-Reply-To: <cover.1705593810.git.code@khaugsbakk.name>
`git maintenance register` registers the repository in the user's global
config. `$XDG_CONFIG_HOME/git/config` is supposed to be used if
`~/.gitconfig` does not exist. However, this command creates a
`~/.gitconfig` file and writes to that one even though the XDG variant
exists.
This used to work correctly until 50a044f1e4 (gc: replace config
subprocesses with API calls, 2022-09-27), when the command started calling
the config API instead of git-config(1).
Also change `unregister` accordingly.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
Notes (series):
v2:
• Add `unregister` tests
• Use subshell when exporting an env. variable
• Style in tests
• Free variables properly
builtin/gc.c | 27 ++++++++++++-------------
t/t7900-maintenance.sh | 45 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 58 insertions(+), 14 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index c078751824c..cb80ced6cb5 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1543,19 +1543,18 @@ static int maintenance_register(int argc, const char **argv, const char *prefix)
if (!found) {
int rc;
- char *user_config = NULL, *xdg_config = NULL;
+ char *global_config_file = NULL;
if (!config_file) {
- git_global_config_paths(&user_config, &xdg_config);
- config_file = user_config;
- if (!user_config)
- die(_("$HOME not set"));
+ global_config_file = git_global_config();
+ config_file = global_config_file;
}
+ if (!config_file)
+ die(_("$HOME not set"));
rc = git_config_set_multivar_in_file_gently(
config_file, "maintenance.repo", maintpath,
CONFIG_REGEX_NONE, 0);
- free(user_config);
- free(xdg_config);
+ free(global_config_file);
if (rc)
die(_("unable to add '%s' value of '%s'"),
@@ -1612,18 +1611,18 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi
if (found) {
int rc;
- char *user_config = NULL, *xdg_config = NULL;
+ char *global_config_file = NULL;
+
if (!config_file) {
- git_global_config_paths(&user_config, &xdg_config);
- config_file = user_config;
- if (!user_config)
- die(_("$HOME not set"));
+ global_config_file = git_global_config();
+ config_file = global_config_file;
}
+ if (!config_file)
+ die(_("$HOME not set"));
rc = git_config_set_multivar_in_file_gently(
config_file, key, NULL, maintpath,
CONFIG_FLAGS_MULTI_REPLACE | CONFIG_FLAGS_FIXED_VALUE);
- free(user_config);
- free(xdg_config);
+ free(global_config_file);
if (rc &&
(!force || rc == CONFIG_NOTHING_SET))
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 00d29871e65..0943dfa18a3 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -67,6 +67,51 @@ test_expect_success 'maintenance.auto config option' '
test_subcommand ! git maintenance run --auto --quiet <false
'
+test_expect_success 'register uses XDG_CONFIG_HOME config if it exists' '
+ test_when_finished rm -r .config/git/config &&
+ (
+ XDG_CONFIG_HOME=.config &&
+ export XDG_CONFIG_HOME &&
+ mkdir -p $XDG_CONFIG_HOME/git &&
+ >$XDG_CONFIG_HOME/git/config &&
+ git maintenance register &&
+ git config --file=$XDG_CONFIG_HOME/git/config --get maintenance.repo >actual &&
+ pwd >expect &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'register does not need XDG_CONFIG_HOME config to exist' '
+ test_when_finished git maintenance unregister &&
+ test_path_is_missing $XDG_CONFIG_HOME/git/config &&
+ git maintenance register &&
+ git config --global --get maintenance.repo >actual &&
+ pwd >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'unregister uses XDG_CONFIG_HOME config if it exists' '
+ test_when_finished rm -r .config/git/config &&
+ (
+ XDG_CONFIG_HOME=.config &&
+ export XDG_CONFIG_HOME &&
+ mkdir -p $XDG_CONFIG_HOME/git &&
+ >$XDG_CONFIG_HOME/git/config &&
+ git maintenance register &&
+ git maintenance unregister &&
+ test_must_fail git config --file=$XDG_CONFIG_HOME/git/config --get maintenance.repo >actual &&
+ test_must_be_empty actual
+ )
+'
+
+test_expect_success 'unregister does not need XDG_CONFIG_HOME config to exist' '
+ test_path_is_missing $XDG_CONFIG_HOME/git/config &&
+ git maintenance register &&
+ git maintenance unregister &&
+ test_must_fail git config --global --get maintenance.repo >actual &&
+ test_must_be_empty actual
+'
+
test_expect_success 'maintenance.<task>.enabled' '
git config maintenance.gc.enabled false &&
git config maintenance.commit-graph.enabled true &&
--
2.43.0
^ permalink raw reply related
* [PATCH v3 3/4] config: factor out global config file retrieval
From: Kristoffer Haugsbakk @ 2024-01-18 16:12 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, ps, stolee, Eric Sunshine, Taylor Blau,
Junio C Hamano
In-Reply-To: <cover.1705593810.git.code@khaugsbakk.name>
Factor out code that retrieves the global config file so that we can use
it in `gc.c` as well.
Use the old name from the previous commit since this function acts
functionally the same as `git_system_config` but for “global”.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
Notes (series):
v3:
• Remove doc on `git_global_config`
• https://lore.kernel.org/git/c87b3d93-74db-4377-a57c-80f766d46e7f@app.fastmail.com/
v2:
• Don’t die; return `NULL`
builtin/config.c | 25 +++----------------------
config.c | 20 ++++++++++++++++++++
config.h | 1 +
3 files changed, 24 insertions(+), 22 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 6fff2655816..08fe36d4997 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -708,30 +708,11 @@ int cmd_config(int argc, const char **argv, const char *prefix)
}
if (use_global_config) {
- char *user_config, *xdg_config;
-
- git_global_config_paths(&user_config, &xdg_config);
- if (!user_config)
- /*
- * It is unknown if HOME/.gitconfig exists, so
- * we do not know if we should write to XDG
- * location; error out even if XDG_CONFIG_HOME
- * is set and points at a sane location.
- */
+ given_config_source.file = git_global_config();
+ if (!given_config_source.file)
die(_("$HOME not set"));
-
given_config_source.scope = CONFIG_SCOPE_GLOBAL;
-
- if (access_or_warn(user_config, R_OK, 0) &&
- xdg_config && !access_or_warn(xdg_config, R_OK, 0)) {
- given_config_source.file = xdg_config;
- free(user_config);
- } else {
- given_config_source.file = user_config;
- free(xdg_config);
- }
- }
- else if (use_system_config) {
+ } else if (use_system_config) {
given_config_source.file = git_system_config();
given_config_source.scope = CONFIG_SCOPE_SYSTEM;
} else if (use_local_config) {
diff --git a/config.c b/config.c
index ebc6a57e1c3..3cfeb3d8bd9 100644
--- a/config.c
+++ b/config.c
@@ -1987,6 +1987,26 @@ char *git_system_config(void)
return system_config;
}
+char *git_global_config(void)
+{
+ char *user_config, *xdg_config;
+
+ git_global_config_paths(&user_config, &xdg_config);
+ if (!user_config) {
+ free(xdg_config);
+ return NULL;
+ }
+
+ if (access_or_warn(user_config, R_OK, 0) && xdg_config &&
+ !access_or_warn(xdg_config, R_OK, 0)) {
+ free(user_config);
+ return xdg_config;
+ } else {
+ free(xdg_config);
+ return user_config;
+ }
+}
+
void git_global_config_paths(char **user_out, char **xdg_out)
{
char *user_config = xstrdup_or_null(getenv("GIT_CONFIG_GLOBAL"));
diff --git a/config.h b/config.h
index e5e523553cc..5dba984f770 100644
--- a/config.h
+++ b/config.h
@@ -382,6 +382,7 @@ int config_error_nonbool(const char *);
#endif
char *git_system_config(void);
+char *git_global_config(void);
void git_global_config_paths(char **user, char **xdg);
int git_config_parse_parameter(const char *, config_fn_t fn, void *data);
--
2.43.0
^ permalink raw reply related
* [PATCH v3 2/4] config: rename global config function
From: Kristoffer Haugsbakk @ 2024-01-18 16:12 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, ps, stolee, Eric Sunshine, Taylor Blau,
Junio C Hamano
In-Reply-To: <cover.1705593810.git.code@khaugsbakk.name>
Rename this function to a more descriptive name since we want to use the
existing name for a new function.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
builtin/config.c | 2 +-
builtin/gc.c | 4 ++--
builtin/var.c | 2 +-
config.c | 4 ++--
config.h | 2 +-
5 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 87d0dc92d99..6fff2655816 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -710,7 +710,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
if (use_global_config) {
char *user_config, *xdg_config;
- git_global_config(&user_config, &xdg_config);
+ git_global_config_paths(&user_config, &xdg_config);
if (!user_config)
/*
* It is unknown if HOME/.gitconfig exists, so
diff --git a/builtin/gc.c b/builtin/gc.c
index 7c11d5ebef0..c078751824c 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1546,7 +1546,7 @@ static int maintenance_register(int argc, const char **argv, const char *prefix)
char *user_config = NULL, *xdg_config = NULL;
if (!config_file) {
- git_global_config(&user_config, &xdg_config);
+ git_global_config_paths(&user_config, &xdg_config);
config_file = user_config;
if (!user_config)
die(_("$HOME not set"));
@@ -1614,7 +1614,7 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi
int rc;
char *user_config = NULL, *xdg_config = NULL;
if (!config_file) {
- git_global_config(&user_config, &xdg_config);
+ git_global_config_paths(&user_config, &xdg_config);
config_file = user_config;
if (!user_config)
die(_("$HOME not set"));
diff --git a/builtin/var.c b/builtin/var.c
index 8cf7dd9e2e5..cf5567208a2 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -90,7 +90,7 @@ static char *git_config_val_global(int ident_flag UNUSED)
char *user, *xdg;
size_t unused;
- git_global_config(&user, &xdg);
+ git_global_config_paths(&user, &xdg);
if (xdg && *xdg) {
normalize_path_copy(xdg, xdg);
strbuf_addf(&buf, "%s\n", xdg);
diff --git a/config.c b/config.c
index d26e16e3ce3..ebc6a57e1c3 100644
--- a/config.c
+++ b/config.c
@@ -1987,7 +1987,7 @@ char *git_system_config(void)
return system_config;
}
-void git_global_config(char **user_out, char **xdg_out)
+void git_global_config_paths(char **user_out, char **xdg_out)
{
char *user_config = xstrdup_or_null(getenv("GIT_CONFIG_GLOBAL"));
char *xdg_config = NULL;
@@ -2040,7 +2040,7 @@ static int do_git_config_sequence(const struct config_options *opts,
data, CONFIG_SCOPE_SYSTEM,
NULL);
- git_global_config(&user_config, &xdg_config);
+ git_global_config_paths(&user_config, &xdg_config);
if (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK))
ret += git_config_from_file_with_options(fn, xdg_config, data,
diff --git a/config.h b/config.h
index 14f881ecfaf..e5e523553cc 100644
--- a/config.h
+++ b/config.h
@@ -382,7 +382,7 @@ int config_error_nonbool(const char *);
#endif
char *git_system_config(void);
-void git_global_config(char **user, char **xdg);
+void git_global_config_paths(char **user, char **xdg);
int git_config_parse_parameter(const char *, config_fn_t fn, void *data);
--
2.43.0
^ permalink raw reply related
* [PATCH v3 1/4] config: format newlines
From: Kristoffer Haugsbakk @ 2024-01-18 16:12 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, ps, stolee, Eric Sunshine, Taylor Blau,
Junio C Hamano
In-Reply-To: <cover.1705593810.git.code@khaugsbakk.name>
Remove unneeded newlines according to `clang-format`.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
Notes (series):
Honestly the formatter changing these lines over and over again was just
annoying. And we're visiting the file anyway.
builtin/config.c | 1 -
config.c | 2 --
2 files changed, 3 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 11a4d4ef141..87d0dc92d99 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -760,7 +760,6 @@ int cmd_config(int argc, const char **argv, const char *prefix)
given_config_source.scope = CONFIG_SCOPE_COMMAND;
}
-
if (respect_includes_opt == -1)
config_options.respect_includes = !given_config_source.file;
else
diff --git a/config.c b/config.c
index 9ff6ae1cb90..d26e16e3ce3 100644
--- a/config.c
+++ b/config.c
@@ -95,7 +95,6 @@ static long config_file_ftell(struct config_source *conf)
return ftell(conf->u.file);
}
-
static int config_buf_fgetc(struct config_source *conf)
{
if (conf->u.buf.pos < conf->u.buf.len)
@@ -3418,7 +3417,6 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
write_err_out:
ret = write_error(get_lock_file_path(&lock));
goto out_free;
-
}
void git_config_set_multivar_in_file(const char *config_filename,
--
2.43.0
^ permalink raw reply related
* [PATCH v3 0/4] maintenance: use XDG config if it exists
From: Kristoffer Haugsbakk @ 2024-01-18 16:12 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, ps, stolee, Eric Sunshine, Taylor Blau,
Junio C Hamano
In-Reply-To: <cover.1697660181.git.code@khaugsbakk.name>
I use the conventional XDG config path for the global configuration. This
path is always used except for `git maintenance register` and
`unregister`.
§ Changes since v2 (by patch)
• config: factor out global config file retrieval
• Remove doc on `git_global_config`
• https://lore.kernel.org/git/c87b3d93-74db-4377-a57c-80f766d46e7f@app.fastmail.com/
§ Patches
• 1–3: Preparatory
• 4: The desired change
§ CC
• Patrick Steinhardt: `config` changes; v1 feedback
• Derrick Stolee: `maintenance` changes
• Eric Sunshine: v1 feedback
• Taylor Blau: v1 feedback
• Junio C Hamano: v2 feedback
§ CI
https://github.com/LemmingAvalanche/git/actions/runs/7521230119
Kristoffer Haugsbakk (4):
config: format newlines
config: rename global config function
config: factor out global config file retrieval
maintenance: use XDG config if it exists
builtin/config.c | 26 +++---------------------
builtin/gc.c | 27 ++++++++++++-------------
builtin/var.c | 2 +-
config.c | 26 ++++++++++++++++++++----
config.h | 3 ++-
t/t7900-maintenance.sh | 45 ++++++++++++++++++++++++++++++++++++++++++
6 files changed, 86 insertions(+), 43 deletions(-)
Range-diff against v2:
1: d5f6c8d62ec = 1: 1c92b772ef4 config: format newlines
2: cbc5fde0094 = 2: 269490794bc config: rename global config function
3: 32e5ec7d866 ! 3: 0643a85892c config: factor out global config file retrieval
@@ Commit message
## Notes (series) ##
+ v3:
+ • Remove doc on `git_global_config`
+ • https://lore.kernel.org/git/c87b3d93-74db-4377-a57c-80f766d46e7f@app.fastmail.com/
v2:
• Don’t die; return `NULL`
@@ config.h: int config_error_nonbool(const char *);
#endif
char *git_system_config(void);
-+/**
-+ * Returns `NULL` if is uncertain whether or not `HOME/.gitconfig` exists.
-+ */
+char *git_global_config(void);
void git_global_config_paths(char **user, char **xdg);
4: 8bd67c5bf01 = 4: e0880af0a31 maintenance: use XDG config if it exists
--
2.43.0
^ permalink raw reply
* Re: [PATCH] merge-ll: expose revision names to custom drivers
From: Antonin Delpeuch @ 2024-01-18 15:42 UTC (permalink / raw)
To: Kristoffer Haugsbakk, Josh Soref; +Cc: git
In-Reply-To: <549875d6-b9a8-4b0d-8eaf-e12b72a20e16@app.fastmail.com>
On 18/01/2024 16:25, Kristoffer Haugsbakk wrote:
> This is gitgitgadget but `git send-email` would probably not pick up the
> CC here since it is not part of the trailer section (because there is a
> blank line between the CC line and the signed-off-by line). Maybe
> gitgitgadget works the same way. Also the space between `CC:` and the
> value is a no-break space but I don’t know if that matters.[1]
Thanks a lot for catching that! Let me fix and re-submit.
Antonin
^ permalink raw reply
* [PATCH v2] merge-ll: expose revision names to custom drivers
From: Antonin Delpeuch via GitGitGadget @ 2024-01-18 15:43 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Kristoffer Haugsbakk, Antonin Delpeuch,
Antonin Delpeuch
In-Reply-To: <pull.1648.git.git.1705587974840.gitgitgadget@gmail.com>
From: Antonin Delpeuch <antonin@delpeuch.eu>
Custom merge drivers need access to the names of the revisions they
are working on, so that the merge conflict markers they introduce
can refer to those revisions. The placeholders '%S', '%X' and '%Y'
are introduced to this end.
Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu>
---
merge-ll: expose revision names to custom drivers
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1648%2Fwetneb%2Fmerge_driver_pathnames-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1648/wetneb/merge_driver_pathnames-v2
Pull-Request: https://github.com/git/git/pull/1648
Range-diff vs v1:
1: e648f9f0696 ! 1: 6dec70529c0 merge-ll: expose revision names to custom drivers
@@ Commit message
can refer to those revisions. The placeholders '%S', '%X' and '%Y'
are introduced to this end.
- CC: Junio C Hamano <gitster@pobox.com>
-
Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu>
## Documentation/gitattributes.txt ##
Documentation/gitattributes.txt | 10 +++++++---
merge-ll.c | 17 ++++++++++++++---
t/t6406-merge-attr.sh | 16 +++++++++++-----
3 files changed, 32 insertions(+), 11 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 201bdf5edbd..108c2e23baa 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -1141,7 +1141,7 @@ command to run to merge ancestor's version (`%O`), current
version (`%A`) and the other branches' version (`%B`). These
three tokens are replaced with the names of temporary files that
hold the contents of these versions when the command line is
-built. Additionally, %L will be replaced with the conflict marker
+built. Additionally, `%L` will be replaced with the conflict marker
size (see below).
The merge driver is expected to leave the result of the merge in
@@ -1159,8 +1159,12 @@ When left unspecified, the driver itself is used for both
internal merge and the final merge.
The merge driver can learn the pathname in which the merged result
-will be stored via placeholder `%P`.
-
+will be stored via placeholder `%P`. Additionally, the names of the
+merge ancestor revision (`%S`), of the current revision (`%X`) and
+of the other branch (`%Y`) can also be supplied. Those are short
+revision names, optionally joined with the paths of the file in each
+revision. Those paths are only present if they differ and are separated
+from the revision by a colon.
`conflict-marker-size`
^^^^^^^^^^^^^^^^^^^^^^
diff --git a/merge-ll.c b/merge-ll.c
index 1df58ebaac0..ae9eb69be14 100644
--- a/merge-ll.c
+++ b/merge-ll.c
@@ -185,9 +185,9 @@ static void create_temp(mmfile_t *src, char *path, size_t len)
static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn,
mmbuffer_t *result,
const char *path,
- mmfile_t *orig, const char *orig_name UNUSED,
- mmfile_t *src1, const char *name1 UNUSED,
- mmfile_t *src2, const char *name2 UNUSED,
+ mmfile_t *orig, const char *orig_name,
+ mmfile_t *src1, const char *name1,
+ mmfile_t *src2, const char *name2,
const struct ll_merge_options *opts,
int marker_size)
{
@@ -222,6 +222,12 @@ static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn,
strbuf_addf(&cmd, "%d", marker_size);
else if (skip_prefix(format, "P", &format))
sq_quote_buf(&cmd, path);
+ else if (skip_prefix(format, "S", &format))
+ sq_quote_buf(&cmd, orig_name);
+ else if (skip_prefix(format, "X", &format))
+ sq_quote_buf(&cmd, name1);
+ else if (skip_prefix(format, "Y", &format))
+ sq_quote_buf(&cmd, name2);
else
strbuf_addch(&cmd, '%');
}
@@ -315,7 +321,12 @@ static int read_merge_config(const char *var, const char *value,
* %B - temporary file name for the other branches' version.
* %L - conflict marker length
* %P - the original path (safely quoted for the shell)
+ * %S - the revision for the merge base
+ * %X - the revision for our version
+ * %Y - the revision for their version
*
+ * If the file is not named indentically in all versions, then each
+ * revision is joined with the corresponding path, separated by a colon.
* The external merge driver should write the results in the
* file named by %A, and signal that it has done with zero exit
* status.
diff --git a/t/t6406-merge-attr.sh b/t/t6406-merge-attr.sh
index 72f8c1722ff..156a1efacfe 100755
--- a/t/t6406-merge-attr.sh
+++ b/t/t6406-merge-attr.sh
@@ -42,11 +42,15 @@ test_expect_success setup '
#!/bin/sh
orig="$1" ours="$2" theirs="$3" exit="$4" path=$5
+ orig_name="$6" our_name="$7" their_name="$8"
(
echo "orig is $orig"
echo "ours is $ours"
echo "theirs is $theirs"
echo "path is $path"
+ echo "orig_name is $orig_name"
+ echo "our_name is $our_name"
+ echo "their_name is $their_name"
echo "=== orig ==="
cat "$orig"
echo "=== ours ==="
@@ -121,7 +125,7 @@ test_expect_success 'custom merge backend' '
git reset --hard anchor &&
git config --replace-all \
- merge.custom.driver "./custom-merge %O %A %B 0 %P" &&
+ merge.custom.driver "./custom-merge %O %A %B 0 %P %S %X %Y" &&
git config --replace-all \
merge.custom.name "custom merge driver for testing" &&
@@ -132,7 +136,8 @@ test_expect_success 'custom merge backend' '
o=$(git unpack-file main^:text) &&
a=$(git unpack-file side^:text) &&
b=$(git unpack-file main:text) &&
- sh -c "./custom-merge $o $a $b 0 text" &&
+ base_revid=$(git rev-parse --short main^) &&
+ sh -c "./custom-merge $o $a $b 0 text $base_revid HEAD main" &&
sed -e 1,3d $a >check-2 &&
cmp check-1 check-2 &&
rm -f $o $a $b
@@ -142,7 +147,7 @@ test_expect_success 'custom merge backend' '
git reset --hard anchor &&
git config --replace-all \
- merge.custom.driver "./custom-merge %O %A %B 1 %P" &&
+ merge.custom.driver "./custom-merge %O %A %B 1 %P %S %X %Y" &&
git config --replace-all \
merge.custom.name "custom merge driver for testing" &&
@@ -159,7 +164,8 @@ test_expect_success 'custom merge backend' '
o=$(git unpack-file main^:text) &&
a=$(git unpack-file anchor:text) &&
b=$(git unpack-file main:text) &&
- sh -c "./custom-merge $o $a $b 0 text" &&
+ base_revid=$(git rev-parse --short main^) &&
+ sh -c "./custom-merge $o $a $b 0 text $base_revid HEAD main" &&
sed -e 1,3d $a >check-2 &&
cmp check-1 check-2 &&
sed -e 1,3d -e 4q $a >check-3 &&
@@ -173,7 +179,7 @@ test_expect_success !WINDOWS 'custom merge driver that is killed with a signal'
git reset --hard anchor &&
git config --replace-all \
- merge.custom.driver "./custom-merge %O %A %B 0 %P" &&
+ merge.custom.driver "./custom-merge %O %A %B 0 %P %S %X %Y" &&
git config --replace-all \
merge.custom.name "custom merge driver for testing" &&
base-commit: 186b115d3062e6230ee296d1ddaa0c4b72a464b5
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH] merge-ll: expose revision names to custom drivers
From: Kristoffer Haugsbakk @ 2024-01-18 15:25 UTC (permalink / raw)
To: Josh Soref; +Cc: Antonin Delpeuch, git
In-Reply-To: <pull.1648.git.git.1705587974840.gitgitgadget@gmail.com>
Hi
> are working on, so that the merge conflict markers they introduce
> can refer to those revisions. The placeholders '%S', '%X' and '%Y'
> are introduced to this end.
>
> CC: Junio C Hamano <gitster@pobox.com>
>
> Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu>
> ---
This is gitgitgadget but `git send-email` would probably not pick up the
CC here since it is not part of the trailer section (because there is a
blank line between the CC line and the signed-off-by line). Maybe
gitgitgadget works the same way. Also the space between `CC:` and the
value is a no-break space but I don’t know if that matters.[1]
(Also see [2] for what it’s worth.)
† 1: On Linux at least this often happens by accident by pressing AltGr+Space.
🔗 2: https://lore.kernel.org/git/xmqqttwzded6.fsf@gitster.g/
--
Kristoffer Haugsbakk
^ permalink raw reply
* [PATCH] merge-ll: expose revision names to custom drivers
From: Antonin Delpeuch via GitGitGadget @ 2024-01-18 14:26 UTC (permalink / raw)
To: git; +Cc: Antonin Delpeuch, Antonin Delpeuch
From: Antonin Delpeuch <antonin@delpeuch.eu>
Custom merge drivers need access to the names of the revisions they
are working on, so that the merge conflict markers they introduce
can refer to those revisions. The placeholders '%S', '%X' and '%Y'
are introduced to this end.
CC: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu>
---
merge-ll: expose revision names to custom drivers
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1648%2Fwetneb%2Fmerge_driver_pathnames-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1648/wetneb/merge_driver_pathnames-v1
Pull-Request: https://github.com/git/git/pull/1648
Documentation/gitattributes.txt | 10 +++++++---
merge-ll.c | 17 ++++++++++++++---
t/t6406-merge-attr.sh | 16 +++++++++++-----
3 files changed, 32 insertions(+), 11 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 201bdf5edbd..108c2e23baa 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -1141,7 +1141,7 @@ command to run to merge ancestor's version (`%O`), current
version (`%A`) and the other branches' version (`%B`). These
three tokens are replaced with the names of temporary files that
hold the contents of these versions when the command line is
-built. Additionally, %L will be replaced with the conflict marker
+built. Additionally, `%L` will be replaced with the conflict marker
size (see below).
The merge driver is expected to leave the result of the merge in
@@ -1159,8 +1159,12 @@ When left unspecified, the driver itself is used for both
internal merge and the final merge.
The merge driver can learn the pathname in which the merged result
-will be stored via placeholder `%P`.
-
+will be stored via placeholder `%P`. Additionally, the names of the
+merge ancestor revision (`%S`), of the current revision (`%X`) and
+of the other branch (`%Y`) can also be supplied. Those are short
+revision names, optionally joined with the paths of the file in each
+revision. Those paths are only present if they differ and are separated
+from the revision by a colon.
`conflict-marker-size`
^^^^^^^^^^^^^^^^^^^^^^
diff --git a/merge-ll.c b/merge-ll.c
index 1df58ebaac0..ae9eb69be14 100644
--- a/merge-ll.c
+++ b/merge-ll.c
@@ -185,9 +185,9 @@ static void create_temp(mmfile_t *src, char *path, size_t len)
static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn,
mmbuffer_t *result,
const char *path,
- mmfile_t *orig, const char *orig_name UNUSED,
- mmfile_t *src1, const char *name1 UNUSED,
- mmfile_t *src2, const char *name2 UNUSED,
+ mmfile_t *orig, const char *orig_name,
+ mmfile_t *src1, const char *name1,
+ mmfile_t *src2, const char *name2,
const struct ll_merge_options *opts,
int marker_size)
{
@@ -222,6 +222,12 @@ static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn,
strbuf_addf(&cmd, "%d", marker_size);
else if (skip_prefix(format, "P", &format))
sq_quote_buf(&cmd, path);
+ else if (skip_prefix(format, "S", &format))
+ sq_quote_buf(&cmd, orig_name);
+ else if (skip_prefix(format, "X", &format))
+ sq_quote_buf(&cmd, name1);
+ else if (skip_prefix(format, "Y", &format))
+ sq_quote_buf(&cmd, name2);
else
strbuf_addch(&cmd, '%');
}
@@ -315,7 +321,12 @@ static int read_merge_config(const char *var, const char *value,
* %B - temporary file name for the other branches' version.
* %L - conflict marker length
* %P - the original path (safely quoted for the shell)
+ * %S - the revision for the merge base
+ * %X - the revision for our version
+ * %Y - the revision for their version
*
+ * If the file is not named indentically in all versions, then each
+ * revision is joined with the corresponding path, separated by a colon.
* The external merge driver should write the results in the
* file named by %A, and signal that it has done with zero exit
* status.
diff --git a/t/t6406-merge-attr.sh b/t/t6406-merge-attr.sh
index 72f8c1722ff..156a1efacfe 100755
--- a/t/t6406-merge-attr.sh
+++ b/t/t6406-merge-attr.sh
@@ -42,11 +42,15 @@ test_expect_success setup '
#!/bin/sh
orig="$1" ours="$2" theirs="$3" exit="$4" path=$5
+ orig_name="$6" our_name="$7" their_name="$8"
(
echo "orig is $orig"
echo "ours is $ours"
echo "theirs is $theirs"
echo "path is $path"
+ echo "orig_name is $orig_name"
+ echo "our_name is $our_name"
+ echo "their_name is $their_name"
echo "=== orig ==="
cat "$orig"
echo "=== ours ==="
@@ -121,7 +125,7 @@ test_expect_success 'custom merge backend' '
git reset --hard anchor &&
git config --replace-all \
- merge.custom.driver "./custom-merge %O %A %B 0 %P" &&
+ merge.custom.driver "./custom-merge %O %A %B 0 %P %S %X %Y" &&
git config --replace-all \
merge.custom.name "custom merge driver for testing" &&
@@ -132,7 +136,8 @@ test_expect_success 'custom merge backend' '
o=$(git unpack-file main^:text) &&
a=$(git unpack-file side^:text) &&
b=$(git unpack-file main:text) &&
- sh -c "./custom-merge $o $a $b 0 text" &&
+ base_revid=$(git rev-parse --short main^) &&
+ sh -c "./custom-merge $o $a $b 0 text $base_revid HEAD main" &&
sed -e 1,3d $a >check-2 &&
cmp check-1 check-2 &&
rm -f $o $a $b
@@ -142,7 +147,7 @@ test_expect_success 'custom merge backend' '
git reset --hard anchor &&
git config --replace-all \
- merge.custom.driver "./custom-merge %O %A %B 1 %P" &&
+ merge.custom.driver "./custom-merge %O %A %B 1 %P %S %X %Y" &&
git config --replace-all \
merge.custom.name "custom merge driver for testing" &&
@@ -159,7 +164,8 @@ test_expect_success 'custom merge backend' '
o=$(git unpack-file main^:text) &&
a=$(git unpack-file anchor:text) &&
b=$(git unpack-file main:text) &&
- sh -c "./custom-merge $o $a $b 0 text" &&
+ base_revid=$(git rev-parse --short main^) &&
+ sh -c "./custom-merge $o $a $b 0 text $base_revid HEAD main" &&
sed -e 1,3d $a >check-2 &&
cmp check-1 check-2 &&
sed -e 1,3d -e 4q $a >check-3 &&
@@ -173,7 +179,7 @@ test_expect_success !WINDOWS 'custom merge driver that is killed with a signal'
git reset --hard anchor &&
git config --replace-all \
- merge.custom.driver "./custom-merge %O %A %B 0 %P" &&
+ merge.custom.driver "./custom-merge %O %A %B 0 %P %S %X %Y" &&
git config --replace-all \
merge.custom.name "custom merge driver for testing" &&
base-commit: 186b115d3062e6230ee296d1ddaa0c4b72a464b5
--
gitgitgadget
^ permalink raw reply related
* [PATCH 2/2] reftable/stack: fix race in up-to-date check
From: Patrick Steinhardt @ 2024-01-18 13:41 UTC (permalink / raw)
To: git
In-Reply-To: <cover.1705585037.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 9609 bytes --]
In 6fdfaf15a0 (reftable/stack: use stat info to avoid re-reading stack
list, 2024-01-11) we have introduced a new mechanism to avoid re-reading
the table list in case stat(3P) figures out that the stack didn't change
since the last time we read it.
While this change significantly improved performance when writing many
refs, it can unfortunately lead to false negatives in very specific
scenarios. Given two processes A and B, there is a feasible sequence of
events that cause us to accidentally treat the table list as up-to-date
even though it changed:
1. A reads the reftable stack and caches its stat info.
2. B updates the stack, appending a new table to "tables.list". This
will both use a new inode and result in a different file size, thus
invalidating A's cache in theory.
3. B decides to auto-compact the stack and merges two tables. The file
size now matches what A has cached again. Furthermore, the
filesystem may decide to recycle the inode number of the file we
have replaced in (2) because it is not in use anymore.
4. A reloads the reftable stack. Neither the inode number nor the
file size changed. If the timestamps did not change either then we
think the cached copy of our stack is up-to-date.
In fact, the commit introduced three related issues:
- Non-POSIX compliant systems may not report proper `st_dev` and
`st_ino` values in stat(3P), which made us rely solely on the
file's potentially coarse-grained mtime and ctime.
- `stat_validity_check()` and friends may end up not comparing
`st_dev` and `st_ino` depending on the "core.checkstat" config,
again reducing the signal to the mtime and ctime.
- `st_ino` can be recycled, rendering the check moot even on
POSIX-compliant systems.
Given that POSIX defines that "The st_ino and st_dev fields taken
together uniquely identify the file within the system", these issues led
to the most important signal to establish file identity to be ignored or
become useless in some cases.
Refactor the code to stop using `stat_validity_check()`. Instead, we
manually stat(3P) the file descriptors to make relevant information
available. On Windows and MSYS2 the result will have both `st_dev` and
`st_ino` set to 0, which allows us to address the first issue by not
using the stat-based cache in that case. It also allows us to make sure
that we always compare `st_dev` and `st_ino`, addressing the second
issue.
The third issue of inode recycling can be addressed by keeping the file
descriptor of "files.list" open during the lifetime of the reftable
stack. As the file will still exist on disk even though it has been
unlinked it is impossible for its inode to be recycled as long as the
file descriptor is still open.
This should address the race in a POSIX-compliant way. The only real
downside is that this mechanism cannot be used on non-POSIX-compliant
systems like Windows. But we at least have the second-level caching
mechanism in place that compares contents of "files.list" with the
currently loaded list of tables.
This new mechanism performs roughly the same as the previous one that
relied on `stat_validity_check()`:
Benchmark 1: update-ref: create many refs (HEAD~)
Time (mean ± σ): 4.754 s ± 0.026 s [User: 2.204 s, System: 2.549 s]
Range (min … max): 4.694 s … 4.802 s 20 runs
Benchmark 2: update-ref: create many refs (HEAD)
Time (mean ± σ): 4.721 s ± 0.020 s [User: 2.194 s, System: 2.527 s]
Range (min … max): 4.691 s … 4.753 s 20 runs
Summary
update-ref: create many refs (HEAD~) ran
1.01 ± 0.01 times faster than update-ref: create many refs (HEAD)
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reftable/stack.c | 99 +++++++++++++++++++++++++++++++++++++++++++----
reftable/stack.h | 4 +-
reftable/system.h | 1 -
3 files changed, 95 insertions(+), 9 deletions(-)
diff --git a/reftable/stack.c b/reftable/stack.c
index 705cfb6caa..77a387a86c 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -66,6 +66,7 @@ int reftable_new_stack(struct reftable_stack **dest, const char *dir,
strbuf_addstr(&list_file_name, "/tables.list");
p->list_file = strbuf_detach(&list_file_name, NULL);
+ p->list_fd = -1;
p->reftable_dir = xstrdup(dir);
p->config = config;
@@ -175,7 +176,12 @@ void reftable_stack_destroy(struct reftable_stack *st)
st->readers_len = 0;
FREE_AND_NULL(st->readers);
}
- stat_validity_clear(&st->list_validity);
+
+ if (st->list_fd >= 0) {
+ close(st->list_fd);
+ st->list_fd = -1;
+ }
+
FREE_AND_NULL(st->list_file);
FREE_AND_NULL(st->reftable_dir);
reftable_free(st);
@@ -375,11 +381,59 @@ static int reftable_stack_reload_maybe_reuse(struct reftable_stack *st,
sleep_millisec(delay);
}
- stat_validity_update(&st->list_validity, fd);
-
out:
- if (err)
- stat_validity_clear(&st->list_validity);
+ /*
+ * Invalidate the stat cache. It is sufficient to only close the file
+ * descriptor and keep the cached stat info because we never use the
+ * latter when the former is negative.
+ */
+ if (st->list_fd >= 0) {
+ close(st->list_fd);
+ st->list_fd = -1;
+ }
+
+ /*
+ * Cache stat information in case it provides a useful signal to us.
+ * According to POSIX, "The st_ino and st_dev fields taken together
+ * uniquely identify the file within the system." That being said,
+ * Windows is not POSIX compliant and we do not have these fields
+ * available. So the information we have there is insufficient to
+ * determine whether two file descriptors point to the same file.
+ *
+ * While we could fall back to using other signals like the file's
+ * mtime, those are not sufficient to avoid races. We thus refrain from
+ * using the stat cache on such systems and fall back to the secondary
+ * caching mechanism, which is to check whether contents of the file
+ * have changed.
+ *
+ * On other systems which are POSIX compliant we must keep the file
+ * descriptor open. This is to avoid a race condition where two
+ * processes access the reftable stack at the same point in time:
+ *
+ * 1. A reads the reftable stack and caches its stat info.
+ *
+ * 2. B updates the stack, appending a new table to "tables.list".
+ * This will both use a new inode and result in a different file
+ * size, thus invalidating A's cache in theory.
+ *
+ * 3. B decides to auto-compact the stack and merges two tables. The
+ * file size now matches what A has cached again. Furthermore, the
+ * filesystem may decide to recycle the inode number of the file
+ * we have replaced in (2) because it is not in use anymore.
+ *
+ * 4. A reloads the reftable stack. Neither the inode number nor the
+ * file size changed. If the timestamps did not change either then
+ * we think the cached copy of our stack is up-to-date.
+ *
+ * By keeping the file descriptor open the inode number cannot be
+ * recycled, mitigating the race.
+ */
+ if (!err && fd >= 0 && !fstat(fd, &st->list_st) &&
+ st->list_st.st_dev && st->list_st.st_ino) {
+ st->list_fd = fd;
+ fd = -1;
+ }
+
if (fd >= 0)
close(fd);
free_names(names);
@@ -396,8 +450,39 @@ static int stack_uptodate(struct reftable_stack *st)
int err;
int i = 0;
- if (stat_validity_check(&st->list_validity, st->list_file))
- return 0;
+ /*
+ * When we have cached stat information available then we use it to
+ * verify whether the file has been rewritten.
+ *
+ * Note that we explicitly do not want to use `stat_validity_check()`
+ * and friends here because they may end up not comparing the `st_dev`
+ * and `st_ino` fields. These functions thus cannot guarantee that we
+ * indeed still have the same file.
+ */
+ if (st->list_fd >= 0) {
+ struct stat list_st;
+
+ if (stat(st->list_file, &list_st) < 0) {
+ /*
+ * It's fine for "tables.list" to not exist. In that
+ * case, we have to refresh when the loaded stack has
+ * any readers.
+ */
+ if (errno == ENOENT)
+ return !!st->readers_len;
+ return REFTABLE_IO_ERROR;
+ }
+
+ /*
+ * When "tables.list" refers to the same file we can assume
+ * that it didn't change. This is because we always use
+ * rename(3P) to update the file and never write to it
+ * directly.
+ */
+ if (st->list_st.st_dev == list_st.st_dev &&
+ st->list_st.st_ino == list_st.st_ino)
+ return 0;
+ }
err = read_lines(st->list_file, &names);
if (err < 0)
diff --git a/reftable/stack.h b/reftable/stack.h
index 3f80cc598a..c1e3efa899 100644
--- a/reftable/stack.h
+++ b/reftable/stack.h
@@ -14,8 +14,10 @@ license that can be found in the LICENSE file or at
#include "reftable-stack.h"
struct reftable_stack {
- struct stat_validity list_validity;
+ struct stat list_st;
char *list_file;
+ int list_fd;
+
char *reftable_dir;
int disable_auto_compact;
diff --git a/reftable/system.h b/reftable/system.h
index 2cc7adf271..6b74a81514 100644
--- a/reftable/system.h
+++ b/reftable/system.h
@@ -12,7 +12,6 @@ license that can be found in the LICENSE file or at
/* This header glues the reftable library to the rest of Git */
#include "git-compat-util.h"
-#include "statinfo.h"
#include "strbuf.h"
#include "hash-ll.h" /* hash ID, sizes.*/
#include "dir.h" /* remove_dir_recursively, for tests.*/
--
2.43.GIT
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 1/2] reftable/stack: unconditionally reload stack after commit
From: Patrick Steinhardt @ 2024-01-18 13:41 UTC (permalink / raw)
To: git
In-Reply-To: <cover.1705585037.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1137 bytes --]
After we have committed an addition to the reftable stack we call
`reftable_stack_reload()` to reload the stack and thus reflect the
changes that were just added. This function will only conditionally
reload the stack in case `stack_uptodate()` tells us that the stack
needs reloading. This check is wasteful though because we already know
that the stack needs reloading.
Call `reftable_stack_reload_maybe_reuse()` instead, which will
unconditionally reload the stack. This is merely a conceptual fix, the
code in question was not found to cause any problems in practice.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reftable/stack.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/reftable/stack.c b/reftable/stack.c
index c28d82299d..705cfb6caa 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -585,7 +585,7 @@ int reftable_addition_commit(struct reftable_addition *add)
add->new_tables = NULL;
add->new_tables_len = 0;
- err = reftable_stack_reload(add->stack);
+ err = reftable_stack_reload_maybe_reuse(add->stack, 1);
if (err)
goto done;
--
2.43.GIT
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 0/2] reftable/stack: fix race in up-to-date check
From: Patrick Steinhardt @ 2024-01-18 13:41 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 1225 bytes --]
Hi,
In 6fdfaf15 (reftable/stack: use stat info to avoid re-reading stack
list, 2024-01-11) we have introduced a new mechanism to avoid re-reading
the table list in case stat(3P) figures out that the stack didn't change
since the last time we read it.
Unfortunately, I've been able to hit a bug there that can happen under
very specific circumstances when a reading and writing process race with
each other. When the writer appends to the stack and then compacts it,
it may happen that we recycle the previously-used inode of "tables.list"
so that the resulting `struct stat` exactly matches the cached value of
the reading process.
The first patch is merely cosmetic, whereas the second patch fixes the
described race. The topic depends on ps/reftable-optimize-io, which has
introduced the issue.
Patrick
Patrick Steinhardt (2):
reftable/stack: unconditionally reload stack after commit
reftable/stack: fix race in up-to-date check
reftable/stack.c | 101 ++++++++++++++++++++++++++++++++++++++++++----
reftable/stack.h | 4 +-
reftable/system.h | 1 -
3 files changed, 96 insertions(+), 10 deletions(-)
base-commit: 718a93ecc06ed59dda4e6a5d91b1c2169275694f
--
2.43.GIT
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: How to execute a command on git am/rebase/cherry pick --abort ?
From: Konstantin Kharlamov @ 2024-01-18 13:30 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <Zaknu0nwBucHVJPP@tanuki>
On Thu, 2024-01-18 at 14:29 +0100, Patrick Steinhardt wrote:
> On Thu, Jan 18, 2024 at 03:53:21PM +0300, Konstantin Kharlamov wrote:
> > (please keep me CC'ed, I'm not subscribed)
> >
> > Hello!
> >
> > There's a well-known problem of git not fully checking out changes
> > while doing e.g. `git checkout` and similar commands when you have
> > submodules. So e.g. if HEAD changes a submodule commit and you do
> > an
> > interactive rebase to HEAD~2, you may be lucky to find a submodule
> > commit change in `git diff` (because if you don't get lucky, you
> > won't
> > notice that and commit the change to the unrelated HEAD~2).
> >
> > As a workaround I have a `git submodule update` inside `post-
> > checkout`
> > hook.
> >
> > Now, the problem is I still often finding myself having the wrong
> > submodule ID, and I tracked down that problem to commands such as
> > `am/rebase/cherry-pick --abort` also not updating the submodule,
> > nor
> > executing `post-checkout`.
> >
> > I looked through `man githooks` but couldn't find any way to
> > execute a
> > `git submodule update` during these aborts.
> >
> > Any ideas how to fix these?
>
> Are you aware of the `submodule.recurse` config? If set, it should
> cause
> git-checkout(1) and many other commands to recurse into submodules
> and
> update them accordingly. This should both make your post-checkout
> hook
> obsolete and should also work with git-cherry-pick(1) et al.
>
> Patrick
Oh, this is amazing, thank you! No, I didn't know about it, will use!
^ permalink raw reply
* Re: How to execute a command on git am/rebase/cherry pick --abort ?
From: Patrick Steinhardt @ 2024-01-18 13:29 UTC (permalink / raw)
To: Konstantin Kharlamov; +Cc: git
In-Reply-To: <d66ef46827fc7391bd74ece943afa2c5245896d6.camel@yandex.ru>
[-- Attachment #1: Type: text/plain, Size: 1338 bytes --]
On Thu, Jan 18, 2024 at 03:53:21PM +0300, Konstantin Kharlamov wrote:
> (please keep me CC'ed, I'm not subscribed)
>
> Hello!
>
> There's a well-known problem of git not fully checking out changes
> while doing e.g. `git checkout` and similar commands when you have
> submodules. So e.g. if HEAD changes a submodule commit and you do an
> interactive rebase to HEAD~2, you may be lucky to find a submodule
> commit change in `git diff` (because if you don't get lucky, you won't
> notice that and commit the change to the unrelated HEAD~2).
>
> As a workaround I have a `git submodule update` inside `post-checkout`
> hook.
>
> Now, the problem is I still often finding myself having the wrong
> submodule ID, and I tracked down that problem to commands such as
> `am/rebase/cherry-pick --abort` also not updating the submodule, nor
> executing `post-checkout`.
>
> I looked through `man githooks` but couldn't find any way to execute a
> `git submodule update` during these aborts.
>
> Any ideas how to fix these?
Are you aware of the `submodule.recurse` config? If set, it should cause
git-checkout(1) and many other commands to recurse into submodules and
update them accordingly. This should both make your post-checkout hook
obsolete and should also work with git-cherry-pick(1) et al.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox