* Re: [PATCH] tree-walk: disallow overflowing modes
From: Jeff King @ 2023-01-22 22:02 UTC (permalink / raw)
To: René Scharfe
Cc: Git List, Junio C Hamano, Ævar Arnfjörð Bjarmason
In-Reply-To: <044bdc8f-fdc9-dfd2-6cbb-941513467524@web.de>
On Sun, Jan 22, 2023 at 11:03:38AM +0100, René Scharfe wrote:
> Am 22.01.23 um 08:50 schrieb Jeff King:
> > On Sat, Jan 21, 2023 at 10:36:09AM +0100, René Scharfe wrote:
> >
> >> When parsing tree entries, reject mode values that don't fit into an
> >> unsigned int.
> >
> > Seems reasonable. I don't think you can cause any interesting mischief
> > here, but it's cheap to check, and finding data problems earlier rather
> > than later is always good.
> >
> > Should it be s/unsigned int/uint16_t/, though?
>
> "mode" is declared as unsigned int, and I was more concerned with
> overflowing that.
Doh, I just did "vim -t get_mode" and ended up in the near-identical
version in fast-import.c, which uses uint16_t. Maybe it would want the
same treatment. ;)
> We could be more strict and reject everything that oversteps
> S_IFMT|ALLPERMS, but the latter is not defined everywhere. But
> permission bits are well-known, so the magic number 07777 should be
> recognizable enough. Like this?
It feels like this level of check should be happening in the fsck
caller. In particular, it's nice for this parsing layer to err on the
forgiving side, because the way you get out of these situations often
involves something like "git ls-tree | git mktree".
So in that sense, even pushing the overflow detection into the fsck
would be nice, but of course it's hard to do, since we have no way to
represent the overflowed value. I guess we could have a "be more
careful" flag that the fsck code would pass, but I'm not sure it's worth
the added complexity.
-Peff
^ permalink raw reply
* nested submodules detection w/o .gitmodules file
From: Andry @ 2023-01-22 20:34 UTC (permalink / raw)
To: git
Hello Git,
I have a pretty long investigation has been started from usage 3dparty projects related directly or indirectly to the git submodules:
`svn externals replacement` : https://github.com/chronoxor/gil/issues/6
`svn complete replacement for externals` : https://github.com/dirk-thomas/vcstool/issues/243
And stumbled on this discussion:
`nested submodules detection w/o .gitmodules file` : https://github.com/gitextensions/gitextensions/discussions/10644 (https://github.com/gitextensions/gitextensions/issues/10642)
The main question here is that, could the git have has submodules without `.submodules` file?
If no, then all side projects which utilizes it's own input file for the externals may subsequentially fail:
https://github.com/chronoxor/gil
https://github.com/dirk-thomas/vcstool
https://github.com/ingydotnet/git-subrepo
If yes, then other projects which does rely on the `.submodules` would have not actual or even invalid state:
https://github.com/gitextensions/gitextensions
Or even the github itself:
`Zip archive to include submodule` : https://github.com/dear-github/dear-github/issues/214
(`[PATCH] archive: add –recurse-submodules to git-archive command` : https://git.github.io/rev_news/2022/11/30/edition-93/, https://lore.kernel.org/git/pull.1359.git.git.1665597148042.gitgitgadget@gmail.com/)
Mine point here is that:
Git database is a primary storage. The `.gitmodules` file is not a primary storage, so can be in not an actual or desync state with the database.
And any application or a 3dparty project must read the database directly.
But another problem here is that the git still does not have a stable API for that.
For example, a submodule can be declared directly from the `.git/config` file in a working copy:
https://git-scm.com/docs/git-submodule#Documentation/git-submodule.txt-init--ltpathgt82308203
https://git-scm.com/docs/gitsubmodules#_active_submodules
So, who is right and what is wrong here?
^ permalink raw reply
* Re: [PATCH v2 1/3] branch: fix die_if_checked_out() when ignore_current_worktree
From: Junio C Hamano @ 2023-01-22 19:58 UTC (permalink / raw)
To: Rubén Justo; +Cc: Git List, Phillip Wood
In-Reply-To: <766b25e1-2d7a-7b5c-10a9-43e545a57dba@gmail.com>
Rubén Justo <rjusto@gmail.com> writes:
>> As the proposed log message explained, updating die_if_checked_out()
>> with this patch would fix a bug---can we demonstrate the existing
>> breakage and protect the fix from future breakages by adding a test
>> or two?
>
> 2/3 and 3/3, I think makes more sense on its own commit.
Hmph, how so? Especially once you split 1/3 into the preliminary
refactoring and the real fix, the fix becomes fairly small and
clear. And the tests to protect the fix would go best in the same
commit.
>> The above comment is about find_shared_symref() which iterates over
>> worktrees and find the one that uses the named symref. Now the
>> comment appears to apply to is_shared_symref() which does not
>> iterate but takes one specific worktree instance. Do their
>> differences necessitate some updates to the comment?
>
> I think the comment still makes sense as is for the new function, both the
> description and the recommendation. I will review it again.
OK. Thanks.
>> > +int is_shared_symref(const struct worktree *wt, const char *symref,
>> > + const char *target)
>> > +{
>>
>> What this function does sound more like "is target in use in this
>> particular worktree by being pointed at by the symref?" IOW, I do
>> not see where "shared" comes into its name from.
>>
>> "HEAD" that is tentatively detached while bisecting or rebasing the
>> "target" branch is still considered to point at the "target", so
>> perhaps symref_points_at_target() or something?
>
> I tried to maintain the terms as much as possible. I'll think about the name
> you suggest.
When you did not change a thing in such a way that it does not
change the relationship between that thing and other things, it
makes perfect sense to keep the same term to refer to the thing.
Otherwise, once the thing starts playing different roles in the
world, there may be a better word to refer to the updated and
improved thing.
^ permalink raw reply
* Re: [PATCH v2 1/1] t0003: Call dd with portable blocksize
From: Torsten Bögershausen @ 2023-01-22 18:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqtu0isfom.fsf@gitster.g>
On Sun, Jan 22, 2023 at 08:21:45AM -0800, Junio C Hamano wrote:
> tboegi@web.de writes:
>
> > From: Torsten Bögershausen <tboegi@web.de>
> >
> > The command `dd bs=101M count=1` is not portable,
> > e.g. dd shipped with MacOs does not understand the 'M'.
>
> Very good piece of information to have here.
>
> > Use `bs=1048576 count=101`, which achives the same, instead.
>
> I'd locally tweak (read: no need to resend) it to
>
> Use `dd bs=1048576 count=101`, which ...
>
> and downcase "Call" on the title line.
>
> A tangent. I wonder how portable
>
> dd bs=1024x1024 count=101
This works all Unix-ish system I have: both on MacOs and FreeBSD.
> dd bs=1kx1k count=101
Works on FreeBSD.
Does not work under MacOS: "dd: bs: illegal numeric value"
> In any case, thanks for the update.
Thanks for the review and the tweaking.
^ permalink raw reply
* Re: [PATCH] MyFirstContribution: refrain from self-iterating too much
From: Junio C Hamano @ 2023-01-22 17:14 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git
In-Reply-To: <xmqqcz76tv6d.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Torsten Bögershausen <tboegi@web.de> writes:
>
>> Or:
>> Re-reading my own stuff, I think that things could have been done
>> in a way like this....
>> Lets wait for more comments before I send out a new version.
>
> Again, very good.
The latter one (different course) needs spearate treatment from the
former (typofix). Small corrections without changing course is
better communicated with "will fix when I reroll" while a drastic
change should warn the readers not to waste their time on the
premature initial version, i.e. "I came up with a drastically
different and better idea; please ignore the one I just posted and
wait for an updated one".
I'll sleep on what I rewrote to see if I or other people find better
phrasing.
Thanks.
^ permalink raw reply
* [PATCH] Documentation: render dash correctly
From: Andrei Rybak @ 2023-01-22 16:56 UTC (permalink / raw)
To: git
Three hyphens are rendered verbatim in documentation, so "--" has to be
used to produce a dash. Fix asciidoc output for dashes. This is
similar to previous commits f0b922473e (Documentation: render special
characters correctly, 2021-07-29) and de82095a95 (doc
hash-function-transition: fix asciidoc output, 2021-02-05).
Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
Documentation/git-apply.txt | 2 +-
Documentation/git-read-tree.txt | 2 +-
Documentation/git.txt | 2 +-
Documentation/technical/hash-function-transition.txt | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index 1d478cbe9b..5e16e6db7e 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -208,7 +208,7 @@ behavior:
* `warn` outputs warnings for a few such errors, but applies the
patch as-is (default).
* `fix` outputs warnings for a few such errors, and applies the
- patch after fixing them (`strip` is a synonym --- the tool
+ patch after fixing them (`strip` is a synonym -- the tool
used to consider only trailing whitespace characters as errors, and the
fix involved 'stripping' them, but modern Gits do more).
* `error` outputs warnings for a few such errors, and refuses
diff --git a/Documentation/git-read-tree.txt b/Documentation/git-read-tree.txt
index 7567955bad..b09707474d 100644
--- a/Documentation/git-read-tree.txt
+++ b/Documentation/git-read-tree.txt
@@ -219,7 +219,7 @@ see which of the "local changes" that you made were carried forward by running
`git diff-index --cached $M`. Note that this does not
necessarily match what `git diff-index --cached $H` would have
produced before such a two tree merge. This is because of cases
-18 and 19 --- if you already had the changes in $M (e.g. maybe
+18 and 19 -- if you already had the changes in $M (e.g. maybe
you picked it up via e-mail in a patch form), `git diff-index
--cached $H` would have told you about the change before this
merge, but it would not show in `git diff-index --cached $M`
diff --git a/Documentation/git.txt b/Documentation/git.txt
index f9a7a4554c..74973d3cc4 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -613,7 +613,7 @@ The file parameters can point at the user's working file
(e.g. `new-file` in "git-diff-files"), `/dev/null` (e.g. `old-file`
when a new file is added), or a temporary file (e.g. `old-file` in the
index). `GIT_EXTERNAL_DIFF` should not worry about unlinking the
-temporary file --- it is removed when `GIT_EXTERNAL_DIFF` exits.
+temporary file -- it is removed when `GIT_EXTERNAL_DIFF` exits.
+
For a path that is unmerged, `GIT_EXTERNAL_DIFF` is called with 1
parameter, <path>.
diff --git a/Documentation/technical/hash-function-transition.txt b/Documentation/technical/hash-function-transition.txt
index e2ac36dd21..ed57481089 100644
--- a/Documentation/technical/hash-function-transition.txt
+++ b/Documentation/technical/hash-function-transition.txt
@@ -562,7 +562,7 @@ hash re-encode during clone and to encourage peers to modernize.
The design described here allows fetches by SHA-1 clients of a
personal SHA-256 repository because it's not much more difficult than
allowing pushes from that repository. This support needs to be guarded
-by a configuration option --- servers like git.kernel.org that serve a
+by a configuration option -- servers like git.kernel.org that serve a
large number of clients would not be expected to bear that cost.
Meaning of signatures
--
2.39.0
^ permalink raw reply related
* Re: t5559 breaks with apache 2.4.55
From: Todd Zullinger @ 2023-01-22 16:33 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <Y8ztIqYgVCPILJlO@coredump.intra.peff.net>
Jeff King wrote:
> I noticed that the test suite now fails after upgrading from apache
> 2.4.54 to 2.4.55 (the latter of which just hit debian unstable). The
> problem is with the http2 tests, specifically t5559.30, where we send a
> large fetch negotiation over http2. The output from curl is (including
> some bits of tracing):
>
> == Info: Received 101, Switching to HTTP/2
> == Info: Using HTTP2, server supports multiplexing
> == Info: Copying HTTP/2 data in stream buffer to connection buffer after upgrade: len=0
> == Info: Closing connection 1
> error: RPC failed; HTTP 101 curl 16 Error in the HTTP2 framing layer
>
> Bisecting within apache's Git repo, the culprit is their 9767274b88,
> which says:
>
> mod_http2: version 2.0.10 of the module, synchronizing changes
> with the gitgub version. This is a partial rewrite of how connections
> and streams are handled.
>
> which seems like a plausible source. But the diff is 8000 lines. It may
> be possible to bisect within the mod_http2 source itself, but I haven't
> tried it yet.
>
> It's also not 100% clear that it's an apache bug. We could be doing
> something weird with git-http-backend, or curl might be doing something
> wrong. Though I tend to doubt it, given the simplicity of the CGI
> interface on the server side and the fact that curl was working reliably
> with older versions of apache.
>
> So I haven't reported the bug further yet. But I thought I'd post this
> here before anybody else wastes time digging in the same hole.
FWIW, I think this is the same issue we discussed about 2
months back, in <Y4fUntdlc1mqwad5@pobox.com>¹.
I haven't done much else with it since then. It's almost
surely either an apache httpd/mod_http2 or curl issue. If I
had to bet, I'd say mod_http2. (But then, it could be curl
and just has yet to be exposed widely because not many are
using the current mod_http2 code.)
¹ https://lore.kernel.org/git/Y4fUntdlc1mqwad5@pobox.com/
--
Todd
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2023, #05; Tue, 17)
From: Junio C Hamano @ 2023-01-22 16:37 UTC (permalink / raw)
To: Philip Oakley; +Cc: git
In-Reply-To: <6aaa2463-a92a-de06-5e16-f0980be3ed3f@iee.email>
Philip Oakley <philipoakley@iee.email> writes:
> I've now sent a re-roll. It primarily updates the documentation, and
> adds a test to cover the wide chars and combined characters.
Thanks!
^ permalink raw reply
* Re: [PATCH] tree-walk: disallow overflowing modes
From: Junio C Hamano @ 2023-01-22 16:36 UTC (permalink / raw)
To: René Scharfe
Cc: Jeff King, Git List, Ævar Arnfjörð Bjarmason
In-Reply-To: <044bdc8f-fdc9-dfd2-6cbb-941513467524@web.de>
René Scharfe <l.s.r@web.de> writes:
> We could be more strict and reject everything that oversteps
> S_IFMT|ALLPERMS, but the latter is not defined everywhere. But
> permission bits are well-known, so the magic number 07777 should be
> recognizable enough. Like this?
I do not quite see the reason why we want to be more strict than we
already are at this point in the code path. Stricter mode check in
reports FSCK_MSG_ZERO_PADDED_FILEMODE and FSCK_MSG_BAD_FILEMODE from
"fsck", which I think is probably sufficient.
Avoiding integer wraparound is a good idea, even if it were
impossible to induce misparsing of the tree data to lead to any
security issues, for the same reason why we check for zero padded
filemode, i.e. such a tree mode will allow the same tree object to
be given different object names.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 1/1] t0003: Call dd with portable blocksize
From: Junio C Hamano @ 2023-01-22 16:21 UTC (permalink / raw)
To: tboegi; +Cc: git
In-Reply-To: <20230122062839.14542-1-tboegi@web.de>
tboegi@web.de writes:
> From: Torsten Bögershausen <tboegi@web.de>
>
> The command `dd bs=101M count=1` is not portable,
> e.g. dd shipped with MacOs does not understand the 'M'.
Very good piece of information to have here.
> Use `bs=1048576 count=101`, which achives the same, instead.
I'd locally tweak (read: no need to resend) it to
Use `dd bs=1048576 count=101`, which ...
and downcase "Call" on the title line.
A tangent. I wonder how portable
dd bs=1024x1024 count=101
dd bs=1kx1k count=101
are in practice. "Two or more positive decimal numbers (with or
without 'k' or 'b') separated by 'x', specifying the product of the
indicated values" is from POSIX, but I haven't used it myself (I
know GNU dd groks it).
In any case, thanks for the update.
^ permalink raw reply
* Re: [PATCH] attr: fix instructions on how to check attrs
From: Junio C Hamano @ 2023-01-22 16:10 UTC (permalink / raw)
To: John Cai via GitGitGadget; +Cc: git, John Cai
In-Reply-To: <pull.1441.git.git.1674356774172.gitgitgadget@gmail.com>
"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: John Cai <johncai86@gmail.com>
>
> The instructions in attr.h describing what functions to call to check
> attributes is missing the index as the first argument to git_check_attr.
>
> Fix this to make it consistent with the actual function signature.
Sounds quite sensible. It would have been very good to explain some
research in the above, like
When 7a400a2c (attr: remove an implicit dependency on the_index,
2018-08-13) started passing an index_state instance to
git_check_attr(), it forgot to update the API documentation that
was in Documentation/technical/api-gitattributes.txt. Later,
3a1b3415 (attr: move doc to attr.h, 2019-11-17) moved the API
documentation to attr.h and made it to a comment, without
realizing the earlier mistake.
or something like that.
Thanks.
> diff --git a/attr.h b/attr.h
> index 2f22dffadb3..47f1111f391 100644
> --- a/attr.h
> +++ b/attr.h
> @@ -45,7 +45,7 @@
> * const char *path;
> *
> * setup_check();
> - * git_check_attr(path, check);
> + * git_check_attr(&the_index, path, check);
> * ------------
> *
> * - Act on `.value` member of the result, left in `check->items[]`:
>
> base-commit: 904d404274fef6695c78a6b055edd184b72e2f9b
^ permalink raw reply
* Re: [PATCH] MyFirstContribution: refrain from self-iterating too much
From: Junio C Hamano @ 2023-01-22 16:01 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git
In-Reply-To: <20230122071156.367jwwt3d5txvkl4@tb-raspi4>
Torsten Bögershausen <tboegi@web.de> writes:
>> +Of course, you still may spot mistakes and rooms for improvements
>> +after you sent your initial patch.
>
> I can't resist the question: After outlining what not to do and why,
> could there be a hint what to do ?
There already is and it is the theme of the next paragraph.
People on the list do not have to see your patch immediately after
you wrote it. Instead of seeing the initial version right now that
is followed by a series of "oops, I like this verison better than
the previous one" rerolls over 2 days, reviewers would appreciate if
a single more polished version came 2 days late and that version was
the only one they need to review.
Wait, re-read what you wrote, fix the problems you find locally, all
without sending it out until you find no more "oops, that would not
work" and simple typos. Sleep on it.
Of course, people are not perfect so they may still find issues
after they sent their patches out.
> It may be, that the author justs spots a simple typo, or there may
> be more heavier changes to be done.
>
> Should the author just respond to her/his patch as a reviewer does ?
> Like:
> Ops, there is a "typax", I should have written "typo".
Follow that with the same "I will fix this typo when I reroll, but
I'll wait for reviews from others" as the other one, and it would be
the second best thing you could do (the best is to avoid having to
say that, of course).
> Or:
> Re-reading my own stuff, I think that things could have been done
> in a way like this....
> Lets wait for more comments before I send out a new version.
Again, very good.
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2023, #05; Tue, 17)
From: Philip Oakley @ 2023-01-22 15:56 UTC (permalink / raw)
To: Junio C Hamano, git
In-Reply-To: <xmqqv8l581ov.fsf@gitster.g>
On 17/01/2023 18:18, Junio C Hamano wrote:
> --------------------------------------------------
> [Stalled]
> * po/pretty-hard-trunc (2022-11-13) 1 commit
> - pretty-formats: add hard truncation, without ellipsis, options
>
> Add a new pretty format which truncates without ellipsis.
>
> Expecting a reroll.
> cf. <093e1dca-b9d4-f1f2-0845-ad6711622cf5@iee.email>
> source: <20221112143616.1429-1-philipoakley@iee.email>
>
Hi Junio,
I've now sent a re-roll. It primarily updates the documentation, and
adds a test to cover the wide chars and combined characters.
https://lore.kernel.org/git/20230119181827.1319-1-philipoakley@iee.email/
Philip
^ permalink raw reply
* [ANNOUNCE] git-sim: a command line tool to visually simulate local git operations
From: Jacob Stopak @ 2023-01-22 15:02 UTC (permalink / raw)
To: git
I've been working on a free and open-source dev tool called "git-sim" which
allows devs to visually simulate Git commands using a git-like syntax directly
in their local repos from the command-line.
Posting it here to put out some feelers and to see if folks in the community might
find it interesting / useful, and if so to get some feedback on how to improve it.
The goal of the tool is to help make Git more accessible to newcomers, as well
as to allows users of all experience levels to get a clear, visual idea of how a
particular Git command will affect the state of their repo before actually
running it.
Think of it as serving a similar purpose as the --dry-run option that some Git
commands have, but where the result is an output image (default) or video
animation of the impact of the Git command in question. My hope is that this
could be a great asset for visual learners.
Git-Sim is written in Python and leverages the GitPython library to interact
with local Git repos and Manim (Math-animation) library to draw and animate the
simulated Git command output.
Here's a link to the codebase/readme on GitHub detailing what Git-Sim does:
https://github.com/initialcommit-com/git-sim
Supported commands so far are: log, status, add, restore, commit, stash, branch,
tag, reset, revert, merge, rebase, cherry-pick
So far the functionality is quite basic, but built out enough that hopefully
folks can get a decent idea of how (and whether) it can be useful. I'm sure
it has bugs and obvious enhancements to be made.
All thoughts/suggestions/criticisms/ideas are welcome.
-Jack
^ permalink raw reply
* Re: [PATCH v2 1/3] branch: fix die_if_checked_out() when ignore_current_worktree
From: Rubén Justo @ 2023-01-22 11:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List, Phillip Wood
In-Reply-To: <xmqqbkmruykg.fsf@gitster.g>
On 21-ene-2023 17:50:55, Junio C Hamano wrote:
> Rubén Justo <rjusto@gmail.com> writes:
>
> > Let's stop using find_shared_symref() in die_if_checked_out(), to handle
> > correctly ignore_current_worktree.
>
> This says what the code stops using, but does not say what it uses
> instead.
I thought the code for that was a better and clearer description. I'll add
some details to the message.
> Factoring is_shared_symref() out of find_shared_symref() is probably
> a good idea that can stand alone without any other change in this
> patch as a single step, and then a second step can update
> die_if_checked_out() using the new function.
OK. I'll split that in two.
> As the proposed log message explained, updating die_if_checked_out()
> with this patch would fix a bug---can we demonstrate the existing
> breakage and protect the fix from future breakages by adding a test
> or two?
2/3 and 3/3, I think makes more sense on its own commit.
> > - const struct worktree *wt;
> > + int i;
> > +
> > + for (i = 0; worktrees[i]; i++)
> > + {
>
> Style. WRite the above on a single line, i.e.
>
> for (i = 0; worktrees[i]; i++) {
Sorry. I'll fix that.
>
> Optionally, we can lose the separate declaration of "i" by using C99
> variable declaration, i.e.
>
> for (int i = 0; worktrees[i]; i++) {
OK. Yes, I was playing with this, changed my mind and ended up with this and
the other style below, mess.
>
> > diff --git a/worktree.c b/worktree.c
> > index aa43c64119..d500d69e4c 100644
> > --- a/worktree.c
> > +++ b/worktree.c
> > @@ -403,6 +403,33 @@ int is_worktree_being_bisected(const struct worktree *wt,
> > * bisect). New commands that do similar things should update this
> > * function as well.
> > */
>
> The above comment is about find_shared_symref() which iterates over
> worktrees and find the one that uses the named symref. Now the
> comment appears to apply to is_shared_symref() which does not
> iterate but takes one specific worktree instance. Do their
> differences necessitate some updates to the comment?
I think the comment still makes sense as is for the new function, both the
description and the recommendation. I will review it again.
>
> > +int is_shared_symref(const struct worktree *wt, const char *symref,
> > + const char *target)
> > +{
>
> What this function does sound more like "is target in use in this
> particular worktree by being pointed at by the symref?" IOW, I do
> not see where "shared" comes into its name from.
>
> "HEAD" that is tentatively detached while bisecting or rebasing the
> "target" branch is still considered to point at the "target", so
> perhaps symref_points_at_target() or something?
>
I tried to maintain the terms as much as possible. I'll think about the name
you suggest.
> > const struct worktree *find_shared_symref(struct worktree **worktrees,
> > const char *symref,
> > const char *target)
> > @@ -411,31 +438,8 @@ const struct worktree *find_shared_symref(struct worktree **worktrees,
> > int i = 0;
> >
> > for (i = 0; worktrees[i]; i++) {
>
> Not a new problem, but the initialization on the declaration of "i"
> is redundant and unnecessary. Again, we can use the C99 style, i.e.
>
> const struct worktree *existing = NULL;
> - int i = 0;
> -
> - for (i = 0; worktrees[i]; i++) {
> + for (int i = 0; worktrees[i]; i++) {
I'll fix this.
>
> > + if (is_shared_symref(worktrees[i], symref, target)) {
> > + existing = worktrees[i];
> > break;
> > }
> > }
> > diff --git a/worktree.h b/worktree.h
> > index 9dcea6fc8c..7889c4761d 100644
> > --- a/worktree.h
> > +++ b/worktree.h
> > @@ -149,6 +149,12 @@ const struct worktree *find_shared_symref(struct worktree **worktrees,
> > const char *symref,
> > const char *target);
> >
> > +/*
> > + * Returns true if a symref points to a ref in a worktree.
> > + */
>
> Make it clear that what you called "a ref" in the above is what is
> called "target" below.
>
Again, that was an attempt to maintain the terms from find_shared_symref().
Thank you for your review.
^ permalink raw reply
* Re: [RFC/PATCH 0/6] hash-object: use fsck to check objects
From: René Scharfe @ 2023-01-22 11:39 UTC (permalink / raw)
To: Jeff King; +Cc: git, Ævar Arnfjörð Bjarmason
In-Reply-To: <Y8zqZH+X6fOoCOYV@coredump.intra.peff.net>
Am 22.01.23 um 08:48 schrieb Jeff King:
> On Sat, Jan 21, 2023 at 10:36:08AM +0100, René Scharfe wrote:
>
>> Am 19.01.23 um 02:39 schrieb Jeff King:
>>>
>>> Though I do find the use of strlen() in decode_tree_entry()
>>> a little suspicious (and that would be true of the current code, as
>>> well, since it powers hash-object's existing parsing check).
>>
>> strlen() won't overrun the buffer because the first check in
>> decode_tree_entry() makes sure there is a NUL in the buffer ahead.
>> If get_mode() crosses it then we exit early.
>
> Yeah, that was what I found after digging deeper (see my patch 7).
>
>> Storing the result in an unsigned int can overflow on platforms where
>> size_t is bigger. That would result in pathlen values being too short
>> for really long paths, but no out-of-bounds access. They are then
>> stored as signed int in struct name_entry and used as such in many
>> places -- that seems like a bad idea, but I didn't actually check them
>> thoroughly.
>
> Yeah, I agree that the use of a signed int there looks questionable. I
> do think it's orthogonal to my series here, as that tree-decoding is
> used by the existing hash-object checks.
Sure.
> But it probably bears further examination, especially because we use it
> for the fsck checks on incoming objects via receive-pack, etc, which are
> meant to be the first line of defense for hosters who might receive
> malicious garbage from users.
>
> We probably ought to reject trees with enormous names via fsck anyway. I
> actually have a patch to do that, but of course it depends on
> decode_tree_entry() to get the length, so there's a bit of
> chicken-and-egg.
Solvable by limiting the search for the end of the string in
decode_tree_entry() by using strnlen(3) or memchr(3) instead of
strlen(3). You just need to define some (configurable?) limit.
> We probably also should outright reject gigantic trees,
> which closes out a whole class of integer truncation problems. I know
> GitHub has rejected trees over 100MB for years for this reason.
Makes sense.
>> get_mode() can overflow "mode" if there are too many octal digits. Do
>> we need to accept more than two handfuls in the first place? I'll send
>> a patch for at least rejecting overflow.
>
> Seems reasonable. I doubt there's an interesting attack here, just
> because the mode isn't used to index anything. If you feed a garbage
> mode that happens to overflow to something useful, you could just as
> easily have sent the useful mode in the first place.
>
>> Hmm, what would be the performance impact of trees with mode fields
>> zero-padded to silly lengths?
>
> Certainly it would waste some time parsing the tree, but you could do
> that already with a long pathname. Or just having a lot of paths in a
> tree. Or a lot of trees.
That's a cup half full/empty thing, perhaps. What's the harm in leading
zeros? vs. Why allow leading zeros?
René
^ permalink raw reply
* Re: [PATCH] tree-walk: disallow overflowing modes
From: René Scharfe @ 2023-01-22 10:03 UTC (permalink / raw)
To: Jeff King
Cc: Git List, Junio C Hamano, Ævar Arnfjörð Bjarmason
In-Reply-To: <Y8zquGar3rLyRdTp@coredump.intra.peff.net>
Am 22.01.23 um 08:50 schrieb Jeff King:
> On Sat, Jan 21, 2023 at 10:36:09AM +0100, René Scharfe wrote:
>
>> When parsing tree entries, reject mode values that don't fit into an
>> unsigned int.
>
> Seems reasonable. I don't think you can cause any interesting mischief
> here, but it's cheap to check, and finding data problems earlier rather
> than later is always good.
>
> Should it be s/unsigned int/uint16_t/, though?
"mode" is declared as unsigned int, and I was more concerned with
overflowing that.
We could be more strict and reject everything that oversteps
S_IFMT|ALLPERMS, but the latter is not defined everywhere. But
permission bits are well-known, so the magic number 07777 should be
recognizable enough. Like this?
--- >8 ---
Subject: [PATCH v2] tree-walk: disallow overflowing modes
When parsing tree entries, reject mode values with bits set outside file
type mask and permission bits.
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
tree-walk.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tree-walk.c b/tree-walk.c
index 74f4d710e8..62da0e5c73 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -18,6 +18,8 @@ static const char *get_mode(const char *str, unsigned int *modep)
if (c < '0' || c > '7')
return NULL;
mode = (mode << 3) + (c - '0');
+ if (mode & ~(S_IFMT | 07777))
+ return NULL;
}
*modep = mode;
return str;
--
2.39.1
^ permalink raw reply related
* t5559 breaks with apache 2.4.55
From: Jeff King @ 2023-01-22 8:00 UTC (permalink / raw)
To: git
I noticed that the test suite now fails after upgrading from apache
2.4.54 to 2.4.55 (the latter of which just hit debian unstable). The
problem is with the http2 tests, specifically t5559.30, where we send a
large fetch negotiation over http2. The output from curl is (including
some bits of tracing):
== Info: Received 101, Switching to HTTP/2
== Info: Using HTTP2, server supports multiplexing
== Info: Copying HTTP/2 data in stream buffer to connection buffer after upgrade: len=0
== Info: Closing connection 1
error: RPC failed; HTTP 101 curl 16 Error in the HTTP2 framing layer
Bisecting within apache's Git repo, the culprit is their 9767274b88,
which says:
mod_http2: version 2.0.10 of the module, synchronizing changes
with the gitgub version. This is a partial rewrite of how connections
and streams are handled.
which seems like a plausible source. But the diff is 8000 lines. It may
be possible to bisect within the mod_http2 source itself, but I haven't
tried it yet.
It's also not 100% clear that it's an apache bug. We could be doing
something weird with git-http-backend, or curl might be doing something
wrong. Though I tend to doubt it, given the simplicity of the CGI
interface on the server side and the fact that curl was working reliably
with older versions of apache.
So I haven't reported the bug further yet. But I thought I'd post this
here before anybody else wastes time digging in the same hole.
-Peff
^ permalink raw reply
* Re: [PATCH] tree-walk: disallow overflowing modes
From: Jeff King @ 2023-01-22 7:50 UTC (permalink / raw)
To: René Scharfe
Cc: Git List, Junio C Hamano, Ævar Arnfjörð Bjarmason
In-Reply-To: <d673fde7-7eb2-6306-86b6-1c1a4c988ee8@web.de>
On Sat, Jan 21, 2023 at 10:36:09AM +0100, René Scharfe wrote:
> When parsing tree entries, reject mode values that don't fit into an
> unsigned int.
Seems reasonable. I don't think you can cause any interesting mischief
here, but it's cheap to check, and finding data problems earlier rather
than later is always good.
Should it be s/unsigned int/uint16_t/, though?
-Peff
^ permalink raw reply
* Re: [RFC/PATCH 0/6] hash-object: use fsck to check objects
From: Jeff King @ 2023-01-22 7:48 UTC (permalink / raw)
To: René Scharfe; +Cc: git, Ævar Arnfjörð Bjarmason
In-Reply-To: <97faa323-a5b9-e459-70d7-3f6318446898@web.de>
On Sat, Jan 21, 2023 at 10:36:08AM +0100, René Scharfe wrote:
> Am 19.01.23 um 02:39 schrieb Jeff King:
> >
> > Though I do find the use of strlen() in decode_tree_entry()
> > a little suspicious (and that would be true of the current code, as
> > well, since it powers hash-object's existing parsing check).
>
> strlen() won't overrun the buffer because the first check in
> decode_tree_entry() makes sure there is a NUL in the buffer ahead.
> If get_mode() crosses it then we exit early.
Yeah, that was what I found after digging deeper (see my patch 7).
> Storing the result in an unsigned int can overflow on platforms where
> size_t is bigger. That would result in pathlen values being too short
> for really long paths, but no out-of-bounds access. They are then
> stored as signed int in struct name_entry and used as such in many
> places -- that seems like a bad idea, but I didn't actually check them
> thoroughly.
Yeah, I agree that the use of a signed int there looks questionable. I
do think it's orthogonal to my series here, as that tree-decoding is
used by the existing hash-object checks.
But it probably bears further examination, especially because we use it
for the fsck checks on incoming objects via receive-pack, etc, which are
meant to be the first line of defense for hosters who might receive
malicious garbage from users.
We probably ought to reject trees with enormous names via fsck anyway. I
actually have a patch to do that, but of course it depends on
decode_tree_entry() to get the length, so there's a bit of
chicken-and-egg. We probably also should outright reject gigantic trees,
which closes out a whole class of integer truncation problems. I know
GitHub has rejected trees over 100MB for years for this reason.
> get_mode() can overflow "mode" if there are too many octal digits. Do
> we need to accept more than two handfuls in the first place? I'll send
> a patch for at least rejecting overflow.
Seems reasonable. I doubt there's an interesting attack here, just
because the mode isn't used to index anything. If you feed a garbage
mode that happens to overflow to something useful, you could just as
easily have sent the useful mode in the first place.
> Hmm, what would be the performance impact of trees with mode fields
> zero-padded to silly lengths?
Certainly it would waste some time parsing the tree, but you could do
that already with a long pathname. Or just having a lot of paths in a
tree. Or a lot of trees.
-Peff
^ permalink raw reply
* Re: [PATCH] MyFirstContribution: refrain from self-iterating too much
From: Torsten Bögershausen @ 2023-01-22 7:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq3583uyk0.fsf@gitster.g>
On Sat, Jan 21, 2023 at 05:51:11PM -0800, Junio C Hamano wrote:
> Finding mistakes in and improving your own patches is a good idea,
> but doing so too quickly is being inconsiderate to reviewers who
> have just seen the initial iteration and taking their time to review
> it. Encourage new developers to perform such a self review before
> they send out their patches, not after.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> Documentation/MyFirstContribution.txt | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git c/Documentation/MyFirstContribution.txt w/Documentation/MyFirstContribution.txt
> index 7c9a037cc2..81dcaedf0c 100644
> --- c/Documentation/MyFirstContribution.txt
> +++ w/Documentation/MyFirstContribution.txt
> @@ -1136,6 +1136,26 @@ index 88f126184c..38da593a60 100644
> [[now-what]]
> == My Patch Got Emailed - Now What?
>
> +You should wait for your patch to be reviewed by other people in the
> +development community. While you are waiting, you may want to
> +re-read what you wrote in the patch you already have sent, as if you
> +are a reviewer who is helping you to improve your patch. But resist
> +the temptation to update the patch and send a new one, until other
> +people had enough time to digest your original patch and give you
> +their reviews. They may be taking time to give you a carefully
> +written review responses and haven't finished it yet. Bombarding
> +them with new versions before they have a chance to react to the
> +first iteration is being rude to them.
> +
> +Of course, you still may spot mistakes and rooms for improvements
> +after you sent your initial patch.
I can't resist the question: After outlining what not to do and why,
could there be a hint what to do ?
It may be, that the author justs spots a simple typo, or there may
be more heavier changes to be done.
Should the author just respond to her/his patch as a reviewer does ?
Like:
Ops, there is a "typax", I should have written "typo".
Or:
Re-reading my own stuff, I think that things could have been done
in a way like this....
Lets wait for more comments before I send out a new version.
>+ Learn from that experience to
> +make sure that you will do such a self-review _before_ sending your
> +patches next time. You do not have to send your patches immediately
> +once you finished writing them. It is not a race. Take your time
> +to self-review them, sleep on them, improve them before sending them
> +out, and do not allow you to send them before you are reasonably
> +sure that you won't find more mistakes in them yourself.
> +
> [[reviewing]]
> === Responding to Reviews
>
^ permalink raw reply
* [PATCH v2 1/1] t0003: Call dd with portable blocksize
From: tboegi @ 2023-01-22 6:28 UTC (permalink / raw)
To: tboegi, git
In-Reply-To: <20230121110505.21362-1-tboegi@web.de>
From: Torsten Bögershausen <tboegi@web.de>
The command `dd bs=101M count=1` is not portable,
e.g. dd shipped with MacOs does not understand the 'M'.
Use `bs=1048576 count=101`, which achives the same, instead.
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
t/t0003-attributes.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index d0284fe2d7..394a08e6d6 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -400,7 +400,7 @@ test_expect_success 'large attributes line ignores trailing content in tree' '
test_expect_success EXPENSIVE 'large attributes file ignored in tree' '
test_when_finished "rm .gitattributes" &&
- dd if=/dev/zero of=.gitattributes bs=101M count=1 2>/dev/null &&
+ dd if=/dev/zero of=.gitattributes bs=1048576 count=101 2>/dev/null &&
git check-attr --all path >/dev/null 2>err &&
echo "warning: ignoring overly large gitattributes file ${SQ}.gitattributes${SQ}" >expect &&
test_cmp expect err
@@ -428,7 +428,7 @@ test_expect_success 'large attributes line ignores trailing content in index' '
test_expect_success EXPENSIVE 'large attributes file ignored in index' '
test_when_finished "git update-index --remove .gitattributes" &&
- blob=$(dd if=/dev/zero bs=101M count=1 2>/dev/null | git hash-object -w --stdin) &&
+ blob=$(dd if=/dev/zero bs=1048576 count=101 2>/dev/null | git hash-object -w --stdin) &&
git update-index --add --cacheinfo 100644,$blob,.gitattributes &&
git check-attr --cached --all path >/dev/null 2>err &&
echo "warning: ignoring overly large gitattributes blob ${SQ}.gitattributes${SQ}" >expect &&
--
2.39.1.254.g904d404274
^ permalink raw reply related
* [PATCH v4 6/9] rebase: clarify the OPT_CMDMODE incompatibilities
From: Elijah Newren via GitGitGadget @ 2023-01-22 6:12 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Elijah Newren, Eric Sunshine, Martin Ågren,
Phillip Wood, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1466.v4.git.1674367961.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
--edit-todo was documented as being incompatible with any of the options
for the apply backend. However, it is also incompatible with any of the
options for the merge backend, and is incompatible with any options that
are not backend specific as well. The same can be said for --continue,
--skip, --abort, --quit, etc.
This is already somewhat implicitly covered by the synopsis, but since
"[<options>]" in the first two variants are vague it might be easy to
miss this. That might not be a big deal, but since the rebase manpage
has to spend so much verbiage about incompatibility of options, making
a separate section for these options that are incompatible with
everything else seems clearer. Do that, and remove the needless
inclusion of --edit-todo in the explicit incompatibility list.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
Documentation/git-rebase.txt | 61 +++++++++++++++++++-----------------
1 file changed, 33 insertions(+), 28 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 8cb075b2bdb..1ba691e4c5f 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -208,6 +208,39 @@ Alternatively, you can undo the 'git rebase' with
git rebase --abort
+MODE OPTIONS
+------------
+
+The options in this section cannot be used with any other option,
+including not with each other:
+
+--continue::
+ Restart the rebasing process after having resolved a merge conflict.
+
+--skip::
+ Restart the rebasing process by skipping the current patch.
+
+--abort::
+ Abort the rebase operation and reset HEAD to the original
+ branch. If `<branch>` was provided when the rebase operation was
+ started, then `HEAD` will be reset to `<branch>`. Otherwise `HEAD`
+ will be reset to where it was when the rebase operation was
+ started.
+
+--quit::
+ Abort the rebase operation but `HEAD` is not reset back to the
+ original branch. The index and working tree are also left
+ unchanged as a result. If a temporary stash entry was created
+ using `--autostash`, it will be saved to the stash list.
+
+--edit-todo::
+ Edit the todo list during an interactive rebase.
+
+--show-current-patch::
+ Show the current patch in an interactive rebase or when rebase
+ is stopped because of conflicts. This is the equivalent of
+ `git show REBASE_HEAD`.
+
OPTIONS
-------
--onto <newbase>::
@@ -249,22 +282,6 @@ See also INCOMPATIBLE OPTIONS below.
<branch>::
Working branch; defaults to `HEAD`.
---continue::
- Restart the rebasing process after having resolved a merge conflict.
-
---abort::
- Abort the rebase operation and reset HEAD to the original
- branch. If `<branch>` was provided when the rebase operation was
- started, then `HEAD` will be reset to `<branch>`. Otherwise `HEAD`
- will be reset to where it was when the rebase operation was
- started.
-
---quit::
- Abort the rebase operation but `HEAD` is not reset back to the
- original branch. The index and working tree are also left
- unchanged as a result. If a temporary stash entry was created
- using `--autostash`, it will be saved to the stash list.
-
--apply::
Use applying strategies to rebase (calling `git-am`
internally). This option may become a no-op in the future
@@ -345,17 +362,6 @@ See also INCOMPATIBLE OPTIONS below.
+
See also INCOMPATIBLE OPTIONS below.
---skip::
- Restart the rebasing process by skipping the current patch.
-
---edit-todo::
- Edit the todo list during an interactive rebase.
-
---show-current-patch::
- Show the current patch in an interactive rebase or when rebase
- is stopped because of conflicts. This is the equivalent of
- `git show REBASE_HEAD`.
-
-m::
--merge::
Using merging strategies to rebase (default).
@@ -651,7 +657,6 @@ are incompatible with the following options:
* --no-keep-empty
* --empty=
* --[no-]reapply-cherry-picks
- * --edit-todo
* --update-refs
* --root when used without --onto
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 7/9] rebase: fix formatting of rebase --reapply-cherry-picks option in docs
From: Elijah Newren via GitGitGadget @ 2023-01-22 6:12 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Elijah Newren, Eric Sunshine, Martin Ågren,
Phillip Wood, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1466.v4.git.1674367961.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
Commit ce5238a690 ("rebase --keep-base: imply --reapply-cherry-picks",
2022-10-17) accidentally added some blank lines that cause extra
paragraphs about --reapply-cherry-picks to be considered not part of
the documentation of that option. Remove the blank lines to make it
clear we are still discussing --reapply-cherry-picks.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
Documentation/git-rebase.txt | 2 --
1 file changed, 2 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 1ba691e4c5f..9f794f5bdcc 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -338,7 +338,6 @@ See also INCOMPATIBLE OPTIONS below.
upstream changes, the behavior towards them is controlled by
the `--empty` flag.)
+
-
In the absence of `--keep-base` (or if `--no-reapply-cherry-picks` is
given), these commits will be automatically dropped. Because this
necessitates reading all upstream commits, this can be expensive in
@@ -347,7 +346,6 @@ read. When using the 'merge' backend, warnings will be issued for each
dropped commit (unless `--quiet` is given). Advice will also be issued
unless `advice.skippedCherryPicks` is set to false (see
linkgit:git-config[1]).
-
+
`--reapply-cherry-picks` allows rebase to forgo reading all upstream
commits, potentially improving performance.
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 4/9] rebase: fix docs about incompatibilities with --root
From: Elijah Newren via GitGitGadget @ 2023-01-22 6:12 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Elijah Newren, Eric Sunshine, Martin Ågren,
Phillip Wood, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1466.v4.git.1674367961.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
In commit 5dacd4abdd ("git-rebase.txt: document incompatible options",
2018-06-25), I added notes about incompatibilities between options for
the apply and merge backends. Unfortunately, I inverted the condition
when --root was incompatible with the apply backend. Fix the
documentation, and add a testcase that verifies the documentation
matches the code.
While at it, the documentation for --root also tried to cover some of
the backend differences between the apply and merge backends in relation
to reapplying cherry picks. The information:
* assumed that the apply backend was the default (it isn't anymore)
* was written before --reapply-cherry-picks became an option
* was written before the detailed information on backend differences
All of these factors make the sentence under --root about reapplying
cherry picks contradict information that is now available elsewhere in
the manual, and the other references are correct. So just strike this
sentence.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
Documentation/git-rebase.txt | 7 ++-----
t/t3422-rebase-incompatible-options.sh | 4 ++++
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 7d01d1412d1..846aeed1b69 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -574,10 +574,7 @@ See also INCOMPATIBLE OPTIONS below.
--root::
Rebase all commits reachable from `<branch>`, instead of
limiting them with an `<upstream>`. This allows you to rebase
- the root commit(s) on a branch. When used with `--onto`, it
- will skip changes already contained in `<newbase>` (instead of
- `<upstream>`) whereas without `--onto` it will operate on every
- change.
+ the root commit(s) on a branch.
+
See also INCOMPATIBLE OPTIONS below.
@@ -656,7 +653,7 @@ are incompatible with the following options:
* --reapply-cherry-picks
* --edit-todo
* --update-refs
- * --root when used in combination with --onto
+ * --root when used without --onto
In addition, the following pairs of options are incompatible:
diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh
index 9b9e78479f6..f86274990b0 100755
--- a/t/t3422-rebase-incompatible-options.sh
+++ b/t/t3422-rebase-incompatible-options.sh
@@ -65,6 +65,10 @@ test_rebase_am_only () {
test_must_fail git rebase $opt --update-refs A
"
+ test_expect_success "$opt incompatible with --root without --onto" "
+ git checkout B^0 &&
+ test_must_fail git rebase $opt --root A
+ "
}
# Check options which imply --apply
--
gitgitgadget
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox