* Re: [PATCH] git: extend --no-lazy-fetch to work across subprocesses
From: Linus Arver @ 2024-02-16 22:30 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Jeff King, Christian Couder
In-Reply-To: <xmqqr0hcglpk.fsf_-_@gitster.g>
FWIW, I see some typos. Otherwise this patch along with your "git:
document GIT_NO_REPLACE_OBJECTS environment variable" one both LGTM for
wording/readability. I must defer to Peff and others for "is this patch
actually doing the right thing?" ;).
Junio C Hamano <gitster@pobox.com> writes:
> Modeling after how the `--no-replace-objects` option is made usable
> across subprocess spawning (e.g., cURL based remote helpers are
> spawned as a separate process while running "git fetch"), allow the
> `--no-lazy-fetch` option to be passed across process boundary.
s/boundary/boundaries
> Do not model how the value of GIT_NO_REPLACE_OBJECTS environment
> variable is ignored, though. Just use the usual git_env_bool() to
> allow "export GIT_NO_LAZY_FETCH=0" and "unset GIT_NO_LAZY_FETCH"
> to be equivalents.
s/equivalents/equivalent
^ permalink raw reply
* Re: [PATCH v2] diff: implement config.diff.renames=copies-harder
From: Sam James @ 2024-02-16 22:16 UTC (permalink / raw)
To: Elijah Newren; +Cc: Sam James, git, Junio C Hamano
In-Reply-To: <CABPp-BFZqa2wyNUs0OfUOokGORjAguCK7-1hqK6U+SUxm8E5Lw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 10957 bytes --]
Elijah Newren <newren@gmail.com> writes:
> Hi,
>
> It helps when providing a new iteration of a patch to do two things:
> (1) provide a range-diff against the previous version to make it
> easier for reviewers to see what has changed
> (2) either reply to the previous submission or link to it in your
> header so reviewers can find previous comments
Thanks, I'll do that for upcoming v3.
>
> In this case, v1 was over at
> https://lore.kernel.org/git/pull.1606.git.1699010701704.gitgitgadget@gmail.com/.
>
> On Tue, Dec 26, 2023 at 12:21 PM Sam James <sam@gentoo.org> wrote:
>>
>> This patch adds a config value for 'diff.renames' called 'copies-harder'
>> which make it so '-C -C' is in effect always passed for 'git log -p',
>> 'git diff', etc.
>>
>> This allows specifying that 'git log -p', 'git diff', etc should always act
>> as if '-C --find-copies-harder' was passed.
>>
>> It has proven this especially useful for certain types of repository (like
>> Gentoo's ebuild repositories) because files are often copies of a previous
>> version:
>>
>> Suppose a directory 'sys-devel/gcc' contains recipes for building
>> GCC, with one file for each supported upstream branch:
>> gcc-13.x.build.recipe
>> gcc-12.x.build.recipe
>> gcc-11.x.build.recipe
>> gcc-10.x.build.recipe
>>
>> gcc-13.x.build.recipe was started as a copy of gcc-12.x.build.recipe
>> (which was started as a copy of gcc-11.x.build.recipe, etc.). Previous versions
>> are kept around to support parallel installation of multiple versions.
>>
>> Being able to easily observe the diff relative to other recipes within the
>> directory has been a quality of life improvement for such repo layouts.
>>
>> Cc: Elijah Newren <newren@gmail.com>
>> Cc: Junio C Hamano <gitster@pobox.com>
>> Signed-off-by: Sam James <sam@gentoo.org>
>
> While the "Cc:" lines don't hurt, note that everything above this
> point is included in the commit message, so we'd typically rather
> those two lines be below the triple-dashed line.
ACK.
>
>> ---
>> v2: Improve the commit message using Elijah Newren's example (it is indeed
>> precisely what I was thinking of, but phrased better), and improve the documentation
>> to explain better what the new config option actually does & refer to git-diff's
>> --find-copies-harder.
>>
>> Documentation/config/diff.txt | 8 +++++---
>> Documentation/config/status.txt | 3 ++-
>> diff.c | 12 +++++++++---
>> diff.h | 1 +
>> diffcore-rename.c | 4 ++--
>> merge-ort.c | 2 +-
>> merge-recursive.c | 2 +-
>> 7 files changed, 21 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
>> index bd5ae0c337..cdd8a74ec0 100644
>> --- a/Documentation/config/diff.txt
>> +++ b/Documentation/config/diff.txt
>> @@ -131,9 +131,11 @@ diff.renames::
>> Whether and how Git detects renames. If set to "false",
>> rename detection is disabled. If set to "true", basic rename
>> detection is enabled. If set to "copies" or "copy", Git will
>> - detect copies, as well. Defaults to true. Note that this
>> - affects only 'git diff' Porcelain like linkgit:git-diff[1] and
>> - linkgit:git-log[1], and not lower level commands such as
>> + detect copies, as well. If set to "copies-harder", Git will spend extra
>> + cycles to find more copies even in unmodified paths, see
>> + '--find-copies-harder' in linkgit:git-diff[1]. Defaults to true.
>> + Note that this affects only 'git diff' Porcelain like linkgit:git-diff[1]
>> + and linkgit:git-log[1], and not lower level commands such as
>> linkgit:git-diff-files[1].
>>
>> diff.suppressBlankEmpty::
>> diff --git a/Documentation/config/status.txt b/Documentation/config/status.txt
>> index 2ff8237f8f..7ca7a4becd 100644
>> --- a/Documentation/config/status.txt
>> +++ b/Documentation/config/status.txt
>> @@ -33,7 +33,8 @@ status.renames::
>> Whether and how Git detects renames in linkgit:git-status[1] and
>> linkgit:git-commit[1] . If set to "false", rename detection is
>> disabled. If set to "true", basic rename detection is enabled.
>> - If set to "copies" or "copy", Git will detect copies, as well.
>> + If set to "copies" or "copy", Git will detect copies, as well. If
>> + set to "copies-harder", Git will try harder to detect copies.
>
> It'd be nice to have the improved text from diff.renames here ("If set
> to "copies-harder", Git will spend extra cycles to find more copies
> even in unmodified paths.").
>
>> Defaults to the value of diff.renames.
>>
>> status.showStash::
>> diff --git a/diff.c b/diff.c
>> index a2def45644..585acf9a99 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -206,8 +206,11 @@ int git_config_rename(const char *var, const char *value)
>> {
>> if (!value)
>> return DIFF_DETECT_RENAME;
>> + if (!strcasecmp(value, "copies-harder"))
>> + return DIFF_DETECT_COPY_HARDER;
>> if (!strcasecmp(value, "copies") || !strcasecmp(value, "copy"))
>> - return DIFF_DETECT_COPY;
>> + return DIFF_DETECT_COPY;
>> +
>
> I pointed out in response to v1 that these last couple lines, while a
> nice cleanup, should be in a separate patch.
>
>> return git_config_bool(var,value) ? DIFF_DETECT_RENAME : 0;
>> }
>>
>> @@ -4849,8 +4852,11 @@ void diff_setup_done(struct diff_options *options)
>> else
>> options->flags.diff_from_contents = 0;
>>
>> - if (options->flags.find_copies_harder)
>> + /* Just fold this in as it makes the patch-to-git smaller */
>> + if (options->flags.find_copies_harder || options->detect_rename == DIFF_DETECT_COPY_HARDER) {
>
> Again, I already responded to v1 that four of your lines were too long
> and needed to be split. All four remain unsplit in your resubmission.
> For future reference, when you resubmit a patch, please either (a)
> first respond to the review explaining why you don't agree with the
> suggested changes, or (b) include the fixes reviewers point out, or
> (c) state in your cover letter or explanation after the '---' why you
> chose to not include the suggested changes.
I apologise for the rookie errors, I don't really have an excuse either.
>
>> + options->flags.find_copies_harder = 1;
>> options->detect_rename = DIFF_DETECT_COPY;
>> + }
>>
>> if (!options->flags.relative_name)
>> options->prefix = NULL;
>> @@ -5281,7 +5287,7 @@ static int diff_opt_find_copies(const struct option *opt,
>> if (*arg != 0)
>> return error(_("invalid argument to %s"), opt->long_name);
>>
>> - if (options->detect_rename == DIFF_DETECT_COPY)
>> + if (options->detect_rename == DIFF_DETECT_COPY || options->detect_rename == DIFF_DETECT_COPY_HARDER)
>> options->flags.find_copies_harder = 1;
>> else
>> options->detect_rename = DIFF_DETECT_COPY;
>> diff --git a/diff.h b/diff.h
>> index 66bd8aeb29..b29e5b777f 100644
>> --- a/diff.h
>> +++ b/diff.h
>> @@ -555,6 +555,7 @@ int git_config_rename(const char *var, const char *value);
>>
>> #define DIFF_DETECT_RENAME 1
>> #define DIFF_DETECT_COPY 2
>> +#define DIFF_DETECT_COPY_HARDER 3
>>
>> #define DIFF_PICKAXE_ALL 1
>> #define DIFF_PICKAXE_REGEX 2
>> diff --git a/diffcore-rename.c b/diffcore-rename.c
>> index 5a6e2bcac7..856291d66f 100644
>> --- a/diffcore-rename.c
>> +++ b/diffcore-rename.c
>> @@ -299,7 +299,7 @@ static int find_identical_files(struct hashmap *srcs,
>> }
>> /* Give higher scores to sources that haven't been used already */
>> score = !source->rename_used;
>> - if (source->rename_used && options->detect_rename != DIFF_DETECT_COPY)
>> + if (source->rename_used && options->detect_rename != DIFF_DETECT_COPY && options->detect_rename != DIFF_DETECT_COPY_HARDER)
>> continue;
>> score += basename_same(source, target);
>> if (score > best_score) {
>> @@ -1405,7 +1405,7 @@ void diffcore_rename_extended(struct diff_options *options,
>> trace2_region_enter("diff", "setup", options->repo);
>> info.setup = 0;
>> assert(!dir_rename_count || strmap_empty(dir_rename_count));
>> - want_copies = (detect_rename == DIFF_DETECT_COPY);
>> + want_copies = (detect_rename == DIFF_DETECT_COPY || detect_rename == DIFF_DETECT_COPY_HARDER);
>> if (dirs_removed && (break_idx || want_copies))
>> BUG("dirs_removed incompatible with break/copy detection");
>> if (break_idx && relevant_sources)
>> diff --git a/merge-ort.c b/merge-ort.c
>> index 6491070d96..7749835465 100644
>> --- a/merge-ort.c
>> +++ b/merge-ort.c
>> @@ -4782,7 +4782,7 @@ static void merge_start(struct merge_options *opt, struct merge_result *result)
>> * sanity check them anyway.
>> */
>> assert(opt->detect_renames >= -1 &&
>> - opt->detect_renames <= DIFF_DETECT_COPY);
>> + opt->detect_renames <= DIFF_DETECT_COPY_HARDER);
>> assert(opt->verbosity >= 0 && opt->verbosity <= 5);
>> assert(opt->buffer_output <= 2);
>> assert(opt->obuf.len == 0);
>> diff --git a/merge-recursive.c b/merge-recursive.c
>> index e3beb0801b..d52dd53660 100644
>> --- a/merge-recursive.c
>> +++ b/merge-recursive.c
>> @@ -3708,7 +3708,7 @@ static int merge_start(struct merge_options *opt, struct tree *head)
>> assert(opt->branch1 && opt->branch2);
>>
>> assert(opt->detect_renames >= -1 &&
>> - opt->detect_renames <= DIFF_DETECT_COPY);
>> + opt->detect_renames <= DIFF_DETECT_COPY_HARDER);
>> assert(opt->detect_directory_renames >= MERGE_DIRECTORY_RENAMES_NONE &&
>> opt->detect_directory_renames <= MERGE_DIRECTORY_RENAMES_TRUE);
>> assert(opt->rename_limit >= -1);
>> --
>> 2.43.0
>
> The patch looks close, but it does continue to violate style
> guidelines and make unrelated changes, like v1. And the wording in
> one of the documentation blurbs could be improved a little. Looking
> forward to a v3 with those fixed.
I've not finished checking yet, but I think
5825268db1058516d05be03d6a8d8d55eea5a943 fixed a bug which I'd been
relying on for something, where I may need to add another option for git
log to suppress copies (for scripts).
Should I send that in a series when doing v3 if it ends up being
required?
thanks,
sam
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 377 bytes --]
^ permalink raw reply
* Re: [PATCH v2] branch: rework the descriptions of rename and copy operations
From: Dragan Simic @ 2024-02-16 21:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, code, rjusto, spectral
In-Reply-To: <xmqqh6i8gk20.fsf@gitster.g>
On 2024-02-16 22:45, Junio C Hamano wrote:
> Dragan Simic <dsimic@manjaro.org> writes:
>
>>> But the halfway modification to the description section in this
>>> patch is not an improvement. It makes some options described there
>>> while -m and -c are completely missing now, making the section
>>> incomplete and coverage of the operating modes of the command
>>> uneven.
>>
>> If I got it right, you'd prefer this patch not to be accepted
>> separately, but as part of the future series that would rework the
>> entire git-branch(1) man page? I'm fine with that as well.
>
> Not necessarily. If you wanted to this this in multiple steps, we
> can first whip the OPTIONS part into a good shape, and then fix the
> DESCRIPTION part.
I'll think a bit more about it, to see what might be our best choice
moving forward.
> What we want to avoid (not limited to this topic) is to say "this
> temporarily makes things worse here, but trust me it will eventually
> become perfect". Removing only -m/-c from the description section
> makes the description section worse than before the patch---we'd be
> better off leaving the original as-is if we are not revamping the
> entire section.
The way you wrote this brought a smile to my face. :) I agree, making
things a bit worse while promising perfection later is rarely justified.
Perhaps only when some nasty bug has to be fixed ASAP.
^ permalink raw reply
* Re: [PATCH v3 0/5] rev-list: allow missing tips with --missing
From: Linus Arver @ 2024-02-16 21:56 UTC (permalink / raw)
To: Christian Couder, git
Cc: Junio C Hamano, Patrick Steinhardt, John Cai, Eric Sunshine,
Christian Couder
In-Reply-To: <20240214142513.4002639-1-christian.couder@gmail.com>
This v3 LGTM.
Reviewed-by: Linus Arver <linusa@google.com>
Thanks!
^ permalink raw reply
* Re: [PATCH v2] branch: rework the descriptions of rename and copy operations
From: Junio C Hamano @ 2024-02-16 21:45 UTC (permalink / raw)
To: Dragan Simic; +Cc: git, code, rjusto, spectral
In-Reply-To: <608b4e81d71a95c820f1e4068382d391@manjaro.org>
Dragan Simic <dsimic@manjaro.org> writes:
>> But the halfway modification to the description section in this
>> patch is not an improvement. It makes some options described there
>> while -m and -c are completely missing now, making the section
>> incomplete and coverage of the operating modes of the command
>> uneven.
>
> If I got it right, you'd prefer this patch not to be accepted
> separately, but as part of the future series that would rework the
> entire git-branch(1) man page? I'm fine with that as well.
Not necessarily. If you wanted to this this in multiple steps, we
can first whip the OPTIONS part into a good shape, and then fix the
DESCRIPTION part.
What we want to avoid (not limited to this topic) is to say "this
temporarily makes things worse here, but trust me it will eventually
become perfect". Removing only -m/-c from the description section
makes the description section worse than before the patch---we'd be
better off leaving the original as-is if we are not revamping the
entire section.
^ permalink raw reply
* Re: [PATCH v2] branch: rework the descriptions of rename and copy operations
From: Dragan Simic @ 2024-02-16 21:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, code, rjusto, spectral
In-Reply-To: <xmqq1q9ci3jt.fsf@gitster.g>
Hello Junio,
On 2024-02-16 20:59, Junio C Hamano wrote:
> Dragan Simic <dsimic@manjaro.org> writes:
>
>> Move the descriptions of the <oldbranch> and <newbranch> arguments to
>> the
>> descriptions of the branch rename and copy operations, where they
>> naturally
>> belong. Also, improve the descriptions of these two branch operations
>> and,
>> for completeness, describe the outcomes of forced operations.
>>
>> Describing the arguments together with their respective operations,
>> instead
>> of describing them separately in a rather unfortunate attempt to
>> squeeze more
>> meaning out of fewer words, flows much better and makes the
>> git-branch(1)
>> man page significantly more usable.
>
> The intention to remove non-option from the OPTIONS enumeration,
> and to explain <new> and <old> used as arguments to -m and -c where
> these options are described, are both very good (heh, after all,
> they are parts of what I envisioned to be the way to go in the
> longer term ;-).
Yes, that's what I plan to work on after this patch is, hopefully,
accepted (see also below). My initial hope was that we'd define
the general outline for the completely reworked git-branch(1) even
further with this patch, which should in turn make the future work
more efficient. I think we're on a good way. :)
>> overridden by using the `--track` and `--no-track` options, and
>> changed later using `git branch --set-upstream-to`.
>>
>> -With a `-m` or `-M` option, <oldbranch> will be renamed to
>> <newbranch>.
>> -If <oldbranch> had a corresponding reflog, it is renamed to match
>> -<newbranch>, and a reflog entry is created to remember the branch
>> -renaming. If <newbranch> exists, -M must be used to force the rename
>> -to happen.
>> -
>> -The `-c` and `-C` options have the exact same semantics as `-m` and
>> -`-M`, except instead of the branch being renamed, it will be copied
>> to a
>> -new name, along with its config and reflog.
>> -
>> With a `-d` or `-D` option, `<branchname>` will be deleted. You may
>> specify more than one branch for deletion. If the branch currently
>> has a reflog then the reflog will also be deleted.
>
> But the halfway modification to the description section in this
> patch is not an improvement. It makes some options described there
> while -m and -c are completely missing now, making the section
> incomplete and coverage of the operating modes of the command
> uneven.
If I got it right, you'd prefer this patch not to be accepted
separately, but as part of the future series that would rework the
entire git-branch(1) man page? I'm fine with that as well.
>> +-m [<oldbranch>] <newbranch>::
>> +--move [<oldbranch>] <newbranch>::
>> + Rename an existing branch `<oldbranch>` to `<newbranch>`; if left
>> + unspecified, `<oldbranch>` defaults to the current branch. The
>> + configuration variables for the `<oldbranch>` branch and its reflog
>> + are also renamed appropriately to be used with `<newbranch>`. In
>> + addition, a reflog entry is created to remember the branch renaming.
>> + Renaming fails if branch `<newbranch>` already exists, but `-M`
>> + or `--move --force` can be used to overwrite the contents of the
>> + existing branch `<newbranch>` while renaming.
>
> OK. This is way more readable than the previous attempts we made.
>
> The description of the single failure mode still worries me (see my
> previous message on this). Here is my attempt:
>
> When the command fails due to an existing '<newbranch>', you
> can use `-M` (or `--move --force`) to force overwriting it.
>
> to hint that there may be other ways for the command to fail, and
> hint that `-M` may not always resolve issues, but I do not know how
> successful it is. I could add
Makes sense. It's intentionally a bit vague, but should work fine.
I'd just replace "the command" with "renaming", and avoid addressing
the reader directly.
> Note that `-M <old> <new>` will not resolve an error if the
> reason why `-m` fails is to protect the other worktree that
> checks out (or otherwise uses) <old> and <new> points at a
> different commit.
>
> but we do not necessarily want to appear to be exhaustive here, so,
> I dunno.
Huh-uh... I'm not sure that such an exhaustive explanation would
make it more clear to the majority of users. Perhaps it's better
to remain a bit vague, at least for now, and omit such details.
>> +-M [<oldbranch>] <newbranch>::
>> Shortcut for `--move --force`.
>
> OK.
>
>> +--copy [<oldbranch>] <newbranch>::
>> + Copy an existing branch `<oldbranch>` to `<newbranch>`; if left
>> + unspecified, `<oldbranch>` defaults to the current branch. The
>> + configuration variables for the `<oldbranch>` branch and its reflog
>> + are also copied appropriately to be used with `<newbranch>`.
>> + Copying fails if branch `<newbranch>` already exists, but `-C`
>> + or `--copy --force` can be used to overwrite the contents of the
>> + existing branch `<newbranch>` while copying.
>
> Exactly the same comment on "other failure modes" applies here.
Noted.
>> -<oldbranch>::
>> - The name of an existing branch. If this option is omitted,
>> - the name of the current branch will be used instead.
>> -
>> -<newbranch>::
>> - The new name for an existing branch. The same restrictions as for
>> - <branchname> apply.
>> -
>
> Removals of these lines are very pleasing ;-).
Oh yes, it's like a clear embodiment of making the current mess
a little bit smaller. :)
^ permalink raw reply
* Re: [BUG] git commit --trailer --verbose puts trailer below scissors line
From: Junio C Hamano @ 2024-02-16 21:19 UTC (permalink / raw)
To: Philippe Blain; +Cc: Git mailing list
In-Reply-To: <8b4738ad-62cd-789e-712e-bd45a151b4ac@gmail.com>
Philippe Blain <levraiphilippeblain@gmail.com> writes:
> I checked that it works OK in 2.42.1 but not in 2.43.2 so it is not
> a new regression in the 2.44 cycle, but I thought I'd write now in case
> someone spots the culprit commit faster than me.
2.43.2 has changes that are meant to be bugfix only without any new
features, taken from 2.44 cycle, so checking 2.43.0 may be worth
doing.
^ permalink raw reply
* [PATCH] git: extend --no-lazy-fetch to work across subprocesses
From: Junio C Hamano @ 2024-02-16 21:09 UTC (permalink / raw)
To: git; +Cc: Jeff King, Christian Couder
In-Reply-To: <xmqq1q9cl3xv.fsf@gitster.g>
Modeling after how the `--no-replace-objects` option is made usable
across subprocess spawning (e.g., cURL based remote helpers are
spawned as a separate process while running "git fetch"), allow the
`--no-lazy-fetch` option to be passed across process boundary.
Do not model how the value of GIT_NO_REPLACE_OBJECTS environment
variable is ignored, though. Just use the usual git_env_bool() to
allow "export GIT_NO_LAZY_FETCH=0" and "unset GIT_NO_LAZY_FETCH"
to be equivalents.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* And this comes on top of the original one plus the documentation
update for GIT_NO_REPLACE_OBJECTS.
Documentation/git.txt | 2 ++
environment.c | 4 ++++
environment.h | 1 +
git.c | 3 +++
t/t0410-partial-clone.sh | 9 +++++++++
5 files changed, 19 insertions(+)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 2f1cb3ef4e..be2829003d 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -183,6 +183,8 @@ If you just want to run git as if it was started in `<path>` then use
Do not fetch missing objects from the promisor remote on
demand. Useful together with `git cat-file -e <object>` to
see if the object is locally available.
+ This is equivalent to setting the `GIT_NO_LAZY_FETCH`
+ environment variable to `1`.
--literal-pathspecs::
Treat pathspecs literally (i.e. no globbing, no pathspec magic).
diff --git a/environment.c b/environment.c
index 9e37bf58c0..afad78a3f8 100644
--- a/environment.c
+++ b/environment.c
@@ -136,6 +136,7 @@ const char * const local_repo_env[] = {
GRAFT_ENVIRONMENT,
INDEX_ENVIRONMENT,
NO_REPLACE_OBJECTS_ENVIRONMENT,
+ NO_LAZY_FETCH_ENVIRONMENT,
GIT_REPLACE_REF_BASE_ENVIRONMENT,
GIT_PREFIX_ENVIRONMENT,
GIT_SHALLOW_FILE_ENVIRONMENT,
@@ -207,6 +208,9 @@ void setup_git_env(const char *git_dir)
shallow_file = getenv(GIT_SHALLOW_FILE_ENVIRONMENT);
if (shallow_file)
set_alternate_shallow_file(the_repository, shallow_file, 0);
+
+ if (git_env_bool(NO_LAZY_FETCH_ENVIRONMENT, 0))
+ fetch_if_missing = 0;
}
int is_bare_repository(void)
diff --git a/environment.h b/environment.h
index e5351c9dd9..74b3124f55 100644
--- a/environment.h
+++ b/environment.h
@@ -35,6 +35,7 @@ const char *getenv_safe(struct strvec *argv, const char *name);
#define EXEC_PATH_ENVIRONMENT "GIT_EXEC_PATH"
#define CEILING_DIRECTORIES_ENVIRONMENT "GIT_CEILING_DIRECTORIES"
#define NO_REPLACE_OBJECTS_ENVIRONMENT "GIT_NO_REPLACE_OBJECTS"
+#define NO_LAZY_FETCH_ENVIRONMENT "GIT_NO_LAZY_FETCH"
#define GIT_REPLACE_REF_BASE_ENVIRONMENT "GIT_REPLACE_REF_BASE"
#define GITATTRIBUTES_FILE ".gitattributes"
#define INFOATTRIBUTES_FILE "info/attributes"
diff --git a/git.c b/git.c
index 28e8bf7497..d11d4dc77b 100644
--- a/git.c
+++ b/git.c
@@ -189,6 +189,9 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
*envchanged = 1;
} else if (!strcmp(cmd, "--no-lazy-fetch")) {
fetch_if_missing = 0;
+ setenv(NO_LAZY_FETCH_ENVIRONMENT, "1", 1);
+ if (envchanged)
+ *envchanged = 1;
} else if (!strcmp(cmd, "--no-replace-objects")) {
disable_replace_refs();
setenv(NO_REPLACE_OBJECTS_ENVIRONMENT, "1", 1);
diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index 5b7bee888d..59629cea1f 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -665,6 +665,15 @@ test_expect_success 'lazy-fetch when accessing object not in the_repository' '
git -C partial.git rev-list --objects --missing=print HEAD >out &&
grep "[?]$FILE_HASH" out &&
+ # The no-lazy-fetch mechanism prevents Git from fetching
+ test_must_fail env GIT_NO_LAZY_FETCH=1 \
+ git -C partial.git cat-file -e "$FILE_HASH" &&
+ test_must_fail git --no-lazy-fetch -C partial.git cat-file -e "$FILE_HASH" &&
+
+ # Sanity check that the file is still missing
+ git -C partial.git rev-list --objects --missing=print HEAD >out &&
+ grep "[?]$FILE_HASH" out &&
+
git -C full cat-file -s "$FILE_HASH" >expect &&
test-tool partial-clone object-info partial.git "$FILE_HASH" >actual &&
test_cmp expect actual &&
--
2.44.0-rc1-17-g3e0d3cd5c7
^ permalink raw reply related
* [BUG] git commit --trailer --verbose puts trailer below scissors line
From: Philippe Blain @ 2024-02-16 21:04 UTC (permalink / raw)
To: Git mailing list
Hello,
I've just found a bug in the current master: running
git commit --trailer="key: value" --verbose
puts the trailer below the diff, and thus below the scissors
line (and so it is discarded).
I checked that it works OK in 2.42.1 but not in 2.43.2 so it is not
a new regression in the 2.44 cycle, but I thought I'd write now in case
someone spots the culprit commit faster than me.
I'll start a bisection now.
Cheers,
Philippe.
^ permalink raw reply
* Re: [PATCH v2 2/4] oidset: refactor oidset_insert_from_set()
From: Linus Arver @ 2024-02-16 20:27 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Patrick Steinhardt, John Cai,
Christian Couder
In-Reply-To: <CAP8UFD0BfdVQ6p8SH6RVRYF4=+-V4PAtKg9LdUEeL_GnSqozzg@mail.gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
> On Fri, Feb 16, 2024 at 2:10 AM Linus Arver <linusa@google.com> wrote:
>>
>> Christian Couder <christian.couder@gmail.com> writes:
>> >> so perhaps the following wording is simpler?
>> >>
>> >> Like oid_insert(), but insert all oids found in 'src'. Calls
>> >> oid_insert() internally.
>> >
>> > (What you suggest would need s/oid_insert/oidset_insert/)
>> >
>> > Yeah, it's a bit simpler and shorter, but on the other hand a reader
>> > might have to read both this and the oidset_insert() doc, so in the
>> > end I am not sure it's a big win for readability. And if they don't
>> > read the oidset_insert() doc, they might miss the fact that we are
>> > copying the oids we insert, which might result in a bug.
>>
>> When functions are built on top of other functions, I think it is good
>> practice to point readers to those underlying functions. In this case
>> the new function is a wrapper around oidset_insert() which does all the
>> real work. Plus the helper function already has some documentation about
>> copying behavior that we already thought was important enough to call
>> out explicitly.
>>
>> So, tying this definition to that (foundational) helper function sounds
>> like a good idea to me in terms of readability. IOW we can inform
>> readers "hey, we're just a wrapper around this other important function
>> --- go there if you're curious about internals" and emphasizing that
>> sort of relationship which may not be immediately obvious to those not
>> familiar with this area would be nice.
>>
>> Alternatively, we could repeat the same comment WRT copying here but
>> that seems redundant and prone to maintenance burdens down the road (if
>> we ever change this behavior we have to change the comment in multiple
>> functions, possibly).
>>
>> > Also your wording ties the implementation with oidset_insert(), which
>> > we might not want if we could find something more performant. See
>> > Junio's comment on this patch saying his initial reaction was that
>> > copying underlying bits may even be more efficient.
>> >
>> > So I prefer not to change this.
>>
>> OK.
>
> I must say that in cases like this, it's difficult to be right for
> sure because it's mostly with enough hindsight that we can tell what
> turned out to be a good decision. Here for example, it might be that
> someone will find something more performant soon or it might turn out
> that the function will never change. We just can't know.
>
> So as long as the wording is clear and good enough, I think there is
> no point in trying to improve it as much as possible. Here both your
> wording and my wording seem clear and good enough to me. Junio might
> change his mind but so far it seems that he found my wording good
> enough too. So in cases like this, it's just simpler to keep current
> wording.
Sounds very reasonable.
> This way I think there is a higher chance that things can be
> merged sooner and that we can all be more efficient.
Thank you for pointing this out. There is definitely a balance between
trying to find the best possible solution (which may require a much
deeper analysis of the codebase, existing usage patterns, future
prospects in this area, etc) and getting something that's good enough.
Somehow I was under the impression that we always wanted the best
possible thing during the review process (regardless of the number of
rerolls), but you make a good point about "code review ergonomics", if
you will. And on top of that I fully agree with all of your other
comments below, so, SGTM. Thanks.
>> >> > +void oidset_insert_from_set(struct oidset *dest, struct oidset *src);
>> >>
>> >> Perhaps "oidset_insert_all" would be a simpler name? I generally prefer
>> >> to reuse any descriptors in comments to guide the names. Plus this
>> >> function used to be called "add_all()" so keeping the "all" naming style
>> >> feels right.
>> >
>> > We already have other related types like 'struct oid-array' and
>> > 'struct oidmap' to store oids, as well as code that inserts many oids
>> > into an oidset from a 'struct ref *' linked list or array in a tight
>> > loop.
>>
>> Thank you for the additional context I was not aware of.
>>
>> > So if we want to add functions inserting all the oids from
>> > instances of such types, how should we call them?
>> >
>> > I would say we should use suffixes like: "_from_set", "_from_map",
>> > "from_array", "_from_ref_list", "_from_ref_array", etc.
>>
>> I agree.
>>
>> However, I would like to point out that the function being added in this
>> patch is a bit special: it is inserting from one "oidset" into another
>> "oidset". IOW the both the dest and src types are the same.
>>
>> For the cases where the types are different, I totally agree that using
>> the suffixes (to encode the type information of the src into the
>> function name itself) is a good idea.
>>
>> So I think it's still fine to use "oidset_insert_all" because the only
>> type in the parameter list is an oidset.
>
> Yeah, here also I think both "oidset_insert_from_set" and
> "oidset_insert_all" are clear and good enough.
>
>> BUT, maybe in our codebase we already use suffixes like this even for
>> cases where the types are the same? I don't know the answer to this
>> question.
>
> I agree that it could be a good thing to be consistent with similar
> structs, but so far for oidmap there is only oidmap_put(), and, for
> oid-array, only oid_array_append(). (And no, I didn't look further
> than this.)
>
>> However if we really wanted to be consistent then maybe we
>> should be using the name oidset_insert_from_oidset() and not
>> oidset_insert_from_set().
>
> Yeah, "oidset_insert_from_oidset" and perhaps
> "oidset_insert_all_from_oidset" would probably be fine too. Junio
> found my wording good enough though, so I think it's just simpler not
> to change it.
>
> Also it's not like it can't be improved later if there is a good
> reason like consistency with other oid related structs that might get
> oidmap_put_all() or oid_array_append_all(). But again we can't predict
> what will happen, so...
^ permalink raw reply
* Re: [PATCH] completion: use awk for filtering the config entries
From: Junio C Hamano @ 2024-02-16 20:12 UTC (permalink / raw)
To: Philippe Blain; +Cc: Beat Bolli, git, Ævar Arnfjörð Bjarmason
In-Reply-To: <fcd3f999-a8d1-9f9d-e8fd-071b38124edc@gmail.com>
Philippe Blain <levraiphilippeblain@gmail.com> writes:
>>> Junio, this goes on top of 'pb/complete-config' which is on next
>>> currently.
>>
>> Alternatively we could redo the topic, squashing this fix in, after
>> the release when we rewind 'next'.
>>
>> Thanks.
>
> Actually you already merged this topic to master in 89400c3615, so it would
> go on top, no ?
Ah, then that's fine. I didn't check what I read above myself
before responding X-<.
^ permalink raw reply
* Re: [PATCH v2] branch: rework the descriptions of rename and copy operations
From: Junio C Hamano @ 2024-02-16 19:59 UTC (permalink / raw)
To: Dragan Simic; +Cc: git, code, rjusto, spectral
In-Reply-To: <6e1c3f2c5816f09aab561bc7dec2b7455d70aaec.1708087213.git.dsimic@manjaro.org>
Dragan Simic <dsimic@manjaro.org> writes:
> Move the descriptions of the <oldbranch> and <newbranch> arguments to the
> descriptions of the branch rename and copy operations, where they naturally
> belong. Also, improve the descriptions of these two branch operations and,
> for completeness, describe the outcomes of forced operations.
>
> Describing the arguments together with their respective operations, instead
> of describing them separately in a rather unfortunate attempt to squeeze more
> meaning out of fewer words, flows much better and makes the git-branch(1)
> man page significantly more usable.
The intention to remove non-option from the OPTIONS enumeration,
and to explain <new> and <old> used as arguments to -m and -c where
these options are described, are both very good (heh, after all,
they are parts of what I envisioned to be the way to go in the
longer term ;-).
> overridden by using the `--track` and `--no-track` options, and
> changed later using `git branch --set-upstream-to`.
>
> -With a `-m` or `-M` option, <oldbranch> will be renamed to <newbranch>.
> -If <oldbranch> had a corresponding reflog, it is renamed to match
> -<newbranch>, and a reflog entry is created to remember the branch
> -renaming. If <newbranch> exists, -M must be used to force the rename
> -to happen.
> -
> -The `-c` and `-C` options have the exact same semantics as `-m` and
> -`-M`, except instead of the branch being renamed, it will be copied to a
> -new name, along with its config and reflog.
> -
> With a `-d` or `-D` option, `<branchname>` will be deleted. You may
> specify more than one branch for deletion. If the branch currently
> has a reflog then the reflog will also be deleted.
But the halfway modification to the description section in this
patch is not an improvement. It makes some options described there
while -m and -c are completely missing now, making the section
incomplete and coverage of the operating modes of the command
uneven.
> +-m [<oldbranch>] <newbranch>::
> +--move [<oldbranch>] <newbranch>::
> + Rename an existing branch `<oldbranch>` to `<newbranch>`; if left
> + unspecified, `<oldbranch>` defaults to the current branch. The
> + configuration variables for the `<oldbranch>` branch and its reflog
> + are also renamed appropriately to be used with `<newbranch>`. In
> + addition, a reflog entry is created to remember the branch renaming.
> + Renaming fails if branch `<newbranch>` already exists, but `-M`
> + or `--move --force` can be used to overwrite the contents of the
> + existing branch `<newbranch>` while renaming.
OK. This is way more readable than the previous attempts we made.
The description of the single failure mode still worries me (see my
previous message on this). Here is my attempt:
When the command fails due to an existing '<newbranch>', you
can use `-M` (or `--move --force`) to force overwriting it.
to hint that there may be other ways for the command to fail, and
hint that `-M` may not always resolve issues, but I do not know how
successful it is. I could add
Note that `-M <old> <new>` will not resolve an error if the
reason why `-m` fails is to protect the other worktree that
checks out (or otherwise uses) <old> and <new> points at a
different commit.
but we do not necessarily want to appear to be exhaustive here, so,
I dunno.
> +-M [<oldbranch>] <newbranch>::
> Shortcut for `--move --force`.
OK.
> +--copy [<oldbranch>] <newbranch>::
> + Copy an existing branch `<oldbranch>` to `<newbranch>`; if left
> + unspecified, `<oldbranch>` defaults to the current branch. The
> + configuration variables for the `<oldbranch>` branch and its reflog
> + are also copied appropriately to be used with `<newbranch>`.
> + Copying fails if branch `<newbranch>` already exists, but `-C`
> + or `--copy --force` can be used to overwrite the contents of the
> + existing branch `<newbranch>` while copying.
Exactly the same comment on "other failure modes" applies here.
> -<oldbranch>::
> - The name of an existing branch. If this option is omitted,
> - the name of the current branch will be used instead.
> -
> -<newbranch>::
> - The new name for an existing branch. The same restrictions as for
> - <branchname> apply.
> -
Removals of these lines are very pleasing ;-).
^ permalink raw reply
* Re: Question about migrating a repository
From: Kristoffer Haugsbakk @ 2024-02-16 19:46 UTC (permalink / raw)
To: Gabor Urban; +Cc: git
In-Reply-To: <CAL=1hhwreG_W_Ch-B5DXioqjUsfkgjayNZbkCk7uOv3vc=TBSQ@mail.gmail.com>
On Fri, Feb 16, 2024, at 20:34, Gabor Urban wrote:
> Hi guys,
>
> I need a bit help.
>
> I have migrated my git repository from my old computer using a bundle.
> (The repo was local with no clones.) That computer will be dismantled
> and thrown away.
>
> I checked and verything is working fine till I get a git status
> report. The most relevant part is:
>
> On branch master
> Your branch is up to date with 'origin/master'.
>
> I would like to make THIS repository to be the "origin". (The other
> will be destroyed.) How could I do that?
>
> Thanks for any help in advance,
`origin/master` is a “remote-tracking branch”. It points to `master` on
the `origin` remote. Or to be precise: it points to a ref that you use
to track this branch from that remote. A remote is some other repository
that you have a link to, like a URL.
That ref (reference) was updated with a command like `git fetch`.
You can get the link to that remote with
```
git remote get-url origin
```
Your own local repository is never a remote like `origin`. You don’t
have to make your own repository into a remote.
Your repository is fine. There’s nothing that you need to do.
>
>
> --
> Urbán Gábor
>
> Linux is like a wigwam: no Gates, no Windows and an Apache inside.
--
Kristoffer Haugsbakk
^ permalink raw reply
* Question about migrating a repository
From: Gabor Urban @ 2024-02-16 19:34 UTC (permalink / raw)
To: git
Hi guys,
I need a bit help.
I have migrated my git repository from my old computer using a bundle.
(The repo was local with no clones.) That computer will be dismantled
and thrown away.
I checked and verything is working fine till I get a git status
report. The most relevant part is:
On branch master
Your branch is up to date with 'origin/master'.
I would like to make THIS repository to be the "origin". (The other
will be destroyed.) How could I do that?
Thanks for any help in advance,
--
Urbán Gábor
Linux is like a wigwam: no Gates, no Windows and an Apache inside.
^ permalink raw reply
* Re: [Improvements on messages 1/5] rebase: trivial fix of error message
From: Alex Henrie @ 2024-02-16 19:05 UTC (permalink / raw)
To: Alexander Shopov; +Cc: git, gitster, worldhello.net
In-Reply-To: <20240216101647.28837-2-ash@kambanaria.org>
On Fri, Feb 16, 2024 at 3:16 AM Alexander Shopov <ash@kambanaria.org> wrote:
>
> Mark --rebase-merges as option rather than variable name
>
> Signed-off-by: Alexander Shopov <ash@kambanaria.org>
> ---
> builtin/rebase.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 4084a6abb8..9c6d971515 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -746,7 +746,7 @@ static void parse_rebase_merges_value(struct rebase_options *options, const char
> else if (!strcmp("rebase-cousins", value))
> options->rebase_cousins = 1;
> else
> - die(_("Unknown rebase-merges mode: %s"), value);
> + die(_("Unknown --rebase-merges mode: %s"), value);
Hi Alexander! The other patches in this series look good to me, but I
don't think this one is right. This error message could be about
either the --rebase-merges command line option or the
rebase.rebaseMerges config option. The word "rebase-merges" is
intentionally ambiguous enough to cover both situations and was not
intended to be left in English in translations.
-Alex
^ permalink raw reply
* Re: [PATCH] completion: use awk for filtering the config entries
From: Philippe Blain @ 2024-02-16 18:47 UTC (permalink / raw)
To: Junio C Hamano, Beat Bolli; +Cc: git, Ævar Arnfjörð Bjarmason
In-Reply-To: <xmqqr0hcjorg.fsf@gitster.g>
Hi Beat and Junio,
Le 2024-02-16 à 12:35, Junio C Hamano a écrit :
> Beat Bolli <dev+git@drbeat.li> writes:
>
>> Commits 1e0ee4087e (completion: add and use
>> __git_compute_first_level_config_vars_for_section, 2024-02-10) and
>> 6e32f718ff (completion: add and use
>> __git_compute_second_level_config_vars_for_section, 2024-02-10)
>> introduced new helpers for config completion.
>>
>> Both helpers use a pipeline of grep and awk to filter the list of config
>> entries. awk is perfectly capable of filtering, so let's eliminate the
>> grep process and move the filtering into the awk script.
>
> Makes sense.
Yes, thanks for improving that!
> I wonder if we can have some simple script sanity
> checker that catches things like this, e.g., catting a single file
> into pipe, grep appearing upstream of awk or sed, etc.
>
>> The "-E" grep option (extended syntax) was not necessary, as $section is
>> a single word.
>>
>> While at it, wrap the over-long lines to make them more readable.
>>
>> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
>> ---
>>
>> Junio, this goes on top of 'pb/complete-config' which is on next
>> currently.
>
> Alternatively we could redo the topic, squashing this fix in, after
> the release when we rewind 'next'.
>
> Thanks.
Actually you already merged this topic to master in 89400c3615, so it would
go on top, no ?
Thanks,
Philippe.
^ permalink raw reply
* Re: [Improvements on messages 0/5] Disambuiguate between options and commands
From: Junio C Hamano @ 2024-02-16 18:43 UTC (permalink / raw)
To: Alexander Shopov; +Cc: git, worldhello.net
In-Reply-To: <20240216101647.28837-1-ash@kambanaria.org>
Alexander Shopov <ash@kambanaria.org> writes:
> These are trivial fixes to messages.
> They make sure commands and options are markes as such.
> This will help translators and end users.
> This will also reduce the special cases Jiang Xin keeps
> for git-po-helper which will ease maintenance.
>
> I am basing these on maint but I have also checked that
> they ar still relevant by cherry picking on top of latest and next.
I've looked at all of them and they looked sensible.
The changes look like this:
- die(_("something option something: %s"), arg);
+ die(_("something --option something: %s"), arg);
It is not a fault of this patch, but wasn't the concensus that the
ideal form would be more like this:
die(_("something %s something: %s"), "--option", arg);
in order to completely avoid tempting translators into touching
"--option", IIRC?
These patches do not make things worse, so I am willing to say they
are strict improvements and the series is a good first step if we
wanted to follow through to eject option names out of translatable
strings later.
Thanks.
^ permalink raw reply
* Re: [PATCH] completion: use awk for filtering the config entries
From: Beat Bolli @ 2024-02-16 18:29 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Philippe Blain, Ævar Arnfjörð Bjarmason
In-Reply-To: <xmqqr0hcjorg.fsf@gitster.g>
On 16.02.24 18:35, Junio C Hamano wrote:
> Beat Bolli <dev+git@drbeat.li> writes:
>
>> Commits 1e0ee4087e (completion: add and use
>> __git_compute_first_level_config_vars_for_section, 2024-02-10) and
>> 6e32f718ff (completion: add and use
>> __git_compute_second_level_config_vars_for_section, 2024-02-10)
>> introduced new helpers for config completion.
>>
>> Both helpers use a pipeline of grep and awk to filter the list of config
>> entries. awk is perfectly capable of filtering, so let's eliminate the
>> grep process and move the filtering into the awk script.
>
> Makes sense. I wonder if we can have some simple script sanity
> checker that catches things like this, e.g., catting a single file
> into pipe, grep appearing upstream of awk or sed, etc.
Yes, there are quite a few cases of these in t/. I'm not sure if it's
worth the churn, though. At least it would make the tests faster on
Windows...
>> The "-E" grep option (extended syntax) was not necessary, as $section is
>> a single word.
>>
>> While at it, wrap the over-long lines to make them more readable.
>>
>> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
>> ---
>>
>> Junio, this goes on top of 'pb/complete-config' which is on next
>> currently.
>
> Alternatively we could redo the topic, squashing this fix in, after
> the release when we rewind 'next'.
As you like. This commit would have to be split to apply to the two
original commits.
>> contrib/completion/git-completion.bash | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index 444b3efa63..fcf1afd75d 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -2673,7 +2673,8 @@ __git_compute_first_level_config_vars_for_section ()
>> __git_compute_config_vars
>> local this_section="__git_first_level_config_vars_for_section_${section}"
>> test -n "${!this_section}" ||
>> - printf -v "__git_first_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars" | grep -E "^${section}\.[a-z]" | awk -F. '{print $2}')"
>> + printf -v "__git_first_level_config_vars_for_section_${section}" %s \
>> + "$(echo "$__git_config_vars" | awk -F. "/^${section}\.[a-z]/ { print \$2 }")"
>> }
>>
>> __git_compute_second_level_config_vars_for_section ()
>> @@ -2682,7 +2683,8 @@ __git_compute_second_level_config_vars_for_section ()
>> __git_compute_config_vars_all
>> local this_section="__git_second_level_config_vars_for_section_${section}"
>> test -n "${!this_section}" ||
>> - printf -v "__git_second_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars_all" | grep -E "^${section}\.<" | awk -F. '{print $3}')"
>> + printf -v "__git_second_level_config_vars_for_section_${section}" %s \
>> + "$(echo "$__git_config_vars_all" | awk -F. "/^${section}\.</ { print \$3 }")"
>> }
>>
>> __git_config_sections=
^ permalink raw reply
* Re: [PATCH] git-difftool--helper: honor `--trust-exit-code` with `--dir-diff`
From: Junio C Hamano @ 2024-02-16 18:12 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Jean-Rémy Falleri, David Aguilar
In-Reply-To: <c071e44c52171b9b19a04d91666be48d762d19bf.1708072576.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> The `--trust-exit-code` option for git-diff-tool(1) was introduced via
> 2b52123fcf (difftool: add support for --trust-exit-code, 2014-10-26).
> When set, it makes us return the exit code of the invoked diff tool when
> diffing multiple files. This patch didn't change the code path where
> `--dir-diff` was passed because we already returned the exit code of the
> diff tool unconditionally in that case.
>
> This was changed a month later via c41d3fedd8 (difftool--helper: add
> explicit exit statement, 2014-11-20), where an explicit `exit 0` was
> added to the end of git-difftool--helper.sh. While the stated intent of
> that commit was merely a cleanup, it had the consequence that we now
> to ignore the exit code of the diff tool when `--dir-diff` was set. This
> change in behaviour is thus very likely an unintended side effect of
> this patch.
>
> Now there are two ways to fix this:
>
> - We can either restore the original behaviour, which unconditionally
> returned the exit code of the diffing tool when `--dir-diff` is
> passed.
>
> - Or we can make the `--dir-diff` case respect the `--trust-exit-code`
> flag.
>
> The fact that we have been ignoring exit codes for 7 years by now makes
> me rather lean towards the latter option. Furthermore, respecting the
> flag in one case but not the other would needlessly make the user
> interface more complex.
>
> Fix the bug so that we also honor `--trust-exit-code` for dir diffs and
> adjust the documentation accordingly.
>
> Reported-by: Jean-Rémy Falleri <jr.falleri@gmail.com>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
Sounds sensible.
The last time David was on list seems to be in April 2023; just in
case let's CC him for an Ack (or something else).
> Documentation/git-difftool.txt | 1 -
> git-difftool--helper.sh | 14 +++++
> t/t7800-difftool.sh | 99 ++++++++++++++++++----------------
> 3 files changed, 68 insertions(+), 46 deletions(-)
>
> diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
> index c05f97aca9..a616f8b2e6 100644
> --- a/Documentation/git-difftool.txt
> +++ b/Documentation/git-difftool.txt
> @@ -105,7 +105,6 @@ instead. `--no-symlinks` is the default on Windows.
> `merge.tool` until a tool is found.
>
> --[no-]trust-exit-code::
> - 'git-difftool' invokes a diff tool individually on each file.
> Errors reported by the diff tool are ignored by default.
> Use `--trust-exit-code` to make 'git-difftool' exit when an
> invoked diff tool returns a non-zero exit code.
> diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
> index e4e820e680..09d8542917 100755
> --- a/git-difftool--helper.sh
> +++ b/git-difftool--helper.sh
> @@ -91,6 +91,20 @@ then
> # ignore the error from the above --- run_merge_tool
> # will diagnose unusable tool by itself
> run_merge_tool "$merge_tool" false
> +
> + status=$?
> + if test $status -ge 126
> + then
> + # Command not found (127), not executable (126) or
> + # exited via a signal (>= 128).
> + exit $status
> + fi
So these errors spawning the tool backend are always reported,
regardless of the trust-exit-code settings. OK.
> + if test "$status" != 0 &&
> + test "$GIT_DIFFTOOL_TRUST_EXIT_CODE" = true
> + then
> + exit $status
> + fi
I found this somehow harder to reason about than necessary. Just
if test "$GIT_DIFFTOOL_TRUST_EXIT_CODE" = true
then
exit $status
fi
would have been a more straight-forward expression of what we want
to happen here, i.e. "if we are told to report the tool's exit
status, we do so, regardless of what the exit status is".
Not that the construct in your patch is wrong---we will exit with 0
at the end even when "trust-exit-code" thing is true and the tool
returned success.
^ permalink raw reply
* Re: [PATCH] completion: use awk for filtering the config entries
From: Junio C Hamano @ 2024-02-16 17:35 UTC (permalink / raw)
To: Beat Bolli; +Cc: git, Philippe Blain, Ævar Arnfjörð Bjarmason
In-Reply-To: <20240216171046.927552-1-dev+git@drbeat.li>
Beat Bolli <dev+git@drbeat.li> writes:
> Commits 1e0ee4087e (completion: add and use
> __git_compute_first_level_config_vars_for_section, 2024-02-10) and
> 6e32f718ff (completion: add and use
> __git_compute_second_level_config_vars_for_section, 2024-02-10)
> introduced new helpers for config completion.
>
> Both helpers use a pipeline of grep and awk to filter the list of config
> entries. awk is perfectly capable of filtering, so let's eliminate the
> grep process and move the filtering into the awk script.
Makes sense. I wonder if we can have some simple script sanity
checker that catches things like this, e.g., catting a single file
into pipe, grep appearing upstream of awk or sed, etc.
> The "-E" grep option (extended syntax) was not necessary, as $section is
> a single word.
>
> While at it, wrap the over-long lines to make them more readable.
>
> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
> ---
>
> Junio, this goes on top of 'pb/complete-config' which is on next
> currently.
Alternatively we could redo the topic, squashing this fix in, after
the release when we rewind 'next'.
Thanks.
>
> contrib/completion/git-completion.bash | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 444b3efa63..fcf1afd75d 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2673,7 +2673,8 @@ __git_compute_first_level_config_vars_for_section ()
> __git_compute_config_vars
> local this_section="__git_first_level_config_vars_for_section_${section}"
> test -n "${!this_section}" ||
> - printf -v "__git_first_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars" | grep -E "^${section}\.[a-z]" | awk -F. '{print $2}')"
> + printf -v "__git_first_level_config_vars_for_section_${section}" %s \
> + "$(echo "$__git_config_vars" | awk -F. "/^${section}\.[a-z]/ { print \$2 }")"
> }
>
> __git_compute_second_level_config_vars_for_section ()
> @@ -2682,7 +2683,8 @@ __git_compute_second_level_config_vars_for_section ()
> __git_compute_config_vars_all
> local this_section="__git_second_level_config_vars_for_section_${section}"
> test -n "${!this_section}" ||
> - printf -v "__git_second_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars_all" | grep -E "^${section}\.<" | awk -F. '{print $3}')"
> + printf -v "__git_second_level_config_vars_for_section_${section}" %s \
> + "$(echo "$__git_config_vars_all" | awk -F. "/^${section}\.</ { print \$3 }")"
> }
>
> __git_config_sections=
^ permalink raw reply
* Re: [PATCH] git: --no-lazy-fetch option
From: Junio C Hamano @ 2024-02-16 17:22 UTC (permalink / raw)
To: git; +Cc: Jeff King, Christian Couder
In-Reply-To: <xmqqv86pslos.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Yuck, I was hoping that we can get away with the tiny change only
> for builtins,but you're right.
Here is a preliminary clean-up only for Documentation. Will not be
queuing before the final, but just so that I won't forget.
------- >8 ------------- >8 ------------- >8 -------
Subject: [PATCH] git: document GIT_NO_REPLACE_OBJECTS environment variable
This variable is used as the primary way to disable the object
replacement mechanism, with the "--no-replace-objects" command line
option as an end-user visible way to set it, but has not been
documented.
The original reason why it was left undocumented might be because it
was meant as an internal implementation detail, but the thing is,
that our tests use the environment variable directly without the
command line option, and there certainly are folks who learned its
use from there, making it impossible to deprecate or change its
behaviour by now.
Add documentation and note that for this variable, unlike many
boolean-looking environment variables, only the presence matters,
not what value it is set to.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git.txt | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git c/Documentation/git.txt i/Documentation/git.txt
index 95f451b22b..2f1cb3ef4e 100644
--- c/Documentation/git.txt
+++ i/Documentation/git.txt
@@ -174,8 +174,10 @@ If you just want to run git as if it was started in `<path>` then use
directory.
--no-replace-objects::
- Do not use replacement refs to replace Git objects. See
- linkgit:git-replace[1] for more information.
+ Do not use replacement refs to replace Git objects.
+ This is equivalent to exporting the `GIT_NO_REPLACE_OBJECTS`
+ environment variable with any value.
+ See linkgit:git-replace[1] for more information.
--no-lazy-fetch::
Do not fetch missing objects from the promisor remote on
^ permalink raw reply related
* [PATCH] completion: use awk for filtering the config entries
From: Beat Bolli @ 2024-02-16 17:10 UTC (permalink / raw)
To: git
Cc: Beat Bolli, Philippe Blain,
Ævar Arnfjörð Bjarmason, Junio C Hamano
Commits 1e0ee4087e (completion: add and use
__git_compute_first_level_config_vars_for_section, 2024-02-10) and
6e32f718ff (completion: add and use
__git_compute_second_level_config_vars_for_section, 2024-02-10)
introduced new helpers for config completion.
Both helpers use a pipeline of grep and awk to filter the list of config
entries. awk is perfectly capable of filtering, so let's eliminate the
grep process and move the filtering into the awk script.
The "-E" grep option (extended syntax) was not necessary, as $section is
a single word.
While at it, wrap the over-long lines to make them more readable.
Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
Junio, this goes on top of 'pb/complete-config' which is on next
currently.
contrib/completion/git-completion.bash | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 444b3efa63..fcf1afd75d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2673,7 +2673,8 @@ __git_compute_first_level_config_vars_for_section ()
__git_compute_config_vars
local this_section="__git_first_level_config_vars_for_section_${section}"
test -n "${!this_section}" ||
- printf -v "__git_first_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars" | grep -E "^${section}\.[a-z]" | awk -F. '{print $2}')"
+ printf -v "__git_first_level_config_vars_for_section_${section}" %s \
+ "$(echo "$__git_config_vars" | awk -F. "/^${section}\.[a-z]/ { print \$2 }")"
}
__git_compute_second_level_config_vars_for_section ()
@@ -2682,7 +2683,8 @@ __git_compute_second_level_config_vars_for_section ()
__git_compute_config_vars_all
local this_section="__git_second_level_config_vars_for_section_${section}"
test -n "${!this_section}" ||
- printf -v "__git_second_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars_all" | grep -E "^${section}\.<" | awk -F. '{print $3}')"
+ printf -v "__git_second_level_config_vars_for_section_${section}" %s \
+ "$(echo "$__git_config_vars_all" | awk -F. "/^${section}\.</ { print \$3 }")"
}
__git_config_sections=
--
2.42.0.583.ga47b40fd90
^ permalink raw reply related
* Re: Hello question on Git for Windows 2.43.0 - GUID and/or SWID tag for this title
From: Konstantin Khomoutov @ 2024-02-16 12:56 UTC (permalink / raw)
To: git@vger.kernel.org
In-Reply-To: <20240216110041.dqz2n5dz43mqtq25@carbon>
[...]
> Therefore, please let me know what your role is with Git for Windows so I
> can send this feedback accordingly and continue working on with our software
> inventory vendor on the issue.
[...]
> So, I would recommend to ask on GfW's Github Discussions [2] or on the
> GfW's dedicated mailing list [3].
[...]
> 2. https://github.com/git-for-windows/git/discussions
> 3. https://groups.google.com/g/git-for-windows/
To make things maybe a bit more clear: this very list our discussion is taking
place is dedicated to the development of the "core" Git, and Git-for-Windows
has started years ago as what one would call a continuous friendly fork of
Git. May changes implemented in GfW get upstreamed into the "core" Git,
but GfW still maintains a set of patches atop of Git, as well as a hefty
amount of infrastructure code (because in order to properly function under
Windows, Git requires lots of supporting software which presence is taken for
granted on Unix-like systems but does not normally exist on Windows).
Maintaining a Windows installer binary is also a task.
In other words, GfW has quite specific requirements to still be a separate,
though very close, project, and because of this is has its own separate
supporting infrastructure: a project and issue tracker on Github and its own
communication venues. These venues are what I and someone other earlier in
this discussion have pointed you at.
^ permalink raw reply
* [PATCH v2] branch: rework the descriptions of rename and copy operations
From: Dragan Simic @ 2024-02-16 12:44 UTC (permalink / raw)
To: git; +Cc: code, gitster, rjusto, spectral
Move the descriptions of the <oldbranch> and <newbranch> arguments to the
descriptions of the branch rename and copy operations, where they naturally
belong. Also, improve the descriptions of these two branch operations and,
for completeness, describe the outcomes of forced operations.
Describing the arguments together with their respective operations, instead
of describing them separately in a rather unfortunate attempt to squeeze more
meaning out of fewer words, flows much better and makes the git-branch(1)
man page significantly more usable.
The subsequent improvements shall continue this approach by either dissolving
as many sentences from the "Description" section into the "Options" section,
or by having those sentences converted into some kind of more readable and
better flowing prose, as already discussed and outlined. [1][2]
[1] https://lore.kernel.org/git/xmqqttmmlahf.fsf@gitster.g/T/#u
[2] https://lore.kernel.org/git/xmqq8r4zln08.fsf@gitster.g/T/#u
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
---
Notes:
This patch was originally named "branch: clarify <oldbranch> and <newbranch>
terms further", submitted and discussed in another thread, [3] but the nature
of the patch has changed, causing the patch subject to be adjusted to match.
Consequently, the version 1 is effectively version 2 of the original patch.
The version 1 of the patch includes detailed feedback from Kyle and Junio,
who suggested moving/adding the argument descriptions to their respective
commands. This resulted in more significant changes to the contents of the
git-branch(1) man page, in an attempt to make it more readable.
The version 2 includes feedback from Kristoffer and Junio, by improving the
wording of the opening sentences in the descriptions of branch rename and
copy operations, and by mentioning the additional reflog entry created while
renaming a branch, which was omitted in the v1 by mistake.
[3] https://lore.kernel.org/git/e2eb777bca8ffeec42bdd684837d28dd52cfc9c3.1707136999.git.dsimic@manjaro.org/T/#u
Documentation/git-branch.txt | 51 ++++++++++++++++--------------------
1 file changed, 23 insertions(+), 28 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 0b0844293235..d52b5e8dbacd 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -72,16 +72,6 @@ the remote-tracking branch. This behavior may be changed via the global
overridden by using the `--track` and `--no-track` options, and
changed later using `git branch --set-upstream-to`.
-With a `-m` or `-M` option, <oldbranch> will be renamed to <newbranch>.
-If <oldbranch> had a corresponding reflog, it is renamed to match
-<newbranch>, and a reflog entry is created to remember the branch
-renaming. If <newbranch> exists, -M must be used to force the rename
-to happen.
-
-The `-c` and `-C` options have the exact same semantics as `-m` and
-`-M`, except instead of the branch being renamed, it will be copied to a
-new name, along with its config and reflog.
-
With a `-d` or `-D` option, `<branchname>` will be deleted. You may
specify more than one branch for deletion. If the branch currently
has a reflog then the reflog will also be deleted.
@@ -128,18 +118,31 @@ Note that 'git branch -f <branchname> [<start-point>]', even with '-f',
refuses to change an existing branch `<branchname>` that is checked out
in another worktree linked to the same repository.
--m::
---move::
- Move/rename a branch, together with its config and reflog.
-
--M::
+-m [<oldbranch>] <newbranch>::
+--move [<oldbranch>] <newbranch>::
+ Rename an existing branch `<oldbranch>` to `<newbranch>`; if left
+ unspecified, `<oldbranch>` defaults to the current branch. The
+ configuration variables for the `<oldbranch>` branch and its reflog
+ are also renamed appropriately to be used with `<newbranch>`. In
+ addition, a reflog entry is created to remember the branch renaming.
+ Renaming fails if branch `<newbranch>` already exists, but `-M`
+ or `--move --force` can be used to overwrite the contents of the
+ existing branch `<newbranch>` while renaming.
+
+-M [<oldbranch>] <newbranch>::
Shortcut for `--move --force`.
--c::
---copy::
- Copy a branch, together with its config and reflog.
-
--C::
+-c [<oldbranch>] <newbranch>::
+--copy [<oldbranch>] <newbranch>::
+ Copy an existing branch `<oldbranch>` to `<newbranch>`; if left
+ unspecified, `<oldbranch>` defaults to the current branch. The
+ configuration variables for the `<oldbranch>` branch and its reflog
+ are also copied appropriately to be used with `<newbranch>`.
+ Copying fails if branch `<newbranch>` already exists, but `-C`
+ or `--copy --force` can be used to overwrite the contents of the
+ existing branch `<newbranch>` while copying.
+
+-C [<oldbranch>] <newbranch>::
Shortcut for `--copy --force`.
--color[=<when>]::
@@ -311,14 +314,6 @@ superproject's "origin/main", but tracks the submodule's "origin/main".
given as a branch name, a commit-id, or a tag. If this
option is omitted, the current HEAD will be used instead.
-<oldbranch>::
- The name of an existing branch. If this option is omitted,
- the name of the current branch will be used instead.
-
-<newbranch>::
- The new name for an existing branch. The same restrictions as for
- <branchname> apply.
-
--sort=<key>::
Sort based on the key given. Prefix `-` to sort in descending
order of the value. You may use the --sort=<key> option
^ permalink raw reply related
* Re: Hello question on Git for Windows 2.43.0 - GUID and/or SWID tag for this title
From: Konstantin Khomoutov @ 2024-02-16 11:00 UTC (permalink / raw)
To: git@vger.kernel.org
In-Reply-To: <LV8PR13MB65609D60ACB8A6EADFBFE3459C4D2@LV8PR13MB6560.namprd13.prod.outlook.com>
On Thu, Feb 15, 2024 at 09:40:47PM +0000, Christian Castro wrote:
>>> I have a question on the GUID and/or SWID tag for Git for Windows 2.43.0.
> Question: Are you a Git for Windows developer, open-source contributor or?
> I ask because I will contact the manufacturer of our inventory product and
> provide them your feedback. But I'd like to know what your role is with Git
> for Windows for as of now I just have a reply from someone named Matthias
> from a live.de email domain. I hope you understand. Truly no offense meant
> on my part.
>
> Therefore, please let me know what your role is with Git for Windows so I
> can send this feedback accordingly and continue working on with our software
> inventory vendor on the issue.
I would say the chief Git-for-Windows maintainer is Johannes Schindelin [1].
But prodding him directly would be impolite: after all, GfW is a F/OSS
project, so none of the folks participating in its development is oblidged to
answer any questions as them are not bound by terms of any contract with the
enterprise you're acting on behalf of - as no such contract exists (please
take no offence - I'm just clearing things up).
So, I would recommend to ask on GfW's Github Discussions [2] or on the
GfW's dedicated mailing list [3].
1. https://github.com/dscho
2. https://github.com/git-for-windows/git/discussions
3. https://groups.google.com/g/git-for-windows/
^ 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