* [PATCH] doc: format.notes specify a ref under refs/notes/ hierarchy
From: Junio C Hamano @ 2023-12-15 22:28 UTC (permalink / raw)
To: git; +Cc: Ramsay Jones
In-Reply-To: <xmqqttojjegr.fsf@gitster.g>
There is no 'ref/notes/' hierarchy. '[format] notes = foo' uses notes
that are found in 'refs/notes/foo'.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* According to my eyeballing "git grep refs/ Documentation" result,
this was the only remaining mention of "ref/" in Documentation/
hierarchy that misspells "refs/".
Documentation/config/format.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git c/Documentation/config/format.txt w/Documentation/config/format.txt
index c98412b697..7410e930e5 100644
--- c/Documentation/config/format.txt
+++ w/Documentation/config/format.txt
@@ -119,7 +119,7 @@ format.notes::
`--notes=<ref>`, where `ref` is the non-boolean value. Defaults
to false.
+
-If one wishes to use the ref `ref/notes/true`, please use that literal
+If one wishes to use the ref `refs/notes/true`, please use that literal
instead.
+
This configuration can be specified multiple times in order to allow
^ permalink raw reply related
* Re: [PATCH 1/5] git.txt: HEAD is not that special
From: Junio C Hamano @ 2023-12-15 22:19 UTC (permalink / raw)
To: Ramsay Jones; +Cc: git, Patrick Steinhardt
In-Reply-To: <xmqq1qbnktnl.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
>
>>> -may contain the SHA-1 name of an object or the name of another ref. Refs
>>> -with names beginning `ref/head/` contain the SHA-1 name of the most
>>> +may contain the SHA-1 name of an object or the name of another ref (the
>>> +latter is called a "symbolic ref").
>>> +Refs with names beginning `ref/head/` contain the SHA-1 name of the most
>>
>> Hmm, s:ref/head:refs/heads: right?
>
> Yeah, right, not a new problem with this change, but is indeed a
> good thing to catch and correct. Thanks for a careful review.
And we have ref/tags/ just below, which I also have fixed locally.
^ permalink raw reply
* Re: [PATCH 1/5] git.txt: HEAD is not that special
From: Junio C Hamano @ 2023-12-15 22:06 UTC (permalink / raw)
To: Ramsay Jones; +Cc: git, Patrick Steinhardt
In-Reply-To: <0c93d426-17c3-434c-bbd0-866c31c23f9d@ramsayjones.plus.com>
Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
>> -may contain the SHA-1 name of an object or the name of another ref. Refs
>> -with names beginning `ref/head/` contain the SHA-1 name of the most
>> +may contain the SHA-1 name of an object or the name of another ref (the
>> +latter is called a "symbolic ref").
>> +Refs with names beginning `ref/head/` contain the SHA-1 name of the most
>
> Hmm, s:ref/head:refs/heads: right?
Yeah, right, not a new problem with this change, but is indeed a
good thing to catch and correct. Thanks for a careful review.
^ permalink raw reply
* Re: [PATCH 1/5] git.txt: HEAD is not that special
From: Ramsay Jones @ 2023-12-15 21:57 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Patrick Steinhardt
In-Reply-To: <20231215203245.3622299-2-gitster@pobox.com>
On 15/12/2023 20:32, Junio C Hamano wrote:
> The introductory text in "git help git" that describes HEAD called
> it "a special ref". It is special compared to the more regular refs
> like refs/heads/master and refs/tags/v1.0.0, but not that special,
> unlike truly special ones like FETCH_HEAD.
>
> Rewrite a few sentences to also introduce the distinction between a
> regular ref that contain the object name and a symbolic ref that
> contain the name of another ref. Update the description of HEAD
> that point at the current branch to use the more correct term, a
> "symbolic ref".
>
> This was found as part of auditing the documentation and in-code
> comments for uses of "special ref" that refer merely a "pseudo ref".
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> Documentation/git.txt | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 2535a30194..880cdc5d7f 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -1025,10 +1025,11 @@ When first created, objects are stored in individual files, but for
> efficiency may later be compressed together into "pack files".
>
> Named pointers called refs mark interesting points in history. A ref
> -may contain the SHA-1 name of an object or the name of another ref. Refs
> -with names beginning `ref/head/` contain the SHA-1 name of the most
> +may contain the SHA-1 name of an object or the name of another ref (the
> +latter is called a "symbolic ref").
> +Refs with names beginning `ref/head/` contain the SHA-1 name of the most
Hmm, s:ref/head:refs/heads: right?
> recent commit (or "head") of a branch under development. SHA-1 names of
> -tags of interest are stored under `ref/tags/`. A special ref named
> +tags of interest are stored under `ref/tags/`. A symbolic ref named
> `HEAD` contains the name of the currently checked-out branch.
>
> The index file is initialized with a list of all paths and, for each
^ permalink raw reply
* Re: [PATCH 0/5] make room for "special ref"
From: Junio C Hamano @ 2023-12-15 21:21 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
In-Reply-To: <20231215203245.3622299-1-gitster@pobox.com>
Junio C Hamano <gitster@pobox.com> writes:
> ... For example, FETCH_HEAD currently stores not
> just a single object name, but can and is used to store multiple
> object names, each with annotations to record where they came from.
> There indeed may be a need to introduce a new term to refer to such
> "special refs".
The "may be" here vaguely hints another possibility. If we manage
to get rid of the "special refs", we do not even have to mention
"special refs", and more importantly, we do not need extra code to
deal with them.
For FETCH_HEAD, for example, I wonder if an update along this line
is possible:
* Teach "git fetch" to store what it writes to FETCH_HEAD to a
different file, under a distinctly different filename (e.g.,
$GIT_DIR/fetched-tips). Demote FETCH_HEAD to a pseudoref, and
store the first object name in that "fetched-tips" file to it.
* Teach "git pull" to learn what it used to learn from FETCH_HEAD
(i.e., list of fetched tips, each annotated with what ref at what
repository it came from and if it is to be merged) from the new
"fetched-tips" file.
The "special" ness of FETCH_HEAD is really an implementation detail
of how "git pull" works and how the findings of "git fetch" are
communicated to "git pull". The general refs API should not have to
worry about it, and the refs backends should not have to worry about
storing more than just an object name (or if it is a symbolic ref,
the target refname).
An end-user command like "git log ORIG_HEAD..FETCH_HEAD" would not
be affected by changes along the above line, because the current
FETCH_HEAD, when used as a revision, will work as if it stores the
single object name that is listed first in the file.
If somebody is reading FETCH_HEAD and acting on its contents (rather
than merely consuming it as a ref of the first object), perhaps
feeding it to "git fmt-merge-msg", they will be broken by such a
change (indeed, our own "git pull" will be broken by the change to
"git fetch", and the second bullet point above is about fixing the
exact fallout from it), but I am not sure if that is a use case worth
worrying about.
Hmm?
^ permalink raw reply
* Re: [PATCH] git-add.txt: add missing short option -A to synopsis
From: Eric Sunshine @ 2023-12-15 21:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Eric Sunshine, git, Benjamin Lehmann
In-Reply-To: <xmqqjzpfkwnr.fsf@gitster.g>
On Fri, Dec 15, 2023 at 4:01 PM Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <ericsunshine@charter.net> writes:
> > With one exception, the synopsis for `git add` consistently lists the
> > short counterpart alongside the long-form of each option (for instance,
> > "[--edit | -e]"). The exception is that -A is not mentioned alongside
> > --all. Fix this inconsistency
>
> I dug the history just in case we deliberately wanted to leave this
> out of the synopsis section, but the side branch leading to the
> merge at 378335b3 (Merge branch 'jc/add-addremove', 2008-07-20) does
> not say we wanted to discourage "-A" (and encourage "--all"). This
> would be a welcome change.
I also dug through the history for the same reason and, like you, did
not find any indication that -A was omitted from the synopsis
deliberately. I probably should have stated as much in the patch
commentary.
^ permalink raw reply
* Re: [PATCH] git-add.txt: add missing short option -A to synopsis
From: Junio C Hamano @ 2023-12-15 21:01 UTC (permalink / raw)
To: Eric Sunshine; +Cc: git, Benjamin Lehmann, Eric Sunshine
In-Reply-To: <20231215204333.1253-1-ericsunshine@charter.net>
Eric Sunshine <ericsunshine@charter.net> writes:
> From: Eric Sunshine <sunshine@sunshineco.com>
>
> With one exception, the synopsis for `git add` consistently lists the
> short counterpart alongside the long-form of each option (for instance,
> "[--edit | -e]"). The exception is that -A is not mentioned alongside
> --all. Fix this inconsistency
>
> Reported-by: Benjamin Lehmann <ben.lehmann@gmail.com>
> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
Thanks.
I dug the history just in case we deliberately wanted to leave this
out of the synopsis section, but the side branch leading to the
merge at 378335b3 (Merge branch 'jc/add-addremove', 2008-07-20) does
not say we wanted to discourage "-A" (and encourage "--all"). This
would be a welcome change.
Will queue.
^ permalink raw reply
* Re: Bug | Documentation | git add -all | Synopsis has minor mistake
From: Eric Sunshine @ 2023-12-15 20:49 UTC (permalink / raw)
To: Benjamin Lehmann; +Cc: git
In-Reply-To: <CAM=w4Pn46nTcWA1e=n4Rms76gCx7iqbRmOWf3=vRmKgtbhqQmA@mail.gmail.com>
On Fri, Dec 15, 2023 at 7:38 AM Benjamin Lehmann <ben.lehmann@gmail.com> wrote:
> The mistake can be found in the synopsis here:
> https://git-scm.com/docs/git-add#Documentation/git-add.txt--A
>
> In the synopsys, the options -all currently reads:
>
> [--[no-]all | --[no-]ignore-removal |
>
> You can see that there is no mention of -A, which is the main way that
> people would use -all perhaps, so it really ought to be included
> correctly in the synopsis.
This seems to be a simple oversight when the --all option was added by
da98053aa6 (git-add --all: documentation, 2008-07-19).
> In addition, the closing square-bracket is missing.
I think this is inaccurate. If you look closely, you will find the
closing bracket after the -u option:
[--[no-]all | --[no-]ignore-removal | [--update | -u]]
meaning that --all, --ignore-removal, and --update are mutually exclusive.
> Hope this was the right place to report this - seemed to be the only option.
This is the correct place. I posted a patch[1] addressing the issue.
[1]: https://lore.kernel.org/git/20231215204333.1253-1-ericsunshine@charter.net/
^ permalink raw reply
* [PATCH] git-add.txt: add missing short option -A to synopsis
From: Eric Sunshine @ 2023-12-15 20:43 UTC (permalink / raw)
To: git; +Cc: Benjamin Lehmann, Eric Sunshine
In-Reply-To: <CAM=w4Pn46nTcWA1e=n4Rms76gCx7iqbRmOWf3=vRmKgtbhqQmA@mail.gmail.com>
From: Eric Sunshine <sunshine@sunshineco.com>
With one exception, the synopsis for `git add` consistently lists the
short counterpart alongside the long-form of each option (for instance,
"[--edit | -e]"). The exception is that -A is not mentioned alongside
--all. Fix this inconsistency
Reported-by: Benjamin Lehmann <ben.lehmann@gmail.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
An alternative would be to collapse the synopsis to:
'git add' <options> [--] [<pathspec>...]
as has been done for other command documentation, however doing so would
throw away at-a-glance clues about which options are mutually exclusive,
so adding the missing -A to the synopsis seems preferable (for now, at
least).
Documentation/git-add.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index ed44c1cb31..3d2e670716 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -9,7 +9,7 @@ SYNOPSIS
--------
[verse]
'git add' [--verbose | -v] [--dry-run | -n] [--force | -f] [--interactive | -i] [--patch | -p]
- [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]] [--sparse]
+ [--edit | -e] [--[no-]all | -A | --[no-]ignore-removal | [--update | -u]] [--sparse]
[--intent-to-add | -N] [--refresh] [--ignore-errors] [--ignore-missing] [--renormalize]
[--chmod=(+|-)x] [--pathspec-from-file=<file> [--pathspec-file-nul]]
[--] [<pathspec>...]
--
2.43.0
^ permalink raw reply related
* [PATCH 5/5] docs: MERGE_AUTOSTASH is not that special
From: Junio C Hamano @ 2023-12-15 20:32 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
In-Reply-To: <20231215203245.3622299-1-gitster@pobox.com>
A handful of manual pages called MERGE_AUTOSTASH a "special ref",
but there is nothing special about it. It merely is yet another
pseudoref.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/merge-options.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index d8f7cd7ca0..3eaefc4e94 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -191,7 +191,7 @@ endif::git-pull[]
--autostash::
--no-autostash::
Automatically create a temporary stash entry before the operation
- begins, record it in the special ref `MERGE_AUTOSTASH`
+ begins, record it in the ref `MERGE_AUTOSTASH`
and apply it after the operation ends. This means
that you can run the operation on a dirty worktree. However, use
with care: the final stash application after a successful
--
2.43.0-76-g1a87c842ec
^ permalink raw reply related
* [PATCH 4/5] docs: AUTO_MERGE is not that special
From: Junio C Hamano @ 2023-12-15 20:32 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
In-Reply-To: <20231215203245.3622299-1-gitster@pobox.com>
A handful of manual pages called AUTO_MERGE a "special ref", but
there is nothing special about it. It merely is yet another
pseudoref.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git-diff.txt | 2 +-
Documentation/git-merge.txt | 2 +-
Documentation/user-manual.txt | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index 08087ffad5..c065f023ec 100644
--- a/Documentation/git-diff.txt
+++ b/Documentation/git-diff.txt
@@ -103,7 +103,7 @@ Just in case you are doing something exotic, it should be
noted that all of the <commit> in the above description, except
in the `--merge-base` case and in the last two forms that use `..`
notations, can be any <tree>. A tree of interest is the one pointed to
-by the special ref `AUTO_MERGE`, which is written by the 'ort' merge
+by the ref named `AUTO_MERGE`, which is written by the 'ort' merge
strategy upon hitting merge conflicts (see linkgit:git-merge[1]).
Comparing the working tree with `AUTO_MERGE` shows changes you've made
so far to resolve textual conflicts (see the examples below).
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index e8ab340319..3e9557a44b 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -196,7 +196,7 @@ happens:
can inspect the stages with `git ls-files -u`). The working
tree files contain the result of the merge operation; i.e. 3-way
merge results with familiar conflict markers `<<<` `===` `>>>`.
-5. A special ref `AUTO_MERGE` is written, pointing to a tree
+5. A ref named `AUTO_MERGE` is written, pointing to a tree
corresponding to the current content of the working tree (including
conflict markers for textual conflicts). Note that this ref is only
written when the 'ort' merge strategy is used (the default).
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index d8dbe6b56d..5d32ff2384 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -1344,7 +1344,7 @@ $ git diff --theirs file.txt # same as the above.
-------------------------------------------------
When using the 'ort' merge strategy (the default), before updating the working
-tree with the result of the merge, Git writes a special ref named AUTO_MERGE
+tree with the result of the merge, Git writes a ref named AUTO_MERGE
reflecting the state of the tree it is about to write. Conflicted paths with
textual conflicts that could not be automatically merged are written to this
tree with conflict markers, just as in the working tree. AUTO_MERGE can thus be
--
2.43.0-76-g1a87c842ec
^ permalink raw reply related
* [PATCH 3/5] refs.h: HEAD is not that special
From: Junio C Hamano @ 2023-12-15 20:32 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
In-Reply-To: <20231215203245.3622299-1-gitster@pobox.com>
In-code comment explains pseudorefs but used a wrong nomenclature
"special ref".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
refs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/refs.h b/refs.h
index 23211a5ea1..ff113bb12a 100644
--- a/refs.h
+++ b/refs.h
@@ -56,7 +56,7 @@ struct worktree;
* Even with RESOLVE_REF_ALLOW_BAD_NAME, names that escape the refs/
* directory and do not consist of all caps and underscores cannot be
* resolved. The function returns NULL for such ref names.
- * Caps and underscores refers to the special refs, such as HEAD,
+ * Caps and underscores refers to the pseudorefs, such as HEAD,
* FETCH_HEAD and friends, that all live outside of the refs/ directory.
*/
#define RESOLVE_REF_READING 0x01
--
2.43.0-76-g1a87c842ec
^ permalink raw reply related
* [PATCH 1/5] git.txt: HEAD is not that special
From: Junio C Hamano @ 2023-12-15 20:32 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
In-Reply-To: <20231215203245.3622299-1-gitster@pobox.com>
The introductory text in "git help git" that describes HEAD called
it "a special ref". It is special compared to the more regular refs
like refs/heads/master and refs/tags/v1.0.0, but not that special,
unlike truly special ones like FETCH_HEAD.
Rewrite a few sentences to also introduce the distinction between a
regular ref that contain the object name and a symbolic ref that
contain the name of another ref. Update the description of HEAD
that point at the current branch to use the more correct term, a
"symbolic ref".
This was found as part of auditing the documentation and in-code
comments for uses of "special ref" that refer merely a "pseudo ref".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git.txt | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 2535a30194..880cdc5d7f 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -1025,10 +1025,11 @@ When first created, objects are stored in individual files, but for
efficiency may later be compressed together into "pack files".
Named pointers called refs mark interesting points in history. A ref
-may contain the SHA-1 name of an object or the name of another ref. Refs
-with names beginning `ref/head/` contain the SHA-1 name of the most
+may contain the SHA-1 name of an object or the name of another ref (the
+latter is called a "symbolic ref").
+Refs with names beginning `ref/head/` contain the SHA-1 name of the most
recent commit (or "head") of a branch under development. SHA-1 names of
-tags of interest are stored under `ref/tags/`. A special ref named
+tags of interest are stored under `ref/tags/`. A symbolic ref named
`HEAD` contains the name of the currently checked-out branch.
The index file is initialized with a list of all paths and, for each
--
2.43.0-76-g1a87c842ec
^ permalink raw reply related
* [PATCH 0/5] make room for "special ref"
From: Junio C Hamano @ 2023-12-15 20:32 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
Patrick's reftable work is progressing nicely and wants to establish
"special ref" as a phrase with some defined meaning that is somewhat
different from a mere "pseudo ref".
A pseudo ref is merely a normal ref with a funny naming convention,
i.e., being outside the refs/ hierarchy and has names with all
uppercase letters (or an underscore). But there truly are refs that
are more than that. For example, FETCH_HEAD currently stores not
just a single object name, but can and is used to store multiple
object names, each with annotations to record where they came from.
There indeed may be a need to introduce a new term to refer to such
"special refs".
Existing documentation, however, uses "special ref" to refer to
pseudo refs without any "special" property, like FETCH_HEAD does.
This series merely corrects such existing uses of the word, to make
room for Patrick's series to introduce (and formally define in the
glossary) "special refs".
Junio C Hamano (5):
git.txt: HEAD is not that special
git-bisect.txt: BISECT_HEAD is not that special
refs.h: HEAD is not that special
docs: AUTO_MERGE is not that special
docs: MERGE_AUTOSTASH is not that special
Documentation/git-bisect.txt | 2 +-
Documentation/git-diff.txt | 2 +-
Documentation/git-merge.txt | 2 +-
Documentation/git.txt | 7 ++++---
Documentation/merge-options.txt | 2 +-
Documentation/user-manual.txt | 2 +-
refs.h | 2 +-
7 files changed, 10 insertions(+), 9 deletions(-)
--
2.43.0-76-g1a87c842ec
^ permalink raw reply
* [PATCH 2/5] git-bisect.txt: BISECT_HEAD is not that special
From: Junio C Hamano @ 2023-12-15 20:32 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
In-Reply-To: <20231215203245.3622299-1-gitster@pobox.com>
The description of "git bisect --no-checkout" called BISECT_HEAD a
"special ref", but there is nothing special about it. It merely is
yet another pseudoref.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git-bisect.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 191b4a42b6..aa02e46224 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -362,7 +362,7 @@ OPTIONS
--no-checkout::
+
Do not checkout the new working tree at each iteration of the bisection
-process. Instead just update a special reference named `BISECT_HEAD` to make
+process. Instead just update the reference named `BISECT_HEAD` to make
it point to the commit that should be tested.
+
This option may be useful when the test you would perform in each step
--
2.43.0-76-g1a87c842ec
^ permalink raw reply related
* Re: [PATCH] Use ^=1 to toggle between 0 and 1
From: Junio C Hamano @ 2023-12-15 17:09 UTC (permalink / raw)
To: Phillip Wood
Cc: Jeff King, René Scharfe, AtariDreams via GitGitGadget, git,
Seija Kijin
In-Reply-To: <99b3a727-36fd-4fa5-a6be-60ae6fc5911e@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
> Even if it unlikely that we would directly compare a boolean variable
> to "true" or "false" it is certainly conceivable that we'd compare two
> boolean variables directly. For the integer fallback to be safe we'd
> need to write
>
> if (!cond_a == !cond_b)
>
> rather than
>
> if (cond_a == cond_b)
Eek, it defeats the benefit of using true Boolean type if we had to
train ourselves to write the former, doesn't it?
> A weather-balloon seems like the safest route forward. We have been
> requiring C99 for two years now [1], hopefully there aren't any
> compilers out that claim to support C99 but don't provide
> "<stdbool.h>" (I did check online and the compiler on NonStop does
> support _Bool).
>
> Best Wishes
>
> Phillip
>
> [1] 7bc341e21b (git-compat-util: add a test balloon for C99 support,
> 2021-12-01)
Nice to be reminded of this one.
The cited commit does not start to use any specific feature from
C99, other than that we now require that the compiler claims C99
conformance by __STDC_VERSION__ set appropriately. The commit log
message says C99 "provides a variety of useful features, including
..., many of which we already use.", which implies that our wish was
to officially allow any and all features in C99 to be used in our
codebase after a successful flight of this test balloon.
Now, I think we saw a successful flight of this test balloon by now.
Is allowing all the C99 the next step we really want to take?
I still personally have an aversion against decl-after-statement and
//-comments, not due to portability reasons at all, but because I
find that the code is easier to read without it. But in principle,
it is powerful to be able to say "OK, as long as the feature is in
C99 you can use it", instead of having to decide on individual
features, and I am not fundamentally against going that route if it
is where people want to go.
Thanks.
^ permalink raw reply
* Re: [PATCH v4] tests: adjust whitespace in chainlint expectations
From: Junio C Hamano @ 2023-12-15 16:44 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Patrick Steinhardt, git
In-Reply-To: <CAPig+cSNZ75WSjxfJFoVPBTkv7yQpxGz1c4ugCyQRLytpNLjig@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
> Aside: I was rather surprised to see this output from git-am when applying v4:
>
> Applying: tests: adjust whitespace in chainlint expectations
> .git/rebase-apply/patch:205: new blank line at EOF.
> +
> .git/rebase-apply/patch:219: new blank line at EOF.
> +
> warning: 2 lines add whitespace errors.
>
> But upon investigating the two "test" files in question,
> dqstring-line-splice.test and dqstring-no-interpolate.test, I recalled
> that I had to play tricks to escape the single-quote context created
> by the Makefile when generating t/chainlinttmp/tests in order to allow
> chainlint.pl to see a double-quoted string. So, the abovementioned
> blank lines are indeed expected output from chainlint.pl given the
> tricks played.
Thanks for being extra careful while reviewing.
^ permalink raw reply
* Re: [PATCH v2 1/2] t5574: test porcelain output of atomic fetch
From: Junio C Hamano @ 2023-12-15 16:47 UTC (permalink / raw)
To: Jiang Xin; +Cc: Patrick Steinhardt, Git List, Jiang Xin
In-Reply-To: <CANYiYbGaJjnuVx7wJshgqiwvpGTmdq2JiOe4S_ph1bgiZ7XTJg@mail.gmail.com>
Jiang Xin <worldhello.net@gmail.com> writes:
>> Nit: you could also do `for opt in "--atomic" ""` directly to get rid of
>> this case statement. Not sure whether this is worth a reroll though,
>> probably not.
>
> Yes, your code is much simpler.
Yup, thanks for careful and helpful reviews, Patrick, on both
patches. And of course, thanks, Jiang, for working on them.
^ permalink raw reply
* Re: What's the recommendation for forgetting all rerere's records?
From: Junio C Hamano @ 2023-12-15 16:30 UTC (permalink / raw)
To: Sean Allred; +Cc: git
In-Reply-To: <m0sf43abw7.fsf@epic96565.epic.com>
Sean Allred <allred.sean@gmail.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>> Sean Allred <allred.sean@gmail.com> writes:
>>> When outside the context of a conflict (no rebase/merge in progress),
>>> what's the intended workflow to clear out the contents of
>>> $GIT_DIR/rr-cache?
>>
>> You could "rm -fr .git/rr-cache/??*" if you want.
>
> Here's my reasoning for not wanting that:
>
>>> I'm wary of recommending `rm -rf "$(git rev-parse --git-dir)/rr-cache"`
>>> -- it's hard to describe this as anything but hacky when the rest of the
>>> experience is handled in porcelain.
It is meant to be ugly ;-); the reason why the Porcelain does not
offer bulk erasure is because we want to strongly discourage it.
>> The "intended" workflow is there will no need to "clear out" at all;
>> you may notice mistaken resolution, and you will remove it when you
>> notice one, but the bulk of the remaining database entries ought to
>> be still correct.
>
> When we noticed mistaken resolutions, rerere had already applied the
> recorded resolution and there was no apparent way to return to the
> conflicted state.
The simplest is to go back to the original state before the merge
and then redo the operation without rerere enabled.
$ git reset --hard
$ git -c rerere.enabled=no merge <whatever arguments you used>
And you can redo the merge manually.
But more generally, after you incorrectly resolved a merge conflict,
whether you fumbled with your editor yourself or you let rerere kick
in to reuse your recorded resolution that you made in the past that
was faulty, before or after you run "git add" to tell the resolution
to the index, you should be able to do
$ git checkout --merge <pathspec>
to tell Git to "unresolve" such paths. Here is the relevant
paragraph from the "git checkout --help":
When checking out paths from the index, this option lets you recreate the
conflicted merge in the specified paths. This option cannot be used when
checking out paths from a tree-ish.
The below is what I just did to demonstrate. You can try the same
if you have our source code. The first attempt will likely to
conflict because you do not have the rerere record, but you can
resolve the conflict in builtin/mv.c the way I did (shown by first
"git diff" in the sample transcript), and run "git rerere" (or just
"git commit -a" should also work) to record the resolution, and then
"git reset --hard" to obtain a sample rerere record you can use to
practice.
# Just a sample merge I know will have a conflict
$ SAMPLE=a59dbae0b3bd; # v2.43.0-rc0~126
# Go to its parent and retry the merge with its second parent
$ git checkout --detach $SAMPLE^1
$ git merge $SAMPLE^2
Auto-merging builtin/mv.c
CONFLICT (content): Merge conflict in builtin/mv.c
Resolved 'builtin/mv.c' using previous resolution.
Automatic merge failed; fix conflicts and then commit the result.
# We have conflict, and rerere kicked in. Because I do not have
# rerere.autoupdate configuration set, I can as "ls-files -u"
# which paths are conflicting, but that is unnecessary (we
# can see the path with conflict with the CONFLICT label above).
$ git ls-files -u
100644 665bd274485f6c76403e9230539e2650073a47f3 1 builtin/mv.c
100644 05e7156034e04d637990cabf105f7fa967b0f2aa 2 builtin/mv.c
100644 80fc7a3c7029603a0fcaf9d15d8432ed799b4909 3 builtin/mv.c
# This is the resolution "rerere" gave me, which is what I did
# back in August this year. If you are following this example,
# you'll first have to edit builtin/mv.c to hand resolve,
# register the resolution to "rerere" database, and then run
# "git reset --hard" to go back to the state before you did the
# "git merge $SAMPLE^2" step to repeat.
$ git diff
diff --cc builtin/mv.c
index 05e7156034,80fc7a3c70..0000000000
--- i/builtin/mv.c
+++ w/builtin/mv.c
@@@ -304,8 -303,8 +304,8 @@@ int cmd_mv(int argc, const char **argv
goto act_on_entry;
}
if (S_ISDIR(st.st_mode)
- && lstat(dst, &st) == 0) {
+ && lstat(dst, &dest_st) == 0) {
- bad = _("cannot move directory over file");
+ bad = _("destination already exists");
goto act_on_entry;
}
# Now the fun command you seem to have missed. You MUST give
# "git checkout --merge" a pathspec. I do not encourage it but
# using "." to say "unresolve everything under the sun" should
# also work.
$ git checkout --merge builtin/mv.c
Recreated 1 merge conflict
# Let's check the result. We have recreated the conflicted
# state in the working tree.
$ git diff
diff --cc builtin/mv.c
index 05e7156034,80fc7a3c70..0000000000
--- i/builtin/mv.c
+++ w/builtin/mv.c
@@@ -304,8 -303,8 +304,16 @@@ int cmd_mv(int argc, const char **argv
goto act_on_entry;
}
if (S_ISDIR(st.st_mode)
++<<<<<<< ours
+ && lstat(dst, &dest_st) == 0) {
+ bad = _("cannot move directory over file");
++||||||| base
++ && lstat(dst, &st) == 0) {
++ bad = _("cannot move directory over file");
++=======
+ && lstat(dst, &st) == 0) {
+ bad = _("destination already exists");
++>>>>>>> theirs
goto act_on_entry;
}
# You should then be able to correct the resolution with your
# editor.
$ edit builtin/mv.c
# If this is one-time fix (you are happy with the original
# resolution and wanted to deviate from it only once this time),
# there is nothing else need to be done. If you want to record
# this as a new resolution, you'd get rid of the old one and
# record this one.
$ git rerere forget builtin/mv.c
$ git rerere
^ permalink raw reply
* Re: [PATCH 00/24] pack-objects: multi-pack verbatim reuse
From: Taylor Blau @ 2023-12-15 15:37 UTC (permalink / raw)
To: Jeff King; +Cc: git, Patrick Steinhardt, Junio C Hamano
In-Reply-To: <20231212081238.GD1117953@coredump.intra.peff.net>
On Tue, Dec 12, 2023 at 03:12:38AM -0500, Jeff King wrote:
> So my question is: how much of what you're seeing is from (1) and (2),
> and how much is from (3)? Because there are other ways to trigger (3),
> such as lowering the window size. For example, if you try your same
> packing example with --window=0, how do the CPU and output size compare
> to the results of your series? (I'd also check peak memory usage).
Interesting question! Here are some preliminary numbers on my machine
(which runs Debian unstable with a Intel Xenon W-2255 CPU @ 3.70GHz and
64GB of RAM).
I ran the following hyperfine command on my testing repository, which
has the Git repository broken up into ~75 packs or so:
$ hyperfine -L v single,multi -L window 0,10 \
--show-output \
-n '{v}-pack reuse, pack.window={window}' \
'git.compile \
-c pack.allowPackReuse={v} \
-c pack.window={window} \
pack-objects --revs --stdout --use-bitmap-index --delta-base-offset <in 2>/dev/null | wc -c'
Which gave the following results for runtime:
Benchmark 1: single-pack reuse, pack.window=0
[...]
Time (mean ± σ): 1.248 s ± 0.004 s [User: 1.160 s, System: 0.188 s]
Range (min … max): 1.244 s … 1.259 s 10 runs
Benchmark 2: multi-pack reuse, pack.window=0
[...]
Time (mean ± σ): 1.075 s ± 0.005 s [User: 0.990 s, System: 0.188 s]
Range (min … max): 1.071 s … 1.088 s 10 runs
Benchmark 3: single-pack reuse, pack.window=10
[...]
Time (mean ± σ): 6.281 s ± 0.024 s [User: 43.727 s, System: 0.492 s]
Range (min … max): 6.252 s … 6.326 s 10 runs
Benchmark 4: multi-pack reuse, pack.window=10
[...]
Time (mean ± σ): 1.028 s ± 0.002 s [User: 1.150 s, System: 0.184 s]
Range (min … max): 1.026 s … 1.032 s 10 runs
Here are the average numbers for the resulting "clone" size in each of
the above configurations:
Benchmark 1: single-pack reuse, pack.window=0
264.443 MB
Benchmark 2: multi-pack reuse, pack.window=0
268.670 MB
Benchmark 3: single-pack reuse, pack.window=10
194.355 MB
Benchmark 4: multi-pack reuse, pack.window=10
266.473 MB
So it looks like setting pack.window=0 (with both single and multi-pack
reuse) yields a similarly sized pack output as multi-pack reuse with any
window setting.
Running the same benchmark as above again, but this time sending the
pack output to /dev/null and instead capturing the maximum RSS value
from `/usr/bin/time -v` gives us the following (averages, in MB):
Benchmark 1: single-pack reuse, pack.window=0
354.224 MB (max RSS)
Benchmark 2: multi-pack reuse, pack.window=0
315.730 MB (max RSS)
Benchmark 3: single-pack reuse, pack.window=10
470.651 MB (max RSS)
Benchmark 4: multi-pack reuse, pack.window=10
328.786 MB (max RSS)
So memory usage is similar between runs except for the single-pack reuse
case with a window size of 10.
It looks like the sweet spot is probably multi-pack reuse with a
small-ish window size, which achieves the best of both worlds (small
pack size, relative to other configurations that reuse large portions of
the pack, and low memory usage).
It's pretty close between multi-pack reuse with a window size of 0 and
a window size of 10. If you want to optimize for pack size, you could
trade a ~4% reduction in pack size for a ~1% increase in peak memory
usage.
Of course, YMMV depending on the repository, packing strategy, and
pack.window configuration (among others) while packing. But this should
give you a general idea of what to expect.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH] Use ^=1 to toggle between 0 and 1
From: Phillip Wood @ 2023-12-15 14:46 UTC (permalink / raw)
To: Jeff King, René Scharfe
Cc: AtariDreams via GitGitGadget, git, Seija Kijin
In-Reply-To: <20231214220503.GA3320432@coredump.intra.peff.net>
On 14/12/2023 22:05, Jeff King wrote:
> On Thu, Dec 14, 2023 at 02:08:31PM +0100, René Scharfe wrote:
>
>>> I don't even know that we'd need much of a weather-balloon patch. I
>>> think it would be valid to do:
>>>
>>> #ifndef bool
>>> #define bool int
>>>
>>> to handle pre-C99 compilers (if there even are any these days). Of
>>> course we probably need some conditional magic to try to "#include
>>> <stdbool.h>" for the actual C99. I guess we could assume C99 by default
>>> and then add NO_STDBOOL as an escape hatch if anybody complains.
>>
>> The semantics are slightly different in edge cases, so that fallback
>> would not be fully watertight. E.g. consider:
>>
>> bool b(bool cond) {return cond == true;}
>> bool b2(void) {return b(2);}
Thanks for bring this up René, I had similar concerns when I saw the
suggestion of using "int" as a fallback.
> Yeah. b2() is wrong for passing "2" to a bool.
I think it depends what you mean by "wrong" §6.3.1.2 of standard is
quite clear that when any non-zero scalar value is converted to _Bool
the result is "1"
> I assumed that the
> compiler would warn of that (at least for people on modern C99
> compilers, not the fallback code), but it doesn't seem to. It's been a
> long time since I've worked on a code base that made us of "bool", but I
> guess that idea is that silently coercing a non-zero int to a bool is
> reasonable in many cases (e.g., "bool found_foo = count_foos()").
I guess it is also consistent with the way "if" and "while" consider a
non-zero scalar value to be "true".
> I guess one could argue that b() is also sub-optimal, as it should just
> say "return cond" or "return !cond" rather than explicitly comparing to
> true/false. But I won't be surprised if it happens from time to time.
Even if it unlikely that we would directly compare a boolean variable to
"true" or "false" it is certainly conceivable that we'd compare two
boolean variables directly. For the integer fallback to be safe we'd
need to write
if (!cond_a == !cond_b)
rather than
if (cond_a == cond_b)
>> A coding rule to not compare bools could mitigate that. Or a rule to
>> only use the values true and false in bool context and to only use
>> logical operators on them.
>
> That seems more complex than we want if our goal is just supporting
> legacy systems that may or may not even exist. Given your example, I'd
> be more inclined to just do a weather-balloon adding <stdbool.h> to
> git-compat-util.h, and using "bool" in a single spot in the code. If
> nobody screams after a few releases, we can consider it OK. If they do,
> it's a trivial patch to convert back.
A weather-balloon seems like the safest route forward. We have been
requiring C99 for two years now [1], hopefully there aren't any
compilers out that claim to support C99 but don't provide "<stdbool.h>"
(I did check online and the compiler on NonStop does support _Bool).
Best Wishes
Phillip
[1] 7bc341e21b (git-compat-util: add a test balloon for C99 support,
2021-12-01)
^ permalink raw reply
* Bug | Documentation | git add -all | Synopsis has minor mistake
From: Benjamin Lehmann @ 2023-12-15 12:38 UTC (permalink / raw)
To: git
Hey.
The mistake can be found in the synopsis here:
https://git-scm.com/docs/git-add#Documentation/git-add.txt--A
In the synopsys, the options -all currently reads:
[--[no-]all | --[no-]ignore-removal |
You can see that there is no mention of -A, which is the main way that
people would use -all perhaps, so it really ought to be included
correctly in the synopsis. In addition, the closing square-bracket is
missing.
Hope this was the right place to report this - seemed to be the only option.
Ben
^ permalink raw reply
* Re: What's the recommendation for forgetting all rerere's records?
From: Sean Allred @ 2023-12-15 12:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sean Allred, git
In-Reply-To: <xmqqcyvgz3ih.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Sean Allred <allred.sean@gmail.com> writes:
>> When outside the context of a conflict (no rebase/merge in progress),
>> what's the intended workflow to clear out the contents of
>> $GIT_DIR/rr-cache?
>
> You could "rm -fr .git/rr-cache/??*" if you want.
Here's my reasoning for not wanting that:
>> I'm wary of recommending `rm -rf "$(git rev-parse --git-dir)/rr-cache"`
>> -- it's hard to describe this as anything but hacky when the rest of the
>> experience is handled in porcelain.
> The "intended" workflow is there will no need to "clear out" at all;
> you may notice mistaken resolution, and you will remove it when you
> notice one, but the bulk of the remaining database entries ought to
> be still correct.
When we noticed mistaken resolutions, rerere had already applied the
recorded resolution and there was no apparent way to return to the
conflicted state. If clearing out the rerere database was never an
intended workflow here, maybe _that's_ my actual question?
It seems likely this 'recovery' workflow should just be documented in
git-rerere.txt (which I'm happy to take on once I learn what that
workflow should be).
--
Sean Allred
^ permalink raw reply
* Re: [PATCH v2 1/2] t5574: test porcelain output of atomic fetch
From: Jiang Xin @ 2023-12-15 11:16 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Git List, Junio C Hamano, Jiang Xin
In-Reply-To: <ZXwi2MA-KUxszfGj@tanuki>
On Fri, Dec 15, 2023 at 5:56 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Thu, Dec 14, 2023 at 08:33:11PM +0800, Jiang Xin wrote:
> > From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> [snip]
> > @@ -91,36 +85,61 @@ test_expect_success 'fetch porcelain output' '
> > git checkout force-updated &&
> > git reset --hard HEAD~ &&
> > test_commit --no-tag force-update-new &&
> > - FORCE_UPDATED_NEW=$(git rev-parse HEAD) &&
> > -
> > - cat >expect <<-EOF &&
> > - - $MAIN_OLD $ZERO_OID refs/forced/deleted-branch
> > - - $MAIN_OLD $ZERO_OID refs/unforced/deleted-branch
> > - $MAIN_OLD $FAST_FORWARD_NEW refs/unforced/fast-forward
> > - ! $FORCE_UPDATED_OLD $FORCE_UPDATED_NEW refs/unforced/force-updated
> > - * $ZERO_OID $MAIN_OLD refs/unforced/new-branch
> > - $MAIN_OLD $FAST_FORWARD_NEW refs/forced/fast-forward
> > - + $FORCE_UPDATED_OLD $FORCE_UPDATED_NEW refs/forced/force-updated
> > - * $ZERO_OID $MAIN_OLD refs/forced/new-branch
> > - $MAIN_OLD $FAST_FORWARD_NEW refs/remotes/origin/fast-forward
> > - + $FORCE_UPDATED_OLD $FORCE_UPDATED_NEW refs/remotes/origin/force-updated
> > - * $ZERO_OID $MAIN_OLD refs/remotes/origin/new-branch
> > - EOF
> > -
> > - # Execute a dry-run fetch first. We do this to assert that the dry-run
> > - # and non-dry-run fetches produces the same output. Execution of the
> > - # fetch is expected to fail as we have a rejected reference update.
> > - test_must_fail git -C porcelain fetch \
> > - --porcelain --dry-run --prune origin $refspecs >actual &&
> > - test_cmp expect actual &&
> > -
> > - # And now we perform a non-dry-run fetch.
> > - test_must_fail git -C porcelain fetch \
> > - --porcelain --prune origin $refspecs >actual 2>stderr &&
> > - test_cmp expect actual &&
> > - test_must_be_empty stderr
> > + FORCE_UPDATED_NEW=$(git rev-parse HEAD)
> > '
> >
> > +for opt in off on
> > +do
> > + case $opt in
> > + on)
> > + opt=--atomic
> > + ;;
> > + off)
> > + opt=
> > + ;;
> > + esac
>
> Nit: you could also do `for opt in "--atomic" ""` directly to get rid of
> this case statement. Not sure whether this is worth a reroll though,
> probably not.
Yes, your code is much simpler.
^ permalink raw reply
* Re: [PATCH v2 1/2] t5574: test porcelain output of atomic fetch
From: Patrick Steinhardt @ 2023-12-15 9:56 UTC (permalink / raw)
To: Jiang Xin; +Cc: Git List, Junio C Hamano, Jiang Xin
In-Reply-To: <210191917bcfa9293622908c291652059576f3e5.1702556642.git.zhiyou.jx@alibaba-inc.com>
[-- Attachment #1: Type: text/plain, Size: 2011 bytes --]
On Thu, Dec 14, 2023 at 08:33:11PM +0800, Jiang Xin wrote:
> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
[snip]
> @@ -91,36 +85,61 @@ test_expect_success 'fetch porcelain output' '
> git checkout force-updated &&
> git reset --hard HEAD~ &&
> test_commit --no-tag force-update-new &&
> - FORCE_UPDATED_NEW=$(git rev-parse HEAD) &&
> -
> - cat >expect <<-EOF &&
> - - $MAIN_OLD $ZERO_OID refs/forced/deleted-branch
> - - $MAIN_OLD $ZERO_OID refs/unforced/deleted-branch
> - $MAIN_OLD $FAST_FORWARD_NEW refs/unforced/fast-forward
> - ! $FORCE_UPDATED_OLD $FORCE_UPDATED_NEW refs/unforced/force-updated
> - * $ZERO_OID $MAIN_OLD refs/unforced/new-branch
> - $MAIN_OLD $FAST_FORWARD_NEW refs/forced/fast-forward
> - + $FORCE_UPDATED_OLD $FORCE_UPDATED_NEW refs/forced/force-updated
> - * $ZERO_OID $MAIN_OLD refs/forced/new-branch
> - $MAIN_OLD $FAST_FORWARD_NEW refs/remotes/origin/fast-forward
> - + $FORCE_UPDATED_OLD $FORCE_UPDATED_NEW refs/remotes/origin/force-updated
> - * $ZERO_OID $MAIN_OLD refs/remotes/origin/new-branch
> - EOF
> -
> - # Execute a dry-run fetch first. We do this to assert that the dry-run
> - # and non-dry-run fetches produces the same output. Execution of the
> - # fetch is expected to fail as we have a rejected reference update.
> - test_must_fail git -C porcelain fetch \
> - --porcelain --dry-run --prune origin $refspecs >actual &&
> - test_cmp expect actual &&
> -
> - # And now we perform a non-dry-run fetch.
> - test_must_fail git -C porcelain fetch \
> - --porcelain --prune origin $refspecs >actual 2>stderr &&
> - test_cmp expect actual &&
> - test_must_be_empty stderr
> + FORCE_UPDATED_NEW=$(git rev-parse HEAD)
> '
>
> +for opt in off on
> +do
> + case $opt in
> + on)
> + opt=--atomic
> + ;;
> + off)
> + opt=
> + ;;
> + esac
Nit: you could also do `for opt in "--atomic" ""` directly to get rid of
this case statement. Not sure whether this is worth a reroll though,
probably not.
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