* [PATCH] commit: detect commits that exist in commit-graph but not in the ODB
From: Patrick Steinhardt @ 2023-10-13 12:37 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Junio C Hamano, Taylor Blau
In-Reply-To: <ZSkCGS3JPEQ71dOF@tanuki>
[-- Attachment #1: Type: text/plain, Size: 4246 bytes --]
Commit graphs can become stale and contain references to commits that do
not exist in the object database anymore. Theoretically, this can lead
to a scenario where we are able to successfully look up any such commit
via the commit graph even though such a lookup would fail if done via
the object database directly.
As the commit graph is mostly intended as a sort of cache to speed up
parsing of commits we do not want to have diverging behaviour in a
repository with and a repository without commit graphs, no matter
whether they are stale or not. As commits are otherwise immutable, the
only thing that we really need to care about is thus the presence or
absence of a commit.
To address potentially stale commit data that may exist in the graph,
our `lookup_commit_in_graph()` function will check for the commit's
existence in both the commit graph, but also in the object database. So
even if we were able to look up the commit's data in the graph, we would
still pretend as if the commit didn't exist if it is missing in the
object database.
We don't have the same safety net in `parse_commit_in_graph_one()`
though. This function is mostly used internally in "commit-graph.c"
itself to validate the commit graph, and this usage is fine. We do
expose its functionality via `parse_commit_in_graph()` though, which
gets called by `repo_parse_commit_internal()`, and that function is in
turn used in many places in our codebase.
For all I can see this function is never used to directly turn an object
ID into a commit object without additional safety checks before or after
this lookup. What it is being used for though is to walk history via the
parent chain of commits. So when commits in the parent chain of a graph
walk are missing it is possible that we wouldn't notice if that missing
commit was part of the commit graph. Thus, a query like `git rev-parse
HEAD~2` can succeed even if the intermittent commit is missing.
It's unclear whether there are additional ways in which such stale
commit graphs can lead to problems. In any case, it feels like this is a
bigger bug waiting to happen when we gain additional direct or indirect
callers of `repo_parse_commit_internal()`. So let's fix the inconsistent
behaviour by checking for object existence via the object database, as
well.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
commit.c | 7 ++++++-
t/t5318-commit-graph.sh | 23 +++++++++++++++++++++++
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/commit.c b/commit.c
index b3223478bc..109e9217e3 100644
--- a/commit.c
+++ b/commit.c
@@ -572,8 +572,13 @@ int repo_parse_commit_internal(struct repository *r,
return -1;
if (item->object.parsed)
return 0;
- if (use_commit_graph && parse_commit_in_graph(r, item))
+ if (use_commit_graph && parse_commit_in_graph(r, item)) {
+ if (!has_object(r, &item->object.oid, 0))
+ return quiet_on_missing ? -1 :
+ error(_("commit %s exists in commit-graph but not in the object database"),
+ oid_to_hex(&item->object.oid));
return 0;
+ }
if (oid_object_info_extended(r, &item->object.oid, &oi, flags) < 0)
return quiet_on_missing ? -1 :
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index ba65f17dd9..25f8e9e2d3 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -821,4 +821,27 @@ test_expect_success 'overflow during generation version upgrade' '
)
'
+test_expect_success 'commit exists in commit-graph but not in object database' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+
+ test_commit A &&
+ test_commit B &&
+ test_commit C &&
+ git commit-graph write --reachable &&
+
+ # Corrupt the repository by deleting the intermittent commit
+ # object. Commands should notice that this object is absent and
+ # thus that the repository is corrupt even if the commit graph
+ # exists.
+ oid=$(git rev-parse B) &&
+ rm .git/objects/"$(test_oid_to_path "$oid")" &&
+
+ test_must_fail git rev-parse HEAD~2 2>error &&
+ grep "error: commit $oid exists in commit-graph but not in the object database" error
+ )
+'
+
test_done
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: [RFC] Define "precious" attribute and support it in `git clean`
From: Oswald Buddenhagen @ 2023-10-13 11:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sebastian Thiel, git, Josh Triplett, Kristoffer Haugsbakk
In-Reply-To: <xmqqttqvg4lw.fsf@gitster.g>
On Thu, Oct 12, 2023 at 09:58:19AM -0700, Junio C Hamano wrote:
>I do not think it will be the end of the world if we don't do so,
>but it would be really really nice if we at least explored a way (or
>two) to make a big enough hole in the syntax to not just add
>"precious", but leave room to later add other traits, without having
>to worry about breaking the backward compatibility again.
>
that would invariably make the syntax more verbose, for dubious gain.
that the extension we're deliberating now (again) was coming (in some
form) was clear for quite a while, while i'm not aware of anything else
that would semantically fit gitignore (*). "other traits" sounds awfully
like scope creep, and would most likely fit gitattributes better.
anyway, such a hypothetical "breaking" change wouldn't have much impact,
because versioned files aren't affected by gitignore. and for the
misclassification to be actually harmful, the user would have to be
unable to notice or correct it.
(*) this got me thinking about things that would fit, and i came up with
a modification of the proposal: one might want to specify just *how*
precious a file is (which i guess would translate to how many times the
extra override option would have to be passed to git-clean). (**)
i guess a suitable syntax for that would be
2>.config
note that even though using the dollar sign to denote "precious" is kind
of intuitive, i'm not using it for two reasons: a) it's not "crazy"
enough to use it at not quite the beginning of a file name (note that
traditionally it isn't even special on windows), and b) the visual
separation of the prefix isn't as good as with the "arrow-like"
character.
(**) actually, one would probably want proper type tagging (e.g., config
files vs. autotools-generated files (which do not belong into a repo,
but do into a tar-ball)). that really does sound a lot like
gitattributes, only that the files aren't versioned.
regards
^ permalink raw reply
* Re: [RFC] Define "precious" attribute and support it in `git clean`
From: Phillip Wood @ 2023-10-13 10:06 UTC (permalink / raw)
To: Junio C Hamano, Sebastian Thiel; +Cc: git, Josh Triplett, Kristoffer Haugsbakk
In-Reply-To: <xmqqttqvg4lw.fsf@gitster.g>
On 12/10/2023 17:58, Junio C Hamano wrote:
> Sebastian Thiel <sebastian.thiel@icloud.com> writes:
Sebastian - thanks for raising this again, it would be really good to
get a solution for handling "ignored but not expendable" files
> I have been regretting that I did not make the quoting syntax not
> obviously extensible in f87f9497 (git-ls-files: --exclude mechanism
> updates., 2005-07-24), which technically was a breaking change (as a
> relative pathname that began with '!' were not special, but after
> the change, it became necessary to '\'-quote it). A relative
> pathname that begins with '$' would be now broken the same way, but
> hopefully the fallout would be minor. I presume you picked '$'
> exactly because of this reason?
>
> I do not think it will be the end of the world if we don't do so,
> but it would be really really nice if we at least explored a way (or
> two) to make a big enough hole in the syntax to not just add
> "precious", but leave room to later add other traits, without having
> to worry about breaking the backward compatibility again. A
> simplest and suboptimal way may be to declare that a path that
> begins with '$' now needs '\'-quoting (just like your proposal),
> reserve '$$' as the precious prefix, and '$' followed by any other
> byte reserved for future use, but there may be better ideas.
One thought I had was that we could abuse the comment syntax to annotate
paths something like
#(keep)
/my-precious-file
would prevent /my-precious-file from being deleted by git clean (and
hopefully unpack-trees()[1]). It means that older versions of git would
treat the file as ignored. If we ever want more than one annotation per
path we could separate them with commas
#(keep,something-else)
/my-file
Strictly speaking it is a backward incompatible change but I doubt there
are many people using comments like that. I also wondered about some
kind of suffix on the file
/my-precious-file #(keep)
but that means that older versions of git would not ignore the file.
Best Wishes
Phillip
[1] Of the cases listed in [2] it is "git checkout" and friends
overwriting ignored files that I worry about more. At least "git clean"
defaults to a dry-run and has in interactive mode to select what gets
deleted.
[2] https://lore.kernel.org/git/xmqqttqytnqb.fsf@gitster.g
^ permalink raw reply
* Re: [RFC] Define "precious" attribute and support it in `git clean`
From: Sebastian Thiel @ 2023-10-13 9:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Josh Triplett, Kristoffer Haugsbakk
In-Reply-To: <xmqqttqvg4lw.fsf@gitster.g>
On 12 Oct 2023, at 18:58, Junio C Hamano wrote:
> I presume you picked '$' > exactly because of this reason?
Yes, and because I thought '$' seems a great fit to represent value.
> I do not think it will be the end of the world if we don't do so,
> but it would be really really nice if we at least explored a way (or
> two) to make a big enough hole in the syntax to not just add
> "precious", but leave room to later add other traits, without having
> to worry about breaking the backward compatibility again. A
> simplest and suboptimal way may be to declare that a path that
> begins with '$' now needs '\'-quoting (just like your proposal),
> reserve '$$' as the precious prefix, and '$' followed by any other
> byte reserved for future use, but there may be better ideas.
Even though I'd love to go with the unextensible option assuming it would last
another 15 years, I can see the appeal of making it extensible from the start.
In a world where '$' is a prefix, I'd also think that it's now possible to
specify exclusion using '$!path' for completeness, if '$$path' marks 'path'
precious.
But if there is now a prefix, I feel that it might as well be chosen so that it
is easier to remember and/or less likely to cause conflicts. I think it must
have been that reason for pathspecs to choose ':' as their prefix, and it seems
to be an equally good choice here.
This would give us the following, taking the Linux kernel as example:
.*
!this-file-is-hidden-and-tracked
:!new-syntax-for-negation-for-completeness
\!an-ignored-file-with-leading-!
\:an-ignored-file-with-leading-:-which-is-technically-breaking
:$.config
:x-invalid-as-:-needs-either-!-or-$-to-follow-it
Now ':$path' would make any path precious, which is `:$.config` in the example
above.
How does that 'feel'? Is the similarity to pathspecs without being pathspecs
an anti-feature maybe?
>> Thus, to make this work, projects that ship the `.gitignore` files would *have
>> to add patterns* that make certain files precious.
>
> Not really. They do not have to do anything if they are content
> with the current Git ecosystem. And users who have precious stuff
> can mark them in the.git/info/excludes no?
Yes, but only if they control all the ignore patterns in their global files. If
the repository decides to exclude a file they deem precious, now it won't be
precious anymore as their ':$make-this-precious' pattern is seen sequentially
after the pattern in the repository.
For instance, tooling-specific ignores are typically fully controlled by the
user, like '/.idea/', which could now easily be made precious with ':$/idea/'.
But as the Linux kernel repository ships with a '.gitignore' file that includes
the '.*' pattern, users won't be able to 'get ahead' of that pattern with their
':$.config' specification.
> The only case that is
> problematic is when the project says 'foo' is ignored and expendable
> but the user thinks otherwise. So to make this work, projects that
> ship the ".gitignore" files have to avoid adding patterns to ignore
> things that it may reasonably be expected for its users to mark
> precious.
Yes, I think my paragraph above is exactly that but with examples to practice
the new syntax-proposal.
>
>> Such opted-in projects would produce `.gitignore` files like these:
>>
>> .*
>> $.config
>
> I would understand if you ignored "*~" or "*.o", but why ignore ".*"?
I don't have an answer, the example is from the Linux Kernel repository was
added in 1e65174a33784 [1].
I am definitely getting excited about the progress the syntax is making :),
thanks for proposing it!
[ Reference ]
1. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1e65174a33784
^ permalink raw reply
* Re: [PATCH 0/3] rev-list: add support for commits in `--missing`
From: Patrick Steinhardt @ 2023-10-13 8:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Karthik Nayak, git
In-Reply-To: <ZSjbYCXfSUtEIkAt@tanuki>
[-- Attachment #1: Type: text/plain, Size: 3996 bytes --]
On Fri, Oct 13, 2023 at 07:53:36AM +0200, Patrick Steinhardt wrote:
> On Thu, Oct 12, 2023 at 09:17:09AM -0700, Junio C Hamano wrote:
> > Patrick Steinhardt <ps@pks.im> writes:
[snip]
> > My assumption also has been that there is no point in running
> > "rev-list --missing" if we know there is no repository corruption,
> > and those who run "rev-list --missing" wants to know if the objects
> > are really available, i.e. even if commit-graph that is out of sync
> > with reality says it exists, if it is not in the object store, they
> > would want to know that.
> >
> > If you can show me that it is not the case, then I may be pursuaded
> > why producing a result that is out of sync with reality _quickly_,
> > instead of taking time to produce a result that matches reality, is
> > a worthy "optimization" to keep.
>
> Note that I'm not saying that it's fine to return wrong results -- this
> is of course a bug that needs to be addressed somehow. After all, things
> working correctly should always trump things working fast. But until now
> it felt more like we were going into the direction of disabling commit
> graphs without checking whether there is an alternative solution that
> allows us to get the best of both worlds, correctness and performance.
>
> So what I'm looking for in this thread is a reason why we _can't_ have
> that, or at least can't have it without unreasonable amounts of work. We
> have helpers like `lookup_commit_in_graph()` that are designed to detect
> stale commit graphs by double-checking whether a commit that has been
> looked up via the commit graph actually exists in the repository. So I'm
> wondering whether this could help us address the issue.
>
> If there is a good reason why all of that is not possible then I'm happy
> to carve in.
I've had a quick look at this problem so that I can solidify my own
train of thought a bit. The issue is `repo_parse_commit_internal()`,
which calls `parse_commit_in_graph()` without verifying that the object
actually exists in the object database. It's the only callsite of that
function outside of "commit-graph.c", as all other external callers
would call `lookup_commit_in_graph()` which _does_ perform the object
existence check.
So I think that the proper way to address the regression would be a
patch similar to the following:
diff --git a/commit.c b/commit.c
index b3223478bc..109e9217e3 100644
--- a/commit.c
+++ b/commit.c
@@ -572,8 +572,13 @@ int repo_parse_commit_internal(struct repository *r,
return -1;
if (item->object.parsed)
return 0;
- if (use_commit_graph && parse_commit_in_graph(r, item))
+ if (use_commit_graph && parse_commit_in_graph(r, item)) {
+ if (!has_object(r, &item->object.oid, 0))
+ return quiet_on_missing ? -1 :
+ error(_("commit %s exists in commit-graph but not in the object database"),
+ oid_to_hex(&item->object.oid));
return 0;
+ }
if (oid_object_info_extended(r, &item->object.oid, &oi, flags) < 0)
return quiet_on_missing ? -1 :
I wouldn't be surprised if there are other edge cases where this can
lead to buggy behaviour.
Also, this issue may not necessarily stem from repository corruption. It
could for example happen that commits are getting garbage collected
without the commit graph having been updated, whatever the reason may be
for this. In that case we would happily continue to return these commits
from the commit graph even though the underlying object has since been
deleted. The repository itself is not corrupt though, we merely look at
an out-of-date commit graph. And for what it's worth, I think that we
should always gracefully handle that case or otherwise the commit graph
becomes less useful overall.
I didn't dig much deeper yet. And while the above patch fixes some of
the test failures, it doesn't fix them all. If we agree that this is the
way to go then I'd be happy to turn this into a proper patch.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: -q option for git to suppress informational messages?
From: Michal Suchánek @ 2023-10-13 6:49 UTC (permalink / raw)
To: Emily Shaffer; +Cc: git
In-Reply-To: <CAJoAoZ=MrjjfH6Noganejey0bAaB=d+jH_rXAqbscPG8E0m3Pw@mail.gmail.com>
Hello,
On Thu, Oct 12, 2023 at 01:46:09PM -0700, Emily Shaffer wrote:
> On Thu, Oct 12, 2023 at 11:53 AM Michal Suchánek <msuchanek@suse.de> wrote:
> >
> > Hello,
> >
> > when using git in scripts I find that many git commands insist on
> > printing informational messages, and the only way to avoid this is to
> > sent the output to /dev/null.
>
> Michal, you might want to investigate further whether there is a
> plumbing-specific command that meets the needs you're concerned about
> instead. Those commands will not have additional human-facing output,
> and have a stronger guarantee around backwards-compatibility than the
> human-facing commands do.
>
> Commands which count as "plumbing" can be located in `git help git` in
> the full list of subcommands; if you're not sure what alternative to
> use, I think you can feel free to describe what you're trying to do
> here and get advice on which plumbing commands to use instead of
> porcelain ones.
I want to replicate the repository data between multiple machines.
Git is 'distributed' VCS but AFAICS it supports replicating repositories
very poorly.
In one direction there is git push which has convenient --mirror option
and also can be scripted to replicate only selected references. AFAICS
there is no lower level command that can be used. There are commands for
uploading data to the remote repository but only push updates
references, no -q option.
In the other direction there is git remote. Of course there is no
cross-reference from git push, cross-references are for losers that
don't know everything about the thing. There are lower level commands
that could be used instead I suppose. Commands for downloading data are
available as well as for updating local references, and for one fetch
does provide a -q option. Also unlike push git remote cannot operate on
commandline arguments alone, it requires configuration. Rewriting it
using lower level commands might bring the benefit of not requiring
that.
However, this whole suggestion somewhat sounds along the lines
- The shape of this chisel is not great, maybe it could be improved?
- The other side of the hammer has an edge, maybe you could do
without a chisel?
Thanks
Michal
^ permalink raw reply
* Re: [PATCH 0/3] rev-list: add support for commits in `--missing`
From: Patrick Steinhardt @ 2023-10-13 5:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Karthik Nayak, git
In-Reply-To: <xmqqy1g7hl2y.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 3888 bytes --]
On Thu, Oct 12, 2023 at 09:17:09AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > Wouldn't this have the potential to significantly regress performance
> > for all those preexisting users of the `--missing` option? The commit
> > graph is quite an important optimization nowadays, and especially in
> > commands where we potentially walk a lot of commits (like we may do
> > here) it can result in queries that are orders of magnitudes faster.
>
> The test fails only when GIT_TEST_COMMIT_GRAPH is on, which updates
> the commit-graph every time a commit is made via "git commit" or
> "git merge".
>
> I'd suggest stepping back and think a bit.
>
> My assumption has been that the failing test emulates this scenario
> that can happen in real life:
>
> * The user creates a new commit.
>
> * A commit graph is written (not as part of GIT_TEST_COMMIT_GRAPH
> that is not realistic, but as part of "maintenance").
>
> * The repository loses some objects due to corruption.
>
> * Now, "--missing=print" is invoked so that the user can view what
> are missing. Or "--missing=allow-primisor" to ensure that the
> repository does not have missing objects other than the ones that
> the promisor will give us if we asked again.
>
> * But because the connectivity of these objects appear in the
> commit graph file, we fail to notice that these objects are
> missing, producing wrong results. If we disabled commit-graph
> while traversal (an earlier writing of it was perfectly OK), then
> "rev-list --missing" would have noticed and reported what the
> user wanted to know.
>
> In other words, the "optimization" you value is working to quickly
> produce a wrong result. Is it "significantly regress"ing if we
> disabled it to obtain the correct result?
It depends, in my opinion. If:
- Wrong results caused by the commit graph are only introduced with
this patch series due to the changed behaviour of `--missing`.
- We disable commit graphs proactively only because of the changed
behaviour of `--missing`.
Then yes, it does feel wrong to me to disable commit graphs and regress
performance for usecases that perviously worked both correct and fast.
> My assumption also has been that there is no point in running
> "rev-list --missing" if we know there is no repository corruption,
> and those who run "rev-list --missing" wants to know if the objects
> are really available, i.e. even if commit-graph that is out of sync
> with reality says it exists, if it is not in the object store, they
> would want to know that.
>
> If you can show me that it is not the case, then I may be pursuaded
> why producing a result that is out of sync with reality _quickly_,
> instead of taking time to produce a result that matches reality, is
> a worthy "optimization" to keep.
Note that I'm not saying that it's fine to return wrong results -- this
is of course a bug that needs to be addressed somehow. After all, things
working correctly should always trump things working fast. But until now
it felt more like we were going into the direction of disabling commit
graphs without checking whether there is an alternative solution that
allows us to get the best of both worlds, correctness and performance.
So what I'm looking for in this thread is a reason why we _can't_ have
that, or at least can't have it without unreasonable amounts of work. We
have helpers like `lookup_commit_in_graph()` that are designed to detect
stale commit graphs by double-checking whether a commit that has been
looked up via the commit graph actually exists in the repository. So I'm
wondering whether this could help us address the issue.
If there is a good reason why all of that is not possible then I'm happy
to carve in.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: why does git set X in LESS env var?
From: Dragan Simic @ 2023-10-13 4:43 UTC (permalink / raw)
To: Christoph Anton Mitterer; +Cc: git
In-Reply-To: <ec91ff19cca3d881d4746208744663c650ebd250.camel@scientia.org>
On 2023-10-13 01:06, Christoph Anton Mitterer wrote:
> On Fri, 2023-10-13 at 00:36 +0200, Dragan Simic wrote:
>> It seems that "--redraw-on-quit" is a possible candidate for
>> replacing "-X" in the set of default options for less(1)
>
> *If* some changes were made to how git handles this, it might perhaps
> be worth to consider not to touch LESS at all, but only add the
> required settings via command line arguments (i.e. -F -R ...).
Actually, that would be wrong. If someone sets $LESS or $PAGER (or
$GIT_PAGER, more specifically), it's up to the utility that invokes the
pager internally not to override the user preferences configured through
these environment variables. That's how everyone can customize the
pager behavior.
> Or perhaps only remove options from it, if they're known to break the
> behaviour with git (like -+R might).
Again, not the way the whole thing with pagination works. If someone
sets their environment variables wrong, it's simply the way they want
it, and it isn't anyone else's business to attempt fixing it
automatically.
> I always feel configuration via env vars is a bit fragile:
> - especially when one has generic names like POSIXLY_CORRECT there's
> some chance that by exporting it to one program, where one wants the
> effect, another program started off by that also gets it
> unintentionally
> - generic terms may be used by multiple programs, causing problems
Well, fragile or not, that's the way it works. It has its downsides for
sure, but it's all about having each utility handle the environment
carefully and document it in its man page(s), so the users can also
carefully craft the values of their customized environment variables.
> Also, if one can set only one LESS var in the environment, not one for
> less "alone", one for less with git, etc. - that is unless for programs
> like bat/delta which have specific own env vars to set the pager.
$LESS can be seen as a global set of the common options for less(1),
which may include the coloring configuration or the enablement of
case-insensitive search, for example, while $MANPAGER, $GIT_PAGER and
$BAT_PAGER may contain utility-specific options for less(1). That's
actually very good, because it makes possible to avoid duplication of
the common options.
> So if I set e.g. LESS to something, than typically only to stuff from
> which I believe it works as expected for any possible users.
> E.g. -F might be such a case.
It's up to everyone to decide what are the common options for less(1)
that they want to set in $LESS.
> But if I do that, git won't touch LESS and set the required -R, so I
> have to do that manually for git, e.g. either via git_config or by
> defining an alias git='LESS=FRX git'.
You don't have to define an alias, there's $GIT_PAGER for that purpose,
as I already explained above.
Moreover, the whole idea of the various utilities touching the $LESS
variable internally is to provide sane defaults to the users that don't
configure $LESS, $PAGER, etc. on their own. Once the user starts to
provide their own environment variable(s), it's no longer up to the
utility to help the user by altering their environment configuration.
> But in both cases it would "break" again, should ever another option be
> needed and added by git to the default LESS (which is however only set
> when it's unset).
> And in case of an alias, there would be the additional problem, that
> it's typically not picked up in non-interactive shells.
As you can see in my replies, it isn't about taking care of the users
who provide their own environment configuration. It's all about
providing the set of sane defaults, for the users with no custom
configuration.
> Long story short, it might make sense for git, to (mostly) ignore LESS
> and rather invoke less with -F -R.
No, that would be wrong on multiple levels, as I already explained in
detail.
> The problem with that in turn would of course be that it doesn't
> automatically propagate down, if e.g. git's pager is set to detla and
> delta in turn runs less.
> However, that's IMO litte concern, since then it's delta's duty to set
> -R (if it think it needs to do so), which it actually does.
I don't know what delta is and how it actually paginates its outputs,
but it should follow the rules of the environment-based pager
configuration that I described in detail above.
^ permalink raw reply
* What's cooking in git.git (Oct 2023, #05; Thu, 12)
From: Junio C Hamano @ 2023-10-12 23:21 UTC (permalink / raw)
To: git
Here are the topics that have been cooking in my tree. Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release). Commits prefixed with '-' are only in 'seen', and
aren't considered "accepted" at all and may be annotated with an URL
to a message that raises issues but they are no means exhaustive. A
topic without enough support may be discarded after a long period of
no activity (of course they can be resubmit when new interests
arise).
There are too many topics marked with "Needs review" label. I can
help reviewing some of them myself, but obviously it will not scale.
Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors. Some
repositories have only a subset of branches.
With maint, master, next, seen, todo:
git://git.kernel.org/pub/scm/git/git.git/
git://repo.or.cz/alt-git.git/
https://kernel.googlesource.com/pub/scm/git/git/
https://github.com/git/git/
https://gitlab.com/git-vcs/git/
With all the integration branches and topics broken out:
https://github.com/gitster/git/
Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):
git://git.kernel.org/pub/scm/git/git-htmldocs.git/
https://github.com/gitster/git-htmldocs.git/
Release tarballs are available at:
https://www.kernel.org/pub/software/scm/git/
--------------------------------------------------
[Graduated to 'master']
* jm/git-status-submodule-states-docfix (2023-10-04) 1 commit
(merged to 'next' on 2023-10-04 at 520b7711a4)
+ git-status.txt: fix minor asciidoc format issue
Docfix.
source: <pull.1591.v3.git.1696386165616.gitgitgadget@gmail.com>
* js/ci-coverity (2023-10-05) 6 commits
(merged to 'next' on 2023-10-05 at 253788f0d1)
+ coverity: detect and report when the token or project is incorrect
+ coverity: allow running on macOS
+ coverity: support building on Windows
+ coverity: allow overriding the Coverity project
+ coverity: cache the Coverity Build Tool
+ ci: add a GitHub workflow to submit Coverity scans
GitHub CI workflow has learned to trigger Coverity check.
source: <pull.1588.v2.git.1695642662.gitgitgadget@gmail.com>
* rs/parse-opt-ctx-cleanup (2023-10-03) 1 commit
(merged to 'next' on 2023-10-04 at d5d0a2ce3b)
+ parse-options: drop unused parse_opt_ctx_t member
Code clean-up.
source: <ebcaa9e1-d306-4c93-adec-3f35d7040531@web.de>
--------------------------------------------------
[New Topics]
* jc/fail-stash-to-store-non-stash (2023-10-11) 1 commit
- stash: be careful what we store
Feeding "git stash store" with a random commit that was not created
by "git stash create" now errors out.
Will merge to 'next'?
source: <xmqqbkd4lwj0.fsf_-_@gitster.g>
* jc/doc-unit-tests-fixup (2023-10-11) 1 commit
- SQUASH???
(this branch uses js/doc-unit-tests and js/doc-unit-tests-with-cmake.)
Quick fix for jc/doc-unit-tests topic to unbreak CI running on 'seen'.
source: <xmqqwmvskf8t.fsf@gitster.g>
* bc/racy-4gb-files (2023-10-12) 2 commits
- Prevent git from rehashing 4GiB files
- t: add a test helper to truncate files
The index file has room only for lower 32-bit of the file size in
the cached stat information, which means cached stat information
will have 0 in its sd_size member for a file whose size is multiple
of 4GiB. This is mistaken for a racily clean path. Avoid it by
storing a bogus sd_size value instead for such files.
Waiting for review response.
source: <20231012160930.330618-1-sandals@crustytoothpaste.net>
* ds/mailmap-entry-update (2023-10-12) 1 commit
(merged to 'next' on 2023-10-12 at 3de300ac62)
+ mailmap: change primary address for Derrick Stolee
Update mailmap entry for Derrick.
Will merge to 'master' immediately.
source: <pull.1592.git.1697131834003.gitgitgadget@gmail.com>
* jc/grep-f-relative-to-cwd (2023-10-12) 1 commit
- grep: -f <path> is relative to $cwd
"cd sub && git grep -f patterns" tried to read "patterns" file at
the top level of the working tree; it has been corrected to read
"sub/patterns" instead.
Needs review.
source: <xmqqedhzg37z.fsf@gitster.g>
--------------------------------------------------
[Stalled]
* pw/rebase-sigint (2023-09-07) 1 commit
- rebase -i: ignore signals when forking subprocesses
If the commit log editor or other external programs (spawned via
"exec" insn in the todo list) receive internactive signal during
"git rebase -i", it caused not just the spawned program but the
"Git" process that spawned them, which is often not what the end
user intended. "git" learned to ignore SIGINT and SIGQUIT while
waiting for these subprocesses.
Expecting a reroll.
cf. <12c956ea-330d-4441-937f-7885ab519e26@gmail.com>
source: <pull.1581.git.1694080982621.gitgitgadget@gmail.com>
* tk/cherry-pick-sequence-requires-clean-worktree (2023-06-01) 1 commit
- cherry-pick: refuse cherry-pick sequence if index is dirty
"git cherry-pick A" that replays a single commit stopped before
clobbering local modification, but "git cherry-pick A..B" did not,
which has been corrected.
Expecting a reroll.
cf. <999f12b2-38d6-f446-e763-4985116ad37d@gmail.com>
source: <pull.1535.v2.git.1685264889088.gitgitgadget@gmail.com>
* jc/diff-cached-fsmonitor-fix (2023-09-15) 3 commits
- diff-lib: fix check_removed() when fsmonitor is active
- Merge branch 'jc/fake-lstat' into jc/diff-cached-fsmonitor-fix
- Merge branch 'js/diff-cached-fsmonitor-fix' into jc/diff-cached-fsmonitor-fix
(this branch uses jc/fake-lstat.)
The optimization based on fsmonitor in the "diff --cached"
codepath is resurrected with the "fake-lstat" introduced earlier.
It is unknown if the optimization is worth resurrecting, but in case...
source: <xmqqr0n0h0tw.fsf@gitster.g>
--------------------------------------------------
[Cooking]
* tb/path-filter-fix (2023-10-10) 17 commits
- bloom: introduce `deinit_bloom_filters()`
- commit-graph: reuse existing Bloom filters where possible
- object.h: fix mis-aligned flag bits table
- commit-graph: drop unnecessary `graph_read_bloom_data_context`
- commit-graph.c: unconditionally load Bloom filters
- bloom: prepare to discard incompatible Bloom filters
- bloom: annotate filters with hash version
- commit-graph: new filter ver. that fixes murmur3
- repo-settings: introduce commitgraph.changedPathsVersion
- t4216: test changed path filters with high bit paths
- t/helper/test-read-graph: implement `bloom-filters` mode
- bloom.h: make `load_bloom_filter_from_graph()` public
- t/helper/test-read-graph.c: extract `dump_graph_info()`
- gitformat-commit-graph: describe version 2 of BDAT
- commit-graph: ensure Bloom filters are read with consistent settings
- revision.c: consult Bloom filters for root commits
- t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
The Bloom filter used for path limited history traversal was broken
on systems whose "char" is unsigned; update the implementation and
bump the format version to 2.
Needs (hopefully final and quick) review.
source: <cover.1696969994.git.me@ttaylorr.com>
* ak/pretty-decorate-more-fix (2023-10-09) 1 commit
(merged to 'next' on 2023-10-12 at 3cbb4c2268)
+ pretty: fix ref filtering for %(decorate) formats
Unlike "git log --pretty=%D", "git log --pretty="%(decorate)" did
not auto-initialize the decoration subsystem, which has been
corrected.
Will merge to 'master'.
source: <20231008202307.1568477-1-andy.koppe@gmail.com>
* en/docfixes (2023-10-09) 25 commits
- documentation: add missing parenthesis
- documentation: add missing quotes
- documentation: add missing fullstops
- documentation: add some commas where they are helpful
- documentation: fix whitespace issues
- documentation: fix capitalization
- documentation: fix punctuation
- documentation: use clearer prepositions
- documentation: add missing hyphens
- documentation: remove unnecessary hyphens
- documentation: add missing article
- documentation: fix choice of article
- documentation: whitespace is already generally plural
- documentation: fix singular vs. plural
- documentation: fix verb vs. noun
- documentation: fix adjective vs. noun
- documentation: fix verb tense
- documentation: employ consistent verb tense for a list
- documentation: fix subject/verb agreement
- documentation: remove extraneous words
- documentation: add missing words
- documentation: fix apostrophe usage
- documentation: fix typos
- documentation: fix small error
- documentation: wording improvements
Documentation typo and grammo fixes.
Needs review.
source: <pull.1595.git.1696747527.gitgitgadget@gmail.com>
* kn/rev-list-missing-fix (2023-10-09) 3 commits
. rev-list: add commit object support in `--missing` option
. rev-list: move `show_commit()` to the bottom
. revision: rename bit to `do_not_die_on_missing_objects`
"git rev-list --missing" did not work for missing commit objects,
which has been corrected.
Needs review.
Seems to break CI job with extra environment settings.
cf. <xmqqil7etndo.fsf@gitster.g>
source: <20231009105528.17777-1-karthik.188@gmail.com>
* sn/cat-file-doc-update (2023-10-09) 1 commit
(merged to 'next' on 2023-10-10 at 6719ba7bd4)
+ doc/cat-file: make synopsis and description less confusing
"git cat-file" documentation updates.
Will merge to 'master'.
source: <20231009175604.2361700-1-stepnem@smrk.net>
* xz/commit-title-soft-limit-doc (2023-10-09) 1 commit
(merged to 'next' on 2023-10-10 at 0bf3d9ed51)
+ doc: correct the 50 characters soft limit (+)
Doc update.
Will merge to 'master'.
source: <pull.1585.v2.git.git.1696778367151.gitgitgadget@gmail.com>
* jk/chunk-bounds (2023-10-09) 20 commits
(merged to 'next' on 2023-10-10 at 21139603ce)
+ chunk-format: drop pair_chunk_unsafe()
+ commit-graph: detect out-of-order BIDX offsets
+ commit-graph: check bounds when accessing BIDX chunk
+ commit-graph: check bounds when accessing BDAT chunk
+ commit-graph: bounds-check generation overflow chunk
+ commit-graph: check size of generations chunk
+ commit-graph: bounds-check base graphs chunk
+ commit-graph: detect out-of-bounds extra-edges pointers
+ commit-graph: check size of commit data chunk
+ midx: check size of revindex chunk
+ midx: bounds-check large offset chunk
+ midx: check size of object offset chunk
+ midx: enforce chunk alignment on reading
+ midx: check size of pack names chunk
+ commit-graph: check consistency of fanout table
+ midx: check size of oid lookup chunk
+ commit-graph: check size of oid fanout chunk
+ midx: stop ignoring malformed oid fanout chunk
+ t: add library for munging chunk-format files
+ chunk-format: note that pair_chunk() is unsafe
The codepaths that read "chunk" formatted files have been corrected
to pay attention to the chunk size and notice broken files.
Will merge to 'master'.
source: <20231009205544.GA3281950@coredump.intra.peff.net>
* ps/rewritten-is-per-worktree-doc (2023-10-10) 1 commit
(merged to 'next' on 2023-10-12 at 501b960e8c)
+ doc/git-worktree: mention "refs/rewritten" as per-worktree refs
Doc update.
Will merge to 'master'.
source: <985ac850eb6e60ae76601acc8bfbcd56f99348b4.1696935657.git.ps@pks.im>
* jc/merge-ort-attr-index-fix (2023-10-09) 1 commit
(merged to 'next' on 2023-10-10 at b139b87502)
+ merge-ort: initialize repo in index state
Fix "git merge-tree" to stop segfaulting when the --attr-source
option is used.
Will merge to 'master'.
source: <pull.1583.v3.git.git.1696857660374.gitgitgadget@gmail.com>
* jk/decoration-and-other-leak-fixes (2023-10-05) 3 commits
(merged to 'next' on 2023-10-06 at 5fc05c94dc)
+ daemon: free listen_addr before returning
+ revision: clear decoration structs during release_revisions()
+ decorate: add clear_decoration() function
Leakfix.
Will merge to 'master'.
source: <20231005212802.GA982892@coredump.intra.peff.net>
* sn/typo-grammo-phraso-fixes (2023-10-05) 5 commits
- t/README: fix multi-prerequisite example
- doc/gitk: s/sticked/stuck/
- git-jump: admit to passing merge mode args to ls-files
- doc/diff-options: improve wording of the log.diffMerges mention
- doc: fix some typos, grammar and wording issues
Many typos, ungrammatical sentences and wrong phrasing have been
fixed.
Needs review.
source: <20231003082107.3002173-1-stepnem@smrk.net>
* so/diff-merges-dd (2023-10-09) 3 commits
- completion: complete '--dd'
- diff-merges: introduce '--dd' option
- diff-merges: improve --diff-merges documentation
"git log" and friends learned "--dd" that is a short-hand for
"--diff-merges=first-parent -p".
Will merge to 'next'?
source: <20231009160535.236523-1-sorganov@gmail.com>
* vd/loose-ref-iteration-optimization (2023-10-09) 4 commits
(merged to 'next' on 2023-10-12 at 99e2f83855)
+ files-backend.c: avoid stat in 'loose_fill_ref_dir'
+ dir.[ch]: add 'follow_symlink' arg to 'get_dtype'
+ dir.[ch]: expose 'get_dtype'
+ ref-cache.c: fix prefix matching in ref iteration
The code to iterate over loose references have been optimized to
reduce the number of lstat() system calls.
Will merge to 'master'.
source: <pull.1594.v2.git.1696888736.gitgitgadget@gmail.com>
* jc/update-list-references-to-lore (2023-10-06) 1 commit
- doc: update list archive reference to use lore.kernel.org
Doc update.
Needs review.
source: <xmqq7cnz741s.fsf@gitster.g>
* cc/git-replay (2023-10-10) 14 commits
- replay: stop assuming replayed branches do not diverge
- replay: add --contained to rebase contained branches
- replay: add --advance or 'cherry-pick' mode
- replay: use standard revision ranges
- replay: make it a minimal server side command
- replay: remove HEAD related sanity check
- replay: remove progress and info output
- replay: add an important FIXME comment about gpg signing
- replay: change rev walking options
- replay: introduce pick_regular_commit()
- replay: die() instead of failing assert()
- replay: start using parse_options API
- replay: introduce new builtin
- t6429: remove switching aspects of fast-rebase
Introduce "git replay", a tool meant on the server side without
working tree to recreate a history.
Needs (hopefully final and quick) review.
source: <20231010123847.2777056-1-christian.couder@gmail.com>
* jk/commit-graph-leak-fixes (2023-10-03) 10 commits
(merged to 'next' on 2023-10-06 at 5d202ef8b9)
+ commit-graph: clear oidset after finishing write
+ commit-graph: free write-context base_graph_name during cleanup
+ commit-graph: free write-context entries before overwriting
+ commit-graph: free graph struct that was not added to chain
+ commit-graph: delay base_graph assignment in add_graph_to_chain()
+ commit-graph: free all elements of graph chain
+ commit-graph: move slab-clearing to close_commit_graph()
+ merge: free result of repo_get_merge_bases()
+ commit-reach: free temporary list in get_octopus_merge_bases()
+ t6700: mark test as leak-free
Leakfix.
Will merge to 'master'.
source: <20231003202504.GA7697@coredump.intra.peff.net>
* tb/repack-max-cruft-size (2023-10-09) 5 commits
(merged to 'next' on 2023-10-09 at 38f039e880)
+ repack: free existing_cruft array after use
(merged to 'next' on 2023-10-06 at b3ca6df3b9)
+ builtin/repack.c: avoid making cruft packs preferred
+ builtin/repack.c: implement support for `--max-cruft-size`
+ builtin/repack.c: parse `--max-pack-size` with OPT_MAGNITUDE
+ t7700: split cruft-related tests to t7704
"git repack" learned "--max-cruft-size" to prevent cruft packs from
growing without bounds.
Will merge to 'master'.
source: <cover.1696293862.git.me@ttaylorr.com>
source: <20231007172031.GA1524950@coredump.intra.peff.net>
source: <035393935108d02aaf8927189b05102f4f74f340.1696370003.git.me@ttaylorr.com>
* ak/color-decorate-symbols (2023-10-03) 1 commit
- decorate: add color.decorate.symbols config option
A new config for coloring.
Needs review.
source: <20231003205442.22963-1-andy.koppe@gmail.com>
* jc/attr-tree-config (2023-10-11) 2 commits
- attr: add attr.tree for setting the treeish to read attributes from
- attr: read attributes from HEAD when bare repo
The attribute subsystem learned to honor `attr.tree` configuration
that specifies which tree to read the .gitattributes files from.
Will merge to 'next'?
source: <pull.1577.v4.git.git.1697044422.gitgitgadget@gmail.com>
* js/submodule-fix-misuse-of-path-and-name (2023-10-03) 6 commits
(merged to 'next' on 2023-10-06 at 1054b6e752)
+ t7420: test that we correctly handle renamed submodules
+ t7419: test that we correctly handle renamed submodules
+ t7419, t7420: use test_cmp_config instead of grepping .gitmodules
+ t7419: actually test the branch switching
+ submodule--helper: return error from set-url when modifying failed
+ submodule--helper: use submodule_from_path in set-{url,branch}
In .gitmodules files, submodules are keyed by their names, and the
path to the submodule whose name is $name is specified by the
submodule.$name.path variable. There were a few codepaths that
mixed the name and path up when consulting the submodule database,
which have been corrected. It took long for these bugs to be found
as the name of a submodule initially is the same as its path, and
the problem does not surface until it is moved to a different path,
which apparently happens very rarely.
Will merge to 'master'.
source: <0a0a157f88321d25fdb0be771a454b3410a449f3.camel@archlinux.org>
* ar/diff-index-merge-base-fix (2023-10-02) 1 commit
(merged to 'next' on 2023-10-06 at 0ff4dfc0e1)
+ diff: fix --merge-base with annotated tags
"git diff --merge-base X other args..." insisted that X must be a
commit and errored out when given an annotated tag that peels to a
commit, but we only need it to be a committish. This has been
corrected.
Will merge to 'master'.
source: <20231001151845.3621551-1-hi@alyssa.is>
* js/update-urls-in-doc-and-comment (2023-09-26) 4 commits
- doc: refer to internet archive
- doc: update links for andre-simon.de
- doc: update links to current pages
- doc: switch links to https
Stale URLs have been updated to their current counterparts (or
archive.org) and HTTP links are replaced with working HTTPS links.
Needs review.
source: <pull.1589.v2.git.1695553041.gitgitgadget@gmail.com>
* la/trailer-cleanups (2023-09-26) 4 commits
- trailer: only use trailer_block_* variables if trailers were found
- trailer: use offsets for trailer_start/trailer_end
- trailer: find the end of the log message
- commit: ignore_non_trailer computes number of bytes to ignore
Code clean-up.
Needs review.
source: <pull.1563.v4.git.1695709372.gitgitgadget@gmail.com>
* eb/hash-transition (2023-10-02) 30 commits
- t1016-compatObjectFormat: add tests to verify the conversion between objects
- t1006: test oid compatibility with cat-file
- t1006: rename sha1 to oid
- test-lib: compute the compatibility hash so tests may use it
- builtin/ls-tree: let the oid determine the output algorithm
- object-file: handle compat objects in check_object_signature
- tree-walk: init_tree_desc take an oid to get the hash algorithm
- builtin/cat-file: let the oid determine the output algorithm
- rev-parse: add an --output-object-format parameter
- repository: implement extensions.compatObjectFormat
- object-file: update object_info_extended to reencode objects
- object-file-convert: convert commits that embed signed tags
- object-file-convert: convert commit objects when writing
- object-file-convert: don't leak when converting tag objects
- object-file-convert: convert tag objects when writing
- object-file-convert: add a function to convert trees between algorithms
- object: factor out parse_mode out of fast-import and tree-walk into in object.h
- cache: add a function to read an OID of a specific algorithm
- tag: sign both hashes
- commit: export add_header_signature to support handling signatures on tags
- commit: convert mergetag before computing the signature of a commit
- commit: write commits for both hashes
- object-file: add a compat_oid_in parameter to write_object_file_flags
- object-file: update the loose object map when writing loose objects
- loose: compatibilty short name support
- loose: add a mapping between SHA-1 and SHA-256 for loose objects
- repository: add a compatibility hash algorithm
- object-names: support input of oids in any supported hash
- oid-array: teach oid-array to handle multiple kinds of oids
- object-file-convert: stubs for converting from one object format to another
Teach a repository to work with both SHA-1 and SHA-256 hash algorithms.
Needs review.
source: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
* jx/remote-archive-over-smart-http (2023-10-04) 4 commits
- archive: support remote archive from stateless transport
- transport-helper: call do_take_over() in connect_helper
- transport-helper: call do_take_over() in process_connect
- transport-helper: no connection restriction in connect_helper
"git archive --remote=<remote>" learned to talk over the smart
http (aka stateless) transport.
Needs review.
source: <cover.1696432593.git.zhiyou.jx@alibaba-inc.com>
* jx/sideband-chomp-newline-fix (2023-10-04) 3 commits
- pkt-line: do not chomp newlines for sideband messages
- pkt-line: memorize sideband fragment in reader
- test-pkt-line: add option parser for unpack-sideband
Sideband demultiplexer fixes.
Needs review.
source: <cover.1696425168.git.zhiyou.jx@alibaba-inc.com>
* ty/merge-tree-strategy-options (2023-10-11) 2 commits
(merged to 'next' on 2023-10-12 at 9b873601df)
+ merge: introduce {copy|clear}_merge_options()
(merged to 'next' on 2023-09-29 at aa65b54416)
+ merge-tree: add -X strategy option
"git merge-tree" learned to take strategy backend specific options
via the "-X" option, like "git merge" does.
Will merge to 'master'.
source: <pull.1565.v6.git.1695522222723.gitgitgadget@gmail.com>
* js/config-parse (2023-09-21) 5 commits
- config-parse: split library out of config.[c|h]
- config.c: accept config_parse_options in git_config_from_stdin
- config: report config parse errors using cb
- config: split do_event() into start and flush operations
- config: split out config_parse_options
The parsing routines for the configuration files have been split
into a separate file.
Needs review.
source: <cover.1695330852.git.steadmon@google.com>
* jc/fake-lstat (2023-09-15) 1 commit
- cache: add fake_lstat()
(this branch is used by jc/diff-cached-fsmonitor-fix.)
A new helper to let us pretend that we called lstat() when we know
our cache_entry is up-to-date via fsmonitor.
Needs review.
source: <xmqqcyykig1l.fsf@gitster.g>
* rs/parse-options-value-int (2023-09-18) 2 commits
- parse-options: use and require int pointer for OPT_CMDMODE
- parse-options: add int value pointer to struct option
A bit of type safety for the "value" pointer used in the
parse-options API.
Not ready yet.
cf. <4014e490-c6c1-453d-b0ed-645220e3e614@web.de>
source: <e6d8a291-03de-cfd3-3813-747fc2cad145@web.de>
* la/trailer-test-and-doc-updates (2023-09-07) 13 commits
(merged to 'next' on 2023-10-06 at 69fef35819)
+ trailer doc: <token> is a <key> or <keyAlias>, not both
+ trailer doc: separator within key suppresses default separator
+ trailer doc: emphasize the effect of configuration variables
+ trailer --unfold help: prefer "reformat" over "join"
+ trailer --parse docs: add explanation for its usefulness
+ trailer --only-input: prefer "configuration variables" over "rules"
+ trailer --parse help: expose aliased options
+ trailer --no-divider help: describe usual "---" meaning
+ trailer: trailer location is a place, not an action
+ trailer doc: narrow down scope of --where and related flags
+ trailer: add tests to check defaulting behavior with --no-* flags
+ trailer test description: this tests --where=after, not --where=before
+ trailer tests: make test cases self-contained
Test coverage for trailers has been improved.
Will merge to 'master'.
source: <pull.1564.v3.git.1694125209.gitgitgadget@gmail.com>
* js/doc-unit-tests (2023-10-09) 3 commits
- ci: run unit tests in CI
- unit tests: add TAP unit test framework
- unit tests: add a project plan document
(this branch is used by jc/doc-unit-tests-fixup and js/doc-unit-tests-with-cmake.)
Process to add some form of low-level unit tests has started.
Expecting a reroll to address CI breakage.
source: <cover.1696889529.git.steadmon@google.com>
* js/doc-unit-tests-with-cmake (2023-10-09) 7 commits
- cmake: handle also unit tests
- cmake: use test names instead of full paths
- cmake: fix typo in variable name
- artifacts-tar: when including `.dll` files, don't forget the unit-tests
- unit-tests: do show relative file paths
- unit-tests: do not mistake `.pdb` files for being executable
- cmake: also build unit tests
(this branch is used by jc/doc-unit-tests-fixup; uses js/doc-unit-tests.)
Update the base topic to work with CMake builds.
Needs review.
source: <pull.1579.v3.git.1695640836.gitgitgadget@gmail.com>
* jc/rerere-cleanup (2023-08-25) 4 commits
- rerere: modernize use of empty strbuf
- rerere: try_merge() should use LL_MERGE_ERROR when it means an error
- rerere: fix comment on handle_file() helper
- rerere: simplify check_one_conflict() helper function
Code clean-up.
Not ready to be reviewed yet.
source: <20230824205456.1231371-1-gitster@pobox.com>
* rj/status-bisect-while-rebase (2023-08-01) 1 commit
- status: fix branch shown when not only bisecting
"git status" is taught to show both the branch being bisected and
being rebased when both are in effect at the same time.
Needs review.
cf. <xmqqtttia3vn.fsf@gitster.g>
source: <48745298-f12b-8efb-4e48-90d2c22a8349@gmail.com>
--------------------------------------------------
[Discarded]
* tb/ci-coverity (2023-09-21) 1 commit
. .github/workflows: add coverity action
GitHub CI workflow has learned to trigger Coverity check.
Superseded by the js/ci-coverity topic.
source: <b23951c569660e1891a7fb3ad2c2ea1952897bd7.1695332105.git.me@ttaylorr.com>
* cw/git-std-lib (2023-09-11) 7 commits
. SQUASH???
. git-std-lib: add test file to call git-std-lib.a functions
. git-std-lib: introduce git standard library
. parse: create new library for parsing strings and env values
. config: correct bad boolean env value error message
. wrapper: remove dependency to Git-specific internal file
. hex-ll: split out functionality from hex
Another libification effort.
Superseded by the cw/prelim-cleanup topic.
cf. <xmqqy1hfrk6p.fsf@gitster.g>
cf. <20230915183927.1597414-1-jonathantanmy@google.com>
source: <20230908174134.1026823-1-calvinwan@google.com>
* so/diff-merges-d (2023-09-11) 2 commits
- diff-merges: introduce '-d' option
- diff-merges: improve --diff-merges documentation
Superseded by the so/diff-merges-dd topic.
source: <20230909125446.142715-1-sorganov@gmail.com>
* kn/rev-list-ignore-missing-links (2023-09-20) 1 commit
. revision: add `--ignore-missing-links` user option
Surface the .ignore_missing_links bit that stops the revision
traversal from stopping and dying when encountering a missing
object to a new command line option of "git rev-list", so that the
objects that are required but are missing can be enumerated.
Superseded by kn/rev-list-missing-fix
source: <20230920104507.21664-1-karthik.188@gmail.com>
^ permalink raw reply
* Re: why does git set X in LESS env var?
From: Christoph Anton Mitterer @ 2023-10-12 23:06 UTC (permalink / raw)
To: Dragan Simic; +Cc: git
In-Reply-To: <60f1922b12a6ef304ffa36c334348e34@manjaro.org>
On Fri, 2023-10-13 at 00:36 +0200, Dragan Simic wrote:
> It seems that "--redraw-on-quit" is a possible candidate for
> replacing
> "-X" in the set of default options for less(1)
*If* some changes were made to how git handles this, it might perhaps
be worth to consider not to touch LESS at all, but only add the
required settings via command line arguments (i.e. -F -R ...).
Or perhaps only remove options from it, if they're known to break the
behaviour with git (like -+R might).
I always feel configuration via env vars is a bit fragile:
- especially when one has generic names like POSIXLY_CORRECT there's
some chance that by exporting it to one program, where one wants the
effect, another program started off by that also gets it
unintentionally
- generic terms may be used by multiple programs, causing problems
Also, if one can set only one LESS var in the environment, not one for
less "alone", one for less with git, etc. - that is unless for programs
like bat/delta which have specific own env vars to set the pager.
So if I set e.g. LESS to something, than typically only to stuff from
which I believe it works as expected for any possible users.
E.g. -F might be such a case.
But if I do that, git won't touch LESS and set the required -R, so I
have to do that manually for git, e.g. either via git_config or by
defining an alias git='LESS=FRX git'.
But in both cases it would "break" again, should ever another option be
needed and added by git to the default LESS (which is however only set
when it's unset).
And in case of an alias, there would be the additional problem, that
it's typically not picked up in non-interactive shells.
Long story short, it might make sense for git, to (mostly) ignore LESS
and rather invoke less with -F -R.
The problem with that in turn would of course be that it doesn't
automatically propagate down, if e.g. git's pager is set to detla and
delta in turn runs less.
However, that's IMO litte concern, since then it's delta's duty to set
-R (if it think it needs to do so), which it actually does.
Cheers,
Chris.
^ permalink raw reply
* Re: [PATCH v2 1/2] t: add a test helper to truncate files
From: Junio C Hamano @ 2023-10-12 22:52 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Taylor Blau, Jason Hatton
In-Reply-To: <20231012160930.330618-2-sandals@crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> + fd = open(argv[1], O_WRONLY | O_CREAT, 0600);
> + if (fd < 0)
> + die_errno("failed to open file %s", argv[1]);
contrib/coccinelle/xopen.cocci tells us to write this simply as
fd = xopen(argv[1], O_WRONLY | O_CREAT, 0600);
https://github.com/git/git/actions/runs/6500777388/job/17656837393#step:4:657
^ permalink raw reply
* Re: why does git set X in LESS env var?
From: Dragan Simic @ 2023-10-12 22:36 UTC (permalink / raw)
To: Christoph Anton Mitterer; +Cc: git
In-Reply-To: <c6cd3133573d5ade6d02b5da1051853a4b3885e1.camel@scientia.org>
On 2023-10-12 23:48, Christoph Anton Mitterer wrote:
> On Thu, 2023-10-12 at 23:15 +0200, Dragan Simic wrote:
>>
>> Ah, I can finally see what are you talking about... Thank you very
>> much
>> for all the testing you've performed and for supplying this screen
>> recording! I can confirm that my environment is also affected, but
>> for
>> some reason I haven't observed it this way before.
>
> Well... perhaps because it's not really "easy" to spot unless one
> carefully reads through the lines (which I guess, one does not that
> often in the terminal "history").
>
> Have a look at my ticket at less, especially:
> https://github.com/gwsw/less/issues/445#issuecomment-1758887183
Great, thanks, it's full of very useful information.
> Where it was confirmed that the issue I describe might happen (and I
> guess is non-fixable?
>
> The whole issue also contains an explanation on why scrolling doesn't
> work when -X is used (but --mouse is not) on VTE terminals (and maybe
> others, though not xterm).
> And it's basically not "fixable" but simply "by design".
It seems that "--redraw-on-quit" is a possible candidate for replacing
"-X" in the set of default options for less(1), and also in other CLI
utilities, but I still need to test it in detail.
>> Huh, that's really worrisome and I'm willing to help you with
>> debugging
>> and fixing this issue. Please, let me perform some debugging and
>> digging around, and I'll come back to you with some further insights,
>
> Well, my assumption (though I'm really not a terminal expert) would be
> that it's not fixable... because less would somehow make sure that
> everything it prints (in the alt screen buffer) is properly
> concatenated in the the regular one.
I did some work on terminal emulators, but I'm also not very much of an
expert when it comes to terminal emulators.
> less upstream made some suggestions:
> https://github.com/gwsw/less/issues/445#issuecomment-1759986293
>
> One would be to change the terminfo entry, which I guess is not really
> feasible as a general solution.
>
> The other would be less’ --redraw-on-quit option.
I agree that the latter seems like a viable solution, but as I already
wrote above, I need to test it first, together with doing some digging
through the less(1) source code.
> Maybe that would be an even better solution for git?
> AFAICS, we could have -F, the VTE mouse scrolling out of the box, plus
> (via that option) the final screen buffer of less printed when exiting.
Exactly, but still needs testing and a detailed insight.
> Would give some context (what one did in the pager, where one was) on
> the regular screen buffer, but (presumably) avoid that mess up... and
> perhaps even prevent unneeded pages of output from the pager, if one
> scrolled down a lot, which maybe aren't even needed?
Yes, it seems to tick all the boxes.
> (Also note, that less` upstream calls `-X` "a risky flag" ;-) )
In general, "-X" seems to be more of a bandaid option.
>> Those people, just as anyone else, can use $PAGER or $GIT_PAGER to
>> configure the pagination the way they like it. In the end, that's
>> also
>> what I do in my environment.
>
> Sure... all I said, that (IMO) in the case of `-X` it's not like with
> `-R` but rater like with `-S`... i.e. neither mode has more right to be
> the default than the other.
^ permalink raw reply
* Re: [PATCH v2 2/2] Prevent git from rehashing 4GiB files
From: Junio C Hamano @ 2023-10-12 22:11 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Taylor Blau, Jason Hatton
In-Reply-To: <ZShsG1SKfrefsCtu@tapette.crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> nor should we support 8- or 16-bit systems. If you'd prefer "uint32_t",
> I can do that.
Going into statinfo.h and updating sd_size to uint32_t is totally
outside the scope of this fix.
> I think using 2^31 is better because it's far away from very small
> values and very large values, which means that it's a hard to modify a
> file which used to have its value as a multiple of 4 GiB and
> accidentally make Git think it was unchanged. Using 1 would make a
> simple "echo > foo" possibly think things were unchanged in some cases,
> which we should avoid.
The reason I gave the extreme "1-byte" was to gauge how much actual
"size" we are relying on the correctness of this change. As mtime
is giving the primary protection from false matching of cached stat
information, I do not think "echo >foo" would be a huge issue. IOW
my stance is 1U<<31 is as good as 1U<<0, so I do not oppose to the
former, either. But in a few years, 64-bit integers may cease to be
too odd, who knows ;-)
^ permalink raw reply
* Re: [PATCH v2 2/2] Prevent git from rehashing 4GiB files
From: brian m. carlson @ 2023-10-12 21:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Taylor Blau, Jason Hatton
In-Reply-To: <xmqqpm1jn2nh.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 3498 bytes --]
On 2023-10-12 at 17:58:42, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
> > An example would be to have a 2^32 sized file in the index of
> > patched git. Patched git would save the file as 2^31 in the cache.
> > An unpatched git would very much see the file has changed in size
> > and force it to rehash the file, which is safe.
>
> The reason why this is "safe" is because an older Git will would
> keep rehashing whether 2^31 or 0 is stored as its sd_size, so the
> change is not making things worse? With older git, "git diff-files"
> will report that such a file is not up to date, and then the user
> will refresh the index, which will store 0 as its sd_file, so
> tentatively "git status" may give a wrong information, but we
> probalby do not care? Is that how the reasoning goes?
Correct. An older Git will rehash it either way, on every git status.
If you run git diff-files on an older Git, it will always show it as
modified (hence why I used that in the tests). The git status will
rehash it and then realize that it isn't modified, not printing any
changes, so the behaviour is not wrong, it's just extremely slow (SHA-1
DC on 4 GiB of data).
If you use a new Git with an old index, git status will still rehash it
the first time and correctly determine that it isn't changed, then write
the 0x80000000 to the index. It just won't rehash it on subsequent
calls to git status.
The only incorrect behaviour (assuming that slow is not incorrect) that
I'm aware of on older Git is that git diff-files marks it as modified
when it's not. There is no incorrect behaviour on a fixed Git.
> > +/*
> > + * Munge st_size into an unsigned int.
> > + */
> > +static unsigned int munge_st_size(off_t st_size) {
> > + unsigned int sd_size = st_size;
> > +
> > + /*
> > + * If the file is an exact multiple of 4 GiB, modify the value so it
> > + * doesn't get marked as racily clean (zero).
> > + */
> > + if (!sd_size && st_size)
> > + return 0x80000000;
> > + else
> > + return sd_size;
> > +}
>
> This assumes typeof(sd_size) aka "unsigned int" is always 32-bit,
> which does not sound reasonable. Reference to 4GiB, 2^32 and 2^31
> in the code and the proposed commit log message need to be qualified
> with "on a platform whose uint is 32-bit" or something, or better
> yet, phrased in a way that is agnostic to the integer size. At
> the very least, the hardcoded 0x80000000 needs to be rethought, I
> am afraid.
unsigned int is 32-bit on every modern 32- or 64-bit platform known to
exist today. I don't believe we support MS-DOS or systems of its era,
nor should we support 8- or 16-bit systems. If you'd prefer "uint32_t",
I can do that.
Note that the problem is reproducible on a stock Ubuntu amd64 system, so
it is definitely a problem on all modern systems.
> Other than that, the workaround for the racy-git avoidance code does
> sound good. I actually wonder if we should always use 1 regardless
> of integer size.
I think using 2^31 is better because it's far away from very small
values and very large values, which means that it's a hard to modify a
file which used to have its value as a multiple of 4 GiB and
accidentally make Git think it was unchanged. Using 1 would make a
simple "echo > foo" possibly think things were unchanged in some cases,
which we should avoid.
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] Documentation/gitformat-pack.txt: fix incorrect MIDX documentation
From: Junio C Hamano @ 2023-10-12 21:54 UTC (permalink / raw)
To: Taylor Blau; +Cc: git, Jeff King
In-Reply-To: <af2742e05dff48c4ba0a9f36d58bcbfc052dca40.1697144959.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
> diff --git a/Documentation/gitformat-pack.txt b/Documentation/gitformat-pack.txt
> index d7153962d4..54000c9412 100644
> --- a/Documentation/gitformat-pack.txt
> +++ b/Documentation/gitformat-pack.txt
> @@ -392,8 +392,9 @@ CHUNK DATA:
> Packfile Names (ID: {'P', 'N', 'A', 'M'})
> Stores the packfile names as concatenated, NUL-terminated strings.
Not a problem this series (neither this or the previous step)
introduces, but I had to read the implementation of
write_midx_pack_names() to find out what "concatenated
NUL-terminated string" really means. The code has a list of
strings, writes each of them as a NUL-terminated string in sequence,
and to align the beginning of the next chunk, NULs are added to make
the whole thing multiple of MIDX_CHUNK_ALIGNMENT bytes.
A naive reader code might implement a loop like so:
while (ptr[0] != '\0') {
endp = strlen(ptr);
... ptr[0..endp] is one pathname ...
ptr = endp + 1;
}
expecting that the terminating NUL of the last entry would be
followed by a NUL, but that is buggy. The sum of the pathname
strings (with one NUL after each) may happen to be multiple of
MIDX_CHUNK_ALIGNMENT bytes, in which case no extra padding NUL bytes
will be there. So the reader also needs to pay attention to the
chunk size to notice when to stop reading. It feels somewhat
suboptimal.
> Packfiles must be listed in lexicographic order for fast lookups by
> - name. This is the only chunk not guaranteed to be a multiple of four
> - bytes in length, so should be the last chunk for alignment reasons.
> + name. Individual entries in this chunk are not guarenteed to be
> + aligned. The chunk is externally padded with zeros to align
> + remaining chunks.
I am not sure what "externally padded" means.
Before write_midx_pack_names() returns, there is a code to pad the
space after it writes those names, which does not look any external
than the bytes used to record the pathnames or NUL bytes used to
terminate these pathnames.
To me, "externally padded" is an appropriate phrase to use if this
function does not bother with the alignment after what it needs to
record, but the caller, i.e., write_chunkfile(), has code to notice
that the number of bytes written by cf->chunks[i].write_fn() it just
called is not a multiple of some required alignment and adds padding
bytes after .write_fn() returned. I do not think that is what is
going on here.
My observations.
* The packfile names chunk is used to record N pathnames; N is not
recorded anywhere explicitly. To record N pathnames, a single
chunk with N names in it is used. It is not like N chunks, each
with one name is used.
* Each of these pathname is recorded literally, followed by a NUL,
in some order, without any extra padding.
* To keep the size of the chunk multiple of alignment requirement,
there may be extra NUL bytes after the names.
This data structure does not allow you to binary search or hash
lookup without an extra table of pointers into this table of strings
at runtime, and once you build such a table, the source being sorted
does not help all that much. So I am not sure how strict the
"lexicographic" requirement needs to be, but of course, starting
strict and loosening later is much easier than starting loose, so
documenting "must be listed" and following that rule is fine.
Thanks.
^ permalink raw reply
* Re: why does git set X in LESS env var?
From: Christoph Anton Mitterer @ 2023-10-12 21:48 UTC (permalink / raw)
To: Dragan Simic; +Cc: git
In-Reply-To: <31b6f4a2b88cc3a2cfa908f82f4f2302@manjaro.org>
On Thu, 2023-10-12 at 23:15 +0200, Dragan Simic wrote:
>
> Ah, I can finally see what are you talking about... Thank you very
> much
> for all the testing you've performed and for supplying this screen
> recording! I can confirm that my environment is also affected, but
> for
> some reason I haven't observed it this way before.
Well... perhaps because it's not really "easy" to spot unless one
carefully reads through the lines (which I guess, one does not that
often in the terminal "history").
Have a look at my ticket at less, especially:
https://github.com/gwsw/less/issues/445#issuecomment-1758887183
Where it was confirmed that the issue I describe might happen (and I
guess is non-fixable?
The whole issue also contains an explanation on why scrolling doesn't
work when -X is used (but --mouse is not) on VTE terminals (and maybe
others, though not xterm).
And it's basically not "fixable" but simply "by design".
> Huh, that's really worrisome and I'm willing to help you with
> debugging
> and fixing this issue. Please, let me perform some debugging and
> digging around, and I'll come back to you with some further insights,
Well, my assumption (though I'm really not a terminal expert) would be
that it's not fixable... because less would somehow make sure that
everything it prints (in the alt screen buffer) is properly
concatenated in the the regular one.
less upstream made some suggestions:
https://github.com/gwsw/less/issues/445#issuecomment-1759986293
One would be to change the terminfo entry, which I guess is not really
feasible as a general solution.
The other would be less’ --redraw-on-quit option.
Maybe that would be an even better solution for git?
AFAICS, we could have -F, the VTE mouse scrolling out of the box, plus
(via that option) the final screen buffer of less printed when exiting.
Would give some context (what one did in the pager, where one was) on
the regular screen buffer, but (presumably) avoid that mess up... and
perhaps even prevent unneeded pages of output from the pager, if one
scrolled down a lot, which maybe aren't even needed?
(Also note, that less` upstream calls `-X` "a risky flag" ;-) )
> Those people, just as anyone else, can use $PAGER or $GIT_PAGER to
> configure the pagination the way they like it. In the end, that's
> also
> what I do in my environment.
Sure... all I said, that (IMO) in the case of `-X` it's not like with
`-R` but rater like with `-S`... i.e. neither mode has more right to be
the default than the other.
Cheers,
Chris.
^ permalink raw reply
* Re: why does git set X in LESS env var?
From: Dragan Simic @ 2023-10-12 21:15 UTC (permalink / raw)
To: Christoph Anton Mitterer; +Cc: git
In-Reply-To: <e1e187ca3d970c18e1a11d51ff93b6cb212bcbaa.camel@scientia.org>
On 2023-10-12 22:23, Christoph Anton Mitterer wrote:
> On Thu, 2023-10-12 at 07:46 +0200, Dragan Simic wrote:
>> Let me repeat that the messed up output you're experiencing isn't
>> normal
>> and has nothing to do with the arguments passed to less(1). That's a
>> separate issue of the terminal emulator(s) you're using, or in issue
>> of
>> your specific environment, and should be debugged and addressed as a
>> separate issue.
>
> As I've told you before it happens at least in gnome-terminal (and thus
> presumably and VTE based terminal), xterm, xfce4-terminal and konsole
> (all current versions of Debian unstable)... with less as of Debian
> unstable as well as 643.
>
> That affects at least on major distro, and there's a good chance that
> it affects any other distro based on Debian (*buntu, etc.).
>
> I further tried on SLES 15 with both gnome-terminal 3.42.2 and xterm
> 330 as well as less 530.
>
> Even tried with the terminal emulator started via env -i and only TERM
> set manually.
>
> *All* cases affected by the same problem I've described before.
>
> Same with the command you've used in your follow-up post, here a video
> of it in HD:
> https://youtu.be/MsxtQgrKM50
Ah, I can finally see what are you talking about... Thank you very much
for all the testing you've performed and for supplying this screen
recording! I can confirm that my environment is also affected, but for
some reason I haven't observed it this way before.
Huh, that's really worrisome and I'm willing to help you with debugging
and fixing this issue. Please, let me perform some debugging and
digging around, and I'll come back to you with some further insights,
>> To me, having inconsistent displaying of the short and long outputs
>> is simply not acceptable.
>
> Which is fine - and as I've said: I personally also tend to prefer it
> like that - but even if the above would be just some bug (which however
> seems to affect all systems I could test on a short notice, except
> yours)... one can IMO still not generally say whether on or the other
> behaviour is generally accepted to be the better one.
>
> Even if output may be just chopped of and thus ambiguously incomplete,
> some people may still prefer to have rather no output at all.
Those people, just as anyone else, can use $PAGER or $GIT_PAGER to
configure the pagination the way they like it. In the end, that's also
what I do in my environment.
>> Perhaps something is wrong with your specific environment, because
>> I see no other reason for this issue.
>
> Well may be, but seems unlikely from my PoV, given that I've now tested
> even on other distros and systems not under my control.
>
> Anyway... I think this got a bit too off-topic here :-D
Well, yes and no. This scrolling-related issue is obviously affecting
numerous git users, which makes it quite relevant for git as a project.
Of course, not directly relevant, but indirectly, yes.
^ permalink raw reply
* [PATCH 2/2] Documentation/gitformat-pack.txt: fix incorrect MIDX documentation
From: Taylor Blau @ 2023-10-12 21:09 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1697144959.git.me@ttaylorr.com>
Back in 32f3c541e3 (multi-pack-index: write pack names in chunk, 2018-07-12)
the MIDX's "Packfile Names" (or "PNAM", for short) chunk was described
as containing an array of string entries. e0d1bcf825 notes that this is
the only chunk in the MIDX format's specification that is not guaranteed
to be 4-byte aligned, and so should be placed last.
This isn't quite accurate: the entries within the PNAM chunk are not
guaranteed to be aligned since they are arbitrary strings, but the
chunk itself is aligned since the ending is padded with NUL bytes.
That external padding has always been there since 32f3c541e3 via
midx.c::write_midx_pack_names(), which ended with:
i = MIDX_CHUNK_ALIGNMENT - (written % MIDX_CHUNK_ALIGNMENT)
if (i < MIDX_CHUNK_ALIGNMENT) {
unsigned char padding[MIDX_CHUNK_ALIGNMENT];
memset(padding, 0, sizeof(padding))
hashwrite(f, padding, i);
written += i;
}
In fact, 32f3c541e3's log message itself describes the chunk in its
first paragraph with:
Since filenames are not well structured, add padding to keep good
alignment in later chunks.
So these have always been externally aligned. Correct the corresponding
part of our documentation to reflect that.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
Documentation/gitformat-pack.txt | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/gitformat-pack.txt b/Documentation/gitformat-pack.txt
index d7153962d4..54000c9412 100644
--- a/Documentation/gitformat-pack.txt
+++ b/Documentation/gitformat-pack.txt
@@ -392,8 +392,9 @@ CHUNK DATA:
Packfile Names (ID: {'P', 'N', 'A', 'M'})
Stores the packfile names as concatenated, NUL-terminated strings.
Packfiles must be listed in lexicographic order for fast lookups by
- name. This is the only chunk not guaranteed to be a multiple of four
- bytes in length, so should be the last chunk for alignment reasons.
+ name. Individual entries in this chunk are not guarenteed to be
+ aligned. The chunk is externally padded with zeros to align
+ remaining chunks.
OID Fanout (ID: {'O', 'I', 'D', 'F'})
The ith entry, F[i], stores the number of OIDs with first
--
2.42.0.349.gf0c1128f8b.dirty
^ permalink raw reply related
* [PATCH 1/2] Documentation/gitformat-pack.txt: fix typo
From: Taylor Blau @ 2023-10-12 21:09 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1697144959.git.me@ttaylorr.com>
e0d1bcf825 (multi-pack-index: add format details, 2018-07-12) describes
the MIDX's "PNAM" chunk as having entries which are "null-terminated
strings".
This is a typo, as strings are terminated with a NUL character, which is
a distinct concept from "NULL" or "null", which we typically reserve for
the void pointer to address 0.
Correct the documentation accordingly.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
Documentation/gitformat-pack.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/gitformat-pack.txt b/Documentation/gitformat-pack.txt
index 870e00f298..d7153962d4 100644
--- a/Documentation/gitformat-pack.txt
+++ b/Documentation/gitformat-pack.txt
@@ -390,7 +390,7 @@ CHUNK LOOKUP:
CHUNK DATA:
Packfile Names (ID: {'P', 'N', 'A', 'M'})
- Stores the packfile names as concatenated, null-terminated strings.
+ Stores the packfile names as concatenated, NUL-terminated strings.
Packfiles must be listed in lexicographic order for fast lookups by
name. This is the only chunk not guaranteed to be a multiple of four
bytes in length, so should be the last chunk for alignment reasons.
--
2.42.0.349.gf0c1128f8b.dirty
^ permalink raw reply related
* [PATCH 0/2] Documentation/gitformat-pack.txt: correct a few issues/typos
From: Taylor Blau @ 2023-10-12 21:09 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
These short couple of patches are some fallout from a larger series[^1]
that I'm working on. I noticed a couple of minor issues in the MIDX
format's documentation, and figured that sending these patches sooner
rather than later would be worthwhile.
The first patch swaps "null" for "NUL" when describing the NUL
character. The second patch corrects a piece of the documentation which
claims the PNAM chunk is not aligned (it is externally padded and thus
guaranteed to be well-aligned).
Thanks in advance for your review.
[^1]: The broader goal of that series is to allow verbatim pack reuse
from multiple packs (instead of just the bitmapped one, or the
preferred pack of a MIDX). This is only possible in certain
circumstances, which require some tweaks to the MIDX format for
bookkeeping reasons, which is how I noticed these issues in the first
place.
Taylor Blau (2):
Documentation/gitformat-pack.txt: fix typo
Documentation/gitformat-pack.txt: fix incorrect MIDX documentation
Documentation/gitformat-pack.txt | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
--
2.42.0.349.gf0c1128f8b.dirty
^ permalink raw reply
* Re: [PATCH] send-email: add --compose-cover option
From: Kristoffer Haugsbakk @ 2023-10-12 20:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ben Dooks, git
In-Reply-To: <xmqqlec7mvl1.fsf@gitster.g>
On Thu, Oct 12, 2023, at 22:31, Junio C Hamano wrote:
> Hmph, I am not sure why a format-patch user would bother with the
> "--compose-cover" option for the cover letter, to be honest. I am a
> format-patch user and never drive it from send-email, just like the
> workflow you use, and the norm to me is to review everything, not
> just the cover letter but also the patches, in my editor session.
> You apparently do not need "--edit-patches" option to review and
> adjust as needed before sending, even though that is what you do
> already. Why do you need a "--compose-cover" option?
The thought was to get an interactive editor session for the subject and
the message and let `format-patch` fill in the range-diff etc., but on
second thought I have to do so many edits (fix mistakes and use the cover
letter in later versions) that an interactive session would probably not
be useful for me, after all. If so I would probably need to use some tool
that makes cover-letter a first-class entity that you can edit multiple
times similar to a commit message, like `git-series` (I'm guessing that's
how it works?).
The upcoming `--description-file`[1] is probably what I'm gonna end up
using.
[1] https://lore.kernel.org/git/20230821170720.577820-1-oswald.buddenhagen@gmx.de/
--
Kristoffer Haugsbakk
^ permalink raw reply
* Re: This list is being migrated to new infrastructure (no action required)
From: Junio C Hamano @ 2023-10-12 20:46 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: git
In-Reply-To: <20231012-magnifier-decipher-4493ec@meerkat>
Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:
> On Thu, Oct 12, 2023 at 04:30:54PM -0400, Konstantin Ryabitsev wrote:
>> This list is being migrated to the new vger infrastructure. This should be a
>> fully transparent process and you don't need to change anything about how you
>> participate with the list or how you receive mail.
>>
>> There will be a brief delay with archives on lore.kernel.org. I will follow up
>> once the archive migration has been completed.
>
> This work is completed now. If you think something isn't working right, please
> report it to helpdesk@kernel.org.
Thanks!
^ permalink raw reply
* Re: -q option for git to suppress informational messages?
From: Emily Shaffer @ 2023-10-12 20:46 UTC (permalink / raw)
To: Michal Suchánek; +Cc: git
In-Reply-To: <20231012185308.GB6241@kitsune.suse.cz>
On Thu, Oct 12, 2023 at 11:53 AM Michal Suchánek <msuchanek@suse.de> wrote:
>
> Hello,
>
> when using git in scripts I find that many git commands insist on
> printing informational messages, and the only way to avoid this is to
> sent the output to /dev/null.
Michal, you might want to investigate further whether there is a
plumbing-specific command that meets the needs you're concerned about
instead. Those commands will not have additional human-facing output,
and have a stronger guarantee around backwards-compatibility than the
human-facing commands do.
Commands which count as "plumbing" can be located in `git help git` in
the full list of subcommands; if you're not sure what alternative to
use, I think you can feel free to describe what you're trying to do
here and get advice on which plumbing commands to use instead of
porcelain ones.
>
> While some select commands support the -q option to suppres
> informational messages many don't.
>
> Since there was recenly some discussion of problems that newcomers could
> work on I suppose this could be added to the list if not there already.
>
> Thanks
>
> Michal
^ permalink raw reply
* Re: This list is being migrated to new infrastructure (no action required)
From: Konstantin Ryabitsev @ 2023-10-12 20:40 UTC (permalink / raw)
To: git
In-Reply-To: <20231012-party-legwarmer-551ab7@meerkat>
On Thu, Oct 12, 2023 at 04:30:54PM -0400, Konstantin Ryabitsev wrote:
> This list is being migrated to the new vger infrastructure. This should be a
> fully transparent process and you don't need to change anything about how you
> participate with the list or how you receive mail.
>
> There will be a brief delay with archives on lore.kernel.org. I will follow up
> once the archive migration has been completed.
This work is completed now. If you think something isn't working right, please
report it to helpdesk@kernel.org.
-K
^ permalink raw reply
* Re: [PATCH] send-email: add --compose-cover option
From: Junio C Hamano @ 2023-10-12 20:31 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: Ben Dooks, git
In-Reply-To: <277a80c5-40c9-4f1e-a68f-96673380012b@app.fastmail.com>
"Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:
> 2. This would be really useful in `format-patch`. One could write the
> cover letter immediately instead of either (1) editing the generated
> one (placeholders) or (2) providing a file.
> - Personally I don't use `send-email` immediately—I use `format-patch`
> to generate everything so that I can review it before any
> sending. But there are many different workflows.
Hmph, I am not sure why a format-patch user would bother with the
"--compose-cover" option for the cover letter, to be honest. I am a
format-patch user and never drive it from send-email, just like the
workflow you use, and the norm to me is to review everything, not
just the cover letter but also the patches, in my editor session.
You apparently do not need "--edit-patches" option to review and
adjust as needed before sending, even though that is what you do
already. Why do you need a "--compose-cover" option?
^ 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