* Re: [PATCH 0/4] osxkeychain: bring in line with other credential helpers
From: M Hickford @ 2024-02-18 20:40 UTC (permalink / raw)
To: gitgitgadget; +Cc: git, mail
In-Reply-To: <pull.1667.git.1708212896.gitgitgadget@gmail.com>
> git-credential-osxkeychain has largely fallen behind other external
> credential helpers in the features it supports, and hasn't received any
> functional changes since 2013. As it stood, osxkeychain failed seven tests
> in the external credential helper test suite:
>
> not ok 8 - helper (osxkeychain) overwrites on store
> not ok 9 - helper (osxkeychain) can forget host
> not ok 11 - helper (osxkeychain) does not erase a password distinct from input
> not ok 15 - helper (osxkeychain) erases all matching credentials
> not ok 18 - helper (osxkeychain) gets password_expiry_utc
> not ok 19 - helper (osxkeychain) overwrites when password_expiry_utc changes
> not ok 21 - helper (osxkeychain) gets oauth_refresh_token
>
> After this set of patches, osxkeychain passes all tests in the external
credential helper test suite.
Great work!
Could these tests run as part of macOS CI?
^ permalink raw reply
* Re: [PATCH] completion: use awk for filtering the config entries
From: Johannes Schindelin @ 2024-02-18 21:58 UTC (permalink / raw)
To: Beat Bolli
Cc: Junio C Hamano, git, Philippe Blain,
Ævar Arnfjörð Bjarmason
In-Reply-To: <4a1d3618-cebe-4c20-89ce-c5dab51af21a@drbeat.li>
Hi Beat,
On Fri, 16 Feb 2024, Beat Bolli wrote:
> On 16.02.24 18:35, Junio C Hamano wrote:
> > Beat Bolli <dev+git@drbeat.li> writes:
> >
> > > Commits 1e0ee4087e (completion: add and use
> > > __git_compute_first_level_config_vars_for_section, 2024-02-10) and
> > > 6e32f718ff (completion: add and use
> > > __git_compute_second_level_config_vars_for_section, 2024-02-10)
> > > introduced new helpers for config completion.
> > >
> > > Both helpers use a pipeline of grep and awk to filter the list of config
> > > entries. awk is perfectly capable of filtering, so let's eliminate the
> > > grep process and move the filtering into the awk script.
> >
> > Makes sense. I wonder if we can have some simple script sanity
> > checker that catches things like this, e.g., catting a single file
> > into pipe, grep appearing upstream of awk or sed, etc.
>
> Yes, there are quite a few cases of these in t/. I'm not sure if it's worth
> the churn, though. At least it would make the tests faster on Windows...
Thank you for caring about the speed on Windows!
Ciao,
Johannes
^ permalink raw reply
* Re: Hello question on Git for Windows 2.43.0 - GUID and/or SWID tag for this title
From: Johannes Schindelin @ 2024-02-18 22:06 UTC (permalink / raw)
To: Konstantin Khomoutov; +Cc: git@vger.kernel.org
In-Reply-To: <20240216110041.dqz2n5dz43mqtq25@carbon>
[-- Attachment #1: Type: text/plain, Size: 1138 bytes --]
Hi,
On Fri, 16 Feb 2024, Konstantin Khomoutov wrote:
> On Thu, Feb 15, 2024 at 09:40:47PM +0000, Christian Castro wrote:
>
> >>> I have a question on the GUID and/or SWID tag for Git for Windows 2.43.0.
>
> > Question: Are you a Git for Windows developer, open-source contributor or?
> > I ask because I will contact the manufacturer of our inventory product and
> > provide them your feedback. But I'd like to know what your role is with Git
> > for Windows for as of now I just have a reply from someone named Matthias
> > from a live.de email domain. I hope you understand. Truly no offense meant
> > on my part.
> >
> > Therefore, please let me know what your role is with Git for Windows so I
> > can send this feedback accordingly and continue working on with our software
> > inventory vendor on the issue.
>
> I would say the chief Git-for-Windows maintainer is Johannes Schindelin [1].
It is: https://gitforwindows.org/governance-model.html
Matthias Aßhauer is a trusted Git for Windows contributor with write
permissions on the repositories, so what he says has a ton of weight.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH] Always check the return value of `repo_read_object_file()`
From: Johannes Schindelin @ 2024-02-18 22:36 UTC (permalink / raw)
To: Teng Long; +Cc: gitgitgadget, git
In-Reply-To: <20240216064326.89551-1-tenglong.tl@alibaba-inc.com>
Hi,
On Fri, 16 Feb 2024, Teng Long wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> wrote on Mon, 05 Feb 2024:
>
> Hi, when I do zh_CN l10n work for 2.44, I found some check changes like:
>
> die(_("unable to read tree %s")
>
> in patchset, some old code for this like work is similar but with parentheses
> surrounded with the OID parameter:
>
> die(_("unable to read tree (%s)")
FWIW I copied the error message from
https://github.com/git/git/blob/v2.43.0/tree-walk.c#L103, but only now
realized that it is untranslated.
> I think it's really a small nit, I don't think it's a requirement to immediately
> optimize, they're just some small printing consistency formatting issues, so make
> some small tips here.
Thank you for paying attention. I agree that it would be good to make
Git's error messages consistent, even if I sadly won't be able to focus on
that due to changes at my dayjob.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH v4 0/3] apply with core.filemode=false
From: Johannes Schindelin @ 2024-02-18 22:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqzfwb53a9.fsf@gitster.g>
Hi Junio,
On Wed, 7 Feb 2024, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Chandra Pratap noticed that "git apply" on a filesystem without
> > executable bit support gives a warning when applying a patch that
> > expects the preimage file to have executable bit on. Dscho noticed
> > that the initial fix by Chandra did not work well when applying a
> > patch in reverse. It turns out that apply.c:reverse_patches()
> > invalidates the "a patch that does not change mode bits have the
> > mode bits in .old_mode member and not in .new_mode member" invariant
> > we rely on.
> >
> > Here is the result of concerted effort.
> >
> > Chandra Pratap (1):
> > apply: ignore working tree filemode when !core.filemode
> >
> > Junio C Hamano (2):
> > apply: correctly reverse patch's pre- and post-image mode bits
> > apply: code simplification
> >
> > apply.c | 16 +++++++++++++---
> > t/t4129-apply-samemode.sh | 27 +++++++++++++++++++++++++++
> > 2 files changed, 40 insertions(+), 3 deletions(-)
>
> Anybody wants to offer a review on this? I actually am fairly
> confortable with these without any additional review, but since I am
> sweeping the "Needs review" topics in the What's cooking report, I
> thought I would ask for this one, too.
I just had a look over all three of the patches, and to me, they look good
to go.
Ciao,
Johannes
^ permalink raw reply
* [PATCH] libsecret: retrieve empty password
From: M Hickford via GitGitGadget @ 2024-02-18 22:51 UTC (permalink / raw)
To: git; +Cc: M Hickford, M Hickford
From: M Hickford <mirth.hickford@gmail.com>
Since 0ce02e2f (credential/libsecret: store new attributes, 2023-06-16)
a test that stores empty username and password fails when
t0303-credential-external.sh is run with
GIT_TEST_CREDENTIAL_HELPER=libsecret.
Retrieve empty password carefully. This fixes test:
ok 14 - helper (libsecret) can store empty username
Signed-off-by: M Hickford <mirth.hickford@gmail.com>
---
libsecret: retrieve empty password
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1676%2Fhickford%2Flibsecret-empty-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1676/hickford/libsecret-empty-v1
Pull-Request: https://github.com/git/git/pull/1676
contrib/credential/libsecret/git-credential-libsecret.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/contrib/credential/libsecret/git-credential-libsecret.c b/contrib/credential/libsecret/git-credential-libsecret.c
index 215a81d8bae..d9e9e4fd524 100644
--- a/contrib/credential/libsecret/git-credential-libsecret.c
+++ b/contrib/credential/libsecret/git-credential-libsecret.c
@@ -164,6 +164,9 @@ static int keyring_get(struct credential *c)
if (g_strv_length(parts) >= 1) {
g_free(c->password);
c->password = g_strdup(parts[0]);
+ } else {
+ g_free(c->password);
+ c->password = strdup("");
}
for (int i = 1; i < g_strv_length(parts); i++) {
if (g_str_has_prefix(parts[i], "password_expiry_utc=")) {
base-commit: 3e0d3cd5c7def4808247caf168e17f2bbf47892b
--
gitgitgadget
^ permalink raw reply related
* Re: Suggested clarification for .gitattributes reference documentation
From: Johannes Schindelin @ 2024-02-18 23:12 UTC (permalink / raw)
To: Matthias Aßhauer
Cc: Torsten Bögershausen, Michael Litwak, brian m. carlson,
git@vger.kernel.org
In-Reply-To: <DB9P250MB0692C605DFA88572D091D959A56E2@DB9P250MB0692.EURP250.PROD.OUTLOOK.COM>
[-- Attachment #1: Type: text/plain, Size: 3706 bytes --]
Hi,
On Sat, 13 Jan 2024, Matthias Aßhauer wrote:
> On Sat, 13 Jan 2024, Torsten Bögershausen wrote:
>
> > On Sat, Jan 13, 2024 at 02:56:27AM +0000, Michael Litwak wrote:
> > > I just installed Git for Windows 2.43.0 and noticed the installer offers
> > > three options for altering the PATH:
> > >
> > > 1) Run git from git bash only
> > >
> > > 2) Run git from git bash, cmd.exe and PowerShell (RECOMMENDED)
> > >
> > > 3) Run git from git bash, cmd.exe and PowerShell with optional utilities
> > > (warning: will override find, sort and other system utilities).
> > >
> > > It turns out iconv.exe is accessible from cmd.exe (Command Prompt) only
> > > when you take the third option. But iconv.exe is NOT optional. It is
> > > required for git to deal with UTF-16LE with BOM text conversions (and
> > > probably for numerous other encoding conversions).
>
> For end users directly calling iconv.exe is definitely optional.
>
> > Plese wait a second - and thanks for bringing this up.
> > To my knowledge the binary iconv.exe (or just iconv under non-Windows)
> > is never called from Git itself.
> > Git is using iconv_open() and friends, which are all inside
> > a library, either the C-library "libc", or "libiconv"
> > (not 100% sure about the naming here)
>
> Exactly. I can't find a single instance of Git for Windows calling iconv.exe
> instead of using the corresponding library functions. [1]
>
> And even if it did, iconv.exe is definitely on the path for git.exe [2] unless
> you're calling /(mingw|clangarm)(64|32)/bin/git.exe directly, in which case
> the solution is to call /cmd/git.exe instead.
Just a quick addition to this: _even if_ you happen to call
`/*[63][42]/bin/git.exe` directly, the `PATH` is adjusted (unless
`MSYSTEM` is set, which would indicate a pilot error without the
corresponding `PATH` components such as `/usr/bin`):
https://github.com/git-for-windows/git/blob/v2.43.0.windows.1/compat/mingw.c#L3529
Ciao,
Johannes
> [1]
> https://github.com/search?q=repo%3Agit-for-windows%2Fgit%20iconv%20NOT%20path%3A%2F%5Et%5C%2F%2F%20NOT%20path%3A%2F%5EDocumentation%5C%2F%2F&type=code
> [2]
> https://github.com/git-for-windows/MINGW-packages/blob/0c91cf2079184ae6a604e8f7a406a47d39305e72/mingw-w64-git/git-wrapper.c#L166-L258
>
> > iconv.exe is not needed in everyday life, or is it ?
> > If yes, when ?
> > iconv.exe is used when you run the test-suite, to verify
> > what Git is doing.
> >
> > Could you elaborate a little bit more,
> > when iconv.exe is missing, and what is happening, please ?
> >
> > >
> > > But when PATH option #2 is chosen, and iconv.exe is unreachable from a
> > > Windows Command Prompt, the git commands which call upon iconv.exe do NOT
> > > indicate the error. The call to iconv.exe fails silently. It is only
> > > later after you commit, push and clone the repo again that you see the
> > > encoding failures.
> > >
> > > And the warning about overriding find and sort must be taken with a grain
> > > of salt, since the Windows versions of those programs are accessed via a
> > > Windows folder which appears earlier in the PATH.
>
> We should probably consider rewording that warning.
>
> > > So this Git for Windows installer screen is misleading. And perhaps
> > > iconv.exe should be relocated so it is accessible even when PATH option #2
> > > is chosen. I intend to submit an issue on the Git for Windows issue
> > > tracker regarding this. I'll also submit an issue about the lack of an
> > > error when running 'git add' for a UTF-16LE with BOM file under PATH
> > > option #2.
> > >
> > > Thanks,
> > > - Michael
> > >
> > []
> >
>
>
^ permalink raw reply
* Re: [PATCH 0/4] osxkeychain: bring in line with other credential helpers
From: Bo Anderson @ 2024-02-18 23:23 UTC (permalink / raw)
To: M Hickford; +Cc: Bo Anderson via GitGitGadget, git
In-Reply-To: <20240218204044.11365-1-mirth.hickford@gmail.com>
> On 18 Feb 2024, at 20:40, M Hickford <mirth.hickford@gmail.com> wrote:
>
>> git-credential-osxkeychain has largely fallen behind other external
>> credential helpers in the features it supports, and hasn't received any
>> functional changes since 2013. As it stood, osxkeychain failed seven tests
>> in the external credential helper test suite:
>>
>> not ok 8 - helper (osxkeychain) overwrites on store
>> not ok 9 - helper (osxkeychain) can forget host
>> not ok 11 - helper (osxkeychain) does not erase a password distinct from input
>> not ok 15 - helper (osxkeychain) erases all matching credentials
>> not ok 18 - helper (osxkeychain) gets password_expiry_utc
>> not ok 19 - helper (osxkeychain) overwrites when password_expiry_utc changes
>> not ok 21 - helper (osxkeychain) gets oauth_refresh_token
>>
>> After this set of patches, osxkeychain passes all tests in the external
> credential helper test suite.
>
> Great work!
>
> Could these tests run as part of macOS CI?
Do we do so for any of the other external credential helpers?
It definitely makes sense in principle. Though the concern perhaps will be that any new features added to the credential helpers and thus its test suite would need adding to each credential helper simultaneously to avoid failing CI. Ideally we would do exactly that, though that requires knowledge on each of the keystore APIs used in each of the credential helpers.
^ permalink raw reply
* [GIT PULL] l10n updates for 2.44.0 round 3
From: Jiang Xin @ 2024-02-19 1:01 UTC (permalink / raw)
To: Junio C Hamano, Git l10n discussion group
Cc: Jiang Xin, Git List, Alexander Shopov, Arkadii Yakovets,
Bagas Sanjaya, Emir SARI, Jean-Noël Avila,
Johannes Schindelin, Jordi Mas, Peter Krefting, Ralf Thielow,
Teng Long, Yi-Jyun Pan
Hi Junio,
Please pull the following l10n updates for Git 2.44.0.
The following changes since commit 3e0d3cd5c7def4808247caf168e17f2bbf47892b:
Merge branch 'jx/dirstat-parseopt-help' (2024-02-15 15:14:48 -0800)
are available in the Git repository at:
git@github.com:git-l10n/git-po.git tags/l10n-2.44.0-rnd3
for you to fetch changes up to 5fdd5b989cbe5096d44e89861a92b2dd47c279d9:
l10n: zh_TW: Git 2.44 (2024-02-18 21:03:43 +0800)
----------------------------------------------------------------
l10n-2.44.0-rnd3
-----BEGIN PGP SIGNATURE-----
iQIzBAABCAAdFiEE37vMEzKDqYvVxs51k24VDd1FMtUFAmXSpeoACgkQk24VDd1F
MtX7RA/9HXk19TQPI/8YHL2Z29Yax2yPcgy+kq/UzXJ3YbNAsEBZyLwqfkv1bi6h
S67Ggc0tmQdE0BDDAwG54+kzCUnRh6WFzHXyU0Mazena+7kxRuyCdmKNj9a9W7Jn
2NLiS37a8KB83nlUn7coIrbFfs7P80J50Ax6oJFSPTEqZM8unNw/QEitufodaju2
XdAbO8wofZZDn+i+HiCQnUT3loV8XxJdCk/ZM7RMtLRLzxKx78GsazLjkbYG1ci1
4yAw3A6M+w1AvppplToZiH4JYvpMg7Box4tow0EKcYL5yOMk9tx2kVAbc26Mpm4Q
IrADtuilhyHr8UI/VrD1frkNW+BByaE3WAJ2IgSzFmOv2eqe2aVtvkmga0DG1zyl
P0ZGAKjfH0cSpEvuG16XGHYQjqp/ulWDedx9bdFJg2iFnjHj7F2DEubm08tUW7OG
cNs6uYGTyfq9VLtvc4qIHFQNtMrUtnYETNK1Sn++11CYWoYpFzdUk75oEB8SkxYq
JPn9xFbhzz5K9vKE1jAp1XIYLaKwD1up71VPCL6bhHyOwvgJ1RcWtbK+h58xqGAw
n+w5epstdOGeKkmtrpYC1R6Y/ejckdk++/K8ml5owFeQ7u1l1zNsonpf9qtjnBLI
utf3YmfkfBd767kAqfeqdcjV9+hLgpaegl8ElWChRLOushfhj18=
=ZD+B
-----END PGP SIGNATURE-----
----------------------------------------------------------------
Alexander Shopov (1):
l10n: bg.po: Updated Bulgarian translation (5610t)
Arkadii Yakovets (3):
l10n: uk: v2.44 localization update
l10n: uk: v2.44 update (round 2)
l10n: uk: v2.44 update (round 3)
Bagas Sanjaya (1):
l10n: po-id for 2.44 (round 1)
Emir SARI (1):
l10n: tr: Update Turkish translations for 2.44
Jean-Noël Avila (1):
l10n: fr.po: v2.44.0 round 3
Jiang Xin (12):
Merge branch 'master' of github.com:git/git
Merge branch 'master' of github.com:git/git
l10n: ci: remove unused param for add-pr-comment@v2
l10n: ci: disable cache for setup-go to suppress warnings
Merge branch 'master' of github.com:nafmo/git-l10n-sv
Merge branch 'catalan-l10n' of github.com:Softcatala/git-po
Merge branch 'fr_2.44.0' of github.com:jnavila/git
Merge branch 'tr-l10n' of github.com:bitigchi/git-po
Merge branch 'master' of github.com:alshopov/git-po
Merge branch '2.44-uk-update' of github.com:arkid15r/git-ukrainian-l10n
Merge branch 'po-id' of github.com:bagasme/git-po
Merge branch 'master' of github.com:ralfth/git
Johannes Schindelin (1):
l10n: bump Actions versions in l10n.yml
Jordi Mas (1):
l10n: Update Catalan translation
Peter Krefting (1):
l10n: sv.po: Update Swedish translation
Ralf Thielow (1):
l10n: Update German translation
Teng Long (1):
l10n: zh_CN: for git 2.44 rounds
Yi-Jyun Pan (1):
l10n: zh_TW: Git 2.44
.github/workflows/l10n.yml | 6 +-
po/bg.po | 391 +++++++++++++++++++++-----------
po/ca.po | 466 ++++++++++++++++++++++++---------------
po/de.po | 392 +++++++++++++++++++++------------
po/fr.po | 427 +++++++++++++++++++++++++----------
po/id.po | 485 ++++++++++++++++++++++++++--------------
po/sv.po | 358 ++++++++++++++++++++----------
po/tr.po | 380 +++++++++++++++++++++-----------
po/uk.po | 429 +++++++++++++++++++++++-------------
po/zh_CN.po | 538 ++++++++++++++++++++++++++++-----------------
po/zh_TW.po | 531 ++++++++++++++++++++++++++++++--------------
11 files changed, 2894 insertions(+), 1509 deletions(-)
--
Jiang Xin
^ permalink raw reply
* Acoustic Instrument
From: Sarenaa Fuller @ 2024-02-19 1:40 UTC (permalink / raw)
To: git
Hello,
I'm giving out my late husband's Yamaha Baby Grand Piano to any passionate instrument lover. Let me know if you are interested or have someone who would like to have the instrument.
Thanks,
Sarenaa Fuller
^ permalink raw reply
* [PATCH 0/2] apply: add unit tests for parse_range
From: Philip Peterson via GitGitGadget @ 2024-02-19 4:45 UTC (permalink / raw)
To: git; +Cc: Philip Peterson
Hello all,
This patchset adds unit tests for apply's parse_range function. Also renames
parse_range to parse_fragment_range.
It was necessary to make the function be non-internal linkage in order to
expose it to the unit testing framework. Because there is another function
in the codebase also called parse_range, I changed this one to a more
specific name as well: parse_fragment_range. There are also several test
cases added (both positive and negative) for the function.
Thank you for your help,
Philip
Philip Peterson (2):
apply: add unit tests for parse_range and rename to
parse_fragment_range
apply: rewrite unit tests with structured cases
Makefile | 1 +
apply.c | 8 ++--
apply.h | 4 ++
t/unit-tests/t-apply.c | 100 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 109 insertions(+), 4 deletions(-)
create mode 100644 t/unit-tests/t-apply.c
base-commit: 186b115d3062e6230ee296d1ddaa0c4b72a464b5
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1677%2Fphilip-peterson%2Fpeterson%2Funit-tests-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1677/philip-peterson/peterson/unit-tests-v1
Pull-Request: https://github.com/git/git/pull/1677
--
gitgitgadget
^ permalink raw reply
* [PATCH 1/2] apply: add unit tests for parse_range and rename to parse_fragment_range
From: Philip Peterson via GitGitGadget @ 2024-02-19 4:45 UTC (permalink / raw)
To: git; +Cc: Philip Peterson, Philip Peterson
In-Reply-To: <pull.1677.git.git.1708317938.gitgitgadget@gmail.com>
From: Philip Peterson <philip.c.peterson@gmail.com>
This patchset makes the parse_range function in apply be non-internal
linkage in order to expose to the unit testing framework. In so doing,
because there is another function called parse_range, I gave this one a more
specific name, parse_fragment_range. Other than that, this commit adds
several test cases (positive and negative) for the function.
Signed-off-by: Philip Peterson <philip.c.peterson@gmail.com>
---
Makefile | 1 +
apply.c | 8 ++---
apply.h | 4 +++
t/unit-tests/t-apply.c | 67 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 76 insertions(+), 4 deletions(-)
create mode 100644 t/unit-tests/t-apply.c
diff --git a/Makefile b/Makefile
index 15990ff3122..369092aedfe 100644
--- a/Makefile
+++ b/Makefile
@@ -1339,6 +1339,7 @@ THIRD_PARTY_SOURCES += compat/regex/%
THIRD_PARTY_SOURCES += sha1collisiondetection/%
THIRD_PARTY_SOURCES += sha1dc/%
+UNIT_TEST_PROGRAMS += t-apply
UNIT_TEST_PROGRAMS += t-basic
UNIT_TEST_PROGRAMS += t-mem-pool
UNIT_TEST_PROGRAMS += t-strbuf
diff --git a/apply.c b/apply.c
index 7608e3301ca..199a1150df6 100644
--- a/apply.c
+++ b/apply.c
@@ -1430,8 +1430,8 @@ static int parse_num(const char *line, unsigned long *p)
return ptr - line;
}
-static int parse_range(const char *line, int len, int offset, const char *expect,
- unsigned long *p1, unsigned long *p2)
+int parse_fragment_range(const char *line, int len, int offset, const char *expect,
+ unsigned long *p1, unsigned long *p2)
{
int digits, ex;
@@ -1530,8 +1530,8 @@ static int parse_fragment_header(const char *line, int len, struct fragment *fra
return -1;
/* Figure out the number of lines in a fragment */
- offset = parse_range(line, len, 4, " +", &fragment->oldpos, &fragment->oldlines);
- offset = parse_range(line, len, offset, " @@", &fragment->newpos, &fragment->newlines);
+ offset = parse_fragment_range(line, len, 4, " +", &fragment->oldpos, &fragment->oldlines);
+ offset = parse_fragment_range(line, len, offset, " @@", &fragment->newpos, &fragment->newlines);
return offset;
}
diff --git a/apply.h b/apply.h
index 7cd38b1443c..bbc5e3caeb5 100644
--- a/apply.h
+++ b/apply.h
@@ -187,3 +187,7 @@ int apply_all_patches(struct apply_state *state,
int options);
#endif
+
+
+int parse_fragment_range(const char *line, int len, int offset, const char *expect,
+ unsigned long *p1, unsigned long *p2);
diff --git a/t/unit-tests/t-apply.c b/t/unit-tests/t-apply.c
new file mode 100644
index 00000000000..ff0abfb2e0b
--- /dev/null
+++ b/t/unit-tests/t-apply.c
@@ -0,0 +1,67 @@
+#include "test-lib.h"
+#include "apply.h"
+
+#define FAILURE -1
+
+static void setup_static(const char *line, int len, int offset,
+ const char *expect, int assert_result,
+ unsigned long assert_p1,
+ unsigned long assert_p2)
+{
+ unsigned long p1 = 9999;
+ unsigned long p2 = 9999;
+ int result = parse_fragment_range(line, len, offset, expect, &p1, &p2);
+ check_int(result, ==, assert_result);
+ check_int(p1, ==, assert_p1);
+ check_int(p2, ==, assert_p2);
+}
+
+int cmd_main(int argc, const char **argv)
+{
+ char* text;
+ int expected_result;
+
+ /* Success */
+ text = "@@ -4,4 +";
+ expected_result = 9;
+ TEST(setup_static(text, strlen(text), 4, " +", expected_result, 4, 4),
+ "well-formed range");
+
+ text = "@@ -4 +8 @@";
+ expected_result = 7;
+ TEST(setup_static(text, strlen(text), 4, " +", expected_result, 4, 1),
+ "non-comma range");
+
+ /* Failure */
+ text = "@@ -X,4 +";
+ expected_result = FAILURE;
+ TEST(setup_static(text, strlen(text), 4, " +", expected_result, 9999, 9999),
+ "non-digit range (first coordinate)");
+
+ text = "@@ -4,X +";
+ expected_result = FAILURE;
+ TEST(setup_static(text, strlen(text), 4, " +", expected_result, 4, 1), // p2 is 1, a little strange but not catastrophic
+ "non-digit range (second coordinate)");
+
+ text = "@@ -4,4 -";
+ expected_result = FAILURE;
+ TEST(setup_static(text, strlen(text), 4, " +", expected_result, 4, 4),
+ "non-expected trailing text");
+
+ text = "@@ -4,4";
+ expected_result = FAILURE;
+ TEST(setup_static(text, strlen(text), 4, " +", expected_result, 4, 4),
+ "not long enough for expected trailing text");
+
+ text = "@@ -4,4";
+ expected_result = FAILURE;
+ TEST(setup_static(text, strlen(text), 7, " +", expected_result, 9999, 9999),
+ "not long enough for offset");
+
+ text = "@@ -4,4";
+ expected_result = FAILURE;
+ TEST(setup_static(text, strlen(text), -1, " +", expected_result, 9999, 9999),
+ "negative offset");
+
+ return test_done();
+}
--
gitgitgadget
^ permalink raw reply related
* [PATCH 2/2] apply: rewrite unit tests with structured cases
From: Philip Peterson via GitGitGadget @ 2024-02-19 4:45 UTC (permalink / raw)
To: git; +Cc: Philip Peterson, Philip Peterson
In-Reply-To: <pull.1677.git.git.1708317938.gitgitgadget@gmail.com>
From: Philip Peterson <philip.c.peterson@gmail.com>
The imperative format was a little hard to read, so I rewrote the test cases
in a declarative style by defining a common structure for each test case and
its assertions.
Signed-off-by: Philip Peterson <philip.c.peterson@gmail.com>
---
t/unit-tests/t-apply.c | 123 ++++++++++++++++++++++++++---------------
1 file changed, 78 insertions(+), 45 deletions(-)
diff --git a/t/unit-tests/t-apply.c b/t/unit-tests/t-apply.c
index ff0abfb2e0b..2b78624b690 100644
--- a/t/unit-tests/t-apply.c
+++ b/t/unit-tests/t-apply.c
@@ -3,65 +3,98 @@
#define FAILURE -1
-static void setup_static(const char *line, int len, int offset,
- const char *expect, int assert_result,
- unsigned long assert_p1,
- unsigned long assert_p2)
+typedef struct test_case {
+ const char *line;
+ const char *expect_suffix;
+ int offset;
+ unsigned long expect_p1;
+ unsigned long expect_p2;
+ int expect_result;
+} test_case;
+
+static void setup_static(struct test_case t)
{
unsigned long p1 = 9999;
unsigned long p2 = 9999;
- int result = parse_fragment_range(line, len, offset, expect, &p1, &p2);
- check_int(result, ==, assert_result);
- check_int(p1, ==, assert_p1);
- check_int(p2, ==, assert_p2);
+ int result = parse_fragment_range(t.line, strlen(t.line), t.offset, t.expect_suffix, &p1, &p2);
+ check_int(result, ==, t.expect_result);
+ check_int(p1, ==, t.expect_p1);
+ check_int(p2, ==, t.expect_p2);
}
int cmd_main(int argc, const char **argv)
{
- char* text;
- int expected_result;
-
- /* Success */
- text = "@@ -4,4 +";
- expected_result = 9;
- TEST(setup_static(text, strlen(text), 4, " +", expected_result, 4, 4),
- "well-formed range");
+ TEST(setup_static((struct test_case) {
+ .line = "@@ -4,4 +",
+ .offset = 4,
+ .expect_suffix = " +",
+ .expect_result = 9,
+ .expect_p1 = 4,
+ .expect_p2 = 4
+ }), "well-formed range");
- text = "@@ -4 +8 @@";
- expected_result = 7;
- TEST(setup_static(text, strlen(text), 4, " +", expected_result, 4, 1),
- "non-comma range");
+ TEST(setup_static((struct test_case) {
+ .line = "@@ -4 +8 @@",
+ .offset = 4,
+ .expect_suffix = " +",
+ .expect_result = 7,
+ .expect_p1 = 4,
+ .expect_p2 = 1
+ }), "non-comma range");
- /* Failure */
- text = "@@ -X,4 +";
- expected_result = FAILURE;
- TEST(setup_static(text, strlen(text), 4, " +", expected_result, 9999, 9999),
- "non-digit range (first coordinate)");
+ TEST(setup_static((struct test_case) {
+ .line = "@@ -X,4 +",
+ .offset = 4,
+ .expect_suffix = " +",
+ .expect_result = FAILURE,
+ .expect_p1 = 9999,
+ .expect_p2 = 9999
+ }), "non-digit range (first coordinate)");
- text = "@@ -4,X +";
- expected_result = FAILURE;
- TEST(setup_static(text, strlen(text), 4, " +", expected_result, 4, 1), // p2 is 1, a little strange but not catastrophic
- "non-digit range (second coordinate)");
+ TEST(setup_static((struct test_case) {
+ .line = "@@ -4,X +",
+ .offset = 4,
+ .expect_suffix = " +",
+ .expect_result = FAILURE,
+ .expect_p1 = 4,
+ .expect_p2 = 1 // A little strange this is 1, but not end of the world
+ }), "non-digit range (second coordinate)");
- text = "@@ -4,4 -";
- expected_result = FAILURE;
- TEST(setup_static(text, strlen(text), 4, " +", expected_result, 4, 4),
- "non-expected trailing text");
+ TEST(setup_static((struct test_case) {
+ .line = "@@ -4,4 -",
+ .offset = 4,
+ .expect_suffix = " +",
+ .expect_result = FAILURE,
+ .expect_p1 = 4,
+ .expect_p2 = 4
+ }), "non-expected trailing text");
- text = "@@ -4,4";
- expected_result = FAILURE;
- TEST(setup_static(text, strlen(text), 4, " +", expected_result, 4, 4),
- "not long enough for expected trailing text");
+ TEST(setup_static((struct test_case) {
+ .line = "@@ -4,4",
+ .offset = 4,
+ .expect_suffix = " +",
+ .expect_result = FAILURE,
+ .expect_p1 = 4,
+ .expect_p2 = 4
+ }), "not long enough for expected trailing text");
- text = "@@ -4,4";
- expected_result = FAILURE;
- TEST(setup_static(text, strlen(text), 7, " +", expected_result, 9999, 9999),
- "not long enough for offset");
+ TEST(setup_static((struct test_case) {
+ .line = "@@ -4,4",
+ .offset = 7,
+ .expect_suffix = " +",
+ .expect_result = FAILURE,
+ .expect_p1 = 9999,
+ .expect_p2 = 9999
+ }), "not long enough for offset");
- text = "@@ -4,4";
- expected_result = FAILURE;
- TEST(setup_static(text, strlen(text), -1, " +", expected_result, 9999, 9999),
- "negative offset");
+ TEST(setup_static((struct test_case) {
+ .line = "@@ -4,4",
+ .offset = -1,
+ .expect_suffix = " +",
+ .expect_result = FAILURE,
+ .expect_p1 = 9999,
+ .expect_p2 = 9999
+ }), "negative offset");
return test_done();
}
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH] libsecret: retrieve empty password
From: Patrick Steinhardt @ 2024-02-19 6:08 UTC (permalink / raw)
To: M Hickford via GitGitGadget; +Cc: git, M Hickford
In-Reply-To: <pull.1676.git.git.1708296694988.gitgitgadget@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1849 bytes --]
On Sun, Feb 18, 2024 at 10:51:34PM +0000, M Hickford via GitGitGadget wrote:
> From: M Hickford <mirth.hickford@gmail.com>
>
> Since 0ce02e2f (credential/libsecret: store new attributes, 2023-06-16)
> a test that stores empty username and password fails when
> t0303-credential-external.sh is run with
> GIT_TEST_CREDENTIAL_HELPER=libsecret.
>
> Retrieve empty password carefully. This fixes test:
>
> ok 14 - helper (libsecret) can store empty username
>
> Signed-off-by: M Hickford <mirth.hickford@gmail.com>
> ---
> libsecret: retrieve empty password
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1676%2Fhickford%2Flibsecret-empty-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1676/hickford/libsecret-empty-v1
> Pull-Request: https://github.com/git/git/pull/1676
>
> contrib/credential/libsecret/git-credential-libsecret.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/contrib/credential/libsecret/git-credential-libsecret.c b/contrib/credential/libsecret/git-credential-libsecret.c
> index 215a81d8bae..d9e9e4fd524 100644
> --- a/contrib/credential/libsecret/git-credential-libsecret.c
> +++ b/contrib/credential/libsecret/git-credential-libsecret.c
> @@ -164,6 +164,9 @@ static int keyring_get(struct credential *c)
> if (g_strv_length(parts) >= 1) {
> g_free(c->password);
> c->password = g_strdup(parts[0]);
> + } else {
> + g_free(c->password);
> + c->password = strdup("");
Shouldn't we use `g_strdup()` here, like we do everywhere else in this
credential helper?
Patrick
> }
> for (int i = 1; i < g_strv_length(parts); i++) {
> if (g_str_has_prefix(parts[i], "password_expiry_utc=")) {
>
> base-commit: 3e0d3cd5c7def4808247caf168e17f2bbf47892b
> --
> gitgitgadget
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] diff.c: use utf8_strwidth() instead of strlen() for display width
From: Patrick Steinhardt @ 2024-02-19 6:23 UTC (permalink / raw)
To: Chandra Pratap via GitGitGadget; +Cc: git, Chandra Pratap, Chandra Pratap
In-Reply-To: <pull.1668.git.1708281443289.gitgitgadget@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1948 bytes --]
On Sun, Feb 18, 2024 at 06:37:23PM +0000, Chandra Pratap via GitGitGadget wrote:
> From: Chandra Pratap <chandrapratap3519@gmail.com>
>
> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
> ---
> diff.c: use utf8_strwidth() instead of strlen() for display width
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1668%2FChand-ra%2Fdiff-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1668/Chand-ra/diff-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1668
>
> diff.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index ccfa1fca0d0..02d60af6749 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -2712,13 +2712,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
> * making the line longer than the maximum width.
> */
>
> - /*
> - * NEEDSWORK: line_prefix is often used for "log --graph" output
> - * and contains ANSI-colored string. utf8_strnwidth() should be
> - * used to correctly count the display width instead of strlen().
> - */
> if (options->stat_width == -1)
> - width = term_columns() - strlen(line_prefix);
> + width = term_columns() - utf8_strwidth(line_prefix);
It would be nice to add a test demonstrating that this indeed fixes an
issue. This would also help to keep this from regressing in the future.
Also, do you know why we didn't use `utf8_strwidth()` right from the
start? It would have saved the writer some time to just use
`utf8_strwidth()` instead of writing a whole paragraph explaining that
we should do it eventually. Makes me wonder whether there is anything
else going on here.
Patrick
> else
> width = options->stat_width ? options->stat_width : 80;
> number_width = decimal_width(max_change) > number_width ?
>
> base-commit: 2996f11c1d11ab68823f0939b6469dedc2b9ab90
> --
> gitgitgadget
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] diff.c: use utf8_strwidth() instead of strlen() for display width
From: Junio C Hamano @ 2024-02-19 7:01 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Chandra Pratap via GitGitGadget, git, Chandra Pratap,
Chandra Pratap
In-Reply-To: <ZdLzxYpY-klokgpI@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
> Also, do you know why we didn't use `utf8_strwidth()` right from the
> start? It would have saved the writer some time to just use
> `utf8_strwidth()` instead of writing a whole paragraph explaining that
> we should do it eventually. Makes me wonder whether there is anything
> else going on here.
I suspect that it is because it is not just that single strlen()
call. The code assumed that byte count and display width were
interchangeable, and use of strlen() there was merely an example.
Starting there, the value returned by strlen() is treated as if it
were interchangeable with a display width, and then later used to
count how many bytes to trim from either end of the string so that
the trimmed string would eventually fit a given display width, which
means that the code to compute how much to trim (which is not that
strlen() the patch in question is touching) would have to compute
the reverse, i.e. "if we need to recover N display columns, how many
bytes do we need to trim from that string?"
^ permalink raw reply
* Re: [PATCH] builtin/stash: configs keepIndex, includeUntracked
From: Jean-Noël Avila @ 2024-02-19 8:04 UTC (permalink / raw)
To: MithicSpirit, git
In-Reply-To: <20240218033146.372727-2-rpc01234@gmail.com>
Hello,
Le 18/02/2024 à 04:30, MithicSpirit a écrit :
> Adds options `stash.keepIndex` and `stash.includeUntracked`, which
> enable `--keep-index` and `--include-untracked` by default (unless it
> would conflict with another option). This is done to facilitate the
> workflow where a user:
>
> . Makes modifications;
> . Selectively adds them with `git add -p`; and
> . Stashes the unadded changes to run tests with only the current
> modifications.
>
> `stash.keepIndex` is especially important for this, as otherwise the
> work done in `git add -p` is lost since applying the stash does not
> restore the index. This problem could also be solved by adding
> functionality to the stash to restore the index specifically, but this
> would create much more complexity and still wouldn't be as convenient
> for that workflow.
>
> Signed-off-by: Ricardo Prado Cunha (MithicSpirit) <rpc01234@gmail.com>
> ---
> This is my first patch to git and my first time using mailing lists for this
> kind of stuff. Please do let me know of any mistakes or gaffes I've made.
>
> Documentation/config/stash.txt | 13 ++++
> builtin/stash.c | 65 ++++++++++++------
> t/t3903-stash.sh | 119 +++++++++++++++++++++++++++++++++
> 3 files changed, 178 insertions(+), 19 deletions(-)
>
> diff --git a/Documentation/config/stash.txt b/Documentation/config/stash.txt
> index ec1edaeba6..d6d9ea7daa 100644
> --- a/Documentation/config/stash.txt
> +++ b/Documentation/config/stash.txt
> @@ -1,6 +1,19 @@
> +stash.includeUntracked::
> + Boolean option that configures whether the `git stash push` and `git
> + stash save` commands also stash untracked files by default. This option
> + is ignored if `--include-untracked`, `--no-include-untracked`, `--all`,
> + `--patch`, or `--staged` are used. Defaults to `false`. See
> + linkgit:git-stash[1].
> +
> +stash.keepIndex::
> + Boolean option that configures whether the `git stash push` and `git
> + stash save` commands also stash changes that have been added to the
> + index. This option is ignored if `--keep-index`, `--no-keep-index`, or
> + `--patch` are used. Defaults to `false`. See linkgit:git-stash[1].
> +
> stash.showIncludeUntracked::
> If this is set to true, the `git stash show` command will show
> the untracked files of a stash entry. Defaults to false. See
> the description of the 'show' command in linkgit:git-stash[1].
>
> stash.showPatch::
> diff --git a/builtin/stash.c b/builtin/stash.c
> index 7fb355bff0..c591a2bf4b 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -836,12 +836,14 @@ static int list_stash(int argc, const char **argv, const char *prefix)
> return run_command(&cp);
> }
>
> static int show_stat = 1;
> static int show_patch;
> static int show_include_untracked;
> +static int default_keep_index;
> +static int default_include_untracked;
>
> static int git_stash_config(const char *var, const char *value,
> const struct config_context *ctx, void *cb)
> {
> if (!strcmp(var, "stash.showstat")) {
> show_stat = git_config_bool(var, value);
> @@ -852,12 +854,20 @@ static int git_stash_config(const char *var, const char *value,
> return 0;
> }
> if (!strcmp(var, "stash.showincludeuntracked")) {
> show_include_untracked = git_config_bool(var, value);
> return 0;
> }
> + if (!strcmp(var, "stash.keepindex")) {
> + default_keep_index = git_config_bool(var, value);
> + return 0;
> + }
> + if (!strcmp(var, "stash.includeuntracked")) {
> + default_include_untracked = git_config_bool(var, value);
> + return 0;
> + }
> return git_diff_basic_config(var, value, ctx, cb);
> }
>
> static void diff_include_untracked(const struct stash_info *info, struct diff_options *diff_opt)
> {
> const struct object_id *oid[] = { &info->w_commit, &info->u_tree };
> @@ -1509,33 +1519,50 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
> int ret = 0;
> struct stash_info info = STASH_INFO_INIT;
> struct strbuf patch = STRBUF_INIT;
> struct strbuf stash_msg_buf = STRBUF_INIT;
> struct strbuf untracked_files = STRBUF_INIT;
>
> - if (patch_mode && keep_index == -1)
> - keep_index = 1;
> -
> - if (patch_mode && include_untracked) {
> - fprintf_ln(stderr, _("Can't use --patch and --include-untracked"
> - " or --all at the same time"));
> - ret = -1;
> - goto done;
> + if (keep_index == -1) {
> + if (patch_mode)
> + keep_index = 1;
> + else
> + keep_index = default_keep_index;
> }
>
> - /* --patch overrides --staged */
> - if (patch_mode)
> + if (patch_mode) {
> + if (include_untracked == -1)
> + include_untracked = 0;
> + else if (include_untracked) {
> + fprintf_ln(stderr,
> + _("Can't use --patch and --include-untracked"
> + " or --all at the same time"));
> + ret = -1;
> + goto done;
> + }
> +
> + /* --patch overrides --staged */
> only_staged = 0;
> -
> - if (only_staged && include_untracked) {
> - fprintf_ln(stderr, _("Can't use --staged and --include-untracked"
> - " or --all at the same time"));
> - ret = -1;
> - goto done;
> }
>
> + if (only_staged) {
> + if (include_untracked == -1)
> + include_untracked = 0;
> + else if (include_untracked) {
> + fprintf_ln(
> + stderr,
> + _("Can't use --staged and --include-untracked"
> + " or --all at the same time"));
> + ret = -1;
> + goto done;
> + }
> + }
> +
> + if (include_untracked == -1)
> + include_untracked = default_include_untracked;
> +
I'm not sure this would be better, but instead of mixing option
compatibility and actually building your logic, why not use a series of
die_for_incompatible_opt3 and the like in order to clear the option
lists, then build your action logic without resorting to special values?
> repo_read_index_preload(the_repository, NULL, 0);
> if (!include_untracked && ps->nr) {
> int i;
> char *ps_matched = xcalloc(ps->nr, 1);
>
> /* TODO: audit for interaction with sparse-index. */
> @@ -1688,13 +1715,13 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
> fprintf_ln(stderr, _("Cannot remove "
> "worktree changes"));
> ret = -1;
> goto done;
> }
>
> - if (keep_index < 1) {
> + if (!keep_index) {
> struct child_process cp = CHILD_PROCESS_INIT;
>
> cp.git_cmd = 1;
> strvec_pushl(&cp.args, "reset", "-q", "--refresh", "--",
> NULL);
> add_pathspecs(&cp.args, ps);
> @@ -1718,13 +1745,13 @@ static int push_stash(int argc, const char **argv, const char *prefix,
> int push_assumed)
> {
> int force_assume = 0;
> int keep_index = -1;
> int only_staged = 0;
> int patch_mode = 0;
> - int include_untracked = 0;
> + int include_untracked = -1;
> int quiet = 0;
> int pathspec_file_nul = 0;
> const char *stash_msg = NULL;
> const char *pathspec_from_file = NULL;
> struct pathspec ps;
> struct option options[] = {
> @@ -1798,13 +1825,13 @@ static int push_stash_unassumed(int argc, const char **argv, const char *prefix)
>
> static int save_stash(int argc, const char **argv, const char *prefix)
> {
> int keep_index = -1;
> int only_staged = 0;
> int patch_mode = 0;
> - int include_untracked = 0;
> + int include_untracked = -1;
> int quiet = 0;
> int ret = 0;
> const char *stash_msg = NULL;
> struct pathspec ps;
> struct strbuf stash_msg_buf = STRBUF_INIT;
> struct option options[] = {
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index 00db82fb24..4ffcca742c 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -1565,7 +1565,126 @@ test_expect_success 'stash apply reports a locked index' '
> EOF
> test_must_fail git stash apply 2>err &&
> test_cmp expect err
> )
> '
>
> +setup_dirty() {
> + echo a >tracked &&
> + echo b >added-modified &&
> + git add tracked added-modified &&
> + git commit -m initial &&
> + echo 1 >>tracked &&
> + echo 2 >>added-modified &&
> + echo c >added-new &&
> + echo d >untracked &&
> + git add added-modified added-new
> +}
> +
> +test_expect_success 'stash.includeuntracked equivalent to --include-untracked' '
> + git init using_opt &&
> + test_when_finished rm -rf using_opt &&
> + (
> + cd using_opt &&
> + setup_dirty &&
> + git stash push &&
> + git stash show -u --patch >../using-opt
> + ) &&
> +
> + test_config stash.includeuntracked true &&
> + git init using_config &&
> + test_when_finished rm -rf using_config &&
> + (
> + cd using_config &&
> + setup_dirty &&
> + git stash push &&
> + git stash show -u --patch >../using-config
> + ) &&
> +
> + test_cmp using-opt using-config
> +'
> +
> +test_expect_success 'stash.includeuntracked yields to --no-include-untracked' '
> + git init no_config &&
> + test_when_finished rm -rf no_config &&
> + (
> + cd no_config &&
> + setup_dirty &&
> + git stash push --no-include-untracked &&
> + git stash show -u --patch >../no-config
> + ) &&
> +
> + test_config stash.includeuntracked true &&
> + git init using_config &&
> + test_when_finished rm -rf using_config &&
> + (
> + cd using_config &&
> + setup_dirty &&
> + git stash push --no-include-untracked &&
> + git stash show -u --patch >../using-config
> + ) &&
> +
> + test_cmp no-config using-config
> +'
> +
> +test_expect_success 'stash.includeuntracked succeeds with --patch' '
> + test_config stash.includeuntracked true &&
> + git stash --patch
> +'
> +
> +test_expect_success 'stash.includeuntracked succeeds with --staged' '
> + test_config stash.includeuntracked true &&
> + git stash --staged
> +'
> +
> +test_expect_success 'stash.keepindex equivalent to --keep-index' '
> + git init using_opt &&
> + test_when_finished rm -rf using_opt &&
> + (
> + cd using_opt &&
> + setup_dirty &&
> + git stash push &&
> + git stash show -u --patch >../using-opt
> + ) &&
> +
> + test_config stash.keepindex true &&
> + git init using_config &&
> + test_when_finished rm -rf using_config &&
> + (
> + cd using_config &&
> + setup_dirty &&
> + git stash push &&
> + git stash show -u --patch >../using-config
> + ) &&
> +
> + test_cmp using-opt using-config
> +'
> +
> +test_expect_success 'stash.keepindex yields to --no-keep-index' '
> + git init no_config &&
> + test_when_finished rm -rf no_config &&
> + (
> + cd no_config &&
> + setup_dirty &&
> + git stash push --no-keep-index &&
> + git stash show -u --patch >../no-config
> + ) &&
> +
> + test_config stash.keepindex true &&
> + git init using_config &&
> + test_when_finished rm -rf using_config &&
> + (
> + cd using_config &&
> + setup_dirty &&
> + git stash push --no-keep-index &&
> + git stash show -u --patch >../using-config
> + ) &&
> +
> + test_cmp no-config using-config
> +'
> +
> +test_expect_success 'stash.keepindex succeeds with --patch' '
> + test_config stash.keepindex true &&
> + git stash --patch
> +'
> +
> test_done
> --
> 2.43.2
>
^ permalink raw reply
* Re: [PATCH 0/5] promise: introduce promises to track success or error
From: Phillip Wood @ 2024-02-19 14:25 UTC (permalink / raw)
To: Philip Peterson via GitGitGadget, git
Cc: Johannes Schindelin, Emily Shaffer, Philip Peterson
In-Reply-To: <pull.1666.git.git.1708241612.gitgitgadget@gmail.com>
Hi Philip
On 18/02/2024 07:33, Philip Peterson via GitGitGadget wrote:
> Hello all, this is my first patchset so thank you for being patient with me
> as I learn the process and conventions of your very fine project. These
> patches are intended as part of the Libification effort, to show how we
> could use a Promise structure to help return values from functions.
I agree that we could do with a better way of propagating errors ups the
call-chain that (a) allows us to pass more detailed information about
the error to the caller and (b) add useful context to the error message
as the stack in unwound. I'm afraid I do not think that the promise
implementation in this patch series is a good way forward for several
reasons.
1) It is hard to see how we can wrap the return value of the function in
a promise and preserve type safety. Even if we used some kind of union
the compiler will not warn us if the caller reads a different member to
the one that the callee set.
2) It obscures the return type of the function and forces callers to
unpack the value from the promise adding complexity to the calling code.
3) It imposes a cost in terms of dynamic memory allocation on code that
is called synchronously and therefore does not need to allocate the
promise on the heap. This adds complexity and is sure to result in
memory leaks.
4) If the function fails we need to propagate the error using
PROMISE_BUBBLE_UP() which forces the caller to add more context to the
error message even if it does covey anything useful to the user. For
example in patch 5 we see
error:
could not find header
caused by:
could not find file diff header
caused by:
git diff header lacks filename information (line 4)" >expected
The error message starts by saying it couldn't find the header and ends
by saying it did actually find the header but it could not parse it.
5) The cover letter talks about adding asynchronous examples in the
future but it is not clear what part of the git code base it is
referring to.
I think we'd be better served by some kind of structured error type like
the failure_result in this patch series that is allocated on the stack
by the caller at the entry point to the library and passed down the call
chain. That avoids the need for lots of dynamic allocations and allows
us to continue allocating "out" parameters on the stack. For example
int f(struct repository *r) {
struct object_id oid;
if (repo_get_oid(r, "HEAD", &oid))
return error(_("could not parse HEAD"))
/* use oid here */
}
would become
int f(struct repository *r, struct error *err) {
struct object_id oid;
if (repo_get_oid(r, "HEAD", &oid))
return error(&err, _("could not parse HEAD"))
/* use oid here */
}
I'm sure this has been discussed in the past but I didn't manage to turn
anything up with a quick search of the archive on lore.kernel.org.
Best Wishes
Phillip
> Problems
> ========
>
> We seek to make libification easier by establishing a pattern for tracking
> whether a function errored in a rich way. Currently, any given function
> could immediately die(), or use error() to print directly to the console,
> bypassing any relevant verbosity checks. The use of die() currently makes
> use of Git as a library inconvenient since it is not graceful.
>
> Additionally, returning using return error(...) (as is commonly done) always
> just returns a generic error value, -1, which provides little information.
>
>
> Approach
> ========
>
> I solve this problem by splitting the single return value into two return
> values: error, and message. However, managing two output variables can
> require some coordination, and this coordination can be abstracted away by
> use of an existing pattern named Promise.
>
>
> Promise Concept
> ===============
>
> A promise is a contract representing "some task" that will eventually
> complete. Initially a promise is considered in a pending state. When it
> completes, one of two codepaths will eventually be entered: reject, or
> resolve. Once resolved or rejected, the promise enters a different state
> representing the result. Reject or resolve may only be called once on a
> given promise.
>
> Until now, everything I described up to this point is consistent with other
> implementations, such as the ECMAScript standard for promises. However, this
> implementation departs from the complexity of those promises. In this
> implementation, promises are simple and canNOT be chained using .then(...)
> and do NOT have any notion of automatic bubbling (via re-entering the
> pending state).
>
>
> Sample output and reproduction
> ==============================
>
> During an error, we can have richer feedback as to what caused the problem.
>
> % git apply garbage.patch
> error:
> could not find header
> caused by:
> patch fragment without header at line 1: @@ -2 +2 @@
>
>
> To reproduce this output, you can use the following patch (garbage.patch):
>
> @@ -2 +2 @@
>
>
>
> Goals
> =====
>
> I would love to get feedback on this approach. This patchset is kept small,
> so as to serve as a minimal proof of concept. It is intended to abstract to
> asynchronous use-cases even though this is only a synchronous one.
> Eventually, any top-level function, such as apply_all_patches(...) would
> return its output via a promise to make the library interface as clean as
> possible, but this patchset does not accomplish this goal. Hopefully it can
> provide a direction to go in to achieve that.
>
>
> Diversion
> =========
>
> While building this patchset, I noted a bug that may not have a concrete
> repro case in the master branch. The bug is that when invoking git am, it
> can call out to git apply, passing many flags but interestingly not the
> --quiet flag. I included a fix for this issue in the patchset.
>
>
> Potential Issue
> ===============
>
> There is one difficulty with this approach, which is the high level of
> repetition in the code required. Tracking which promise is which is its own
> source of complexity and may make mistakes more prone to happen. If anyone
> has suggestions for how to make the code cleaner, I would love to hear.
>
> Thank you, Philip
>
> Philip Peterson (5):
> promise: add promise pattern to track success/error from operations
> apply: use new promise structures in git-apply logic as a proving
> ground
> apply: update t4012 test suite
> apply: pass through quiet flag to fix t4150
> am: update test t4254 by adding the new error text
>
> Makefile | 1 +
> apply.c | 133 +++++++++++++++++++++++++++--------------
> apply.h | 9 ++-
> builtin/am.c | 5 ++
> promise.c | 89 +++++++++++++++++++++++++++
> promise.h | 71 ++++++++++++++++++++++
> range-diff.c | 14 +++--
> t/t4012-diff-binary.sh | 4 +-
> t/t4254-am-corrupt.sh | 9 ++-
> 9 files changed, 279 insertions(+), 56 deletions(-)
> create mode 100644 promise.c
> create mode 100644 promise.h
>
>
> base-commit: 2996f11c1d11ab68823f0939b6469dedc2b9ab90
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1666%2Fphilip-peterson%2Fpeterson%2Femail-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1666/philip-peterson/peterson/email-v1
> Pull-Request: https://github.com/git/git/pull/1666
^ permalink raw reply
* [PATCH 0/6] reflog: introduce subcommand to list reflogs
From: Patrick Steinhardt @ 2024-02-19 14:35 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 3051 bytes --]
Hi,
this patch series introduces a new `git reflog list` subcommand that
lists all reflogs of the current repository. This addresses an issue
with discoverability as there is no way for a user to learn about which
reflogs exist. While this isn't all that bad with the "files" backend as
a user could in the worst case figure out which reflogs exist by walking
the ".git/logs" directory, with the "reftable" backend it's basically
impossible to achieve this.
While I think this is sufficient motivation to have such a subcommand
nowadays already, I think the need for such a thing will grow in the
future. It was noted in multiple threads that we may eventually want to
lift the artificial limitations in the "reftable" backend where reflogs
are deleted together with their refs. This limitation is inherited from
the "files" backend, which may otherwise hit issues with directory/file
conflicts if it didn't delete reflogs.
Once that limitation is lifted for the "reftable" backend though, it
will become even more important to give users the tools to discover
reflogs that do not have a corresponding ref.
The series is structured as follows:
- Patches 1-3 extend the dir iterator so that it can sort directory
entries lexicographically. This is required such that we can list
reflogs with deterministic ordering.
- Patch 4 refactors the reflog iterator interface to demonstrate that
the object ID and flags aren't needed nowadays, and patch 5 builds
on top of that and stops resolving the refs altogether. This allows
us to also surface reflogs of broken refs.
- Patch 6 introduces the new subcommand.
The series depends on Junio's ps/reftable-backend at 8a0bebdeae
(refs/reftable: fix leak when copying reflog fails, 2024-02-08): the
change in behaviour in patches 4 and 5 apply to both backends.
Patrick
Patrick Steinhardt (6):
dir-iterator: pass name to `prepare_next_entry_data()` directly
dir-iterator: support iteration in sorted order
refs/files: sort reflogs returned by the reflog iterator
refs: drop unused params from the reflog iterator callback
refs: stop resolving ref corresponding to reflogs
builtin/reflog: introduce subcommand to list reflogs
Documentation/git-reflog.txt | 3 ++
builtin/fsck.c | 4 +-
builtin/reflog.c | 37 +++++++++++++-
dir-iterator.c | 93 ++++++++++++++++++++++++++--------
dir-iterator.h | 3 ++
refs.c | 23 +++++++--
refs.h | 11 +++-
refs/files-backend.c | 22 ++------
refs/reftable-backend.c | 12 +----
revision.c | 4 +-
t/helper/test-ref-store.c | 18 ++++---
t/t0600-reffiles-backend.sh | 24 ++++-----
t/t1405-main-ref-store.sh | 8 +--
t/t1406-submodule-ref-store.sh | 8 +--
t/t1410-reflog.sh | 69 +++++++++++++++++++++++++
15 files changed, 251 insertions(+), 88 deletions(-)
--
2.44.0-rc1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH 1/6] dir-iterator: pass name to `prepare_next_entry_data()` directly
From: Patrick Steinhardt @ 2024-02-19 14:35 UTC (permalink / raw)
To: git
In-Reply-To: <cover.1708353264.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1694 bytes --]
When adding the next directory entry for `struct dir_iterator` we pass
the complete `struct dirent *` to `prepare_next_entry_data()` even
though we only need the entry's name.
Refactor the code to pass in the name, only. This prepares for a
subsequent commit where we introduce the ability to iterate through
dir entries in an ordered manner.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
dir-iterator.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/dir-iterator.c b/dir-iterator.c
index 278b04243a..f58a97e089 100644
--- a/dir-iterator.c
+++ b/dir-iterator.c
@@ -94,15 +94,15 @@ static int pop_level(struct dir_iterator_int *iter)
/*
* Populate iter->base with the necessary information on the next iteration
- * entry, represented by the given dirent de. Return 0 on success and -1
+ * entry, represented by the given name. Return 0 on success and -1
* otherwise, setting errno accordingly.
*/
static int prepare_next_entry_data(struct dir_iterator_int *iter,
- struct dirent *de)
+ const char *name)
{
int err, saved_errno;
- strbuf_addstr(&iter->base.path, de->d_name);
+ strbuf_addstr(&iter->base.path, name);
/*
* We have to reset these because the path strbuf might have
* been realloc()ed at the previous strbuf_addstr().
@@ -159,7 +159,7 @@ int dir_iterator_advance(struct dir_iterator *dir_iterator)
if (is_dot_or_dotdot(de->d_name))
continue;
- if (prepare_next_entry_data(iter, de)) {
+ if (prepare_next_entry_data(iter, de->d_name)) {
if (errno != ENOENT && iter->flags & DIR_ITERATOR_PEDANTIC)
goto error_out;
continue;
--
2.44.0-rc1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 2/6] dir-iterator: support iteration in sorted order
From: Patrick Steinhardt @ 2024-02-19 14:35 UTC (permalink / raw)
To: git
In-Reply-To: <cover.1708353264.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 5586 bytes --]
The `struct dir_iterator` is a helper that allows us to iterate through
directory entries. This iterator returns entries in the exact same order
as readdir(3P) does -- or in other words, it guarantees no specific
order at all.
This is about to become problematic as we are introducing a new reflog
subcommand to list reflogs. As the "files" backend uses the directory
iterator to enumerate reflogs, returning reflog names and exposing them
to the user would inherit the indeterministic ordering. Naturally, it
would make for a terrible user interface to show a list with no
discernible order. While this could be handled at a higher level by the
new subcommand itself by collecting and ordering the reflogs, this would
be inefficient and introduce latency when there are many reflogs.
Instead, introduce a new option into the directory iterator that asks
for its entries to be yielded in lexicographical order. If set, the
iterator will read all directory entries greedily end sort them before
we start to iterate over them.
While this will of course also incur overhead as we cannot yield the
directory entries immediately, it should at least be more efficient than
having to sort the complete list of reflogs as we only need to sort one
directory at a time.
This functionality will be used in a follow-up commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
dir-iterator.c | 87 ++++++++++++++++++++++++++++++++++++++++----------
dir-iterator.h | 3 ++
2 files changed, 73 insertions(+), 17 deletions(-)
diff --git a/dir-iterator.c b/dir-iterator.c
index f58a97e089..396c28178f 100644
--- a/dir-iterator.c
+++ b/dir-iterator.c
@@ -2,9 +2,12 @@
#include "dir.h"
#include "iterator.h"
#include "dir-iterator.h"
+#include "string-list.h"
struct dir_iterator_level {
DIR *dir;
+ struct string_list entries;
+ size_t entries_idx;
/*
* The length of the directory part of path at this level
@@ -72,6 +75,40 @@ static int push_level(struct dir_iterator_int *iter)
return -1;
}
+ string_list_init_dup(&level->entries);
+ level->entries_idx = 0;
+
+ /*
+ * When the iterator is sorted we read and sort all directory entries
+ * directly.
+ */
+ if (iter->flags & DIR_ITERATOR_SORTED) {
+ while (1) {
+ struct dirent *de;
+
+ errno = 0;
+ de = readdir(level->dir);
+ if (!de) {
+ if (errno && errno != ENOENT) {
+ warning_errno("error reading directory '%s'",
+ iter->base.path.buf);
+ return -1;
+ }
+
+ break;
+ }
+
+ if (is_dot_or_dotdot(de->d_name))
+ continue;
+
+ string_list_append(&level->entries, de->d_name);
+ }
+ string_list_sort(&level->entries);
+
+ closedir(level->dir);
+ level->dir = NULL;
+ }
+
return 0;
}
@@ -88,6 +125,7 @@ static int pop_level(struct dir_iterator_int *iter)
warning_errno("error closing directory '%s'",
iter->base.path.buf);
level->dir = NULL;
+ string_list_clear(&level->entries, 0);
return --iter->levels_nr;
}
@@ -136,30 +174,43 @@ int dir_iterator_advance(struct dir_iterator *dir_iterator)
/* Loop until we find an entry that we can give back to the caller. */
while (1) {
- struct dirent *de;
struct dir_iterator_level *level =
&iter->levels[iter->levels_nr - 1];
+ struct dirent *de;
+ const char *name;
strbuf_setlen(&iter->base.path, level->prefix_len);
- errno = 0;
- de = readdir(level->dir);
-
- if (!de) {
- if (errno) {
- warning_errno("error reading directory '%s'",
- iter->base.path.buf);
- if (iter->flags & DIR_ITERATOR_PEDANTIC)
- goto error_out;
- } else if (pop_level(iter) == 0) {
- return dir_iterator_abort(dir_iterator);
+
+ if (level->dir) {
+ errno = 0;
+ de = readdir(level->dir);
+ if (!de) {
+ if (errno) {
+ warning_errno("error reading directory '%s'",
+ iter->base.path.buf);
+ if (iter->flags & DIR_ITERATOR_PEDANTIC)
+ goto error_out;
+ } else if (pop_level(iter) == 0) {
+ return dir_iterator_abort(dir_iterator);
+ }
+ continue;
}
- continue;
- }
- if (is_dot_or_dotdot(de->d_name))
- continue;
+ if (is_dot_or_dotdot(de->d_name))
+ continue;
- if (prepare_next_entry_data(iter, de->d_name)) {
+ name = de->d_name;
+ } else {
+ if (level->entries_idx >= level->entries.nr) {
+ if (pop_level(iter) == 0)
+ return dir_iterator_abort(dir_iterator);
+ continue;
+ }
+
+ name = level->entries.items[level->entries_idx++].string;
+ }
+
+ if (prepare_next_entry_data(iter, name)) {
if (errno != ENOENT && iter->flags & DIR_ITERATOR_PEDANTIC)
goto error_out;
continue;
@@ -188,6 +239,8 @@ int dir_iterator_abort(struct dir_iterator *dir_iterator)
warning_errno("error closing directory '%s'",
iter->base.path.buf);
}
+
+ string_list_clear(&level->entries, 0);
}
free(iter->levels);
diff --git a/dir-iterator.h b/dir-iterator.h
index 479e1ec784..6d438809b6 100644
--- a/dir-iterator.h
+++ b/dir-iterator.h
@@ -54,8 +54,11 @@
* and ITER_ERROR is returned immediately. In both cases, a meaningful
* warning is emitted. Note: ENOENT errors are always ignored so that
* the API users may remove files during iteration.
+ *
+ * - DIR_ITERATOR_SORTED: sort directory entries alphabetically.
*/
#define DIR_ITERATOR_PEDANTIC (1 << 0)
+#define DIR_ITERATOR_SORTED (1 << 1)
struct dir_iterator {
/* The current path: */
--
2.44.0-rc1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 3/6] refs/files: sort reflogs returned by the reflog iterator
From: Patrick Steinhardt @ 2024-02-19 14:35 UTC (permalink / raw)
To: git
In-Reply-To: <cover.1708353264.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 3433 bytes --]
We use a directory iterator to return reflogs via the reflog iterator.
This iterator returns entries in the same order as readdir(3P) would and
will thus yield reflogs with no discernible order.
Set the new `DIR_ITERATOR_SORTED` flag that was introduced in the
preceding commit so that the order is deterministic. While the effect of
this can only been observed in a test tool, a subsequent commit will
start to expose this functionality to users via a new `git reflog list`
subcommand.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
refs/files-backend.c | 4 ++--
t/t0600-reffiles-backend.sh | 4 ++--
t/t1405-main-ref-store.sh | 2 +-
t/t1406-submodule-ref-store.sh | 2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 75dcc21ecb..2ffc63185f 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2193,7 +2193,7 @@ static struct ref_iterator *reflog_iterator_begin(struct ref_store *ref_store,
strbuf_addf(&sb, "%s/logs", gitdir);
- diter = dir_iterator_begin(sb.buf, 0);
+ diter = dir_iterator_begin(sb.buf, DIR_ITERATOR_SORTED);
if (!diter) {
strbuf_release(&sb);
return empty_ref_iterator_begin();
@@ -2202,7 +2202,7 @@ static struct ref_iterator *reflog_iterator_begin(struct ref_store *ref_store,
CALLOC_ARRAY(iter, 1);
ref_iterator = &iter->base;
- base_ref_iterator_init(ref_iterator, &files_reflog_iterator_vtable, 0);
+ base_ref_iterator_init(ref_iterator, &files_reflog_iterator_vtable, 1);
iter->dir_iterator = diter;
iter->ref_store = ref_store;
strbuf_release(&sb);
diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index e6a5f1868f..4f860285cc 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -287,7 +287,7 @@ test_expect_success 'for_each_reflog()' '
mkdir -p .git/worktrees/wt/logs/refs/bisect &&
echo $ZERO_OID > .git/worktrees/wt/logs/refs/bisect/wt-random &&
- $RWT for-each-reflog | cut -d" " -f 2- | sort >actual &&
+ $RWT for-each-reflog | cut -d" " -f 2- >actual &&
cat >expected <<-\EOF &&
HEAD 0x1
PSEUDO-WT 0x0
@@ -297,7 +297,7 @@ test_expect_success 'for_each_reflog()' '
EOF
test_cmp expected actual &&
- $RMAIN for-each-reflog | cut -d" " -f 2- | sort >actual &&
+ $RMAIN for-each-reflog | cut -d" " -f 2- >actual &&
cat >expected <<-\EOF &&
HEAD 0x1
PSEUDO-MAIN 0x0
diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index 976bd71efb..cfb583f544 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -74,7 +74,7 @@ test_expect_success 'verify_ref(new-main)' '
'
test_expect_success 'for_each_reflog()' '
- $RUN for-each-reflog | sort -k2 | cut -d" " -f 2- >actual &&
+ $RUN for-each-reflog | cut -d" " -f 2- >actual &&
cat >expected <<-\EOF &&
HEAD 0x1
refs/heads/main 0x0
diff --git a/t/t1406-submodule-ref-store.sh b/t/t1406-submodule-ref-store.sh
index e6a7f7334b..40332e23cc 100755
--- a/t/t1406-submodule-ref-store.sh
+++ b/t/t1406-submodule-ref-store.sh
@@ -63,7 +63,7 @@ test_expect_success 'verify_ref(new-main)' '
'
test_expect_success 'for_each_reflog()' '
- $RUN for-each-reflog | sort | cut -d" " -f 2- >actual &&
+ $RUN for-each-reflog | cut -d" " -f 2- >actual &&
cat >expected <<-\EOF &&
HEAD 0x1
refs/heads/main 0x0
--
2.44.0-rc1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 4/6] refs: drop unused params from the reflog iterator callback
From: Patrick Steinhardt @ 2024-02-19 14:35 UTC (permalink / raw)
To: git
In-Reply-To: <cover.1708353264.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 12618 bytes --]
The ref and reflog iterators share much of the same underlying code to
iterate over the corresponding entries. This results in some weird code
because the reflog iterator also exposes an object ID as well as a flag
to the callback function. Neither of these fields do refer to the reflog
though -- they refer to the corresponding ref with the same name. This
is quite misleading. In practice at least the object ID cannot really be
implemented in any other way as a reflog does not have a specific object
ID in the first place. This is further stressed by the fact that none of
the callbacks except for our test helper make use of these fields.
Split up the infrastucture so that ref and reflog iterators use separate
callback signatures. This allows us to drop the nonsensical fields from
the reflog iterator.
Note that internally, the backends still use the same shared infra to
iterate over both types. As the backends should never end up being
called directly anyway, this is not much of a problem and thus kept
as-is for simplicity's sake.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/fsck.c | 4 +---
builtin/reflog.c | 3 +--
refs.c | 23 +++++++++++++++++++----
refs.h | 11 +++++++++--
refs/files-backend.c | 8 +-------
refs/reftable-backend.c | 8 +-------
revision.c | 4 +---
t/helper/test-ref-store.c | 18 ++++++++++++------
t/t0600-reffiles-backend.sh | 24 ++++++++++++------------
t/t1405-main-ref-store.sh | 8 ++++----
t/t1406-submodule-ref-store.sh | 8 ++++----
11 files changed, 65 insertions(+), 54 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index a7cf94f67e..f892487c9b 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -509,9 +509,7 @@ static int fsck_handle_reflog_ent(struct object_id *ooid, struct object_id *noid
return 0;
}
-static int fsck_handle_reflog(const char *logname,
- const struct object_id *oid UNUSED,
- int flag UNUSED, void *cb_data)
+static int fsck_handle_reflog(const char *logname, void *cb_data)
{
struct strbuf refname = STRBUF_INIT;
diff --git a/builtin/reflog.c b/builtin/reflog.c
index a5a4099f61..3a0c4d4322 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -60,8 +60,7 @@ struct worktree_reflogs {
struct string_list reflogs;
};
-static int collect_reflog(const char *ref, const struct object_id *oid UNUSED,
- int flags UNUSED, void *cb_data)
+static int collect_reflog(const char *ref, void *cb_data)
{
struct worktree_reflogs *cb = cb_data;
struct worktree *worktree = cb->worktree;
diff --git a/refs.c b/refs.c
index fff343c256..6d76000f13 100644
--- a/refs.c
+++ b/refs.c
@@ -2516,18 +2516,33 @@ int refs_verify_refname_available(struct ref_store *refs,
return ret;
}
-int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data)
+struct do_for_each_reflog_help {
+ each_reflog_fn *fn;
+ void *cb_data;
+};
+
+static int do_for_each_reflog_helper(struct repository *r UNUSED,
+ const char *refname,
+ const struct object_id *oid UNUSED,
+ int flags,
+ void *cb_data)
+{
+ struct do_for_each_reflog_help *hp = cb_data;
+ return hp->fn(refname, hp->cb_data);
+}
+
+int refs_for_each_reflog(struct ref_store *refs, each_reflog_fn fn, void *cb_data)
{
struct ref_iterator *iter;
- struct do_for_each_ref_help hp = { fn, cb_data };
+ struct do_for_each_reflog_help hp = { fn, cb_data };
iter = refs->be->reflog_iterator_begin(refs);
return do_for_each_repo_ref_iterator(the_repository, iter,
- do_for_each_ref_helper, &hp);
+ do_for_each_reflog_helper, &hp);
}
-int for_each_reflog(each_ref_fn fn, void *cb_data)
+int for_each_reflog(each_reflog_fn fn, void *cb_data)
{
return refs_for_each_reflog(get_main_ref_store(the_repository), fn, cb_data);
}
diff --git a/refs.h b/refs.h
index 303c5fac4d..895579aeb7 100644
--- a/refs.h
+++ b/refs.h
@@ -534,12 +534,19 @@ int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn, void *cb_dat
/* youngest entry first */
int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn, void *cb_data);
+/*
+ * The signature for the callback function for the {refs_,}for_each_reflog()
+ * functions below. The memory pointed to by the refname argument is only
+ * guaranteed to be valid for the duration of a single callback invocation.
+ */
+typedef int each_reflog_fn(const char *refname, void *cb_data);
+
/*
* Calls the specified function for each reflog file until it returns nonzero,
* and returns the value. Reflog file order is unspecified.
*/
-int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data);
-int for_each_reflog(each_ref_fn fn, void *cb_data);
+int refs_for_each_reflog(struct ref_store *refs, each_reflog_fn fn, void *cb_data);
+int for_each_reflog(each_reflog_fn fn, void *cb_data);
#define REFNAME_ALLOW_ONELEVEL 1
#define REFNAME_REFSPEC_PATTERN 2
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 2ffc63185f..2b3c99b00d 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2116,10 +2116,8 @@ static int files_for_each_reflog_ent(struct ref_store *ref_store,
struct files_reflog_iterator {
struct ref_iterator base;
-
struct ref_store *ref_store;
struct dir_iterator *dir_iterator;
- struct object_id oid;
};
static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)
@@ -2130,8 +2128,6 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)
int ok;
while ((ok = dir_iterator_advance(diter)) == ITER_OK) {
- int flags;
-
if (!S_ISREG(diter->st.st_mode))
continue;
if (diter->basename[0] == '.')
@@ -2141,14 +2137,12 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)
if (!refs_resolve_ref_unsafe(iter->ref_store,
diter->relative_path, 0,
- &iter->oid, &flags)) {
+ NULL, NULL)) {
error("bad ref for %s", diter->path.buf);
continue;
}
iter->base.refname = diter->relative_path;
- iter->base.oid = &iter->oid;
- iter->base.flags = flags;
return ITER_OK;
}
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index a14f2ad7f4..889bb1f1ba 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -1637,7 +1637,6 @@ struct reftable_reflog_iterator {
struct reftable_ref_store *refs;
struct reftable_iterator iter;
struct reftable_log_record log;
- struct object_id oid;
char *last_name;
int err;
};
@@ -1648,8 +1647,6 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator)
(struct reftable_reflog_iterator *)ref_iterator;
while (!iter->err) {
- int flags;
-
iter->err = reftable_iterator_next_log(&iter->iter, &iter->log);
if (iter->err)
break;
@@ -1663,7 +1660,7 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator)
continue;
if (!refs_resolve_ref_unsafe(&iter->refs->base, iter->log.refname,
- 0, &iter->oid, &flags)) {
+ 0, NULL, NULL)) {
error(_("bad ref for %s"), iter->log.refname);
continue;
}
@@ -1671,8 +1668,6 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator)
free(iter->last_name);
iter->last_name = xstrdup(iter->log.refname);
iter->base.refname = iter->log.refname;
- iter->base.oid = &iter->oid;
- iter->base.flags = flags;
break;
}
@@ -1725,7 +1720,6 @@ static struct reftable_reflog_iterator *reflog_iterator_for_stack(struct reftabl
iter = xcalloc(1, sizeof(*iter));
base_ref_iterator_init(&iter->base, &reftable_reflog_iterator_vtable, 1);
iter->refs = refs;
- iter->base.oid = &iter->oid;
ret = refs->err;
if (ret)
diff --git a/revision.c b/revision.c
index 2424c9bd67..ac45c6d8f2 100644
--- a/revision.c
+++ b/revision.c
@@ -1686,9 +1686,7 @@ static int handle_one_reflog_ent(struct object_id *ooid, struct object_id *noid,
return 0;
}
-static int handle_one_reflog(const char *refname_in_wt,
- const struct object_id *oid UNUSED,
- int flag UNUSED, void *cb_data)
+static int handle_one_reflog(const char *refname_in_wt, void *cb_data)
{
struct all_refs_cb *cb = cb_data;
struct strbuf refname = STRBUF_INIT;
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 702ec1f128..7a0f6cac53 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -221,15 +221,21 @@ static int cmd_verify_ref(struct ref_store *refs, const char **argv)
return ret;
}
+static int each_reflog(const char *refname, void *cb_data UNUSED)
+{
+ printf("%s\n", refname);
+ return 0;
+}
+
static int cmd_for_each_reflog(struct ref_store *refs,
const char **argv UNUSED)
{
- return refs_for_each_reflog(refs, each_ref, NULL);
+ return refs_for_each_reflog(refs, each_reflog, NULL);
}
-static int each_reflog(struct object_id *old_oid, struct object_id *new_oid,
- const char *committer, timestamp_t timestamp,
- int tz, const char *msg, void *cb_data UNUSED)
+static int each_reflog_ent(struct object_id *old_oid, struct object_id *new_oid,
+ const char *committer, timestamp_t timestamp,
+ int tz, const char *msg, void *cb_data UNUSED)
{
printf("%s %s %s %" PRItime " %+05d%s%s", oid_to_hex(old_oid),
oid_to_hex(new_oid), committer, timestamp, tz,
@@ -241,14 +247,14 @@ static int cmd_for_each_reflog_ent(struct ref_store *refs, const char **argv)
{
const char *refname = notnull(*argv++, "refname");
- return refs_for_each_reflog_ent(refs, refname, each_reflog, refs);
+ return refs_for_each_reflog_ent(refs, refname, each_reflog_ent, refs);
}
static int cmd_for_each_reflog_ent_reverse(struct ref_store *refs, const char **argv)
{
const char *refname = notnull(*argv++, "refname");
- return refs_for_each_reflog_ent_reverse(refs, refname, each_reflog, refs);
+ return refs_for_each_reflog_ent_reverse(refs, refname, each_reflog_ent, refs);
}
static int cmd_reflog_exists(struct ref_store *refs, const char **argv)
diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 4f860285cc..56a3196b83 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -287,23 +287,23 @@ test_expect_success 'for_each_reflog()' '
mkdir -p .git/worktrees/wt/logs/refs/bisect &&
echo $ZERO_OID > .git/worktrees/wt/logs/refs/bisect/wt-random &&
- $RWT for-each-reflog | cut -d" " -f 2- >actual &&
+ $RWT for-each-reflog >actual &&
cat >expected <<-\EOF &&
- HEAD 0x1
- PSEUDO-WT 0x0
- refs/bisect/wt-random 0x0
- refs/heads/main 0x0
- refs/heads/wt-main 0x0
+ HEAD
+ PSEUDO-WT
+ refs/bisect/wt-random
+ refs/heads/main
+ refs/heads/wt-main
EOF
test_cmp expected actual &&
- $RMAIN for-each-reflog | cut -d" " -f 2- >actual &&
+ $RMAIN for-each-reflog >actual &&
cat >expected <<-\EOF &&
- HEAD 0x1
- PSEUDO-MAIN 0x0
- refs/bisect/random 0x0
- refs/heads/main 0x0
- refs/heads/wt-main 0x0
+ HEAD
+ PSEUDO-MAIN
+ refs/bisect/random
+ refs/heads/main
+ refs/heads/wt-main
EOF
test_cmp expected actual
'
diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index cfb583f544..3eee758bce 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -74,11 +74,11 @@ test_expect_success 'verify_ref(new-main)' '
'
test_expect_success 'for_each_reflog()' '
- $RUN for-each-reflog | cut -d" " -f 2- >actual &&
+ $RUN for-each-reflog >actual &&
cat >expected <<-\EOF &&
- HEAD 0x1
- refs/heads/main 0x0
- refs/heads/new-main 0x0
+ HEAD
+ refs/heads/main
+ refs/heads/new-main
EOF
test_cmp expected actual
'
diff --git a/t/t1406-submodule-ref-store.sh b/t/t1406-submodule-ref-store.sh
index 40332e23cc..c01f0f14a1 100755
--- a/t/t1406-submodule-ref-store.sh
+++ b/t/t1406-submodule-ref-store.sh
@@ -63,11 +63,11 @@ test_expect_success 'verify_ref(new-main)' '
'
test_expect_success 'for_each_reflog()' '
- $RUN for-each-reflog | cut -d" " -f 2- >actual &&
+ $RUN for-each-reflog >actual &&
cat >expected <<-\EOF &&
- HEAD 0x1
- refs/heads/main 0x0
- refs/heads/new-main 0x0
+ HEAD
+ refs/heads/main
+ refs/heads/new-main
EOF
test_cmp expected actual
'
--
2.44.0-rc1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 5/6] refs: stop resolving ref corresponding to reflogs
From: Patrick Steinhardt @ 2024-02-19 14:35 UTC (permalink / raw)
To: git
In-Reply-To: <cover.1708353264.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 3407 bytes --]
The reflog iterator tries to resolve the corresponding ref for every
reflog that it is about to yield. Historically, this was done due to
multiple reasons:
- It ensures that the refname is safe because we end up calling
`check_refname_format()`. Also, non-conformant refnames are skipped
altogether.
- The iterator used to yield the resolved object ID as well as its
flags to the callback. This info was never used though, and the
corresponding parameters were dropped in the preceding commit.
- When a ref is corrupt then the reflog is not emitted at all.
We're about to introduce a new `git reflog list` subcommand that will
print all reflogs that the refdb knows about. Skipping over reflogs
whose refs are corrupted would be quite counterproductive in this case
as the user would have no way to learn about reflogs which may still
exist in their repository to help and rescue such a corrupted ref. Thus,
the only remaining reason for why we'd want to resolve the ref is to
verify its refname.
Refactor the code to call `check_refname_format()` directly instead of
trying to resolve the ref. This is significantly more efficient given
that we don't have to hit the object database anymore to list reflogs.
And second, it ensures that we end up showing reflogs of broken refs,
which will help to make the reflog more useful.
Note that this really only impacts the case where the corresponding ref
is corrupt. Reflogs for nonexistent refs would have been returned to the
caller beforehand already as we did not pass `RESOLVE_REF_READING` to
the function, and thus `refs_resolve_ref_unsafe()` would have returned
successfully in that case.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
refs/files-backend.c | 12 ++----------
refs/reftable-backend.c | 6 ++----
2 files changed, 4 insertions(+), 14 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 2b3c99b00d..741148087d 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2130,17 +2130,9 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)
while ((ok = dir_iterator_advance(diter)) == ITER_OK) {
if (!S_ISREG(diter->st.st_mode))
continue;
- if (diter->basename[0] == '.')
+ if (check_refname_format(diter->basename,
+ REFNAME_ALLOW_ONELEVEL))
continue;
- if (ends_with(diter->basename, ".lock"))
- continue;
-
- if (!refs_resolve_ref_unsafe(iter->ref_store,
- diter->relative_path, 0,
- NULL, NULL)) {
- error("bad ref for %s", diter->path.buf);
- continue;
- }
iter->base.refname = diter->relative_path;
return ITER_OK;
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 889bb1f1ba..efbbf23c72 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -1659,11 +1659,9 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator)
if (iter->last_name && !strcmp(iter->log.refname, iter->last_name))
continue;
- if (!refs_resolve_ref_unsafe(&iter->refs->base, iter->log.refname,
- 0, NULL, NULL)) {
- error(_("bad ref for %s"), iter->log.refname);
+ if (check_refname_format(iter->log.refname,
+ REFNAME_ALLOW_ONELEVEL))
continue;
- }
free(iter->last_name);
iter->last_name = xstrdup(iter->log.refname);
--
2.44.0-rc1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 6/6] builtin/reflog: introduce subcommand to list reflogs
From: Patrick Steinhardt @ 2024-02-19 14:35 UTC (permalink / raw)
To: git
In-Reply-To: <cover.1708353264.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 6086 bytes --]
While the git-reflog(1) command has subcommands to show reflog entries
or check for reflog existence, it does not have any subcommands that
would allow the user to enumerate all existing reflogs. This makes it
quite hard to discover which reflogs a repository has. While this can
be worked around with the "files" backend by enumerating files in the
".git/logs" directory, users of the "reftable" backend don't enjoy such
a luxury.
Introduce a new subcommand `git reflog list` that lists all reflogs the
repository knows of to fill this gap.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git-reflog.txt | 3 ++
builtin/reflog.c | 34 ++++++++++++++++++
t/t1410-reflog.sh | 69 ++++++++++++++++++++++++++++++++++++
3 files changed, 106 insertions(+)
diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt
index ec64cbff4c..a929c52982 100644
--- a/Documentation/git-reflog.txt
+++ b/Documentation/git-reflog.txt
@@ -10,6 +10,7 @@ SYNOPSIS
--------
[verse]
'git reflog' [show] [<log-options>] [<ref>]
+'git reflog list'
'git reflog expire' [--expire=<time>] [--expire-unreachable=<time>]
[--rewrite] [--updateref] [--stale-fix]
[--dry-run | -n] [--verbose] [--all [--single-worktree] | <refs>...]
@@ -39,6 +40,8 @@ actions, and in addition the `HEAD` reflog records branch switching.
`git reflog show` is an alias for `git log -g --abbrev-commit
--pretty=oneline`; see linkgit:git-log[1] for more information.
+The "list" subcommand lists all refs which have a corresponding reflog.
+
The "expire" subcommand prunes older reflog entries. Entries older
than `expire` time, or entries older than `expire-unreachable` time
and not reachable from the current tip, are removed from the reflog.
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 3a0c4d4322..63cd4d8b29 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -7,11 +7,15 @@
#include "wildmatch.h"
#include "worktree.h"
#include "reflog.h"
+#include "refs.h"
#include "parse-options.h"
#define BUILTIN_REFLOG_SHOW_USAGE \
N_("git reflog [show] [<log-options>] [<ref>]")
+#define BUILTIN_REFLOG_LIST_USAGE \
+ N_("git reflog list")
+
#define BUILTIN_REFLOG_EXPIRE_USAGE \
N_("git reflog expire [--expire=<time>] [--expire-unreachable=<time>]\n" \
" [--rewrite] [--updateref] [--stale-fix]\n" \
@@ -29,6 +33,11 @@ static const char *const reflog_show_usage[] = {
NULL,
};
+static const char *const reflog_list_usage[] = {
+ BUILTIN_REFLOG_LIST_USAGE,
+ NULL,
+};
+
static const char *const reflog_expire_usage[] = {
BUILTIN_REFLOG_EXPIRE_USAGE,
NULL
@@ -46,6 +55,7 @@ static const char *const reflog_exists_usage[] = {
static const char *const reflog_usage[] = {
BUILTIN_REFLOG_SHOW_USAGE,
+ BUILTIN_REFLOG_LIST_USAGE,
BUILTIN_REFLOG_EXPIRE_USAGE,
BUILTIN_REFLOG_DELETE_USAGE,
BUILTIN_REFLOG_EXISTS_USAGE,
@@ -238,6 +248,29 @@ static int cmd_reflog_show(int argc, const char **argv, const char *prefix)
return cmd_log_reflog(argc, argv, prefix);
}
+static int show_reflog(const char *refname, void *cb_data UNUSED)
+{
+ printf("%s\n", refname);
+ return 0;
+}
+
+static int cmd_reflog_list(int argc, const char **argv, const char *prefix)
+{
+ struct option options[] = {
+ OPT_END()
+ };
+ struct ref_store *ref_store;
+
+ argc = parse_options(argc, argv, prefix, options, reflog_list_usage, 0);
+ if (argc)
+ return error(_("%s does not accept arguments: '%s'"),
+ "list", argv[0]);
+
+ ref_store = get_main_ref_store(the_repository);
+
+ return refs_for_each_reflog(ref_store, show_reflog, NULL);
+}
+
static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
{
struct cmd_reflog_expire_cb cmd = { 0 };
@@ -417,6 +450,7 @@ int cmd_reflog(int argc, const char **argv, const char *prefix)
parse_opt_subcommand_fn *fn = NULL;
struct option options[] = {
OPT_SUBCOMMAND("show", &fn, cmd_reflog_show),
+ OPT_SUBCOMMAND("list", &fn, cmd_reflog_list),
OPT_SUBCOMMAND("expire", &fn, cmd_reflog_expire),
OPT_SUBCOMMAND("delete", &fn, cmd_reflog_delete),
OPT_SUBCOMMAND("exists", &fn, cmd_reflog_exists),
diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index d2f5f42e67..6d8d5a253d 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -436,4 +436,73 @@ test_expect_success 'empty reflog' '
test_must_be_empty err
'
+test_expect_success 'list reflogs' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ git reflog list >actual &&
+ test_must_be_empty actual &&
+
+ test_commit A &&
+ cat >expect <<-EOF &&
+ HEAD
+ refs/heads/main
+ EOF
+ git reflog list >actual &&
+ test_cmp expect actual &&
+
+ git branch b &&
+ cat >expect <<-EOF &&
+ HEAD
+ refs/heads/b
+ refs/heads/main
+ EOF
+ git reflog list >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'reflog list returns error with additional args' '
+ cat >expect <<-EOF &&
+ error: list does not accept arguments: ${SQ}bogus${SQ}
+ EOF
+ test_must_fail git reflog list bogus 2>err &&
+ test_cmp expect err
+'
+
+test_expect_success 'reflog for symref with unborn target can be listed' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit A &&
+ git symbolic-ref HEAD refs/heads/unborn &&
+ cat >expect <<-EOF &&
+ HEAD
+ refs/heads/main
+ EOF
+ git reflog list >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'reflog with invalid object ID can be listed' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit A &&
+ test-tool ref-store main update-ref msg refs/heads/missing \
+ $(test_oid deadbeef) "$ZERO_OID" REF_SKIP_OID_VERIFICATION &&
+ cat >expect <<-EOF &&
+ HEAD
+ refs/heads/main
+ refs/heads/missing
+ EOF
+ git reflog list >actual &&
+ test_cmp expect actual
+ )
+'
+
test_done
--
2.44.0-rc1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ 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