* [PATCH v3] mingw: replace deprecated GetVersion with RtlGetVersion
From: Rose via GitGitGadget @ 2023-01-21 19:50 UTC (permalink / raw)
To: git; +Cc: Rose, Seija Kijin
In-Reply-To: <pull.1438.v2.git.git.1674149773693.gitgitgadget@gmail.com>
From: Seija Kijin <doremylover123@gmail.com>
The previous way is deprecated and returns
the wrong value in Windows 8 and up,
returning the manifest Windows data
as opposed to the actual Windows data.
RtlGetVersion is the correct way
to get the Windows version now.
Note: ntdll does not need to be
manually loaded into the runtime,
as this is the one special library
that is automatically loaded upon launch.
Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
mingw: replace deprecated GetVersion with RtlGetVersion
GetVersion has its behavior changed in Windows 8 and above anyway, so
this is the right way to do it now.
The previous way returns the wrong value in Windows 8 and up, returning
the manifest Windows data as opposed to the actual Windows data.
Signed-off-by: Seija Kijin doremylover123@gmail.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1438%2FAtariDreams%2Fmingw-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1438/AtariDreams/mingw-v3
Pull-Request: https://github.com/git/git/pull/1438
Range-diff vs v2:
1: e5457905028 ! 1: 31f778a6b34 mingw: replace deprecated GetVersion with RtlGetVersion
@@ Commit message
returning the manifest Windows data
as opposed to the actual Windows data.
- RtlGetVersion is the correct way to get the Windows version now.
+ RtlGetVersion is the correct way
+ to get the Windows version now.
+
+ Note: ntdll does not need to be
+ manually loaded into the runtime,
+ as this is the one special library
+ that is automatically loaded upon launch.
Signed-off-by: Seija Kijin <doremylover123@gmail.com>
@@ compat/mingw.c: int wmain(int argc, const wchar_t **wargv)
{
- unsigned v = (unsigned)GetVersion();
+ union winprocaddr RtlGetVersionInternal;
-+ OSVERSIONINFOA version;
++ OSVERSIONINFOW version;
++
++ memset(&version, 0, sizeof(version));
++ version.dwOSVersionInfoSize = sizeof(version);
+
++ /* RtlGetVersion always gets the true Windows version, even when running
++ * under Windows's compatibility mode*/
+ RtlGetVersionInternal.procaddr =
+ GetProcAddress(GetModuleHandleW(L"ntdll.dll"), "RtlGetVersion");
-+ if (!RtlGetVersionInternal.procaddr) {
-+ /* if this is reached, something is seriously, seriously wrong
-+ */
-+ perror("Could not call RtlGetVersion in ntdll.dll");
-+ abort();
-+ }
+
-+ version.dwOSVersionInfoSize = sizeof(OSVERSIONINFO);
-+ RtlGetVersionInternal.procGetVersion((PRTL_OSVERSIONINFOW)&version);
++ if (RtlGetVersionInternal.procaddr) {
++ RtlGetVersionInternal.procGetVersion((PRTL_OSVERSIONINFOW)&version);
++ } else {
++ /* Should not happen, but just in case, fallback to deprecated
++ * GetVersionExW */
++ GetVersionExW(&version);
++ }
+
memset(buf, 0, sizeof(*buf));
xsnprintf(buf->sysname, sizeof(buf->sysname), "Windows");
compat/mingw.c | 39 +++++++++++++++++++++++++++++++++------
1 file changed, 33 insertions(+), 6 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index af397e68a1d..b1d75c93cfe 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -3081,15 +3081,42 @@ int wmain(int argc, const wchar_t **wargv)
return exit_status;
}
+/*
+ * for RtlGetVersion in uname
+ */
+
+typedef NTSTATUS(WINAPI *RtlGetVersionPtr)(PRTL_OSVERSIONINFOW);
+union winprocaddr {
+ FARPROC procaddr;
+ RtlGetVersionPtr procGetVersion;
+};
+
int uname(struct utsname *buf)
{
- unsigned v = (unsigned)GetVersion();
+ union winprocaddr RtlGetVersionInternal;
+ OSVERSIONINFOW version;
+
+ memset(&version, 0, sizeof(version));
+ version.dwOSVersionInfoSize = sizeof(version);
+
+ /* RtlGetVersion always gets the true Windows version, even when running
+ * under Windows's compatibility mode*/
+ RtlGetVersionInternal.procaddr =
+ GetProcAddress(GetModuleHandleW(L"ntdll.dll"), "RtlGetVersion");
+
+ if (RtlGetVersionInternal.procaddr) {
+ RtlGetVersionInternal.procGetVersion((PRTL_OSVERSIONINFOW)&version);
+ } else {
+ /* Should not happen, but just in case, fallback to deprecated
+ * GetVersionExW */
+ GetVersionExW(&version);
+ }
+
memset(buf, 0, sizeof(*buf));
xsnprintf(buf->sysname, sizeof(buf->sysname), "Windows");
- xsnprintf(buf->release, sizeof(buf->release),
- "%u.%u", v & 0xff, (v >> 8) & 0xff);
- /* assuming NT variants only.. */
- xsnprintf(buf->version, sizeof(buf->version),
- "%u", (v >> 16) & 0x7fff);
+ xsnprintf(buf->release, sizeof(buf->release), "%lu.%lu",
+ version.dwMajorVersion, version.dwMinorVersion);
+ xsnprintf(buf->version, sizeof(buf->version), "%lu",
+ version.dwBuildNumber);
return 0;
}
base-commit: 904d404274fef6695c78a6b055edd184b72e2f9b
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v3 5/7] rebase: add coverage of other incompatible options
From: Phillip Wood @ 2023-01-21 19:25 UTC (permalink / raw)
To: Elijah Newren via GitGitGadget, git
Cc: Derrick Stolee, Elijah Newren, Eric Sunshine, Martin Ågren,
Phillip Wood
In-Reply-To: <d5ef5870-baac-14d4-65a5-deb94a848011@dunelm.org.uk>
On 21/01/2023 15:20, Phillip Wood wrote:
>> @@ -1238,6 +1238,17 @@ int cmd_rebase(int argc, const char **argv,
>> const char *prefix)
>> if (options.fork_point < 0)
>> options.fork_point = 0;
>> }
>> + /*
>> + * reapply_cherry_picks is slightly weird. It starts out with a
>> + * value of -1. It will be assigned a value of keep_base below and
>> + * then handled specially. The apply backend is hardcoded to
>> + * behave like reapply_cherry_picks==1,
>
> I think it is hard coded to 0 not 1. We generate the patches with
>
> git format-patch --right-only $upstream...$head
Sorry I somhow managed to omit --cherry-pick from that, it should have been
git format-patch --right-only --cherry-pick $upstream...$head
Phillip
> so cherry-picks will not be reapplied. I'm hardly an impartial observer
> but I think the current check is correct. We support
>
> --whitespace=fix --no-reapply-cherry-picks
> --whitespace=fix --keep-base --reapply-cherry-picks
>
> but not
>
> --whitespace=fix --reapply-cherry-picks
>
>> @@ -1525,6 +1529,9 @@ int cmd_rebase(int argc, const char **argv,
>> const char *prefix)
>> if (options.update_refs)
>> imply_merge(&options, "--update-refs");
>> + if (options.autosquash)
>> + imply_merge(&options, "--autosquash");
>
> Thanks for adding this, it maybe merits a mention in the commit message
> though as it is a change in behavior for users who have
> rebase.autosquash set and try to use the apply backend.
>
> Best Wishes
>
> Phillip
>
>> if (options.type == REBASE_UNSPECIFIED) {
>> if (!strcmp(options.default_backend, "merge"))
>> imply_merge(&options, "--merge");
>> diff --git a/t/t3422-rebase-incompatible-options.sh
>> b/t/t3422-rebase-incompatible-options.sh
>> index f86274990b0..c830025470f 100755
>> --- a/t/t3422-rebase-incompatible-options.sh
>> +++ b/t/t3422-rebase-incompatible-options.sh
>> @@ -50,6 +50,11 @@ test_rebase_am_only () {
>> test_must_fail git rebase $opt --strategy-option=ours A
>> "
>> + test_expect_success "$opt incompatible with --autosquash" "
>> + git checkout B^0 &&
>> + test_must_fail git rebase $opt --autosquash A
>> + "
>> +
>> test_expect_success "$opt incompatible with --interactive" "
>> git checkout B^0 &&
>> test_must_fail git rebase $opt --interactive A
>> @@ -60,6 +65,21 @@ test_rebase_am_only () {
>> test_must_fail git rebase $opt --exec 'true' A
>> "
>> + test_expect_success "$opt incompatible with --keep-empty" "
>> + git checkout B^0 &&
>> + test_must_fail git rebase $opt --keep-empty A
>> + "
>> +
>> + test_expect_success "$opt incompatible with --empty=..." "
>> + git checkout B^0 &&
>> + test_must_fail git rebase $opt --empty=ask A
>> + "
>> +
>> + test_expect_success "$opt incompatible with
>> --no-reapply-cherry-picks" "
>> + git checkout B^0 &&
>> + test_must_fail git rebase $opt --no-reapply-cherry-picks A
>> + "
>> +
>> test_expect_success "$opt incompatible with --update-refs" "
>> git checkout B^0 &&
>> test_must_fail git rebase $opt --update-refs A
^ permalink raw reply
* [BUG] `git stash` exits without output when lockfile present
From: Keith Layne @ 2023-01-21 18:07 UTC (permalink / raw)
To: git
What did you do before the bug happened? (Steps to reproduce your issue)
In clean repo root:
$ touch foo
$ touch .git/index.lock
$ git stash
What did you expect to happen? (Expected behavior)
An error message to stderr explaining that .git/index.lock exists, etc.
What happened instead? (Actual behavior)
`git stash` has an exit code of 1, with no output on stdout or stderr.
What's different between what you expected and what actually happened?
I didn't notice at first that the stash failed, and I was briefly
confused. I guess I've never tried to stash and gotten this error
before, but whenever I see the "lockfile exists" error I feel like the
UX is very consistent with other commands. I expected stash to work the
same way.
I apologize if this is a dupe, I don't know how to search the archives.
[System Info]
git version:
git version 2.34.1
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 5.19.16-76051916-generic
#202210150742~1666053244~22.04~cf07008 SMP PREEMPT_DYNAMIC Tue O x86_64
compiler info: gnuc: 11.2
libc info: glibc: 2.35
$SHELL (typically, interactive shell): /bin/zsh
[Enabled Hooks]
^ permalink raw reply
* Re: [PATCH v3 7/7] rebase: fix formatting of rebase --reapply-cherry-picks option in docs
From: Phillip Wood @ 2023-01-21 15:21 UTC (permalink / raw)
To: Elijah Newren via GitGitGadget, git
Cc: Derrick Stolee, Elijah Newren, Eric Sunshine, Martin Ågren,
Phillip Wood
In-Reply-To: <0e8b06163f2b21748a19f32d515aecb16cd4574b.1674266126.git.gitgitgadget@gmail.com>
Hi Elijah
On 21/01/2023 01:55, Elijah Newren via GitGitGadget wrote:
> 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.
Thanks for clearing up my mess!
Phillip
> 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 00a9e22bc32..140c984d0ea 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -331,7 +331,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
> @@ -340,7 +339,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.
^ permalink raw reply
* Re: [PATCH v3 5/7] rebase: add coverage of other incompatible options
From: Phillip Wood @ 2023-01-21 15:20 UTC (permalink / raw)
To: Elijah Newren via GitGitGadget, git
Cc: Derrick Stolee, Elijah Newren, Eric Sunshine, Martin Ågren,
Phillip Wood
In-Reply-To: <48c40c0dda00eeb8b9bdc5ba9372b46964eea14a.1674266126.git.gitgitgadget@gmail.com>
Hi Elijah
On 21/01/2023 01:55, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
>
> The git-rebase manual noted several sets of incompatible options, but
> we were missing tests for a few of these. Further, we were missing
> code checks for some of these, which could result in command line
> options being silently ignored.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
> builtin/rebase.c | 21 ++++++++++++++-------
> t/t3422-rebase-incompatible-options.sh | 20 ++++++++++++++++++++
> 2 files changed, 34 insertions(+), 7 deletions(-)
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 2a5e0e8a7a0..6dcdb59bb02 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -1238,6 +1238,17 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
> if (options.fork_point < 0)
> options.fork_point = 0;
> }
> + /*
> + * reapply_cherry_picks is slightly weird. It starts out with a
> + * value of -1. It will be assigned a value of keep_base below and
> + * then handled specially. The apply backend is hardcoded to
> + * behave like reapply_cherry_picks==1,
I think it is hard coded to 0 not 1. We generate the patches with
git format-patch --right-only $upstream...$head
so cherry-picks will not be reapplied. I'm hardly an impartial observer
but I think the current check is correct. We support
--whitespace=fix --no-reapply-cherry-picks
--whitespace=fix --keep-base --reapply-cherry-picks
but not
--whitespace=fix --reapply-cherry-picks
> @@ -1525,6 +1529,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
> if (options.update_refs)
> imply_merge(&options, "--update-refs");
>
> + if (options.autosquash)
> + imply_merge(&options, "--autosquash");
Thanks for adding this, it maybe merits a mention in the commit message
though as it is a change in behavior for users who have
rebase.autosquash set and try to use the apply backend.
Best Wishes
Phillip
> if (options.type == REBASE_UNSPECIFIED) {
> if (!strcmp(options.default_backend, "merge"))
> imply_merge(&options, "--merge");
> diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh
> index f86274990b0..c830025470f 100755
> --- a/t/t3422-rebase-incompatible-options.sh
> +++ b/t/t3422-rebase-incompatible-options.sh
> @@ -50,6 +50,11 @@ test_rebase_am_only () {
> test_must_fail git rebase $opt --strategy-option=ours A
> "
>
> + test_expect_success "$opt incompatible with --autosquash" "
> + git checkout B^0 &&
> + test_must_fail git rebase $opt --autosquash A
> + "
> +
> test_expect_success "$opt incompatible with --interactive" "
> git checkout B^0 &&
> test_must_fail git rebase $opt --interactive A
> @@ -60,6 +65,21 @@ test_rebase_am_only () {
> test_must_fail git rebase $opt --exec 'true' A
> "
>
> + test_expect_success "$opt incompatible with --keep-empty" "
> + git checkout B^0 &&
> + test_must_fail git rebase $opt --keep-empty A
> + "
> +
> + test_expect_success "$opt incompatible with --empty=..." "
> + git checkout B^0 &&
> + test_must_fail git rebase $opt --empty=ask A
> + "
> +
> + test_expect_success "$opt incompatible with --no-reapply-cherry-picks" "
> + git checkout B^0 &&
> + test_must_fail git rebase $opt --no-reapply-cherry-picks A
> + "
> +
> test_expect_success "$opt incompatible with --update-refs" "
> git checkout B^0 &&
> test_must_fail git rebase $opt --update-refs A
^ permalink raw reply
* Re: [PATCH v3 3/7] rebase: remove --allow-empty-message from incompatible opts
From: Phillip Wood @ 2023-01-21 15:09 UTC (permalink / raw)
To: Elijah Newren via GitGitGadget, git
Cc: Derrick Stolee, Elijah Newren, Eric Sunshine, Martin Ågren,
Phillip Wood
In-Reply-To: <f4fbfd40d4599542b041081880e89075f4ff792b.1674266126.git.gitgitgadget@gmail.com>
Hi Elijah
On 21/01/2023 01:55, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
>
> --allow-empty-message was turned into a no-op and even documented
> as such; the flag is simply ignored. Since the flag is ignored, it
> shouldn't be documented as being incompatible with other flags.
The patch looks fine. Just to note that
#leftoverbits - I notice there is some code in builtin/rebase.c,
builtin/revert.c and sequencer.[ch] related to this option that could be
removed. The setting seems to be completely ignored by the sequencer and
so could be removed from struct replay_opts.
Best Wishes
Phillip
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
> Documentation/git-rebase.txt | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 00d21d7287d..3929535c0cd 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -640,7 +640,6 @@ are incompatible with the following options:
> * --merge
> * --strategy
> * --strategy-option
> - * --allow-empty-message
> * --[no-]autosquash
> * --rebase-merges
> * --interactive
^ permalink raw reply
* Git worktree bug
From: Kusal Kithul-Godage @ 2023-01-21 14:06 UTC (permalink / raw)
To: git
Bug summary
--force-with-lease option not working with worktree based repository
What did you do before the bug happened? (Steps to reproduce your issue)
git clone --bare repo.git
cd repo.git
git worktree add master
cd master
touch file
git add file
git commit -m "Commit"
git push
touch file2
git add file2
git commit --amend -m "Commit"
git push --force-with-lease
What did you expect to happen? (Expected behavior)
+ (forced update)
What happened instead? (Actual behavior)
! [rejected] (stale info)
What's different between what you expected and what actually happened?
The git push command might be sending incorrect metadata as the remote
thinks I have stale info
Anything else you want to add:
It'd also be nice if the 'git branch' command and related commands
behaved the same in worktree based repos as normal repos - ie. only
list local branches
[System Info]
git version:
git version 2.39.0
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon
uname: Darwin 22.2.0 Darwin Kernel Version 22.2.0: Fri Nov 11 02:08:47
PST 2022; root:xnu-8792.61.2~4/RELEASE_X86_64 x86_64
compiler info: clang: 14.0.0 (clang-1400.0.29.202)
libc info: no libc information available
$SHELL (typically, interactive shell): /usr/local/bin/zsh
^ permalink raw reply
* [PATCH v1 1/1] t0003: Call dd with portable blocksize
From: tboegi @ 2023-01-21 11:05 UTC (permalink / raw)
To: tboegi, git
From: Torsten Bögershausen <tboegi@web.de>
The command `dd -bs=101M count=1` is not portable.
Use `bs=1048576 count=101`, which does 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
* Re: [CI]: Is t7527 known to be flakey?
From: SZEDER Gábor @ 2023-01-21 10:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff Hostetler, edecosta, git
In-Reply-To: <xmqqtu0lzzn2.fsf@gitster.g>
On Thu, Jan 19, 2023 at 06:52:01PM -0800, Junio C Hamano wrote:
> The said test failed its linux-musl job in its first attempt, but
> re-running the failed job passed.
>
> https://github.com/git/git/actions/runs/3963948890/jobs/6792356234
> (seen@e096683 attempt #1 linux-musl)
>
> https://github.com/git/git/actions/runs/3963948890/jobs/6792850313
> (seen@e096683 attempt #2 linux-musl)
t7527 is quite slow, even with the right selection of test cases, but
this little tweak makes it much faster:
diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
index 0e497ba98d..4210ef644c 100755
--- a/t/t7527-builtin-fsmonitor.sh
+++ b/t/t7527-builtin-fsmonitor.sh
@@ -676,7 +676,10 @@ test_expect_success 'cleanup worktrees' '
# cause incorrect results when the untracked-cache is enabled.
test_lazy_prereq UNTRACKED_CACHE '
- git update-index --test-untracked-cache
+ # This check takes a very long time, but I know it works on
+ # my system, so let's fake it.
+ #git update-index --test-untracked-cache
+ true
'
test_expect_success 'Matrix: setup for untracked-cache,fsmonitor matrix' '
This with the right selection of test cases makes --stress
practicable, and the test tends to fail after a handful of
repetitions:
./t7527-builtin-fsmonitor.sh --stress -r 8,23-58
I saw different failures in multiple test cases, e.g.:
Unexpected output in case 55:
expecting success of 7527.55 'Matrix[uc:true][fsm:true] move_directory_contents_deeper':
matrix_clean_up_repo &&
$fn &&
if test $uc = false && test $fsm = false
then
git status --porcelain=v1 >.git/expect.$fn
else
git status --porcelain=v1 >.git/actual.$fn &&
test_cmp .git/expect.$fn .git/actual.$fn
fi
+ matrix_clean_up_repo
+ git reset --hard HEAD
HEAD is now at 1d1edcb initial
+ git clean -fd
+ move_directory_contents_deeper
+ mkdir T1/_new_
+ mv T1/F1 T1/F2 T1/T2 T1/_new_
+ test true = false
+ git status --porcelain=v1
error: read error: Connection reset by peer
error: could not read IPC response
+ test_cmp .git/expect.move_directory_contents_deeper .git/actual.move_directory_contents_deeper
+ test 2 -ne 2
+ eval diff -u "$@"
+ diff -u .git/expect.move_directory_contents_deeper .git/actual.move_directory_contents_deeper
--- .git/expect.move_directory_contents_deeper 2023-01-21 09:47:12.677410349 +0000
+++ .git/actual.move_directory_contents_deeper 2023-01-21 09:47:14.045448573 +0000
@@ -7,3 +7,4 @@
D T1/T2/T3/T4/F1
D T1/T2/T3/T4/F2
?? T1/_new_/
+?? dir1
error: last command exited with $?=1
not ok 55 - Matrix[uc:true][fsm:true] move_directory_contents_deeper
SIGPIPE in 'git status' cases 42, 43 and 55:
expecting success of 7527.42 'Matrix[uc:false][fsm:true] move_directory_contents_deeper':
matrix_clean_up_repo &&
$fn &&
if test $uc = false && test $fsm = false
then
git status --porcelain=v1 >.git/expect.$fn
else
git status --porcelain=v1 >.git/actual.$fn &&
test_cmp .git/expect.$fn .git/actual.$fn
fi
+ matrix_clean_up_repo
+ git reset --hard HEAD
HEAD is now at 1d1edcb initial
+ git clean -fd
+ move_directory_contents_deeper
+ mkdir T1/_new_
+ mv T1/F1 T1/F2 T1/T2 T1/_new_
+ test false = false
+ test true = false
+ git status --porcelain=v1
error: last command exited with $?=141
not ok 42 - Matrix[uc:false][fsm:true] move_directory_contents_deeper
expecting success of 7527.43 'Matrix[uc:false][fsm:true] move_directory_up':
matrix_clean_up_repo &&
$fn &&
if test $uc = false && test $fsm = false
then
git status --porcelain=v1 >.git/expect.$fn
else
git status --porcelain=v1 >.git/actual.$fn &&
test_cmp .git/expect.$fn .git/actual.$fn
fi
+ matrix_clean_up_repo
+ git reset --hard HEAD
HEAD is now at 1d1edcb initial
+ git clean -fd
Removing T1/_new_/
+ move_directory_up
+ mv T1/T2/T3 T1
+ test false = false
+ test true = false
+ git status --porcelain=v1
error: last command exited with $?=141
not ok 43 - Matrix[uc:false][fsm:true] move_directory_up
expecting success of 7527.55 'Matrix[uc:true][fsm:true] move_directory_contents_deeper':
matrix_clean_up_repo &&
$fn &&
if test $uc = false && test $fsm = false
then
git status --porcelain=v1 >.git/expect.$fn
else
git status --porcelain=v1 >.git/actual.$fn &&
test_cmp .git/expect.$fn .git/actual.$fn
fi
+ matrix_clean_up_repo
+ git reset --hard HEAD
HEAD is now at 1d1edcb initial
+ git clean -fd
+ move_directory_contents_deeper
+ mkdir T1/_new_
+ mv T1/F1 T1/F2 T1/T2 T1/_new_
+ test true = false
+ git status --porcelain=v1
error: last command exited with $?=141
I find it interesting that the output of 'git status' is redirected to
a file in all these cases, and yet it gets a SIGPIPE.
I also saw the test hang a couple of times, e.g.:
$ ./t7527-builtin-fsmonitor.sh --stress -r 8,23-58
OK 6.1
OK 7.1
OK 1.1
OK 2.1
OK 3.1
OK 5.1
OK 4.1
OK 0.1
OK 6.2
OK 1.2
OK 2.2
OK 7.2
OK 5.2
OK 0.2
OK 4.2
OK 6.3
OK 7.3
OK 2.3
OK 0.3
OK 4.3
OK 6.4
OK 7.4
OK 2.4
OK 0.4
OK 4.4
OK 6.5
OK 7.5
OK 2.5
OK 0.5
OK 4.5
OK 6.6
OK 7.6
OK 2.6
OK 0.6
OK 4.6
OK 6.7
OK 7.7
OK 2.7
OK 0.7
OK 4.7
OK 6.8
OK 7.8
OK 2.8
OK 0.8
OK 4.8
OK 6.9
OK 7.9
OK 2.9
OK 0.9
OK 4.9
OK 6.10
OK 7.10
OK 2.10
OK 0.10
OK 4.10
OK 6.11
OK 7.11
OK 2.11
OK 0.11
OK 4.11
OK 6.12
OK 7.12
OK 2.12
OK 0.12
OK 4.12
OK 6.13
OK 7.13
OK 2.13
OK 0.13
OK 6.14
OK 4.13
OK 7.14
OK 2.14
OK 0.14
OK 6.15
OK 4.14
OK 2.15
OK 7.15
OK 0.15
FAIL 7.16
OK 6.16
OK 2.16
OK 4.15
OK 0.16
At this point the test script should print the log of the failed job,
but it hangs instead, as there are a number of stuck fsmonitor--daemon
and status processes (notice how the stress test starts with 8 jobs,
but the last repetition only has 4):
$ ps aux |grep git
szeder 1857100 0.0 0.1 72272 4452 pts/2 Sl+ 21:40 0:00 /home/szeder/src/git/git fsmonitor--daemon run --detach --ipc-threads=8
szeder 1857779 0.0 0.1 6560 4152 pts/2 S+ 21:40 0:00 /home/szeder/src/git/git status --porcelain=v1
szeder 1860020 0.0 0.1 88664 4312 pts/2 Sl+ 21:40 0:00 /home/szeder/src/git/git fsmonitor--daemon run --detach --ipc-threads=8
szeder 1860668 0.0 0.1 6560 4040 pts/2 S+ 21:40 0:00 /home/szeder/src/git/git status --porcelain=v1
szeder 1860749 0.0 0.1 96860 4528 pts/2 Sl+ 21:40 0:00 /home/szeder/src/git/git fsmonitor--daemon run --detach --ipc-threads=8
szeder 1861281 0.0 0.1 6560 4272 pts/2 S+ 21:40 0:00 /home/szeder/src/git/git status --porcelain=v1
^ permalink raw reply
* [PATCH] tree-walk: disallow overflowing modes
From: René Scharfe @ 2023-01-21 9:36 UTC (permalink / raw)
To: Git List
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason
When parsing tree entries, reject mode values that don't fit into an
unsigned int.
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..5e7bc38600 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -17,6 +17,8 @@ static const char *get_mode(const char *str, unsigned int *modep)
while ((c = *str++) != ' ') {
if (c < '0' || c > '7')
return NULL;
+ if ((mode << 3) >> 3 != mode)
+ return NULL;
mode = (mode << 3) + (c - '0');
}
*modep = mode;
--
2.39.1
^ permalink raw reply related
* Re: [RFC/PATCH 0/6] hash-object: use fsck to check objects
From: René Scharfe @ 2023-01-21 9:36 UTC (permalink / raw)
To: Jeff King, git; +Cc: Ævar Arnfjörð Bjarmason
In-Reply-To: <Y8ifa7hyqxSbL92U@coredump.intra.peff.net>
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.
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.
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.
Hmm, what would be the performance impact of trees with mode fields
zero-padded to silly lengths?
René
^ permalink raw reply
* Re: [PATCH v2 1/2] rebase: remove completely useless -C option
From: Elijah Newren @ 2023-01-21 4:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Elijah Newren via GitGitGadget, git, Derrick Stolee
In-Reply-To: <xmqqh6wlxjuo.fsf@gitster.g>
On Fri, Jan 20, 2023 at 8:16 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> > On Fri, Jan 20, 2023 at 4:05 AM Junio C Hamano <gitster@pobox.com> wrote:
> >> ...
> >> That is, even when a patch does not cleanly apply with full context
> >> in the incoming diff, by requiring *smaller* number of lines to
> >> match, the diff *could* be forced to apply with reduced precision?
> >
> > Oh! Reducing the number of lines of context to pay attention to never
> > even occurred to me for whatever reason. I'll drop the patch.
>
> Yup. "completely useless" on the title is less than half correct,
> but still correct for a minor use case where -C is used to use
> *more* context lines than the default.
>
> We could update "rebase --apply" codepath to increase the context
> lines generated by format-patch. That would make the "completely
> useless" totally incorrect ;-)
haha :-)
I'd probably go for that, but since in my mind the plan is still to
deprecate and remove the apply backend as you suggested at [1], I'm
not particularly motivated to improve/extend options specific to the
apply backend of rebase.
[1] https://lore.kernel.org/git/xmqqa78d2qmk.fsf@gitster-ct.c.googlers.com/
^ permalink raw reply
* [PATCH] name-rev: stop including taggerdate in naming of commits
From: Elijah Newren via GitGitGadget @ 2023-01-21 4:28 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Elijah Newren
From: Elijah Newren <newren@gmail.com>
Commit 7550424804 ("name-rev: include taggerdate in considering the best
name", 2016-04-22) introduced the idea of using taggerdate in the
criteria for selecting the best name. At the time, a certain commit in
linux.git -- namely, aed06b9cfcab -- was being named by name-rev as
v4.6-rc1~9^2~792
which, while correct, felt very suboptimal. Some investigation found
that tweaking the MERGE_TRAVERSAL_WEIGHT to lower it could give
alternate answers such as
v3.13-rc7~9^2~14^2~42
or
v3.13~5^2~4^2~2^2~1^2~42
A manual solution involving looking at tagger dates came up with
v3.13-rc1~65^2^2~42
which was then implemented in name-rev.
It turns out that this taggerdate heuristic isn't needed due to a
subsequent change to fix the naming logic in 3656f84278 ("name-rev:
prefer shorter names over following merges", 2021-12-04). Simply
removing the taggerdate heuristic from the calculation nowadays
still causes us to get the optimal answer on that particular commit
of interest in linux.git, namely:
v3.13-rc1~65^2^2~42
Further, the taggerdate heuristic is causing bugs of its own. I was
pointed to a case in a private repository where name-rev reports a name
of the form
v2022.10.02~86
when users expected to see one of the form
v2022.10.01~2
(I've modified the names and numbers a bit from the real testcase.) As
you can probably guess, v2022.10.01 was created after v2022.10.02 (by a
few hours), even though it pointed to an older commit. While the
condition is unusual even in the repository in question, it is not the
only problematic set of tags in that repository. The taggerdate logic
was a workaround that is no longer needed, and is now causing suboptimal
results in other cases.
As such, remove the taggerdate in the comparison. However, note that
"taggerdate" is actually also used to store commit dates since
ef1e74065c ("name-rev: favor describing with tags and use committer date
to tiebreak", 2017-03-29), where it is used as a fallback tiebreaker
when distances are equal. We do not want to remove that fallback
tiebreaker, we are only removing the use of actual taggerdates as a
primary criteria overridding effective distance calculations.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
name-rev: stop including taggerdate in naming of commits
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1468%2Fnewren%2Ffix-name-rev-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1468/newren/fix-name-rev-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1468
builtin/name-rev.c | 4 +---
t/t6120-describe.sh | 6 ++++++
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 15535e914a6..df50abcdeb9 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -113,9 +113,7 @@ static int is_better_name(struct rev_name *name,
* based on the older tag, even if it is farther away.
*/
if (from_tag && name->from_tag)
- return (name->taggerdate > taggerdate ||
- (name->taggerdate == taggerdate &&
- name_distance > new_distance));
+ return name_distance > new_distance;
/*
* We know that at least one of them is a non-tag at this point.
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 9a35e783a75..c9afcef2018 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -657,4 +657,10 @@ test_expect_success 'setup: describe commits with disjoint bases 2' '
check_describe -C disjoint2 "B-3-gHASH" HEAD
+test_expect_success 'setup misleading taggerdates' '
+ GIT_COMMITTER_DATE="2006-12-12 12:31" git tag -a -m "another tag" newer-tag-older-commit unique-file~1
+'
+
+check_describe newer-tag-older-commit~1 --contains unique-file~2
+
test_done
base-commit: 221222b278e713054e65cbbbcb2b1ac85483ea89
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 7/7] rebase: fix formatting of rebase --reapply-cherry-picks option in docs
From: Elijah Newren via GitGitGadget @ 2023-01-21 1:55 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.v3.git.1674266126.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 00a9e22bc32..140c984d0ea 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -331,7 +331,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
@@ -340,7 +339,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 v3 6/7] rebase: clarify the OPT_CMDMODE incompatibilities
From: Elijah Newren via GitGitGadget @ 2023-01-21 1:55 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.v3.git.1674266126.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 3f12d242d83..00a9e22bc32 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -201,6 +201,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>::
@@ -242,22 +275,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
@@ -338,17 +355,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).
@@ -644,7 +650,6 @@ are incompatible with the following options:
* --no-keep-empty
* --empty=
* --reapply-cherry-picks
- * --edit-todo
* --update-refs
* --root when used without --onto
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 5/7] rebase: add coverage of other incompatible options
From: Elijah Newren via GitGitGadget @ 2023-01-21 1:55 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.v3.git.1674266126.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
The git-rebase manual noted several sets of incompatible options, but
we were missing tests for a few of these. Further, we were missing
code checks for some of these, which could result in command line
options being silently ignored.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
builtin/rebase.c | 21 ++++++++++++++-------
t/t3422-rebase-incompatible-options.sh | 20 ++++++++++++++++++++
2 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 2a5e0e8a7a0..6dcdb59bb02 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1238,6 +1238,17 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
if (options.fork_point < 0)
options.fork_point = 0;
}
+ /*
+ * reapply_cherry_picks is slightly weird. It starts out with a
+ * value of -1. It will be assigned a value of keep_base below and
+ * then handled specially. The apply backend is hardcoded to
+ * behave like reapply_cherry_picks==1, so if it has that value, we
+ * can just ignore the flag with the apply backend. Thus, we only
+ * really need to throw an error and require the merge backend if
+ * reapply_cherry_picks==0.
+ */
+ if (options.reapply_cherry_picks == 0)
+ imply_merge(&options, "--no-reapply-cherry-picks");
/*
* --keep-base defaults to --reapply-cherry-picks to avoid losing
* commits when using this option.
@@ -1420,13 +1431,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
if (options.empty != EMPTY_UNSPECIFIED)
imply_merge(&options, "--empty");
- /*
- * --keep-base implements --reapply-cherry-picks by altering upstream so
- * it works with both backends.
- */
- if (options.reapply_cherry_picks && !keep_base)
- imply_merge(&options, "--reapply-cherry-picks");
-
if (gpg_sign)
options.gpg_sign_opt = xstrfmt("-S%s", gpg_sign);
@@ -1525,6 +1529,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
if (options.update_refs)
imply_merge(&options, "--update-refs");
+ if (options.autosquash)
+ imply_merge(&options, "--autosquash");
+
if (options.type == REBASE_UNSPECIFIED) {
if (!strcmp(options.default_backend, "merge"))
imply_merge(&options, "--merge");
diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh
index f86274990b0..c830025470f 100755
--- a/t/t3422-rebase-incompatible-options.sh
+++ b/t/t3422-rebase-incompatible-options.sh
@@ -50,6 +50,11 @@ test_rebase_am_only () {
test_must_fail git rebase $opt --strategy-option=ours A
"
+ test_expect_success "$opt incompatible with --autosquash" "
+ git checkout B^0 &&
+ test_must_fail git rebase $opt --autosquash A
+ "
+
test_expect_success "$opt incompatible with --interactive" "
git checkout B^0 &&
test_must_fail git rebase $opt --interactive A
@@ -60,6 +65,21 @@ test_rebase_am_only () {
test_must_fail git rebase $opt --exec 'true' A
"
+ test_expect_success "$opt incompatible with --keep-empty" "
+ git checkout B^0 &&
+ test_must_fail git rebase $opt --keep-empty A
+ "
+
+ test_expect_success "$opt incompatible with --empty=..." "
+ git checkout B^0 &&
+ test_must_fail git rebase $opt --empty=ask A
+ "
+
+ test_expect_success "$opt incompatible with --no-reapply-cherry-picks" "
+ git checkout B^0 &&
+ test_must_fail git rebase $opt --no-reapply-cherry-picks A
+ "
+
test_expect_success "$opt incompatible with --update-refs" "
git checkout B^0 &&
test_must_fail git rebase $opt --update-refs A
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 4/7] rebase: fix docs about incompatibilities with --root
From: Elijah Newren via GitGitGadget @ 2023-01-21 1:55 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.v3.git.1674266126.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 3929535c0cd..3f12d242d83 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -567,10 +567,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.
@@ -649,7 +646,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
* [PATCH v3 3/7] rebase: remove --allow-empty-message from incompatible opts
From: Elijah Newren via GitGitGadget @ 2023-01-21 1:55 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.v3.git.1674266126.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
--allow-empty-message was turned into a no-op and even documented
as such; the flag is simply ignored. Since the flag is ignored, it
shouldn't be documented as being incompatible with other flags.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
Documentation/git-rebase.txt | 1 -
1 file changed, 1 deletion(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 00d21d7287d..3929535c0cd 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -640,7 +640,6 @@ are incompatible with the following options:
* --merge
* --strategy
* --strategy-option
- * --allow-empty-message
* --[no-]autosquash
* --rebase-merges
* --interactive
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 2/7] rebase: flag --apply and --merge as incompatible
From: Elijah Newren via GitGitGadget @ 2023-01-21 1:55 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.v3.git.1674266126.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
Previously, we flagged options which implied --apply as being
incompatible with options which implied --merge. But if both options
were given explicitly, then we didn't flag the incompatibility. The
same is true with --apply and --interactive. Add the check, and add
some testcases to verify these are also caught.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
builtin/rebase.c | 12 ++++++++++--
t/t3422-rebase-incompatible-options.sh | 3 +++
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index accd62fce48..2a5e0e8a7a0 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -920,6 +920,9 @@ static int parse_opt_am(const struct option *opt, const char *arg, int unset)
BUG_ON_OPT_NEG(unset);
BUG_ON_OPT_ARG(arg);
+ if (opts->type != REBASE_UNSPECIFIED && opts->type != REBASE_APPLY)
+ die(_("apply options and merge options cannot be used together"));
+
opts->type = REBASE_APPLY;
return 0;
@@ -933,8 +936,10 @@ static int parse_opt_merge(const struct option *opt, const char *arg, int unset)
BUG_ON_OPT_NEG(unset);
BUG_ON_OPT_ARG(arg);
- if (!is_merge(opts))
- opts->type = REBASE_MERGE;
+ if (opts->type != REBASE_UNSPECIFIED && opts->type != REBASE_MERGE)
+ die(_("apply options and merge options cannot be used together"));
+
+ opts->type = REBASE_MERGE;
return 0;
}
@@ -948,6 +953,9 @@ static int parse_opt_interactive(const struct option *opt, const char *arg,
BUG_ON_OPT_NEG(unset);
BUG_ON_OPT_ARG(arg);
+ if (opts->type != REBASE_UNSPECIFIED && opts->type != REBASE_MERGE)
+ die(_("apply options and merge options cannot be used together"));
+
opts->type = REBASE_MERGE;
opts->flags |= REBASE_INTERACTIVE_EXPLICIT;
diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh
index 9da39cd91c2..9b9e78479f6 100755
--- a/t/t3422-rebase-incompatible-options.sh
+++ b/t/t3422-rebase-incompatible-options.sh
@@ -67,7 +67,10 @@ test_rebase_am_only () {
}
+# Check options which imply --apply
test_rebase_am_only --whitespace=fix
test_rebase_am_only -C4
+# Also check an explicit --apply
+test_rebase_am_only --apply
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 1/7] rebase: mark --update-refs as requiring the merge backend
From: Elijah Newren via GitGitGadget @ 2023-01-21 1:55 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.v3.git.1674266126.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
--update-refs is built in terms of the sequencer, which requires the
merge backend. It was already marked as incompatible with the apply
backend in the git-rebase manual, but the code didn't check for this
incompatibility and warn the user. Check and error now.
While at it, fix a typo in t3422...and fix some misleading wording
(most options which used to be am-specific have since been implemented
in the merge backend as well).
Signed-off-by: Elijah Newren <newren@gmail.com>
---
Documentation/git-rebase.txt | 2 ++
builtin/rebase.c | 3 +++
t/t3422-rebase-incompatible-options.sh | 15 ++++++++++-----
3 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index f9675bd24e6..00d21d7287d 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -623,6 +623,8 @@ start would be overridden by the presence of
+
If the configuration variable `rebase.updateRefs` is set, then this option
can be used to override and disable this setting.
++
+See also INCOMPATIBLE OPTIONS below.
INCOMPATIBLE OPTIONS
--------------------
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 1481c5b6a5b..accd62fce48 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1514,6 +1514,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
}
}
+ if (options.update_refs)
+ imply_merge(&options, "--update-refs");
+
if (options.type == REBASE_UNSPECIFIED) {
if (!strcmp(options.default_backend, "merge"))
imply_merge(&options, "--merge");
diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh
index 6dabb05a2ad..9da39cd91c2 100755
--- a/t/t3422-rebase-incompatible-options.sh
+++ b/t/t3422-rebase-incompatible-options.sh
@@ -25,11 +25,11 @@ test_expect_success 'setup' '
'
#
-# Rebase has lots of useful options like --whitepsace=fix, which are
-# actually all built in terms of flags to git-am. Since neither
-# --merge nor --interactive (nor any options that imply those two) use
-# git-am, using them together will result in flags like --whitespace=fix
-# being ignored. Make sure rebase warns the user and aborts instead.
+# Rebase has a couple options which are specific to the apply backend,
+# and several options which are specific to the merge backend. Flags
+# from the different sets cannot work together, and we do not want to
+# just ignore one of the sets of flags. Make sure rebase warns the
+# user and aborts instead.
#
test_rebase_am_only () {
@@ -60,6 +60,11 @@ test_rebase_am_only () {
test_must_fail git rebase $opt --exec 'true' A
"
+ test_expect_success "$opt incompatible with --update-refs" "
+ git checkout B^0 &&
+ test_must_fail git rebase $opt --update-refs A
+ "
+
}
test_rebase_am_only --whitespace=fix
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 0/7] rebase: fix several code/testing/documentation issues around flag incompatibilities
From: Elijah Newren via GitGitGadget @ 2023-01-21 1:55 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Elijah Newren, Eric Sunshine, Martin Ågren,
Phillip Wood, Elijah Newren
In-Reply-To: <pull.1466.v2.git.1674190573.gitgitgadget@gmail.com>
We had a report about --update-refs being ignored when --whitespace=fix was
passed, confusing an end user. These were already marked as incompatible in
the manual, but the code didn't check for the incompatibility and provide an
error to the user.
Folks brought up other flags tangentially when reviewing an earlier round of
this series, and I found we had more that were missing checks, and that we
were missing some testcases, and that the documentation was wrong on some of
the relevant options. So, I ended up getting lots of little fixes to
straighten these all out.
Left out of this series:
* If an option like rebase.autosquash or rebase.updateRefs are selected and
the user specifies e.g. --whitespace=fix, we should either (a) tailor the
error message better to point out that it's a config option that is
incompatible with their command line flag, or (b) implement
--whitespace=fix for the merge backend so we can just deprecate and
eventually remove the apply backend[1].
[1] See "longer term goal" at
https://lore.kernel.org/git/xmqqa78d2qmk.fsf@gitster-ct.c.googlers.com/
Changes since v2:
* Remove the extra patch and reword the comment in t3422 more thoroughly.
* Add tests for other flag incompatibilities that were missing
* Add code that was missing for checking various flag incompatibilities
* Fix incorrect claims in the documentation around incompatible options
* A few other miscellaneous fixups noticed while doing all the above
Changes since v1:
* Add a patch nuking the -C option to rebase (fixes confusion around the
comment in t3422 and acknowledges the fact that the option is totally and
utterly useless and always has been. It literally never affects the
results of a rebase.)
Signed-off-by: Elijah Newren newren@gmail.com
Elijah Newren (7):
rebase: mark --update-refs as requiring the merge backend
rebase: flag --apply and --merge as incompatible
rebase: remove --allow-empty-message from incompatible opts
rebase: fix docs about incompatibilities with --root
rebase: add coverage of other incompatible options
rebase: clarify the OPT_CMDMODE incompatibilities
rebase: fix formatting of rebase --reapply-cherry-picks option in docs
Documentation/git-rebase.txt | 73 +++++++++++++-------------
builtin/rebase.c | 36 +++++++++----
t/t3422-rebase-incompatible-options.sh | 42 +++++++++++++--
3 files changed, 101 insertions(+), 50 deletions(-)
base-commit: 221222b278e713054e65cbbbcb2b1ac85483ea89
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1466%2Fnewren%2Frebase-update-refs-imply-merge-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1466/newren/rebase-update-refs-imply-merge-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1466
Range-diff vs v2:
2: 2e44d0b7e57 ! 1: 9089834adea rebase: mark --update-refs as requiring the merge backend
@@ Commit message
--update-refs is built in terms of the sequencer, which requires the
merge backend. It was already marked as incompatible with the apply
backend in the git-rebase manual, but the code didn't check for this
- incompatibility and warn the user. Check and warn now.
+ incompatibility and warn the user. Check and error now.
- While at it, fix a typo in t3422...and fix some misleading wording (all
- useful options other than --whitespace=fix have long since been
- implemented in the merge backend).
+ While at it, fix a typo in t3422...and fix some misleading wording
+ (most options which used to be am-specific have since been implemented
+ in the merge backend as well).
Signed-off-by: Elijah Newren <newren@gmail.com>
+ ## Documentation/git-rebase.txt ##
+@@ Documentation/git-rebase.txt: start would be overridden by the presence of
+ +
+ If the configuration variable `rebase.updateRefs` is set, then this option
+ can be used to override and disable this setting.
+++
++See also INCOMPATIBLE OPTIONS below.
+
+ INCOMPATIBLE OPTIONS
+ --------------------
+
## builtin/rebase.c ##
@@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix)
}
@@ t/t3422-rebase-incompatible-options.sh: test_expect_success 'setup' '
-# --merge nor --interactive (nor any options that imply those two) use
-# git-am, using them together will result in flags like --whitespace=fix
-# being ignored. Make sure rebase warns the user and aborts instead.
-+# Rebase has a useful option, --whitespace=fix, which is actually
-+# built in terms of flags to git-am. Since neither --merge nor
-+# --interactive (nor any options that imply those two) use git-am,
-+# using them together will result in --whitespace=fix being ignored.
-+# Make sure rebase warns the user and aborts instead.
++# Rebase has a couple options which are specific to the apply backend,
++# and several options which are specific to the merge backend. Flags
++# from the different sets cannot work together, and we do not want to
++# just ignore one of the sets of flags. Make sure rebase warns the
++# user and aborts instead.
#
test_rebase_am_only () {
1: a0f8f5fac1c ! 2: a8b5a0e4fb0 rebase: remove completely useless -C option
@@ Metadata
Author: Elijah Newren <newren@gmail.com>
## Commit message ##
- rebase: remove completely useless -C option
+ rebase: flag --apply and --merge as incompatible
- The `-C` option to rebase was introduced with 67dad687ad ("add -C[NUM]
- to git-am", 2007-02-08). Based on feedback on the patch, the author
- added the -C option not just to git-am but also to git-rebase. But did
- it such that the option was just passed through to git-am (which passes
- it along to git-apply), with no corresponding option to format-patch.
-
- As per the git-apply documentation for the `-C` option:
- Ensure at least <n> lines of surrounding context match...When fewer
- lines of surrounding context exist they all must match.
-
- The fact that format-patch was not passed a -U option to increase the
- number of context lines meant that there would still only be 3 lines of
- context to match on. So, anyone attempting to use this option in
- git-rebase would just be spinning their wheels and wasting time. I was
- one such user a number of years ago.
-
- Since this option can at best waste users time and is nothing more than
- a confusing no-op, and has never been anything other than a confusing
- no-op, and no one has ever bothered to create a testcase for it that
- goes beyond option parsing, simply excise the option.
+ Previously, we flagged options which implied --apply as being
+ incompatible with options which implied --merge. But if both options
+ were given explicitly, then we didn't flag the incompatibility. The
+ same is true with --apply and --interactive. Add the check, and add
+ some testcases to verify these are also caught.
Signed-off-by: Elijah Newren <newren@gmail.com>
- ## Documentation/git-rebase.txt ##
-@@ Documentation/git-rebase.txt: include::rerere-options.txt[]
- Allows the pre-rebase hook to run, which is the default. This option can
- be used to override `--no-verify`. See also linkgit:githooks[5].
+ ## builtin/rebase.c ##
+@@ builtin/rebase.c: static int parse_opt_am(const struct option *opt, const char *arg, int unset)
+ BUG_ON_OPT_NEG(unset);
+ BUG_ON_OPT_ARG(arg);
---C<n>::
-- Ensure at least `<n>` lines of surrounding context match before
-- and after each change. When fewer lines of surrounding
-- context exist they all must match. By default no context is
-- ever ignored. Implies `--apply`.
--+
--See also INCOMPATIBLE OPTIONS below.
--
- --no-ff::
- --force-rebase::
- -f::
-@@ Documentation/git-rebase.txt: The following options:
++ if (opts->type != REBASE_UNSPECIFIED && opts->type != REBASE_APPLY)
++ die(_("apply options and merge options cannot be used together"));
++
+ opts->type = REBASE_APPLY;
- * --apply
- * --whitespace
-- * -C
+ return 0;
+@@ builtin/rebase.c: static int parse_opt_merge(const struct option *opt, const char *arg, int unset)
+ BUG_ON_OPT_NEG(unset);
+ BUG_ON_OPT_ARG(arg);
- are incompatible with the following options:
+- if (!is_merge(opts))
+- opts->type = REBASE_MERGE;
++ if (opts->type != REBASE_UNSPECIFIED && opts->type != REBASE_MERGE)
++ die(_("apply options and merge options cannot be used together"));
++
++ opts->type = REBASE_MERGE;
-
- ## builtin/rebase.c ##
-@@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix)
- N_("ignore author date and use current date")),
- OPT_HIDDEN_BOOL(0, "ignore-date", &options.ignore_date,
- N_("synonym of --reset-author-date")),
-- OPT_PASSTHRU_ARGV('C', NULL, &options.git_am_opts, N_("n"),
-- N_("passed to 'git apply'"), 0),
- OPT_BOOL(0, "ignore-whitespace", &ignore_whitespace,
- N_("ignore changes in whitespace")),
- OPT_PASSTHRU_ARGV(0, "whitespace", &options.git_am_opts,
-@@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix)
- if (!strcmp(option, "--whitespace=fix") ||
- !strcmp(option, "--whitespace=strip"))
- allow_preemptive_ff = 0;
-- else if (skip_prefix(option, "-C", &p)) {
-- while (*p)
-- if (!isdigit(*(p++)))
-- die(_("switch `C' expects a "
-- "numerical value"));
-- } else if (skip_prefix(option, "--whitespace=", &p)) {
-+ else if (skip_prefix(option, "--whitespace=", &p)) {
- if (*p && strcmp(p, "warn") && strcmp(p, "nowarn") &&
- strcmp(p, "error") && strcmp(p, "error-all"))
- die("Invalid whitespace option: '%s'", p);
-
- ## t/t3406-rebase-message.sh ##
-@@ t/t3406-rebase-message.sh: test_expect_success 'rebase --onto outputs the invalid ref' '
- test_i18ngrep "invalid-ref" err
- '
+ return 0;
+ }
+@@ builtin/rebase.c: static int parse_opt_interactive(const struct option *opt, const char *arg,
+ BUG_ON_OPT_NEG(unset);
+ BUG_ON_OPT_ARG(arg);
+
++ if (opts->type != REBASE_UNSPECIFIED && opts->type != REBASE_MERGE)
++ die(_("apply options and merge options cannot be used together"));
++
+ opts->type = REBASE_MERGE;
+ opts->flags |= REBASE_INTERACTIVE_EXPLICIT;
--test_expect_success 'error out early upon -C<n> or --whitespace=<bad>' '
-- test_must_fail git rebase -Cnot-a-number HEAD 2>err &&
-- test_i18ngrep "numerical value" err &&
-- test_must_fail git rebase --whitespace=bad HEAD 2>err &&
-- test_i18ngrep "Invalid whitespace option" err
--'
--
- write_reflog_expect () {
- if test $mode = --apply
- then
## t/t3422-rebase-incompatible-options.sh ##
@@ t/t3422-rebase-incompatible-options.sh: test_rebase_am_only () {
+
}
++# Check options which imply --apply
test_rebase_am_only --whitespace=fix
--test_rebase_am_only -C4
+ test_rebase_am_only -C4
++# Also check an explicit --apply
++test_rebase_am_only --apply
test_done
-: ----------- > 3: f4fbfd40d45 rebase: remove --allow-empty-message from incompatible opts
-: ----------- > 4: a1e61ac8f21 rebase: fix docs about incompatibilities with --root
-: ----------- > 5: 48c40c0dda0 rebase: add coverage of other incompatible options
-: ----------- > 6: 8664cce6cf7 rebase: clarify the OPT_CMDMODE incompatibilities
-: ----------- > 7: 0e8b06163f2 rebase: fix formatting of rebase --reapply-cherry-picks option in docs
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH v2 2/2] rebase: mark --update-refs as requiring the merge backend
From: Elijah Newren @ 2023-01-21 1:34 UTC (permalink / raw)
To: phillip.wood
Cc: Elijah Newren via GitGitGadget, git, Derrick Stolee,
Junio C Hamano, Philippe Blain
In-Reply-To: <bb75c8e1-05d3-1359-e06a-ee013ae677da@dunelm.org.uk>
On Fri, Jan 20, 2023 at 8:46 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Elijah
>
> Thanks for working on this
>
> On 20/01/2023 04:56, Elijah Newren via GitGitGadget wrote:
> > From: Elijah Newren <newren@gmail.com>
> >
> > --update-refs is built in terms of the sequencer, which requires the
> > merge backend. It was already marked as incompatible with the apply
> > backend in the git-rebase manual, but the code didn't check for this
> > incompatibility and warn the user. Check and warn now.
>
> Strictly speaking we die rather than warn but I don't think that
> warrants a re-roll.
Oh, good catch. I'm re-rolling anyway, so I might as well fix this.
> I just had a quick look to see how easy it would be
> to add the advice Stolee's patch had if the user has set
> rebase.updaterefs but does not pass "--no-update-refs" when using the
> apply backend but it looks a bit fiddly unfortunately as we could die in
> imply_merge() or later on.
Yeah, and it gets even more finicky than that. If the user specifies
_any_ merge-specific options on the command line together with an
apply-specific option, then there's no point bringing up
rebase.updaterefs (or rebase.autosquash). We only want to bring up
those config options if they are the only reasons for getting a
backends-are-incompatible error message.
> Thinking more generally, imply_merge() does a good job of telling the
> user which option is incompatible with "--apply" but if the user passes
> a merge option with "--whitespace=fix" and omits "--apply" then we just
> print a generic message saying "apply options and merge options cannot
> be used together" which isn't terribly helpful to the user (doubly so
> when the merge option come from a config setting).
That's not specific to --whitespace=fix (it also happens with -C, and
in the past happened with other options that used to only work with
the apply backend). In particular, it's whenever both backends are
implied -- in those cases, we don't try to keep track of which options
implied it and thus only provide a very generic error message.
> I've also noticed that "--autosquash" is ignored if we end up using the
> apply backend. That's a separate issue but shares the "this may have
> come from a config setting rather than a command line argument" problem.
Yeah, Stolee also pointed this one out...and --autosquash was missing
the same incompatible-with-apply-options warnings too.
> All in all I'm not sure if it is friendlier to die when the user has
> rebsase.updaterefs set and they try to rebase with "--whitespace=fix" or
> if it is better just to ignore the config in that case. If we can find a
> way to print some help when we die in that case it would be nicer for
> the user.
I think ignoring it would be worse, as I argued over at [1]. But
another thing to keep in mind is that we can eventually make the
question obsolete by deprecating and eventually removing the apply
backend, as suggested by Junio[2]. That would allow us to remove all
the incompatibility checking and simplify the manual.
[1] https://lore.kernel.org/git/CABPp-BHDhpSVpuaubTP=smWaf7FBmpzB-_Frh0Dn5oN+vx0xzw@mail.gmail.com/
[2] See "longer term goal" of
https://lore.kernel.org/git/xmqqa78d2qmk.fsf@gitster-ct.c.googlers.com/
^ permalink raw reply
* Re: [PATCH v4] checkout/switch: disallow checking out same branch in multiple worktrees
From: Carlo Arenas @ 2023-01-20 22:12 UTC (permalink / raw)
To: phillip.wood
Cc: git, pclouds, gitster, Jinwook Jeong, Eric Sunshine,
Rubén Justo, Phillip Wood
In-Reply-To: <8f24fc3c-c30f-dc70-5a94-5ee4ed3de102@dunelm.org.uk>
On Fri, Jan 20, 2023 at 7:08 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> On 20/01/2023 11:35, Carlo Marcelo Arenas Belón wrote:
> > Commands `git switch -C` and `git checkout -B` neglect to check whether
> > the provided branch is already checked out in some other worktree, as
> > shown by the following:
> >
> > $ git worktree list
> > .../foo beefb00f [main]
> > $ git worktree add ../other
> > Preparing worktree (new branch 'other')
> > HEAD is now at beefb00f first
> > $ cd ../other
> > $ git switch -C main
> > Switched to and reset branch 'main'
> > $ git worktree list
> > .../foo beefb00f [main]
> > .../other beefb00f [main]
> >
> > Fix this problem by teaching `git switch -C` and `git checkout -B` to
> > check whether the branch in question is already checked out elsewhere.
> >
> > Unlike what it is done for `git switch` and `git checkout`, that have
> > an historical exception to ignore other worktrees if the branch to
> > check is the current one (as required when called as part of other
> > tools), the logic implemented is more strict and will require the user
> > to invoke the command with `--ignore-other-worktrees` to explicitly
> > indicate they want the risky behaviour.
> >
> > This matches the current behaviour of `git branch -f` and is safer; for
> > more details see the tests in t2400.
>
> As I said before, it would be much easier for everyone else to
> understand the changes if you wrote out what they were rather than
> saying "look at the tests". I do appreciate the improved test
> descriptions though - thanks for that.
Apologies on that, I tried to come up with something that would
describe the change of behaviour other than the paragraph above and
couldn't come out with a better explanation except reading the tests
(which I know is complicated by the fact they are interlinked).
The behaviour I am changing is also not documented (other than by the
test) and might have been added as a quirk to keep the scripted rebase
and bisect going when confronted with branches that were checked out
in multiple worktrees, and as such might had not be intended for
`switch`, and might not be needed anymore either.
Using`checkout` for simplicity, but also applies to `switch`,
% git worktree list
.../base 6a45aba [main]
% git worktree add -f ../other main
Preparing worktree (checking out 'main')
HEAD is now at 6a45aba init
% cd ../other
% git checkout main
Already on 'main'
% git checkout -B main
fatal: 'main' is already checked out at '.../base'
% git checkout --ignore-other-worktrees -B main
Already on 'main'
The change of behaviour only applies to -B and it actually matches the
documentation better.
> > @@ -1533,13 +1534,13 @@ static int checkout_branch(struct checkout_opts *opts,
> > if (!opts->can_switch_when_in_progress)
> > die_if_some_operation_in_progress();
> >
> > - if (new_branch_info->path && !opts->force_detach && !opts->new_branch &&
> > - !opts->ignore_other_worktrees) {
> > + if (!opts->ignore_other_worktrees && !opts->force_detach &&
> > + check_branch_path && ref_exists(check_branch_path)) {
>
> I think check_branch_path is NULL if opts->ignore_other_worktrees is set
> so we could maybe lose "!opts->ignore_other_worktrees" here (or possibly
> below where you set check_branch_path).
opts->ignore_other_worktrees was kept from the original expression;
you are correct that is not needed anymore, but I thought it didn't
hurt and made the code intention clearer (meaning it is obvious to
anyone new to the code that this code will be skipped if that flag is
set), would using an assert or a comment be a better option?
> > /*
> > * Extract branch name from command line arguments, so
> > * all that is left is pathspecs.
> > @@ -1741,6 +1751,9 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
> > new_branch_info, opts, &rev);
> > argv += n;
> > argc -= n;
> > +
> > + if (!opts->ignore_other_worktrees && !check_branch_path && new_branch_info->path)
> > + check_branch_path = xstrdup(new_branch_info->path);
>
> I'm a bit confused what this is doing.
The branch we are interested in might come from 2 places, either it is
a parameter to -b, which was picked up before, or it is the argument
to the command itself, which was detected above.
If both are provided, we want to make sure to use the one from -b, or
will have the bug you sharply spotted before, which was frankly
embarrassing.
> > diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
> > index d587e0b20d..7ab7e87440 100755
> > @@ -133,17 +136,34 @@ test_expect_success SYMLINKS 'die the same branch is already checked out (symlin
> > test_must_fail git -C here checkout newmain
> > '
> >
> > -test_expect_success 'not die the same branch is already checked out' '
> > +test_expect_success 'allow creating multiple worktrees for same branch with force' '
> > + git worktree add --force anothernewmain newmain
> > +'
> > +
> > +test_expect_success 'allow checkout/reset from the conflicted branch' '
>
> I'm not sure what "the conflicted branch" means (it reminds we of merge
> conflicts).
by "conflicted" I meant one that is checked out in more than one worktree
> Is this just testing that "checkout -b/B <branch>
> <start-point>" works?
yes, but most importantly that we chose the right branch to check if
both are provided and <start-point> is also a branch
> > (
> > cd here &&
> > - git worktree add --force anothernewmain newmain
> > + git checkout -b conflictedmain newmain &&
> > + git checkout -B conflictedmain newmain &&
> > + git switch -C conflictedmain newmain
> > + )
> > +'
> > +
> > +test_expect_success 'and not die on re-checking out current branch even if conflicted' '
>
> I think 'allow re-checking out ...' would be clearer, again I'm not sure
> what's conflicted here.
ok
> > + (
> > + cd there &&
> > + git checkout newmain &&
> > + git switch newmain
> > )
> > '
> >
> > -test_expect_success 'not die on re-checking out current branch' '
> > +test_expect_failure 'unless using force without --ignore-other-worktrees' '
>
> This test passes for me - what's the reason for changing from
> test_expect_success to test_expect_failure?
It also works for me, and for Junio, but somehow it didn't in the last
runs from the CI and you could reproduce locally by going to the tree
created above in the example I provided and doing:
% cd ../base
% git checkout -B main
which should fail after finding that main is already checked out in
`other`, but does not because when looking at the worktrees would
first find the current one and not die, never aware there is another
worktree with that same branch.
the bug is the same one that Rubén is trying to address for rebase and
that you commented on as well and that was mentioned before in this
thread:
https://lore.kernel.org/git/eeb0c778-af0a-235c-f009-bca3abafdb15@gmail.com/
Carlo
^ permalink raw reply
* [PATCH v7 12/12] credential: add WWW-Authenticate header to cred requests
From: Matthew John Cheetham via GitGitGadget @ 2023-01-20 22:08 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Lessley Dennington, Matthew John Cheetham,
M Hickford, Jeff Hostetler, Glen Choo, Victoria Dye,
Ævar Arnfjörð Bjarmason, Matthew John Cheetham,
Matthew John Cheetham
In-Reply-To: <pull.1352.v7.git.1674252530.gitgitgadget@gmail.com>
From: Matthew John Cheetham <mjcheetham@outlook.com>
Add the value of the WWW-Authenticate response header to credential
requests. Credential helpers that understand and support HTTP
authentication and authorization can use this standard header (RFC 2616
Section 14.47 [1]) to generate valid credentials.
WWW-Authenticate headers can contain information pertaining to the
authority, authentication mechanism, or extra parameters/scopes that are
required.
The current I/O format for credential helpers only allows for unique
names for properties/attributes, so in order to transmit multiple header
values (with a specific order) we introduce a new convention whereby a
C-style array syntax is used in the property name to denote multiple
ordered values for the same property.
In this case we send multiple `wwwauth[]` properties where the order
that the repeated attributes appear in the conversation reflects the
order that the WWW-Authenticate headers appeared in the HTTP response.
Add a set of tests to exercise the HTTP authentication header parsing
and the interop with credential helpers. Credential helpers will receive
WWW-Authenticate information in credential requests.
[1] https://datatracker.ietf.org/doc/html/rfc2616#section-14.47
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
---
Documentation/git-credential.txt | 19 ++-
credential.c | 11 ++
t/lib-credential-helper.sh | 27 ++++
t/t5556-http-auth.sh | 242 +++++++++++++++++++++++++++++++
4 files changed, 298 insertions(+), 1 deletion(-)
create mode 100644 t/lib-credential-helper.sh
diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt
index ac2818b9f66..50759153ef1 100644
--- a/Documentation/git-credential.txt
+++ b/Documentation/git-credential.txt
@@ -113,7 +113,13 @@ separated by an `=` (equals) sign, followed by a newline.
The key may contain any bytes except `=`, newline, or NUL. The value may
contain any bytes except newline or NUL.
-In both cases, all bytes are treated as-is (i.e., there is no quoting,
+Attributes with keys that end with C-style array brackets `[]` can have
+multiple values. Each instance of a multi-valued attribute forms an
+ordered list of values - the order of the repeated attributes defines
+the order of the values. An empty multi-valued attribute (`key[]=\n`)
+acts to clear any previous entries and reset the list.
+
+In all cases, all bytes are treated as-is (i.e., there is no quoting,
and one cannot transmit a value with newline or NUL in it). The list of
attributes is terminated by a blank line or end-of-file.
@@ -160,6 +166,17 @@ empty string.
Components which are missing from the URL (e.g., there is no
username in the example above) will be left unset.
+`wwwauth[]`::
+
+ When an HTTP response is received by Git that includes one or more
+ 'WWW-Authenticate' authentication headers, these will be passed by Git
+ to credential helpers.
++
+Each 'WWW-Authenticate' header value is passed as a multi-valued
+attribute 'wwwauth[]', where the order of the attributes is the same as
+they appear in the HTTP response. This attribute is 'one-way' from Git
+to pass additional information to credential helpers.
+
Unrecognised attributes are silently discarded.
GIT
diff --git a/credential.c b/credential.c
index 897b4679333..9f39ebc3c7e 100644
--- a/credential.c
+++ b/credential.c
@@ -263,6 +263,16 @@ static void credential_write_item(FILE *fp, const char *key, const char *value,
fprintf(fp, "%s=%s\n", key, value);
}
+static void credential_write_strvec(FILE *fp, const char *key,
+ const struct strvec *vec)
+{
+ char *full_key = xstrfmt("%s[]", key);
+ for (size_t i = 0; i < vec->nr; i++) {
+ credential_write_item(fp, full_key, vec->v[i], 0);
+ }
+ free(full_key);
+}
+
void credential_write(const struct credential *c, FILE *fp)
{
credential_write_item(fp, "protocol", c->protocol, 1);
@@ -270,6 +280,7 @@ void credential_write(const struct credential *c, FILE *fp)
credential_write_item(fp, "path", c->path, 0);
credential_write_item(fp, "username", c->username, 0);
credential_write_item(fp, "password", c->password, 0);
+ credential_write_strvec(fp, "wwwauth", &c->wwwauth_headers);
}
static int run_credential_helper(struct credential *c,
diff --git a/t/lib-credential-helper.sh b/t/lib-credential-helper.sh
new file mode 100644
index 00000000000..8b0e4414234
--- /dev/null
+++ b/t/lib-credential-helper.sh
@@ -0,0 +1,27 @@
+setup_credential_helper() {
+ test_expect_success 'setup credential helper' '
+ CREDENTIAL_HELPER="$TRASH_DIRECTORY/credential-helper.sh" &&
+ export CREDENTIAL_HELPER &&
+ echo $CREDENTIAL_HELPER &&
+
+ write_script "$CREDENTIAL_HELPER" <<-\EOF
+ cmd=$1
+ teefile=$cmd-query.cred
+ catfile=$cmd-reply.cred
+ sed -n -e "/^$/q" -e "p" >> $teefile
+ if test "$cmd" = "get"; then
+ cat $catfile
+ fi
+ EOF
+ '
+}
+
+set_credential_reply() {
+ cat >"$TRASH_DIRECTORY/$1-reply.cred"
+}
+
+expect_credential_query() {
+ cat >"$TRASH_DIRECTORY/$1-expect.cred" &&
+ test_cmp "$TRASH_DIRECTORY/$1-expect.cred" \
+ "$TRASH_DIRECTORY/$1-query.cred"
+}
diff --git a/t/t5556-http-auth.sh b/t/t5556-http-auth.sh
index 2c16c8f72a5..93b7c178da6 100755
--- a/t/t5556-http-auth.sh
+++ b/t/t5556-http-auth.sh
@@ -4,6 +4,7 @@ test_description='test http auth header and credential helper interop'
TEST_NO_CREATE_REPO=1
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-credential-helper.sh
test_set_port GIT_TEST_HTTP_PROTOCOL_PORT
@@ -33,6 +34,8 @@ test_expect_success 'setup repos' '
git -C "$REPO_DIR" branch -M main
'
+setup_credential_helper
+
run_http_server_worker() {
(
cd "$REPO_DIR"
@@ -101,6 +104,7 @@ per_test_cleanup () {
stop_http_server &&
rm -f OUT.* &&
rm -f IN.* &&
+ rm -f *.cred &&
rm -f auth.config
}
@@ -218,4 +222,242 @@ test_expect_success 'http auth anonymous no challenge' '
git ls-remote $ORIGIN_URL
'
+test_expect_success 'http auth www-auth headers to credential helper basic valid' '
+ test_when_finished "per_test_cleanup" &&
+ # base64("alice:secret-passwd")
+ USERPASS64=YWxpY2U6c2VjcmV0LXBhc3N3ZA== &&
+ export USERPASS64 &&
+
+ cat >auth.config <<-EOF &&
+ [auth]
+ challenge = basic:realm=\"example.com\"
+ token = basic:$USERPASS64
+ EOF
+
+ start_http_server --auth-config="$TRASH_DIRECTORY/auth.config" &&
+
+ set_credential_reply get <<-EOF &&
+ protocol=http
+ host=$HOST_PORT
+ username=alice
+ password=secret-passwd
+ EOF
+
+ git -c "credential.helper=!\"$CREDENTIAL_HELPER\"" ls-remote $ORIGIN_URL &&
+
+ expect_credential_query get <<-EOF &&
+ protocol=http
+ host=$HOST_PORT
+ wwwauth[]=basic realm="example.com"
+ EOF
+
+ expect_credential_query store <<-EOF
+ protocol=http
+ host=$HOST_PORT
+ username=alice
+ password=secret-passwd
+ EOF
+'
+
+test_expect_success 'http auth www-auth headers to credential helper ignore case valid' '
+ test_when_finished "per_test_cleanup" &&
+ # base64("alice:secret-passwd")
+ USERPASS64=YWxpY2U6c2VjcmV0LXBhc3N3ZA== &&
+ export USERPASS64 &&
+
+ cat >auth.config <<-EOF &&
+ [auth]
+ challenge = basic:realm=\"example.com\"
+ token = basic:$USERPASS64
+ extraHeader = wWw-aUtHeNtIcAtE: bEaRer auThoRiTy=\"id.example.com\"
+ EOF
+
+ start_http_server --auth-config="$TRASH_DIRECTORY/auth.config" &&
+
+ set_credential_reply get <<-EOF &&
+ protocol=http
+ host=$HOST_PORT
+ username=alice
+ password=secret-passwd
+ EOF
+
+ git -c "credential.helper=!\"$CREDENTIAL_HELPER\"" ls-remote $ORIGIN_URL &&
+
+ expect_credential_query get <<-EOF &&
+ protocol=http
+ host=$HOST_PORT
+ wwwauth[]=basic realm="example.com"
+ wwwauth[]=bEaRer auThoRiTy="id.example.com"
+ EOF
+
+ expect_credential_query store <<-EOF
+ protocol=http
+ host=$HOST_PORT
+ username=alice
+ password=secret-passwd
+ EOF
+'
+
+test_expect_success 'http auth www-auth headers to credential helper continuation hdr' '
+ test_when_finished "per_test_cleanup" &&
+ # base64("alice:secret-passwd")
+ USERPASS64=YWxpY2U6c2VjcmV0LXBhc3N3ZA== &&
+ export USERPASS64 &&
+
+ cat >auth.config <<-EOF &&
+ [auth]
+ challenge = "bearer:authority=\"id.example.com\"\\n q=1\\n \\t p=0"
+ challenge = basic:realm=\"example.com\"
+ token = basic:$USERPASS64
+ EOF
+
+ start_http_server --auth-config="$TRASH_DIRECTORY/auth.config" &&
+
+ set_credential_reply get <<-EOF &&
+ protocol=http
+ host=$HOST_PORT
+ username=alice
+ password=secret-passwd
+ EOF
+
+ git -c "credential.helper=!\"$CREDENTIAL_HELPER\"" ls-remote $ORIGIN_URL &&
+
+ expect_credential_query get <<-EOF &&
+ protocol=http
+ host=$HOST_PORT
+ wwwauth[]=bearer authority="id.example.com" q=1 p=0
+ wwwauth[]=basic realm="example.com"
+ EOF
+
+ expect_credential_query store <<-EOF
+ protocol=http
+ host=$HOST_PORT
+ username=alice
+ password=secret-passwd
+ EOF
+'
+
+test_expect_success 'http auth www-auth headers to credential helper empty continuation hdrs' '
+ test_when_finished "per_test_cleanup" &&
+ # base64("alice:secret-passwd")
+ USERPASS64=YWxpY2U6c2VjcmV0LXBhc3N3ZA== &&
+ export USERPASS64 &&
+
+ cat >auth.config <<-EOF &&
+ [auth]
+ challenge = basic:realm=\"example.com\"
+ token = basic:$USERPASS64
+ extraheader = "WWW-Authenticate:"
+ extraheader = " "
+ extraheader = " bearer authority=\"id.example.com\""
+ EOF
+
+ start_http_server --auth-config="$TRASH_DIRECTORY/auth.config" &&
+
+ set_credential_reply get <<-EOF &&
+ protocol=http
+ host=$HOST_PORT
+ username=alice
+ password=secret-passwd
+ EOF
+
+ git -c "credential.helper=!\"$CREDENTIAL_HELPER\"" ls-remote $ORIGIN_URL &&
+
+ expect_credential_query get <<-EOF &&
+ protocol=http
+ host=$HOST_PORT
+ wwwauth[]=basic realm="example.com"
+ wwwauth[]=bearer authority="id.example.com"
+ EOF
+
+ expect_credential_query store <<-EOF
+ protocol=http
+ host=$HOST_PORT
+ username=alice
+ password=secret-passwd
+ EOF
+'
+
+test_expect_success 'http auth www-auth headers to credential helper custom schemes' '
+ test_when_finished "per_test_cleanup" &&
+ # base64("alice:secret-passwd")
+ USERPASS64=YWxpY2U6c2VjcmV0LXBhc3N3ZA== &&
+ export USERPASS64 &&
+
+ cat >auth.config <<-EOF &&
+ [auth]
+ challenge = "foobar:alg=test widget=1"
+ challenge = "bearer:authority=\"id.example.com\" q=1 p=0"
+ challenge = basic:realm=\"example.com\"
+ token = basic:$USERPASS64
+ EOF
+
+ start_http_server --auth-config="$TRASH_DIRECTORY/auth.config" &&
+
+ set_credential_reply get <<-EOF &&
+ protocol=http
+ host=$HOST_PORT
+ username=alice
+ password=secret-passwd
+ EOF
+
+ git -c "credential.helper=!\"$CREDENTIAL_HELPER\"" ls-remote $ORIGIN_URL &&
+
+ expect_credential_query get <<-EOF &&
+ protocol=http
+ host=$HOST_PORT
+ wwwauth[]=foobar alg=test widget=1
+ wwwauth[]=bearer authority="id.example.com" q=1 p=0
+ wwwauth[]=basic realm="example.com"
+ EOF
+
+ expect_credential_query store <<-EOF
+ protocol=http
+ host=$HOST_PORT
+ username=alice
+ password=secret-passwd
+ EOF
+'
+
+test_expect_success 'http auth www-auth headers to credential helper invalid' '
+ test_when_finished "per_test_cleanup" &&
+ # base64("alice:secret-passwd")
+ USERPASS64=YWxpY2U6c2VjcmV0LXBhc3N3ZA== &&
+ export USERPASS64 &&
+
+ cat >auth.config <<-EOF &&
+ [auth]
+ challenge = "bearer:authority=\"id.example.com\" q=1 p=0"
+ challenge = basic:realm=\"example.com\"
+ token = basic:$USERPASS64
+ EOF
+
+ start_http_server --auth-config="$TRASH_DIRECTORY/auth.config" &&
+
+ set_credential_reply get <<-EOF &&
+ protocol=http
+ host=$HOST_PORT
+ username=alice
+ password=invalid-passwd
+ EOF
+
+ test_must_fail git -c "credential.helper=!\"$CREDENTIAL_HELPER\"" ls-remote $ORIGIN_URL &&
+
+ expect_credential_query get <<-EOF &&
+ protocol=http
+ host=$HOST_PORT
+ wwwauth[]=bearer authority="id.example.com" q=1 p=0
+ wwwauth[]=basic realm="example.com"
+ EOF
+
+ expect_credential_query erase <<-EOF
+ protocol=http
+ host=$HOST_PORT
+ username=alice
+ password=invalid-passwd
+ wwwauth[]=bearer authority="id.example.com" q=1 p=0
+ wwwauth[]=basic realm="example.com"
+ EOF
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v7 10/12] http: replace unsafe size_t multiplication with st_mult
From: Matthew John Cheetham via GitGitGadget @ 2023-01-20 22:08 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Lessley Dennington, Matthew John Cheetham,
M Hickford, Jeff Hostetler, Glen Choo, Victoria Dye,
Ævar Arnfjörð Bjarmason, Matthew John Cheetham,
Matthew John Cheetham
In-Reply-To: <pull.1352.v7.git.1674252530.gitgitgadget@gmail.com>
From: Matthew John Cheetham <mjcheetham@outlook.com>
Replace direct multiplication of two size_t parameters in curl response
stream handling callback functions with `st_mult` to guard against
overflows.
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
---
http.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/http.c b/http.c
index 8a5ba3f4776..a2a80318bb2 100644
--- a/http.c
+++ b/http.c
@@ -146,7 +146,7 @@ static int http_schannel_use_ssl_cainfo;
size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
{
- size_t size = eltsize * nmemb;
+ size_t size = st_mult(eltsize, nmemb);
struct buffer *buffer = buffer_;
if (size > buffer->buf.len - buffer->posn)
@@ -176,7 +176,7 @@ curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp)
size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
{
- size_t size = eltsize * nmemb;
+ size_t size = st_mult(eltsize, nmemb);
struct strbuf *buffer = buffer_;
strbuf_add(buffer, ptr, size);
--
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