* Re: [PATCH 1/2] http: only reject basic auth credentials once they have been tried
From: Junio C Hamano @ 2024-02-04 22:47 UTC (permalink / raw)
To: Quentin Bouget; +Cc: git
In-Reply-To: <20240204185427.39664-2-ypsah@devyard.org>
Quentin Bouget <ypsah@devyard.org> writes:
> else if (results->http_code == 401) {
> - if (http_auth.username && http_auth.password) {
> - credential_reject(&http_auth);
> - return HTTP_NOAUTH;
> - } else {
> + if ((http_auth_methods & CURLAUTH_GSSNEGOTIATE) == CURLAUTH_GSSNEGOTIATE) {
> http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
> if (results->auth_avail) {
> http_auth_methods &= results->auth_avail;
> http_auth_methods_restricted = 1;
> }
> return HTTP_REAUTH;
> }
> + if (http_auth.username && http_auth.password)
> + credential_reject(&http_auth);
> + return HTTP_NOAUTH;
A few comments and questions.
* GSSNEGOTIATE is a synonym for NEGOTIATE since cURL 7.38.0
(released in Sep 2014); currently the earliest version we claim
to support is 7.19.5 (released May 2009) without imap-send, and
we require 7.34.0 (released Dec 2013) with imap-send, so for now,
it is prudent that this patch uses GSSNEGOTIATE.
* Is it something that the client code of libcURL can rely on that
these CURLAUTH_FOO macros are bitmasks [*]? If so, wouldn't
if ((http_auth_methods & CURLAUTH_GSSNEGOTIATE))
be clear enough (and less risk of making typo)?
* When we see 401, the first thing we do in the new code is to see
if GSS is enabled in auth_methods, and if so we drop it from
auth_methods (to prevent us from trying it again) and say REAUTH.
- What assures us that the presense of GSS bit in auth_methods
mean we tried GSS to get this 401? Could it be that we tried
basic and seeing 401 from that, but we haven't tried GSS and we
could retry with GSS now? Is it commonly known that GSS is
always tried first before Basic/Digest when both are availble,
or something like that?
- When auth_avail was given by the cURL library, we further limit
the auth_methods (after dropping GSS) and say REAUTH. This is
not a new to the updated code, but can it happen that the
resulting restricted auth_methods bitmap becomes empty (i.e.
REAUTH would be useless)?
Thanks.
[References]
* https://github.com/curl/curl/blob/b8c003832d730bb2f4b9de4204675ca5d9f7a903/include/curl/curl.h#L787C4-L787C64
^ permalink raw reply
* Re: [PATCH 2/2] http: prevent redirect from dropping credentials during reauth
From: Junio C Hamano @ 2024-02-04 22:51 UTC (permalink / raw)
To: Quentin Bouget; +Cc: git
In-Reply-To: <20240204185427.39664-3-ypsah@devyard.org>
Quentin Bouget <ypsah@devyard.org> writes:
> During a re-authentication (second attempt at authenticating with a
> remote, e.g. after a failed GSSAPI attempt), git allows the remote to
> provide credential overrides in the redirect URL and unconditionnaly
> drops the current HTTP credentials in favors of those, even when there
> aren't any.
>
> This commit makes it so HTTP credentials are only overridden when the
> redirect URL actually contains credentials itself.
"This commit makes it so" -> "Make it so"
> + char *username = NULL, *password = NULL;
> +
> + if (http_auth.username)
> + username = xstrdup(http_auth.username);
> + if (http_auth.password)
> + password = xstrdup(http_auth.password);
Not a huge deal, but we have xstrdup_or_null() helper function
exactly for a use case like this.
> credential_from_url(&http_auth, options->base_url->buf);
> +
> + if (http_auth.username)
> + free(username);
> + else if (username)
> + http_auth.username = username;
> +
> + if (http_auth.password)
> + free(password);
> + else if (password)
> + http_auth.password = password;
This is an interesting change. I wonder what breaks if we
completely ignored such credential materials forced by the remote
via a redirect?
> url = options->effective_url->buf;
> }
> }
Thanks.
^ permalink raw reply
* RE: [PATCH 2/2] http: prevent redirect from dropping credentials during reauth
From: rsbecker @ 2024-02-04 23:01 UTC (permalink / raw)
To: 'Quentin Bouget', git
In-Reply-To: <20240204185427.39664-3-ypsah@devyard.org>
On Sunday, February 4, 2024 1:54 PM, Quentin Bouget wrote:
>During a re-authentication (second attempt at authenticating with a remote,
e.g.
>after a failed GSSAPI attempt), git allows the remote to provide credential
overrides
>in the redirect URL and unconditionnaly drops the current HTTP credentials
in favors
>of those, even when there aren't any.
>
>This commit makes it so HTTP credentials are only overridden when the
redirect URL
>actually contains credentials itself.
>
>Signed-off-by: Quentin Bouget <ypsah@devyard.org>
>---
> http.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
>diff --git a/http.c b/http.c
>index ccea19ac47..caba9cac1e 100644
>--- a/http.c
>+++ b/http.c
>@@ -2160,7 +2160,25 @@ static int http_request_reauth(const char *url,
> if (options && options->effective_url && options->base_url) {
> if (update_url_from_redirect(options->base_url,
> url, options->effective_url)) {
>+ char *username = NULL, *password = NULL;
>+
>+ if (http_auth.username)
>+ username = xstrdup(http_auth.username);
>+ if (http_auth.password)
>+ password = xstrdup(http_auth.password);
>+
> credential_from_url(&http_auth,
options->base_url->buf);
>+
>+ if (http_auth.username)
>+ free(username);
>+ else if (username)
>+ http_auth.username = username;
>+
>+ if (http_auth.password)
>+ free(password);
>+ else if (password)
>+ http_auth.password = password;
>+
> url = options->effective_url->buf;
> }
> }
I am wondering whether this is a good idea. Having credentials in a redirect
seems like it might be a vector for going somewhere other than what you want
to do, with credentials you do not necessarily want. Others might no better
than I on this, but would potentially lead to a CVE? I would prefer to see
credentials in a redirect rejected rather than used.
--Randall
^ permalink raw reply
* git bug
From: moti sd @ 2024-02-04 23:42 UTC (permalink / raw)
To: git
[-- Attachment #1.1: Type: text/plain, Size: 1985 bytes --]
Dear Git Team,
I am writing to report a potential bug encountered while using the "git
stash" command. The issue was observed during a troubleshooting. Please
find the details below:
*Problem Description:*
The "git stash" command is not providing any feedback or indication of
execution, even though modifications are present and the stash operation is
attempted.
*Steps to Reproduce:*
Make modifications to a file.
Execute "git stash" to stash changes.
*Expected Behavior:*
The "git stash" command should provide feedback on the stash operation,
confirming whether it was successful or if there were any errors.
*Additional Information:*
The issue was initially observed when executing "git commit -a --amend" and
pressing Ctrl+Z to exit. Subsequent attempts to use "git stash" resulted in
an error.
Testing with a provided ZIP file containing a problematic repository
confirmed the issue's recurrence.
*Investigation and Findings:*
Upon investigation, it was discovered that the use of Ctrl+Z after "git
commit -a --amend" might be the root cause.
Deleting a lock file left behind by the incomplete git command resolved the
issue, indicating a potential bug in how "git stash" handles incomplete
commands.
*Recommendation:*
The "git stash" command should provide meaningful feedback or error
messages to users, even in cases where the operation is incomplete or
encounters issues.
*Steps to Reproduce the Bug:*
Execute "git commit -a --amend."
Press Ctrl+Z to exit the command.
Attempt "git stash."
*Attachment:*
A ZIP file containing a problematic repository is attached for your
reference.
*Impact:*
The bug affects users' ability to use the "git stash" command effectively,
leading to potential confusion and hindering workflow.
Resolution:
I appreciate your attention to this matter, and I believe addressing this
bug will enhance the overall user experience with Git. If further
information or testing is required, please feel free to reach out.
Best regards,
Moti
[-- Attachment #1.2: Type: text/html, Size: 2332 bytes --]
[-- Attachment #2: git-bugreport-2024-02-05-0015.txt --]
[-- Type: text/plain, Size: 1262 bytes --]
Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.
What did you do before the bug happened? (Steps to reproduce your issue)
I executed "git commit -a --amend," where I pressed Ctrl+Z to exit. Later, when trying again, an error occurred.
What did you expect to happen? (Expected behavior)
I intended to execute the "git stash" command, but it is not functioning as expected. I wish to receive an error message to better understand the issue and address it accordingly. The command is silent, whereas during a commit, error messages are displayed, providing helpful feedback for resolution.
What happened instead? (Actual behavior)
Due to .git/lock file, git stash does not behave normally, and the error message does not show up
Please review the rest of the bug report below.
You can delete any lines you don't wish to share.
[System Info]
git version:
git version 2.38.1.windows.1
cpu: x86_64
built from commit: b85c8f604d375d4d773a36842964e8a7ec056aae
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon
uname: Windows 10.0 22631
compiler info: gnuc: 12.2
libc info: no libc information available
$SHELL (typically, interactive shell): <unset>
[Enabled Hooks]
[-- Attachment #3: git-diagnostics-2024-02-05-0015.zip --]
[-- Type: application/x-zip-compressed, Size: 844 bytes --]
[-- Attachment #4: 02-stash-amend-wd.no-quiere-stashear.zip --]
[-- Type: application/x-zip-compressed, Size: 28391 bytes --]
^ permalink raw reply
* Re: single-char options
From: Oswald Buddenhagen @ 2024-02-04 13:47 UTC (permalink / raw)
To: Martin Guy; +Cc: git
In-Reply-To: <0e88ac8ad60d2da689d0a308cc59a02d468bc15f.camel@gmail.com>
On Fri, Feb 02, 2024 at 01:45:55PM +0000, Martin Guy wrote:
>I'm sure this must have been thought of before and rejected, but us RSI
>sufferers would like to be able to say "git rebase -c" for "--continue"
>not only for speed and ease, but for all the other RSI sufferers out
>there.
>
you don't need to re-type the commands.
in bash (or any other readline-using shell) just type ctrl-r 'onti' (or
anything else sufficiently unique) and maybe ctrl-r a few times more.
it's very rare that i type repeat commands from scratch. in fact, to the
point that i often spend more time hopping through the history than it
would have taken to re-type the command, heh.
^ permalink raw reply
* Re: Migrate away from vger to GitHub or (on-premise) GitLab?
From: Oswald Buddenhagen @ 2024-02-04 15:12 UTC (permalink / raw)
To: Michal Suchánek
Cc: phillip.wood, Patrick Steinhardt, brian m. carlson, Hans Meiser,
Dragan Simic, Konstantin Ryabitsev, git@vger.kernel.org
In-Reply-To: <20240202115004.GV9696@kitsune.suse.cz>
On Fri, Feb 02, 2024 at 12:50:04PM +0100, Michal Suchánek wrote:
>Given the open nature of lore it should be feasible to provide
>additional interfaces on top of it that cater to people used to PRs
>on popular forge web UIs without hijacking the whole project and the
>existing tools and interfaces. For some reason people are set on
>replacing it as a whole, and removing the interfaces they personally
>don't use,
>calling them obosolete.
>
because they positively *are*.
when i started, patch-based code reviews were the norm, and i'm still
using them for my small project with almost no external contributions.
but after working with gerrit code review for over a decade, i find it
mind-boggling that people are still voluntarily subjecting themselves to
mail-based reviews for serious high-volume work.
it doesn't matter just how super-proficient you got with your old tools.
there is just no way you'll get anywhere near as efficient as you would
with the new ones, if you just were interested enough to learn them.
migrating the workflows that are worth keeping isn't such a bit deal.
i'll note that i don't consider github-like forges to be adequate tools
for serious work, as they seem to intentionally discourage producing
polished commits.
the gerrit project is unfortunately not interested in building a proper
forge, but luckily there is a bridge to github (hosted version available
at gerrithub.io).
^ permalink raw reply
* Re: Migrate away from vger to GitHub or (on-premise) GitLab?
From: Oswald Buddenhagen @ 2024-02-05 1:04 UTC (permalink / raw)
To: Michal Suchánek
Cc: phillip.wood, Patrick Steinhardt, brian m. carlson, Hans Meiser,
Dragan Simic, Konstantin Ryabitsev, git@vger.kernel.org
In-Reply-To: <20240204154714.GZ9696@kitsune.suse.cz>
On Sun, Feb 04, 2024 at 04:47:14PM +0100, Michal Suchánek wrote:
>On Sun, Feb 04, 2024 at 04:12:02PM +0100, Oswald Buddenhagen wrote:
>> but after working with gerrit code review for over a decade, i find
>> it
>> mind-boggling that people are still voluntarily subjecting themselves to
>> mail-based reviews for serious high-volume work.
>
>I have yet to see gerrit in action. Very few projects use it so it's
>difficult to gauge what tradeoffs compared to e-mail based workflow it
>does provide.
>
from my just slightly biased perspective ;-) i can't see any significant
trade-offs except for some set-up cost (that will quickly pay for
itself).
in fact, my gerrit workflow is still "e-mail based", in that everything
is driven by the notification mails, only that i "branch out" to the
browser whenever something interesting happens.
for the CLI hardliners there are gertty and emacs egerrit, but i see no
point in using either despite being a heavy CLI and TUI user. given how
"much" attention these tools get despite there being literally tens of
thousands of regular gerrit users, i'm inclined to think that there is
indeed very little demand.
gerrit also has an incoming email gateway, but i'm not sure how advanced
it is - at some point it required well-formed html replies as input.
if one really wants to, one can install a webhook or event stream
watcher that posts all activity to a mailing list (and i don't mean
_the_ list, because it would be just noise on top of everyone's
individual notifications).
>> migrating the workflows that are worth keeping isn't such a bit deal.
>
>Have you migrated them to gerrit already, and t[a]ught all the git
>contributors how to use them from gerrit?
>
that challenge is sort of meaningless, because the only workflow within
the git project that i'm aware of that would affect "all the git
contributors" is the interaction with gerrit itself. which has a very
steep, but also extremely short learning curve. and there are tools to
meke it more pleasant - https://wiki.qt.io/Git-gpush-scripts (yep,
shameless self-promotion here).
i'm not aware of any pre-integration build bots, so nothing to do on
that front except for some mirroring adjustments (gerrit insists on
being its own authoritative git server).
gitgitgadget would just become obsolete, to be replaced by the github
integration plugin.
my main concern is with the maintainer workflow:
the way gerrit is usually used, the contributor determines the target
branch, and the changes are merged directly to it after they are
approved. unclean merges must also be eliminated during review. that
works just fine, but it doesn't match the refs and merge commit messages
junio produces. and while aggregating pending changes into `seen` would
be still perfectly possible (each change including its dependencies is
just a ref), it would be somewhat awkward due to the naming and location
of the change refs.
to reproduce the existing merge workflow more faithfully,
- junio would have to monitor incoming changes, manually create an empty
branches for each topic, and change the target branch of all changes
in each topic
- the gerrit-side integration would then happen into that branch
- junio would then proceed with manually merging the branch and
direct-pushing (that is, not creating a review for it) the merge into
next or maint
this is reallly just the current workflow, and can be equally automated,
just with slightly different tooling. only it's ... weird for gerrit,
artificially creating a bottleneck. the gerrit integration workflow is
naturally decentralized.
personally, i would just switch to the usual gerrit workflow, and at
least for `seen` use a merge-free workflow -- with gpick (gpush
complement, see link above) it's absolutely trivial to track all
interesting branches stacked onto each other.
>Somobody has to do it.
>
yes. and if nobody does, then everybody keeps paying the cost of not
doing it. that might incentivize Somebody (TM) with the resources and a
vested interest.
>Also can you migrate away from gerrit once it becomes defunct or new,
>better alternative emerges?
>
>Recently it seems that forges offer a 'download your project data'
>option, probably as a result of GDPR. What use is such data blob though?
>
current gerrit keeps the meta data in (yet more) awkwardly named refs
containing plain-text files, so that's no issue. one could render it
into a read-only view, or convert it.
>An e-mail archive is that: an archive. It's a medium that you can read
>with a wealth of software today, and 100 years from now. An achivable
>data format.
>
with a mailing list archiving the event stream, we'd have that.
>Compare that with the 'download your data' blob from a forge. Can it be
>uploaded even to a diffferent instance of the same forge to restore your
>project elsewhere? Interpreted by any tool othar than the correct
>vintage of that same forge? Deos even more than one instance of the
>forge exist?
>
a review meta-data standard is being discussed from time to time, but it
hasn't gone anywhere yet.
but looking at it from a practical perspective, with a list-based
archive the situation wouldn't be any worse than it is right now if
gerrit was to suddenly disappear.
during my tenure at the qt project i established a commit policy (and
deployed tooling to help enforce it) that presumes that gerrit could be
replaced at any time, so commit messages are not supposed to refer to
other commits by gerrit change ids or review urls. (in principle, this
works even for pending and abandoned changes, as each patchset (revision
of a change) keeps its commit and therefore sha1 forever.)
of course that's not practical for regular mailing list posts, but one
can't have everything ...
>And even if you do convert to gerrit it's unlikely to satisfy the "Why
>are you not using github or gitlab" crowd. It's not one of the big,
>popular forges they are familiar with, the UX is significantly
>different.
>
i can confirm that in the qt project this is indeed absolutely the case,
and the last related thread isn't even cold yet.
but why should the people with standards care? lowering the barrier to
entry is all dandy, but not when it causes significant detriment to the
workflow of those who do most work.
note that the original request in this thread was for "structured
discussion", and gerrit would absolutely provide that, among other
things.
^ permalink raw reply
* Re: [PATCH 2/2] http: prevent redirect from dropping credentials during reauth
From: Quentin Bouget @ 2024-02-05 3:01 UTC (permalink / raw)
To: brian m. carlson; +Cc: git
In-Reply-To: <ZcARb4YNCD4NLJku@tapette.crustytoothpaste.net>
On Sun Feb 4, 2024 at 11:36 PM CET, brian m. carlson wrote:
> On 2024-02-04 at 18:54:27, Quentin Bouget wrote:
> > During a re-authentication (second attempt at authenticating with a
> > remote, e.g. after a failed GSSAPI attempt), git allows the remote to
> > provide credential overrides in the redirect URL and unconditionnaly
> > drops the current HTTP credentials in favors of those, even when there
> > aren't any.
> >
> > This commit makes it so HTTP credentials are only overridden when the
> > redirect URL actually contains credentials itself.
>
> I don't think your proposed change is safe. Credentials are supposed to
> be tied to a certain site and may even be tied to a specific repository,
> and if there's a redirect, then we need to re-fetch credentials or we
> could leak credentials to the wrong site by reusing them. Your change
> would therefore introduce a security vulnerability.
Good point, I had not considered the security implications.
I can see libcurl only reuses credentials after a redirect if the
hostname has not changed: [1]
By default, libcurl only sends credentials and Authentication
headers to the initial hostname as given in the original URL, to
avoid leaking username + password to other sites.
Does it sound OK if I use the credentials provided by the redirect when
there are any (out of consistency with the current implementation), and
only allow reusing the current credentials when the redirect and the
original URLs share the same hostname?
If so, is there a helper to extract the hostname out of a URL in git?
I can see credential.c does this itself, otherwise, libcurl provides its
own helpers for this (curl_url(), curl_url_set(), curl_url_get()). [2]
> I should also point out that in general we are trying to make it less
> easy and less convenient for people to use credentials in the URL
> because that always necessitates insecure storage. There have in fact
> been proposals to remove that functionality entirely.
Apologies, I feel like I may have given the impression I wanted to
configure credentials in git's configuration files, which is not the
case.
My use case is to `git push` a tag from a CI/CD pipeline to trigger a
release, similar to how I do it here. [3]
Or is this the kind of use case you are trying to discourage?
[1] https://curl.se/libcurl/c/CURLOPT_UNRESTRICTED_AUTH.html
[2] https://curl.se/libcurl/c/curl_url_get.html
[3] https://gitlab.com/ypsah/simple-git-versioning/-/blob/main/.gitlab-ci.yml?ref_type=heads#L110
^ permalink raw reply
* Re: [PATCH 2/2] http: prevent redirect from dropping credentials during reauth
From: Quentin Bouget @ 2024-02-05 3:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq8r3zonat.fsf@gitster.g>
On Sun Feb 4, 2024 at 11:51 PM CET, Junio C Hamano wrote:
> Quentin Bouget <ypsah@devyard.org> writes:
>
> > During a re-authentication (second attempt at authenticating with a
> > remote, e.g. after a failed GSSAPI attempt), git allows the remote to
> > provide credential overrides in the redirect URL and unconditionnaly
> > drops the current HTTP credentials in favors of those, even when there
> > aren't any.
> >
> > This commit makes it so HTTP credentials are only overridden when the
> > redirect URL actually contains credentials itself.
>
> "This commit makes it so" -> "Make it so"
Will change.
> > + char *username = NULL, *password = NULL;
> > +
> > + if (http_auth.username)
> > + username = xstrdup(http_auth.username);
> > + if (http_auth.password)
> > + password = xstrdup(http_auth.password);
>
> Not a huge deal, but we have xstrdup_or_null() helper function
> exactly for a use case like this.
Thanks, will change.
> > credential_from_url(&http_auth, options->base_url->buf);
> > +
> > + if (http_auth.username)
> > + free(username);
> > + else if (username)
> > + http_auth.username = username;
> > +
> > + if (http_auth.password)
> > + free(password);
> > + else if (password)
> > + http_auth.password = password;
>
> This is an interesting change. I wonder what breaks if we
> completely ignored such credential materials forced by the remote
> via a redirect?
Me too. Maybe the original author would know. Is it OK to Cc them in
this case?
> > url = options->effective_url->buf;
> > }
> > }
Thanks,
Quentin
^ permalink raw reply
* Re: [PATCH 2/2] http: prevent redirect from dropping credentials during reauth
From: Quentin Bouget @ 2024-02-05 3:12 UTC (permalink / raw)
To: rsbecker, git
In-Reply-To: <017a01da57be$28748aa0$795d9fe0$@nexbridge.com>
On Mon Feb 5, 2024 at 12:01 AM CET, wrote:
> On Sunday, February 4, 2024 1:54 PM, Quentin Bouget wrote:
> > During a re-authentication (second attempt at authenticating with a remote, e.g.
> > after a failed GSSAPI attempt), git allows the remote to provide credential overrides
> > in the redirect URL and unconditionnaly drops the current HTTP credentials in favors
> > of those, even when there aren't any.
> >
> > This commit makes it so HTTP credentials are only overridden when the redirect URL
> > actually contains credentials itself.
> >
> > Signed-off-by: Quentin Bouget <ypsah@devyard.org>
> > ---
> > http.c | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/http.c b/http.c
> > index ccea19ac47..caba9cac1e 100644
> > --- a/http.c
> > +++ b/http.c
> > @@ -2160,7 +2160,25 @@ static int http_request_reauth(const char *url,
> > if (options && options->effective_url && options->base_url) {
> > if (update_url_from_redirect(options->base_url,
> > url, options->effective_url)) {
> > + char *username = NULL, *password = NULL;
> > +
> > + if (http_auth.username)
> > + username = xstrdup(http_auth.username);
> > + if (http_auth.password)
> > + password = xstrdup(http_auth.password);
> > +
> > credential_from_url(&http_auth, options->base_url->buf);
> > +
> > + if (http_auth.username)
> > + free(username);
> > + else if (username)
> > + http_auth.username = username;
> > +
> > + if (http_auth.password)
> > + free(password);
> > + else if (password)
> > + http_auth.password = password;
> > +
> > url = options->effective_url->buf;
> > }
> > }
>
> I am wondering whether this is a good idea. Having credentials in a redirect
> seems like it might be a vector for going somewhere other than what you want
> to do, with credentials you do not necessarily want. Others might no better
> than I on this, but would potentially lead to a CVE? I would prefer to see
> credentials in a redirect rejected rather than used.
I guess this can be controlled by setting http.followRedirects to false.
I am not sure there is generally more danger in following a redirect URL
with or without the provided credentials. Note that this is also how git
currently behaves and while I am not aware of any concrete use case
myself, I am also not confident there isn't any.
That being said, Brian M. Carlson pointed out that reusing credentials
from the initial URL for the redirect URL is quite dangerous.
In my reply, I have suggested to switch to implementing the same
behaviour as libcurl when it comes to reusing credentials: if the
hostname of the redirect is the same as the original URL, reuse the
credentials, otherwise drop them. [1]
I would still retain the (seemingly strange) current behaviour of
favoring credentials in the redirect URL over anything else.
Thanks,
Quentin
[1] https://curl.se/libcurl/c/CURLOPT_UNRESTRICTED_AUTH.html
^ permalink raw reply
* Re: [PATCH 1/2] http: only reject basic auth credentials once they have been tried
From: Quentin Bouget @ 2024-02-05 3:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqfry7onhf.fsf@gitster.g>
On Sun Feb 4, 2024 at 11:47 PM CET, Junio C Hamano wrote:
> Quentin Bouget <ypsah@devyard.org> writes:
>
> > else if (results->http_code == 401) {
> > - if (http_auth.username && http_auth.password) {
> > - credential_reject(&http_auth);
> > - return HTTP_NOAUTH;
> > - } else {
> > + if ((http_auth_methods & CURLAUTH_GSSNEGOTIATE) == CURLAUTH_GSSNEGOTIATE) {
> > http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
> > if (results->auth_avail) {
> > http_auth_methods &= results->auth_avail;
> > http_auth_methods_restricted = 1;
> > }
> > return HTTP_REAUTH;
> > }
> > + if (http_auth.username && http_auth.password)
> > + credential_reject(&http_auth);
> > + return HTTP_NOAUTH;
>
> A few comments and questions.
>
> * GSSNEGOTIATE is a synonym for NEGOTIATE since cURL 7.38.0
> (released in Sep 2014); currently the earliest version we claim
> to support is 7.19.5 (released May 2009) without imap-send, and
> we require 7.34.0 (released Dec 2013) with imap-send, so for now,
> it is prudent that this patch uses GSSNEGOTIATE.
Agreed
> * Is it something that the client code of libcURL can rely on that
> these CURLAUTH_FOO macros are bitmasks [*]? If so, wouldn't
>
> if ((http_auth_methods & CURLAUTH_GSSNEGOTIATE))
>
> be clear enough (and less risk of making typo)?
>
> [References]
>
> * https://github.com/curl/curl/blob/b8c003832d730bb2f4b9de4204675ca5d9f7a903/include/curl/curl.h#L787C4-L787C64
I had not considered the risk of typos, fair enough. Will change.
> * When we see 401, the first thing we do in the new code is to see
> if GSS is enabled in auth_methods, and if so we drop it from
> auth_methods (to prevent us from trying it again) and say REAUTH.
>
> - What assures us that the presense of GSS bit in auth_methods
> mean we tried GSS to get this 401? Could it be that we tried
> basic and seeing 401 from that, but we haven't tried GSS and we
> could retry with GSS now? Is it commonly known that GSS is
> always tried first before Basic/Digest when both are availble,
> or something like that?
libcurl's documentation on CURLOPT_HTTPAUTH says:
If more than one bit is set, libcurl first queries the host to
see which authentication methods it supports and then picks the
best one you allow it to use.
And then:
CURLAUTH_NEGOTIATE
HTTP Negotiate (SPNEGO) authentication. Negotiate
authentication is defined in RFC 4559 and is the most
secure way to perform authentication over HTTP.
Which hints at CURLAUTH_NEGOTIATE (aka. GSSNEGOTIATE as you pointed out)
being the prefered auth method.
The current implementation confirms this. [1]
> - When auth_avail was given by the cURL library, we further limit
> the auth_methods (after dropping GSS) and say REAUTH. This is
> not a new to the updated code, but can it happen that the
> resulting restricted auth_methods bitmap becomes empty (i.e.
> REAUTH would be useless)?
I am not familiar enough with SPNEGO to say. Seems plausible.
Should I instead do:
return (http_auth_methods & CURLAUTH_ANY) ? HTTP_REAUTH : HTTP_NOAUTH;
Thanks,
Quentin
[1] https://github.com/curl/curl/blob/b8c003832d730bb2f4b9de4204675ca5d9f7a903/lib/http.c#L373
^ permalink raw reply
* Re: [PATCH 1/2] http: only reject basic auth credentials once they have been tried
From: Patrick Steinhardt @ 2024-02-05 5:47 UTC (permalink / raw)
To: Quentin Bouget; +Cc: git
In-Reply-To: <20240204185427.39664-2-ypsah@devyard.org>
[-- Attachment #1: Type: text/plain, Size: 2579 bytes --]
On Sun, Feb 04, 2024 at 07:54:26PM +0100, Quentin Bouget wrote:
> When CURLAUTH_GSSNEGOTIATE is enabled, it is currently assumed that
> the provided username/password relate to a GSSAPI auth attempt.
> In practice, forges such as gitlab can be deployed with HTTP basic auth
> and GSSAPI auth both listening on the same port, meaning just because
> the server supports GSSAPI and failed an authentication attempt using
> the provided credentials, it does not mean the credentials are not valid
> HTTP basic auth credentials.
>
> This is documented as a long running bug here [1] and breaks token-based
> authentication when the token is provided in the remote's URL itself.
>
> This commit makes it so credentials are only dropped once they have been
> tried both as GSSAPI credentials and HTTP basic auth credentials.
>
> [1] https://gitlab.com/gitlab-org/gitlab/-/blob/b0e0d25646d1992fefda863febdcba8d4c7a1bbf/doc/integration/kerberos.md#L250
Do you think it's feasible to add a test for this? We already have a
bunch of tests for authentication with Apache's httpd in t5563, so if we
could extend t/lib-httpd.sh to set up `mod_auth_gssapi` that would be
great.
I didn't try though, and it could just as well be that this would
require a full-fledged Kerberos setup, which would be a deal breaker I
guess. I ain't got enough familiarity with `mod_auth_gssapi` to tell.
Patrick
> Signed-off-by: Quentin Bouget <ypsah@devyard.org>
> ---
> http.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/http.c b/http.c
> index e73b136e58..ccea19ac47 100644
> --- a/http.c
> +++ b/http.c
> @@ -1758,10 +1758,7 @@ static int handle_curl_result(struct slot_results *results)
> } else if (missing_target(results))
> return HTTP_MISSING_TARGET;
> else if (results->http_code == 401) {
> - if (http_auth.username && http_auth.password) {
> - credential_reject(&http_auth);
> - return HTTP_NOAUTH;
> - } else {
> + if ((http_auth_methods & CURLAUTH_GSSNEGOTIATE) == CURLAUTH_GSSNEGOTIATE) {
> http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
> if (results->auth_avail) {
> http_auth_methods &= results->auth_avail;
> @@ -1769,6 +1766,9 @@ static int handle_curl_result(struct slot_results *results)
> }
> return HTTP_REAUTH;
> }
> + if (http_auth.username && http_auth.password)
> + credential_reject(&http_auth);
> + return HTTP_NOAUTH;
> } else {
> if (results->http_connectcode == 407)
> credential_reject(&proxy_auth);
> --
> 2.43.0
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v3 0/2] refs: introduce reftable backend
From: Patrick Steinhardt @ 2024-02-05 6:02 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Han-Wen Nienhuys, Karthik Nayak
In-Reply-To: <cover.1706601199.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 3034 bytes --]
Hi,
this is the third version of my patch series that introduces the
reftable backend.
The only real change in this version is that I've pulled in
kn/for-all-refs at 693e807311 (for-each-ref: avoid filtering on empty
pattern, 2024-01-29) as a dependency. This series introduces a new
DO_FOR_EACH_INCLUDE_ALL_REFS flag that we need to handle in the reftable
backend. With this change all tests should pass again.
Patrick
Patrick Steinhardt (2):
refs: introduce reftable backend
ci: add jobs to test with the reftable backend
.github/workflows/main.yml | 9 +
.gitlab-ci.yml | 9 +
Documentation/ref-storage-format.txt | 2 +
.../technical/repository-version.txt | 5 +-
Makefile | 1 +
ci/lib.sh | 2 +-
ci/run-build-and-tests.sh | 3 +
contrib/workdir/git-new-workdir | 2 +-
path.c | 2 +-
path.h | 1 +
refs.c | 1 +
refs/refs-internal.h | 1 +
refs/reftable-backend.c | 2297 +++++++++++++++++
repository.h | 5 +-
t/t0610-reftable-basics.sh | 887 +++++++
t/t0611-reftable-httpd.sh | 26 +
t/test-lib.sh | 2 +
17 files changed, 3248 insertions(+), 7 deletions(-)
create mode 100644 refs/reftable-backend.c
create mode 100755 t/t0610-reftable-basics.sh
create mode 100755 t/t0611-reftable-httpd.sh
Range-diff against v2:
1: d6548dcfad ! 1: d83e66e980 refs: introduce reftable backend
@@ refs/reftable-backend.c (new)
+{
+ switch (log->value_type) {
+ case REFTABLE_LOG_UPDATE:
-+ /* when we write log records, the hashes are owned by a struct
-+ * oid */
++ /*
++ * When we write log records, the hashes are owned by the
++ * caller and thus shouldn't be free'd.
++ */
+ log->value.update.old_hash = NULL;
+ log->value.update.new_hash = NULL;
+ break;
@@ refs/reftable-backend.c (new)
+ break;
+
+ /*
-+ * The files backend only lists references contained in
-+ * "refs/". We emulate the same behaviour here and thus skip
-+ * all references that don't start with this prefix.
++ * Unless DO_FOR_EACH_INCLUDE_ALL_REFS is set, we only list
++ * refs starting with "refs/" to mimic the "files" backend.
+ */
-+ if (!starts_with(iter->ref.refname, "refs/"))
++ if (!(iter->flags & DO_FOR_EACH_INCLUDE_ALL_REFS) &&
++ !starts_with(iter->ref.refname, "refs/"))
+ continue;
+
+ if (iter->prefix &&
2: 63eafc9f5b = 2: 146bb95c03 ci: add jobs to test with the reftable backend
--
2.43.GIT
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v3 1/2] refs: introduce reftable backend
From: Patrick Steinhardt @ 2024-02-05 6:02 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Han-Wen Nienhuys, Karthik Nayak
In-Reply-To: <cover.1707109509.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 118111 bytes --]
Due to scalability issues, Shawn Pearce has originally proposed a new
"reftable" format more than six years ago [1]. Initially, this new
format was implemented in JGit with promising results. Around two years
ago, we have then added the "reftable" library to the Git codebase via
a4bbd13be3 (Merge branch 'hn/reftable', 2021-12-15). With this we have
landed all the low-level code to read and write reftables. Notably
missing though was the integration of this low-level code into the Git
code base in the form of a new ref backend that ties all of this
together.
This gap is now finally closed by introducing a new "reftable" backend
into the Git codebase. This new backend promises to bring some notable
improvements to Git repositories:
- It becomes possible to do truly atomic writes where either all refs
are committed to disk or none are. This was not possible with the
"files" backend because ref updates were split across multiple loose
files.
- The disk space required to store many refs is reduced, both compared
to loose refs and packed-refs. This is enabled both by the reftable
format being a binary format, which is more compact, and by prefix
compression.
- We can ignore filesystem-specific behaviour as ref names are not
encoded via paths anymore. This means there is no need to handle
case sensitivity on Windows systems or Unicode precomposition on
macOS.
- There is no need to rewrite the complete refdb anymore every time a
ref is being deleted like it was the case for packed-refs. This
means that ref deletions are now constant time instead of scaling
linearly with the number of refs.
- We can ignore file/directory conflicts so that it becomes possible
to store both "refs/heads/foo" and "refs/heads/foo/bar".
- Due to this property we can retain reflogs for deleted refs. We have
previously been deleting reflogs together with their refs to avoid
file/directory conflicts, which is not necessary anymore.
- We can properly enumerate all refs. With the "files" backend it is
not easily possible to distinguish between refs and non-refs because
they may live side by side in the gitdir.
Not all of these improvements are realized with the current "reftable"
backend implementation. At this point, the new backend is supposed to be
a drop-in replacement for the "files" backend that is used by basically
all Git repositories nowadays. It strives for 1:1 compatibility, which
means that a user can expect the same behaviour regardless of whether
they use the "reftable" backend or the "files" backend for most of the
part.
Most notably, this means we artificially limit the capabilities of the
"reftable" backend to match the limits of the "files" backend. It is not
possible to create refs that would end up with file/directory conflicts,
we do not retain reflogs, we perform stricter-than-necessary checks.
This is done intentionally due to two main reasons:
- It makes it significantly easier to land the "reftable" backend as
tests behave the same. It would be tough to argue for each and every
single test that doesn't pass with the "reftable" backend.
- It ensures compatibility between repositories that use the "files"
backend and repositories that use the "reftable" backend. Like this,
hosters can migrate their repositories to use the "reftable" backend
without causing issues for clients that use the "files" backend in
their clones.
It is expected that these artificial limitations may eventually go away
in the long term.
Performance-wise things very much depend on the actual workload. The
following benchmarks compare the "files" and "reftable" backends in the
current version:
- Creating N refs in separate transactions shows that the "files"
backend is ~50% faster. This is not surprising given that creating a
ref only requires us to create a single loose ref. The "reftable"
backend will also perform auto compaction on updates. In real-world
workloads we would likely also want to perform pack loose refs,
which would likely change the picture.
Benchmark 1: update-ref: create refs sequentially (refformat = files, refcount = 1)
Time (mean ± σ): 2.1 ms ± 0.3 ms [User: 0.6 ms, System: 1.7 ms]
Range (min … max): 1.8 ms … 4.3 ms 133 runs
Benchmark 2: update-ref: create refs sequentially (refformat = reftable, refcount = 1)
Time (mean ± σ): 2.7 ms ± 0.1 ms [User: 0.6 ms, System: 2.2 ms]
Range (min … max): 2.4 ms … 2.9 ms 132 runs
Benchmark 3: update-ref: create refs sequentially (refformat = files, refcount = 1000)
Time (mean ± σ): 1.975 s ± 0.006 s [User: 0.437 s, System: 1.535 s]
Range (min … max): 1.969 s … 1.980 s 3 runs
Benchmark 4: update-ref: create refs sequentially (refformat = reftable, refcount = 1000)
Time (mean ± σ): 2.611 s ± 0.013 s [User: 0.782 s, System: 1.825 s]
Range (min … max): 2.597 s … 2.622 s 3 runs
Benchmark 5: update-ref: create refs sequentially (refformat = files, refcount = 100000)
Time (mean ± σ): 198.442 s ± 0.241 s [User: 43.051 s, System: 155.250 s]
Range (min … max): 198.189 s … 198.670 s 3 runs
Benchmark 6: update-ref: create refs sequentially (refformat = reftable, refcount = 100000)
Time (mean ± σ): 294.509 s ± 4.269 s [User: 104.046 s, System: 190.326 s]
Range (min … max): 290.223 s … 298.761 s 3 runs
- Creating N refs in a single transaction shows that the "files"
backend is significantly slower once we start to write many refs.
The "reftable" backend only needs to update two files, whereas the
"files" backend needs to write one file per ref.
Benchmark 1: update-ref: create many refs (refformat = files, refcount = 1)
Time (mean ± σ): 1.9 ms ± 0.1 ms [User: 0.4 ms, System: 1.4 ms]
Range (min … max): 1.8 ms … 2.6 ms 151 runs
Benchmark 2: update-ref: create many refs (refformat = reftable, refcount = 1)
Time (mean ± σ): 2.5 ms ± 0.1 ms [User: 0.7 ms, System: 1.7 ms]
Range (min … max): 2.4 ms … 3.4 ms 148 runs
Benchmark 3: update-ref: create many refs (refformat = files, refcount = 1000)
Time (mean ± σ): 152.5 ms ± 5.2 ms [User: 19.1 ms, System: 133.1 ms]
Range (min … max): 148.5 ms … 167.8 ms 15 runs
Benchmark 4: update-ref: create many refs (refformat = reftable, refcount = 1000)
Time (mean ± σ): 58.0 ms ± 2.5 ms [User: 28.4 ms, System: 29.4 ms]
Range (min … max): 56.3 ms … 72.9 ms 40 runs
Benchmark 5: update-ref: create many refs (refformat = files, refcount = 1000000)
Time (mean ± σ): 152.752 s ± 0.710 s [User: 20.315 s, System: 131.310 s]
Range (min … max): 152.165 s … 153.542 s 3 runs
Benchmark 6: update-ref: create many refs (refformat = reftable, refcount = 1000000)
Time (mean ± σ): 51.912 s ± 0.127 s [User: 26.483 s, System: 25.424 s]
Range (min … max): 51.769 s … 52.012 s 3 runs
- Deleting a ref in a fully-packed repository shows that the "files"
backend scales with the number of refs. The "reftable" backend has
constant-time deletions.
Benchmark 1: update-ref: delete ref (refformat = files, refcount = 1)
Time (mean ± σ): 1.7 ms ± 0.1 ms [User: 0.4 ms, System: 1.2 ms]
Range (min … max): 1.6 ms … 2.1 ms 316 runs
Benchmark 2: update-ref: delete ref (refformat = reftable, refcount = 1)
Time (mean ± σ): 1.8 ms ± 0.1 ms [User: 0.4 ms, System: 1.3 ms]
Range (min … max): 1.7 ms … 2.1 ms 294 runs
Benchmark 3: update-ref: delete ref (refformat = files, refcount = 1000)
Time (mean ± σ): 2.0 ms ± 0.1 ms [User: 0.5 ms, System: 1.4 ms]
Range (min … max): 1.9 ms … 2.5 ms 287 runs
Benchmark 4: update-ref: delete ref (refformat = reftable, refcount = 1000)
Time (mean ± σ): 1.9 ms ± 0.1 ms [User: 0.5 ms, System: 1.3 ms]
Range (min … max): 1.8 ms … 2.1 ms 217 runs
Benchmark 5: update-ref: delete ref (refformat = files, refcount = 1000000)
Time (mean ± σ): 229.8 ms ± 7.9 ms [User: 182.6 ms, System: 46.8 ms]
Range (min … max): 224.6 ms … 245.2 ms 6 runs
Benchmark 6: update-ref: delete ref (refformat = reftable, refcount = 1000000)
Time (mean ± σ): 2.0 ms ± 0.0 ms [User: 0.6 ms, System: 1.3 ms]
Range (min … max): 2.0 ms … 2.1 ms 3 runs
- Listing all refs shows no significant advantage for either of the
backends. The "files" backend is a bit faster, but not by a
significant margin. When repositories are not packed the "reftable"
backend outperforms the "files" backend because the "reftable"
backend performs auto-compaction.
Benchmark 1: show-ref: print all refs (refformat = files, refcount = 1, packed = true)
Time (mean ± σ): 1.6 ms ± 0.1 ms [User: 0.4 ms, System: 1.1 ms]
Range (min … max): 1.5 ms … 2.0 ms 1729 runs
Benchmark 2: show-ref: print all refs (refformat = reftable, refcount = 1, packed = true)
Time (mean ± σ): 1.6 ms ± 0.1 ms [User: 0.4 ms, System: 1.1 ms]
Range (min … max): 1.5 ms … 1.8 ms 1816 runs
Benchmark 3: show-ref: print all refs (refformat = files, refcount = 1000, packed = true)
Time (mean ± σ): 4.3 ms ± 0.1 ms [User: 0.9 ms, System: 3.3 ms]
Range (min … max): 4.1 ms … 4.6 ms 645 runs
Benchmark 4: show-ref: print all refs (refformat = reftable, refcount = 1000, packed = true)
Time (mean ± σ): 4.5 ms ± 0.2 ms [User: 1.0 ms, System: 3.3 ms]
Range (min … max): 4.2 ms … 5.9 ms 643 runs
Benchmark 5: show-ref: print all refs (refformat = files, refcount = 1000000, packed = true)
Time (mean ± σ): 2.537 s ± 0.034 s [User: 0.488 s, System: 2.048 s]
Range (min … max): 2.511 s … 2.627 s 10 runs
Benchmark 6: show-ref: print all refs (refformat = reftable, refcount = 1000000, packed = true)
Time (mean ± σ): 2.712 s ± 0.017 s [User: 0.653 s, System: 2.059 s]
Range (min … max): 2.692 s … 2.752 s 10 runs
Benchmark 7: show-ref: print all refs (refformat = files, refcount = 1, packed = false)
Time (mean ± σ): 1.6 ms ± 0.1 ms [User: 0.4 ms, System: 1.1 ms]
Range (min … max): 1.5 ms … 1.9 ms 1834 runs
Benchmark 8: show-ref: print all refs (refformat = reftable, refcount = 1, packed = false)
Time (mean ± σ): 1.6 ms ± 0.1 ms [User: 0.4 ms, System: 1.1 ms]
Range (min … max): 1.4 ms … 2.0 ms 1840 runs
Benchmark 9: show-ref: print all refs (refformat = files, refcount = 1000, packed = false)
Time (mean ± σ): 13.8 ms ± 0.2 ms [User: 2.8 ms, System: 10.8 ms]
Range (min … max): 13.3 ms … 14.5 ms 208 runs
Benchmark 10: show-ref: print all refs (refformat = reftable, refcount = 1000, packed = false)
Time (mean ± σ): 4.5 ms ± 0.2 ms [User: 1.2 ms, System: 3.3 ms]
Range (min … max): 4.3 ms … 6.2 ms 624 runs
Benchmark 11: show-ref: print all refs (refformat = files, refcount = 1000000, packed = false)
Time (mean ± σ): 12.127 s ± 0.129 s [User: 2.675 s, System: 9.451 s]
Range (min … max): 11.965 s … 12.370 s 10 runs
Benchmark 12: show-ref: print all refs (refformat = reftable, refcount = 1000000, packed = false)
Time (mean ± σ): 2.799 s ± 0.022 s [User: 0.735 s, System: 2.063 s]
Range (min … max): 2.769 s … 2.836 s 10 runs
- Printing a single ref shows no real difference between the "files"
and "reftable" backends.
Benchmark 1: show-ref: print single ref (refformat = files, refcount = 1)
Time (mean ± σ): 1.5 ms ± 0.1 ms [User: 0.4 ms, System: 1.0 ms]
Range (min … max): 1.4 ms … 1.8 ms 1779 runs
Benchmark 2: show-ref: print single ref (refformat = reftable, refcount = 1)
Time (mean ± σ): 1.6 ms ± 0.1 ms [User: 0.4 ms, System: 1.1 ms]
Range (min … max): 1.4 ms … 2.5 ms 1753 runs
Benchmark 3: show-ref: print single ref (refformat = files, refcount = 1000)
Time (mean ± σ): 1.5 ms ± 0.1 ms [User: 0.3 ms, System: 1.1 ms]
Range (min … max): 1.4 ms … 1.9 ms 1840 runs
Benchmark 4: show-ref: print single ref (refformat = reftable, refcount = 1000)
Time (mean ± σ): 1.6 ms ± 0.1 ms [User: 0.4 ms, System: 1.1 ms]
Range (min … max): 1.5 ms … 2.0 ms 1831 runs
Benchmark 5: show-ref: print single ref (refformat = files, refcount = 1000000)
Time (mean ± σ): 1.6 ms ± 0.1 ms [User: 0.4 ms, System: 1.1 ms]
Range (min … max): 1.5 ms … 2.1 ms 1848 runs
Benchmark 6: show-ref: print single ref (refformat = reftable, refcount = 1000000)
Time (mean ± σ): 1.6 ms ± 0.1 ms [User: 0.4 ms, System: 1.1 ms]
Range (min … max): 1.5 ms … 2.1 ms 1762 runs
So overall, performance depends on the usecases. Except for many
sequential writes the "reftable" backend is roughly on par or
significantly faster than the "files" backend though. Given that the
"files" backend has received 18 years of optimizations by now this can
be seen as a win. Furthermore, we can expect that the "reftable" backend
will grow faster over time when attention turns more towards
optimizations.
The complete test suite passes, except for those tests explicitly marked
to require the REFFILES prerequisite. Some tests in t0610 are marked as
failing because they depend on still-in-flight bug fixes. Tests can be
run with the new backend by setting the GIT_TEST_DEFAULT_REF_FORMAT
environment variable to "reftable".
There is a single known conceptual incompatibility with the dumb HTTP
transport. As "info/refs" SHOULD NOT contain the HEAD reference, and
because the "HEAD" file is not valid anymore, it is impossible for the
remote client to figure out the default branch without changing the
protocol. This shortcoming needs to be handled in a subsequent patch
series.
As the reftable library has already been introduced a while ago, this
commit message will not go into the details of how exactly the on-disk
format works. Please refer to our preexisting technical documentation at
Documentation/technical/reftable for this.
[1]: https://public-inbox.org/git/CAJo=hJtyof=HRy=2sLP0ng0uZ4=S-DpZ5dR1aF+VHVETKG20OQ@mail.gmail.com/
Original-idea-by: Shawn Pearce <spearce@spearce.org>
Based-on-patch-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/ref-storage-format.txt | 2 +
.../technical/repository-version.txt | 5 +-
Makefile | 1 +
contrib/workdir/git-new-workdir | 2 +-
path.c | 2 +-
path.h | 1 +
refs.c | 1 +
refs/refs-internal.h | 1 +
refs/reftable-backend.c | 2297 +++++++++++++++++
repository.h | 5 +-
t/t0610-reftable-basics.sh | 887 +++++++
t/t0611-reftable-httpd.sh | 26 +
t/test-lib.sh | 2 +
13 files changed, 3226 insertions(+), 6 deletions(-)
create mode 100644 refs/reftable-backend.c
create mode 100755 t/t0610-reftable-basics.sh
create mode 100755 t/t0611-reftable-httpd.sh
diff --git a/Documentation/ref-storage-format.txt b/Documentation/ref-storage-format.txt
index 1a65cac468..14fff8a9c6 100644
--- a/Documentation/ref-storage-format.txt
+++ b/Documentation/ref-storage-format.txt
@@ -1 +1,3 @@
* `files` for loose files with packed-refs. This is the default.
+* `reftable` for the reftable format. This format is experimental and its
+ internals are subject to change.
diff --git a/Documentation/technical/repository-version.txt b/Documentation/technical/repository-version.txt
index 27be3741e6..47281420fc 100644
--- a/Documentation/technical/repository-version.txt
+++ b/Documentation/technical/repository-version.txt
@@ -103,5 +103,6 @@ GIT_COMMON_DIR/worktrees/<id>/config.worktree)
==== `refStorage`
-Specifies the file format for the ref database. The only valid value
-is `files` (loose references with a packed-refs file).
+Specifies the file format for the ref database. The valid values are
+`files` (loose references with a packed-refs file) and `reftable` (see
+Documentation/technical/reftable.txt).
diff --git a/Makefile b/Makefile
index 0f748a52e6..b302516173 100644
--- a/Makefile
+++ b/Makefile
@@ -1127,6 +1127,7 @@ LIB_OBJS += reflog.o
LIB_OBJS += refs.o
LIB_OBJS += refs/debug.o
LIB_OBJS += refs/files-backend.o
+LIB_OBJS += refs/reftable-backend.o
LIB_OBJS += refs/iterator.o
LIB_OBJS += refs/packed-backend.o
LIB_OBJS += refs/ref-cache.o
diff --git a/contrib/workdir/git-new-workdir b/contrib/workdir/git-new-workdir
index 888c34a521..989197aace 100755
--- a/contrib/workdir/git-new-workdir
+++ b/contrib/workdir/git-new-workdir
@@ -79,7 +79,7 @@ trap cleanup $siglist
# create the links to the original repo. explicitly exclude index, HEAD and
# logs/HEAD from the list since they are purely related to the current working
# directory, and should not be shared.
-for x in config refs logs/refs objects info hooks packed-refs remotes rr-cache svn
+for x in config refs logs/refs objects info hooks packed-refs remotes rr-cache svn reftable
do
# create a containing directory if needed
case $x in
diff --git a/path.c b/path.c
index 0fb527918b..8bb223c92c 100644
--- a/path.c
+++ b/path.c
@@ -871,7 +871,7 @@ const char *enter_repo(const char *path, int strict)
return NULL;
}
-static int calc_shared_perm(int mode)
+int calc_shared_perm(int mode)
{
int tweak;
diff --git a/path.h b/path.h
index b3233c51fa..e053effef2 100644
--- a/path.h
+++ b/path.h
@@ -181,6 +181,7 @@ const char *git_path_shallow(struct repository *r);
int ends_with_path_components(const char *path, const char *components);
int validate_headref(const char *ref);
+int calc_shared_perm(int mode);
int adjust_shared_perm(const char *path);
char *interpolate_path(const char *path, int real_home);
diff --git a/refs.c b/refs.c
index 3190df8d81..7e6970ca1d 100644
--- a/refs.c
+++ b/refs.c
@@ -35,6 +35,7 @@
*/
static const struct ref_storage_be *refs_backends[] = {
[REF_STORAGE_FORMAT_FILES] = &refs_be_files,
+ [REF_STORAGE_FORMAT_REFTABLE] = &refs_be_reftable,
};
static const struct ref_storage_be *find_ref_storage_backend(unsigned int ref_storage_format)
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index be15c19c02..2285abe1ec 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -700,6 +700,7 @@ struct ref_storage_be {
};
extern struct ref_storage_be refs_be_files;
+extern struct ref_storage_be refs_be_reftable;
extern struct ref_storage_be refs_be_packed;
/*
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
new file mode 100644
index 0000000000..207cf2f8e9
--- /dev/null
+++ b/refs/reftable-backend.c
@@ -0,0 +1,2297 @@
+#include "../git-compat-util.h"
+#include "../abspath.h"
+#include "../chdir-notify.h"
+#include "../environment.h"
+#include "../gettext.h"
+#include "../hash.h"
+#include "../hex.h"
+#include "../iterator.h"
+#include "../ident.h"
+#include "../lockfile.h"
+#include "../object.h"
+#include "../path.h"
+#include "../refs.h"
+#include "../reftable/reftable-stack.h"
+#include "../reftable/reftable-record.h"
+#include "../reftable/reftable-error.h"
+#include "../reftable/reftable-iterator.h"
+#include "../reftable/reftable-merged.h"
+#include "../setup.h"
+#include "../strmap.h"
+#include "refs-internal.h"
+
+/*
+ * Used as a flag in ref_update::flags when the ref_update was via an
+ * update to HEAD.
+ */
+#define REF_UPDATE_VIA_HEAD (1 << 8)
+
+struct reftable_ref_store {
+ struct ref_store base;
+
+ /*
+ * The main stack refers to the common dir and thus contains common
+ * refs as well as refs of the main repository.
+ */
+ struct reftable_stack *main_stack;
+ /*
+ * The worktree stack refers to the gitdir in case the refdb is opened
+ * via a worktree. It thus contains the per-worktree refs.
+ */
+ struct reftable_stack *worktree_stack;
+ /*
+ * Map of worktree stacks by their respective worktree names. The map
+ * is populated lazily when we try to resolve `worktrees/$worktree` refs.
+ */
+ struct strmap worktree_stacks;
+ struct reftable_write_options write_options;
+
+ unsigned int store_flags;
+ int err;
+};
+
+/*
+ * Downcast ref_store to reftable_ref_store. Die if ref_store is not a
+ * reftable_ref_store. required_flags is compared with ref_store's store_flags
+ * to ensure the ref_store has all required capabilities. "caller" is used in
+ * any necessary error messages.
+ */
+static struct reftable_ref_store *reftable_be_downcast(struct ref_store *ref_store,
+ unsigned int required_flags,
+ const char *caller)
+{
+ struct reftable_ref_store *refs;
+
+ if (ref_store->be != &refs_be_reftable)
+ BUG("ref_store is type \"%s\" not \"reftables\" in %s",
+ ref_store->be->name, caller);
+
+ refs = (struct reftable_ref_store *)ref_store;
+
+ if ((refs->store_flags & required_flags) != required_flags)
+ BUG("operation %s requires abilities 0x%x, but only have 0x%x",
+ caller, required_flags, refs->store_flags);
+
+ return refs;
+}
+
+/*
+ * Some refs are global to the repository (refs/heads/{*}), while others are
+ * local to the worktree (eg. HEAD, refs/bisect/{*}). We solve this by having
+ * multiple separate databases (ie. multiple reftable/ directories), one for
+ * the shared refs, one for the current worktree refs, and one for each
+ * additional worktree. For reading, we merge the view of both the shared and
+ * the current worktree's refs, when necessary.
+ *
+ * This function also optionally assigns the rewritten reference name that is
+ * local to the stack. This translation is required when using worktree refs
+ * like `worktrees/$worktree/refs/heads/foo` as worktree stacks will store
+ * those references in their normalized form.
+ */
+static struct reftable_stack *stack_for(struct reftable_ref_store *store,
+ const char *refname,
+ const char **rewritten_ref)
+{
+ const char *wtname;
+ int wtname_len;
+
+ if (!refname)
+ return store->main_stack;
+
+ switch (parse_worktree_ref(refname, &wtname, &wtname_len, rewritten_ref)) {
+ case REF_WORKTREE_OTHER: {
+ static struct strbuf wtname_buf = STRBUF_INIT;
+ struct strbuf wt_dir = STRBUF_INIT;
+ struct reftable_stack *stack;
+
+ /*
+ * We're using a static buffer here so that we don't need to
+ * allocate the worktree name whenever we look up a reference.
+ * This could be avoided if the strmap interface knew how to
+ * handle keys with a length.
+ */
+ strbuf_reset(&wtname_buf);
+ strbuf_add(&wtname_buf, wtname, wtname_len);
+
+ /*
+ * There is an edge case here: when the worktree references the
+ * current worktree, then we set up the stack once via
+ * `worktree_stacks` and once via `worktree_stack`. This is
+ * wasteful, but in the reading case it shouldn't matter. And
+ * in the writing case we would notice that the stack is locked
+ * already and error out when trying to write a reference via
+ * both stacks.
+ */
+ stack = strmap_get(&store->worktree_stacks, wtname_buf.buf);
+ if (!stack) {
+ strbuf_addf(&wt_dir, "%s/worktrees/%s/reftable",
+ store->base.repo->commondir, wtname_buf.buf);
+
+ store->err = reftable_new_stack(&stack, wt_dir.buf,
+ store->write_options);
+ assert(store->err != REFTABLE_API_ERROR);
+ strmap_put(&store->worktree_stacks, wtname_buf.buf, stack);
+ }
+
+ strbuf_release(&wt_dir);
+ return stack;
+ }
+ case REF_WORKTREE_CURRENT:
+ /*
+ * If there is no worktree stack then we're currently in the
+ * main worktree. We thus return the main stack in that case.
+ */
+ if (!store->worktree_stack)
+ return store->main_stack;
+ return store->worktree_stack;
+ case REF_WORKTREE_MAIN:
+ case REF_WORKTREE_SHARED:
+ return store->main_stack;
+ default:
+ BUG("unhandled worktree reference type");
+ }
+}
+
+static int should_write_log(struct ref_store *refs, const char *refname)
+{
+ if (log_all_ref_updates == LOG_REFS_UNSET)
+ log_all_ref_updates = is_bare_repository() ? LOG_REFS_NONE : LOG_REFS_NORMAL;
+
+ switch (log_all_ref_updates) {
+ case LOG_REFS_NONE:
+ return refs_reflog_exists(refs, refname);
+ case LOG_REFS_ALWAYS:
+ return 1;
+ case LOG_REFS_NORMAL:
+ if (should_autocreate_reflog(refname))
+ return 1;
+ return refs_reflog_exists(refs, refname);
+ default:
+ BUG("unhandled core.logAllRefUpdates value %d", log_all_ref_updates);
+ }
+}
+
+static void clear_reftable_log_record(struct reftable_log_record *log)
+{
+ switch (log->value_type) {
+ case REFTABLE_LOG_UPDATE:
+ /*
+ * When we write log records, the hashes are owned by the
+ * caller and thus shouldn't be free'd.
+ */
+ log->value.update.old_hash = NULL;
+ log->value.update.new_hash = NULL;
+ break;
+ case REFTABLE_LOG_DELETION:
+ break;
+ }
+ reftable_log_record_release(log);
+}
+
+static void fill_reftable_log_record(struct reftable_log_record *log)
+{
+ const char *info = git_committer_info(0);
+ struct ident_split split = {0};
+ int sign = 1;
+
+ if (split_ident_line(&split, info, strlen(info)))
+ BUG("failed splitting committer info");
+
+ reftable_log_record_release(log);
+ log->value_type = REFTABLE_LOG_UPDATE;
+ log->value.update.name =
+ xstrndup(split.name_begin, split.name_end - split.name_begin);
+ log->value.update.email =
+ xstrndup(split.mail_begin, split.mail_end - split.mail_begin);
+ log->value.update.time = atol(split.date_begin);
+ if (*split.tz_begin == '-') {
+ sign = -1;
+ split.tz_begin++;
+ }
+ if (*split.tz_begin == '+') {
+ sign = 1;
+ split.tz_begin++;
+ }
+
+ log->value.update.tz_offset = sign * atoi(split.tz_begin);
+}
+
+static int read_ref_without_reload(struct reftable_stack *stack,
+ const char *refname,
+ struct object_id *oid,
+ struct strbuf *referent,
+ unsigned int *type)
+{
+ struct reftable_ref_record ref = {0};
+ int ret;
+
+ ret = reftable_stack_read_ref(stack, refname, &ref);
+ if (ret)
+ goto done;
+
+ if (ref.value_type == REFTABLE_REF_SYMREF) {
+ strbuf_reset(referent);
+ strbuf_addstr(referent, ref.value.symref);
+ *type |= REF_ISSYMREF;
+ } else if (reftable_ref_record_val1(&ref)) {
+ oidread(oid, reftable_ref_record_val1(&ref));
+ } else {
+ /* We got a tombstone, which should not happen. */
+ BUG("unhandled reference value type %d", ref.value_type);
+ }
+
+done:
+ assert(ret != REFTABLE_API_ERROR);
+ reftable_ref_record_release(&ref);
+ return ret;
+}
+
+static struct ref_store *reftable_be_init(struct repository *repo,
+ const char *gitdir,
+ unsigned int store_flags)
+{
+ struct reftable_ref_store *refs = xcalloc(1, sizeof(*refs));
+ struct strbuf path = STRBUF_INIT;
+ int is_worktree;
+ mode_t mask;
+
+ mask = umask(0);
+ umask(mask);
+
+ base_ref_store_init(&refs->base, repo, gitdir, &refs_be_reftable);
+ strmap_init(&refs->worktree_stacks);
+ refs->store_flags = store_flags;
+ refs->write_options.block_size = 4096;
+ refs->write_options.hash_id = repo->hash_algo->format_id;
+ refs->write_options.default_permissions = calc_shared_perm(0666 & ~mask);
+
+ /*
+ * Set up the main reftable stack that is hosted in GIT_COMMON_DIR.
+ * This stack contains both the shared and the main worktree refs.
+ *
+ * Note that we don't try to resolve the path in case we have a
+ * worktree because `get_common_dir_noenv()` already does it for us.
+ */
+ is_worktree = get_common_dir_noenv(&path, gitdir);
+ if (!is_worktree) {
+ strbuf_reset(&path);
+ strbuf_realpath(&path, gitdir, 0);
+ }
+ strbuf_addstr(&path, "/reftable");
+ refs->err = reftable_new_stack(&refs->main_stack, path.buf,
+ refs->write_options);
+ if (refs->err)
+ goto done;
+
+ /*
+ * If we're in a worktree we also need to set up the worktree reftable
+ * stack that is contained in the per-worktree GIT_DIR.
+ *
+ * Ideally, we would also add the stack to our worktree stack map. But
+ * we have no way to figure out the worktree name here and thus can't
+ * do it efficiently.
+ */
+ if (is_worktree) {
+ strbuf_reset(&path);
+ strbuf_addf(&path, "%s/reftable", gitdir);
+
+ refs->err = reftable_new_stack(&refs->worktree_stack, path.buf,
+ refs->write_options);
+ if (refs->err)
+ goto done;
+ }
+
+ chdir_notify_reparent("reftables-backend $GIT_DIR", &refs->base.gitdir);
+
+done:
+ assert(refs->err != REFTABLE_API_ERROR);
+ strbuf_release(&path);
+ return &refs->base;
+}
+
+static int reftable_be_init_db(struct ref_store *ref_store,
+ int flags UNUSED,
+ struct strbuf *err UNUSED)
+{
+ struct reftable_ref_store *refs =
+ reftable_be_downcast(ref_store, REF_STORE_WRITE, "init_db");
+ struct strbuf sb = STRBUF_INIT;
+
+ strbuf_addf(&sb, "%s/reftable", refs->base.gitdir);
+ safe_create_dir(sb.buf, 1);
+ strbuf_reset(&sb);
+
+ strbuf_addf(&sb, "%s/HEAD", refs->base.gitdir);
+ write_file(sb.buf, "ref: refs/heads/.invalid");
+ adjust_shared_perm(sb.buf);
+ strbuf_reset(&sb);
+
+ strbuf_addf(&sb, "%s/refs", refs->base.gitdir);
+ safe_create_dir(sb.buf, 1);
+ strbuf_reset(&sb);
+
+ strbuf_addf(&sb, "%s/refs/heads", refs->base.gitdir);
+ write_file(sb.buf, "this repository uses the reftable format");
+ adjust_shared_perm(sb.buf);
+
+ strbuf_release(&sb);
+ return 0;
+}
+
+struct reftable_ref_iterator {
+ struct ref_iterator base;
+ struct reftable_ref_store *refs;
+ struct reftable_iterator iter;
+ struct reftable_ref_record ref;
+ struct object_id oid;
+
+ const char *prefix;
+ unsigned int flags;
+ int err;
+};
+
+static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator)
+{
+ struct reftable_ref_iterator *iter =
+ (struct reftable_ref_iterator *)ref_iterator;
+ struct reftable_ref_store *refs = iter->refs;
+
+ while (!iter->err) {
+ int flags = 0;
+
+ iter->err = reftable_iterator_next_ref(&iter->iter, &iter->ref);
+ if (iter->err)
+ break;
+
+ /*
+ * Unless DO_FOR_EACH_INCLUDE_ALL_REFS is set, we only list
+ * refs starting with "refs/" to mimic the "files" backend.
+ */
+ if (!(iter->flags & DO_FOR_EACH_INCLUDE_ALL_REFS) &&
+ !starts_with(iter->ref.refname, "refs/"))
+ continue;
+
+ if (iter->prefix &&
+ strncmp(iter->prefix, iter->ref.refname, strlen(iter->prefix))) {
+ iter->err = 1;
+ break;
+ }
+
+ if (iter->flags & DO_FOR_EACH_PER_WORKTREE_ONLY &&
+ parse_worktree_ref(iter->ref.refname, NULL, NULL, NULL) !=
+ REF_WORKTREE_CURRENT)
+ continue;
+
+ switch (iter->ref.value_type) {
+ case REFTABLE_REF_VAL1:
+ oidread(&iter->oid, iter->ref.value.val1);
+ break;
+ case REFTABLE_REF_VAL2:
+ oidread(&iter->oid, iter->ref.value.val2.value);
+ break;
+ case REFTABLE_REF_SYMREF:
+ if (!refs_resolve_ref_unsafe(&iter->refs->base, iter->ref.refname,
+ RESOLVE_REF_READING, &iter->oid, &flags))
+ oidclr(&iter->oid);
+ break;
+ default:
+ BUG("unhandled reference value type %d", iter->ref.value_type);
+ }
+
+ if (is_null_oid(&iter->oid))
+ flags |= REF_ISBROKEN;
+
+ if (check_refname_format(iter->ref.refname, REFNAME_ALLOW_ONELEVEL)) {
+ if (!refname_is_safe(iter->ref.refname))
+ die(_("refname is dangerous: %s"), iter->ref.refname);
+ oidclr(&iter->oid);
+ flags |= REF_BAD_NAME | REF_ISBROKEN;
+ }
+
+ if (iter->flags & DO_FOR_EACH_OMIT_DANGLING_SYMREFS &&
+ flags & REF_ISSYMREF &&
+ flags & REF_ISBROKEN)
+ continue;
+
+ if (!(iter->flags & DO_FOR_EACH_INCLUDE_BROKEN) &&
+ !ref_resolves_to_object(iter->ref.refname, refs->base.repo,
+ &iter->oid, flags))
+ continue;
+
+ iter->base.refname = iter->ref.refname;
+ iter->base.oid = &iter->oid;
+ iter->base.flags = flags;
+
+ break;
+ }
+
+ if (iter->err > 0) {
+ if (ref_iterator_abort(ref_iterator) != ITER_DONE)
+ return ITER_ERROR;
+ return ITER_DONE;
+ }
+
+ if (iter->err < 0) {
+ ref_iterator_abort(ref_iterator);
+ return ITER_ERROR;
+ }
+
+ return ITER_OK;
+}
+
+static int reftable_ref_iterator_peel(struct ref_iterator *ref_iterator,
+ struct object_id *peeled)
+{
+ struct reftable_ref_iterator *iter =
+ (struct reftable_ref_iterator *)ref_iterator;
+
+ if (iter->ref.value_type == REFTABLE_REF_VAL2) {
+ oidread(peeled, iter->ref.value.val2.target_value);
+ return 0;
+ }
+
+ return -1;
+}
+
+static int reftable_ref_iterator_abort(struct ref_iterator *ref_iterator)
+{
+ struct reftable_ref_iterator *iter =
+ (struct reftable_ref_iterator *)ref_iterator;
+ reftable_ref_record_release(&iter->ref);
+ reftable_iterator_destroy(&iter->iter);
+ free(iter);
+ return ITER_DONE;
+}
+
+static struct ref_iterator_vtable reftable_ref_iterator_vtable = {
+ .advance = reftable_ref_iterator_advance,
+ .peel = reftable_ref_iterator_peel,
+ .abort = reftable_ref_iterator_abort
+};
+
+static struct reftable_ref_iterator *ref_iterator_for_stack(struct reftable_ref_store *refs,
+ struct reftable_stack *stack,
+ const char *prefix,
+ int flags)
+{
+ struct reftable_merged_table *merged_table;
+ struct reftable_ref_iterator *iter;
+ int ret;
+
+ iter = xcalloc(1, sizeof(*iter));
+ base_ref_iterator_init(&iter->base, &reftable_ref_iterator_vtable, 1);
+ iter->prefix = prefix;
+ iter->base.oid = &iter->oid;
+ iter->flags = flags;
+ iter->refs = refs;
+
+ ret = refs->err;
+ if (ret)
+ goto done;
+
+ ret = reftable_stack_reload(stack);
+ if (ret)
+ goto done;
+
+ merged_table = reftable_stack_merged_table(stack);
+
+ ret = reftable_merged_table_seek_ref(merged_table, &iter->iter, prefix);
+ if (ret)
+ goto done;
+
+done:
+ iter->err = ret;
+ return iter;
+}
+
+static enum iterator_selection iterator_select(struct ref_iterator *iter_worktree,
+ struct ref_iterator *iter_common,
+ void *cb_data UNUSED)
+{
+ if (iter_worktree && !iter_common) {
+ /*
+ * Return the worktree ref if there are no more common refs.
+ */
+ return ITER_SELECT_0;
+ } else if (iter_common) {
+ /*
+ * In case we have pending worktree and common refs we need to
+ * yield them based on their lexicographical order. Worktree
+ * refs that have the same name as common refs shadow the
+ * latter.
+ */
+ if (iter_worktree) {
+ int cmp = strcmp(iter_worktree->refname,
+ iter_common->refname);
+ if (cmp < 0)
+ return ITER_SELECT_0;
+ else if (!cmp)
+ return ITER_SELECT_0_SKIP_1;
+ }
+
+ /*
+ * We now know that the lexicographically-next ref is a common
+ * ref. When the common ref is a shared one we return it.
+ */
+ if (parse_worktree_ref(iter_common->refname, NULL, NULL,
+ NULL) == REF_WORKTREE_SHARED)
+ return ITER_SELECT_1;
+
+ /*
+ * Otherwise, if the common ref is a per-worktree ref we skip
+ * it because it would belong to the main worktree, not ours.
+ */
+ return ITER_SKIP_1;
+ } else {
+ return ITER_DONE;
+ }
+}
+
+static struct ref_iterator *reftable_be_iterator_begin(struct ref_store *ref_store,
+ const char *prefix,
+ const char **exclude_patterns,
+ unsigned int flags)
+{
+ struct reftable_ref_iterator *main_iter, *worktree_iter;
+ struct reftable_ref_store *refs;
+ unsigned int required_flags = REF_STORE_READ;
+
+ if (!(flags & DO_FOR_EACH_INCLUDE_BROKEN))
+ required_flags |= REF_STORE_ODB;
+ refs = reftable_be_downcast(ref_store, required_flags, "ref_iterator_begin");
+
+ main_iter = ref_iterator_for_stack(refs, refs->main_stack, prefix, flags);
+
+ /*
+ * The worktree stack is only set when we're in an actual worktree
+ * right now. If we aren't, then we return the common reftable
+ * iterator, only.
+ */
+ if (!refs->worktree_stack)
+ return &main_iter->base;
+
+ /*
+ * Otherwise we merge both the common and the per-worktree refs into a
+ * single iterator.
+ */
+ worktree_iter = ref_iterator_for_stack(refs, refs->worktree_stack, prefix, flags);
+ return merge_ref_iterator_begin(1, &worktree_iter->base, &main_iter->base,
+ iterator_select, NULL);
+}
+
+static int reftable_be_read_raw_ref(struct ref_store *ref_store,
+ const char *refname,
+ struct object_id *oid,
+ struct strbuf *referent,
+ unsigned int *type,
+ int *failure_errno)
+{
+ struct reftable_ref_store *refs =
+ reftable_be_downcast(ref_store, REF_STORE_READ, "read_raw_ref");
+ struct reftable_stack *stack = stack_for(refs, refname, &refname);
+ int ret;
+
+ if (refs->err < 0)
+ return refs->err;
+
+ ret = reftable_stack_reload(stack);
+ if (ret)
+ return ret;
+
+ ret = read_ref_without_reload(stack, refname, oid, referent, type);
+ if (ret < 0)
+ return ret;
+ if (ret > 0) {
+ *failure_errno = ENOENT;
+ return -1;
+ }
+
+ return 0;
+}
+
+static int reftable_be_read_symbolic_ref(struct ref_store *ref_store,
+ const char *refname,
+ struct strbuf *referent)
+{
+ struct reftable_ref_store *refs =
+ reftable_be_downcast(ref_store, REF_STORE_READ, "read_symbolic_ref");
+ struct reftable_stack *stack = stack_for(refs, refname, &refname);
+ struct reftable_ref_record ref = {0};
+ int ret;
+
+ ret = reftable_stack_reload(stack);
+ if (ret)
+ return ret;
+
+ ret = reftable_stack_read_ref(stack, refname, &ref);
+ if (ret == 0 && ref.value_type == REFTABLE_REF_SYMREF)
+ strbuf_addstr(referent, ref.value.symref);
+ else
+ ret = -1;
+
+ reftable_ref_record_release(&ref);
+ return ret;
+}
+
+/*
+ * Return the refname under which update was originally requested.
+ */
+static const char *original_update_refname(struct ref_update *update)
+{
+ while (update->parent_update)
+ update = update->parent_update;
+ return update->refname;
+}
+
+struct reftable_transaction_update {
+ struct ref_update *update;
+ struct object_id current_oid;
+};
+
+struct write_transaction_table_arg {
+ struct reftable_ref_store *refs;
+ struct reftable_stack *stack;
+ struct reftable_addition *addition;
+ struct reftable_transaction_update *updates;
+ size_t updates_nr;
+ size_t updates_alloc;
+ size_t updates_expected;
+};
+
+struct reftable_transaction_data {
+ struct write_transaction_table_arg *args;
+ size_t args_nr, args_alloc;
+};
+
+static void free_transaction_data(struct reftable_transaction_data *tx_data)
+{
+ if (!tx_data)
+ return;
+ for (size_t i = 0; i < tx_data->args_nr; i++) {
+ reftable_addition_destroy(tx_data->args[i].addition);
+ free(tx_data->args[i].updates);
+ }
+ free(tx_data->args);
+ free(tx_data);
+}
+
+/*
+ * Prepare transaction update for the given reference update. This will cause
+ * us to lock the corresponding reftable stack for concurrent modification.
+ */
+static int prepare_transaction_update(struct write_transaction_table_arg **out,
+ struct reftable_ref_store *refs,
+ struct reftable_transaction_data *tx_data,
+ struct ref_update *update,
+ struct strbuf *err)
+{
+ struct reftable_stack *stack = stack_for(refs, update->refname, NULL);
+ struct write_transaction_table_arg *arg = NULL;
+ size_t i;
+ int ret;
+
+ /*
+ * Search for a preexisting stack update. If there is one then we add
+ * the update to it, otherwise we set up a new stack update.
+ */
+ for (i = 0; !arg && i < tx_data->args_nr; i++)
+ if (tx_data->args[i].stack == stack)
+ arg = &tx_data->args[i];
+
+ if (!arg) {
+ struct reftable_addition *addition;
+
+ ret = reftable_stack_reload(stack);
+ if (ret)
+ return ret;
+
+ ret = reftable_stack_new_addition(&addition, stack);
+ if (ret) {
+ if (ret == REFTABLE_LOCK_ERROR)
+ strbuf_addstr(err, "cannot lock references");
+ return ret;
+ }
+
+ ALLOC_GROW(tx_data->args, tx_data->args_nr + 1,
+ tx_data->args_alloc);
+ arg = &tx_data->args[tx_data->args_nr++];
+ arg->refs = refs;
+ arg->stack = stack;
+ arg->addition = addition;
+ arg->updates = NULL;
+ arg->updates_nr = 0;
+ arg->updates_alloc = 0;
+ arg->updates_expected = 0;
+ }
+
+ arg->updates_expected++;
+
+ if (out)
+ *out = arg;
+
+ return 0;
+}
+
+/*
+ * Queue a reference update for the correct stack. We potentially need to
+ * handle multiple stack updates in a single transaction when it spans across
+ * multiple worktrees.
+ */
+static int queue_transaction_update(struct reftable_ref_store *refs,
+ struct reftable_transaction_data *tx_data,
+ struct ref_update *update,
+ struct object_id *current_oid,
+ struct strbuf *err)
+{
+ struct write_transaction_table_arg *arg = NULL;
+ int ret;
+
+ if (update->backend_data)
+ BUG("reference update queued more than once");
+
+ ret = prepare_transaction_update(&arg, refs, tx_data, update, err);
+ if (ret < 0)
+ return ret;
+
+ ALLOC_GROW(arg->updates, arg->updates_nr + 1,
+ arg->updates_alloc);
+ arg->updates[arg->updates_nr].update = update;
+ oidcpy(&arg->updates[arg->updates_nr].current_oid, current_oid);
+ update->backend_data = &arg->updates[arg->updates_nr++];
+
+ return 0;
+}
+
+static int reftable_be_transaction_prepare(struct ref_store *ref_store,
+ struct ref_transaction *transaction,
+ struct strbuf *err)
+{
+ struct reftable_ref_store *refs =
+ reftable_be_downcast(ref_store, REF_STORE_WRITE|REF_STORE_MAIN, "ref_transaction_prepare");
+ struct strbuf referent = STRBUF_INIT, head_referent = STRBUF_INIT;
+ struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
+ struct reftable_transaction_data *tx_data = NULL;
+ struct object_id head_oid;
+ unsigned int head_type = 0;
+ size_t i;
+ int ret;
+
+ ret = refs->err;
+ if (ret < 0)
+ goto done;
+
+ tx_data = xcalloc(1, sizeof(*tx_data));
+
+ /*
+ * Preprocess all updates. For one we check that there are no duplicate
+ * reference updates in this transaction. Second, we lock all stacks
+ * that will be modified during the transaction.
+ */
+ for (i = 0; i < transaction->nr; i++) {
+ ret = prepare_transaction_update(NULL, refs, tx_data,
+ transaction->updates[i], err);
+ if (ret)
+ goto done;
+
+ string_list_append(&affected_refnames,
+ transaction->updates[i]->refname);
+ }
+
+ /*
+ * Now that we have counted updates per stack we can preallocate their
+ * arrays. This avoids having to reallocate many times.
+ */
+ for (i = 0; i < tx_data->args_nr; i++) {
+ CALLOC_ARRAY(tx_data->args[i].updates, tx_data->args[i].updates_expected);
+ tx_data->args[i].updates_alloc = tx_data->args[i].updates_expected;
+ }
+
+ /*
+ * Fail if a refname appears more than once in the transaction.
+ * This code is taken from the files backend and is a good candidate to
+ * be moved into the generic layer.
+ */
+ string_list_sort(&affected_refnames);
+ if (ref_update_reject_duplicates(&affected_refnames, err)) {
+ ret = TRANSACTION_GENERIC_ERROR;
+ goto done;
+ }
+
+ ret = read_ref_without_reload(stack_for(refs, "HEAD", NULL), "HEAD", &head_oid,
+ &head_referent, &head_type);
+ if (ret < 0)
+ goto done;
+
+ for (i = 0; i < transaction->nr; i++) {
+ struct ref_update *u = transaction->updates[i];
+ struct object_id current_oid = {0};
+ struct reftable_stack *stack;
+ const char *rewritten_ref;
+
+ stack = stack_for(refs, u->refname, &rewritten_ref);
+
+ /* Verify that the new object ID is valid. */
+ if ((u->flags & REF_HAVE_NEW) && !is_null_oid(&u->new_oid) &&
+ !(u->flags & REF_SKIP_OID_VERIFICATION) &&
+ !(u->flags & REF_LOG_ONLY)) {
+ struct object *o = parse_object(refs->base.repo, &u->new_oid);
+ if (!o) {
+ strbuf_addf(err,
+ _("trying to write ref '%s' with nonexistent object %s"),
+ u->refname, oid_to_hex(&u->new_oid));
+ ret = -1;
+ goto done;
+ }
+
+ if (o->type != OBJ_COMMIT && is_branch(u->refname)) {
+ strbuf_addf(err, _("trying to write non-commit object %s to branch '%s'"),
+ oid_to_hex(&u->new_oid), u->refname);
+ ret = -1;
+ goto done;
+ }
+ }
+
+ /*
+ * When we update the reference that HEAD points to we enqueue
+ * a second log-only update for HEAD so that its reflog is
+ * updated accordingly.
+ */
+ if (head_type == REF_ISSYMREF &&
+ !(u->flags & REF_LOG_ONLY) &&
+ !(u->flags & REF_UPDATE_VIA_HEAD) &&
+ !strcmp(rewritten_ref, head_referent.buf)) {
+ struct ref_update *new_update;
+
+ /*
+ * First make sure that HEAD is not already in the
+ * transaction. This check is O(lg N) in the transaction
+ * size, but it happens at most once per transaction.
+ */
+ if (string_list_has_string(&affected_refnames, "HEAD")) {
+ /* An entry already existed */
+ strbuf_addf(err,
+ _("multiple updates for 'HEAD' (including one "
+ "via its referent '%s') are not allowed"),
+ u->refname);
+ ret = TRANSACTION_NAME_CONFLICT;
+ goto done;
+ }
+
+ new_update = ref_transaction_add_update(
+ transaction, "HEAD",
+ u->flags | REF_LOG_ONLY | REF_NO_DEREF,
+ &u->new_oid, &u->old_oid, u->msg);
+ string_list_insert(&affected_refnames, new_update->refname);
+ }
+
+ ret = read_ref_without_reload(stack, rewritten_ref,
+ ¤t_oid, &referent, &u->type);
+ if (ret < 0)
+ goto done;
+ if (ret > 0 && (!(u->flags & REF_HAVE_OLD) || is_null_oid(&u->old_oid))) {
+ /*
+ * The reference does not exist, and we either have no
+ * old object ID or expect the reference to not exist.
+ * We can thus skip below safety checks as well as the
+ * symref splitting. But we do want to verify that
+ * there is no conflicting reference here so that we
+ * can output a proper error message instead of failing
+ * at a later point.
+ */
+ ret = refs_verify_refname_available(ref_store, u->refname,
+ &affected_refnames, NULL, err);
+ if (ret < 0)
+ goto done;
+
+ /*
+ * There is no need to write the reference deletion
+ * when the reference in question doesn't exist.
+ */
+ if (u->flags & REF_HAVE_NEW && !is_null_oid(&u->new_oid)) {
+ ret = queue_transaction_update(refs, tx_data, u,
+ ¤t_oid, err);
+ if (ret)
+ goto done;
+ }
+
+ continue;
+ }
+ if (ret > 0) {
+ /* The reference does not exist, but we expected it to. */
+ strbuf_addf(err, _("cannot lock ref '%s': "
+ "unable to resolve reference '%s'"),
+ original_update_refname(u), u->refname);
+ ret = -1;
+ goto done;
+ }
+
+ if (u->type & REF_ISSYMREF) {
+ /*
+ * The reftable stack is locked at this point already,
+ * so it is safe to call `refs_resolve_ref_unsafe()`
+ * here without causing races.
+ */
+ const char *resolved = refs_resolve_ref_unsafe(&refs->base, u->refname, 0,
+ ¤t_oid, NULL);
+
+ if (u->flags & REF_NO_DEREF) {
+ if (u->flags & REF_HAVE_OLD && !resolved) {
+ strbuf_addf(err, _("cannot lock ref '%s': "
+ "error reading reference"), u->refname);
+ ret = -1;
+ goto done;
+ }
+ } else {
+ struct ref_update *new_update;
+ int new_flags;
+
+ new_flags = u->flags;
+ if (!strcmp(rewritten_ref, "HEAD"))
+ new_flags |= REF_UPDATE_VIA_HEAD;
+
+ /*
+ * If we are updating a symref (eg. HEAD), we should also
+ * update the branch that the symref points to.
+ *
+ * This is generic functionality, and would be better
+ * done in refs.c, but the current implementation is
+ * intertwined with the locking in files-backend.c.
+ */
+ new_update = ref_transaction_add_update(
+ transaction, referent.buf, new_flags,
+ &u->new_oid, &u->old_oid, u->msg);
+ new_update->parent_update = u;
+
+ /*
+ * Change the symbolic ref update to log only. Also, it
+ * doesn't need to check its old OID value, as that will be
+ * done when new_update is processed.
+ */
+ u->flags |= REF_LOG_ONLY | REF_NO_DEREF;
+ u->flags &= ~REF_HAVE_OLD;
+
+ if (string_list_has_string(&affected_refnames, new_update->refname)) {
+ strbuf_addf(err,
+ _("multiple updates for '%s' (including one "
+ "via symref '%s') are not allowed"),
+ referent.buf, u->refname);
+ ret = TRANSACTION_NAME_CONFLICT;
+ goto done;
+ }
+ string_list_insert(&affected_refnames, new_update->refname);
+ }
+ }
+
+ /*
+ * Verify that the old object matches our expectations. Note
+ * that the error messages here do not make a lot of sense in
+ * the context of the reftable backend as we never lock
+ * individual refs. But the error messages match what the files
+ * backend returns, which keeps our tests happy.
+ */
+ if (u->flags & REF_HAVE_OLD && !oideq(¤t_oid, &u->old_oid)) {
+ if (is_null_oid(&u->old_oid))
+ strbuf_addf(err, _("cannot lock ref '%s': "
+ "reference already exists"),
+ original_update_refname(u));
+ else if (is_null_oid(¤t_oid))
+ strbuf_addf(err, _("cannot lock ref '%s': "
+ "reference is missing but expected %s"),
+ original_update_refname(u),
+ oid_to_hex(&u->old_oid));
+ else
+ strbuf_addf(err, _("cannot lock ref '%s': "
+ "is at %s but expected %s"),
+ original_update_refname(u),
+ oid_to_hex(¤t_oid),
+ oid_to_hex(&u->old_oid));
+ ret = -1;
+ goto done;
+ }
+
+ /*
+ * If all of the following conditions are true:
+ *
+ * - We're not about to write a symref.
+ * - We're not about to write a log-only entry.
+ * - Old and new object ID are different.
+ *
+ * Then we're essentially doing a no-op update that can be
+ * skipped. This is not only for the sake of efficiency, but
+ * also skips writing unneeded reflog entries.
+ */
+ if ((u->type & REF_ISSYMREF) ||
+ (u->flags & REF_LOG_ONLY) ||
+ (u->flags & REF_HAVE_NEW && !oideq(¤t_oid, &u->new_oid))) {
+ ret = queue_transaction_update(refs, tx_data, u,
+ ¤t_oid, err);
+ if (ret)
+ goto done;
+ }
+ }
+
+ transaction->backend_data = tx_data;
+ transaction->state = REF_TRANSACTION_PREPARED;
+
+done:
+ assert(ret != REFTABLE_API_ERROR);
+ if (ret < 0) {
+ free_transaction_data(tx_data);
+ transaction->state = REF_TRANSACTION_CLOSED;
+ if (!err->len)
+ strbuf_addf(err, _("reftable: transaction prepare: %s"),
+ reftable_error_str(ret));
+ }
+ string_list_clear(&affected_refnames, 0);
+ strbuf_release(&referent);
+ strbuf_release(&head_referent);
+
+ return ret;
+}
+
+static int reftable_be_transaction_abort(struct ref_store *ref_store,
+ struct ref_transaction *transaction,
+ struct strbuf *err)
+{
+ struct reftable_transaction_data *tx_data = transaction->backend_data;
+ free_transaction_data(tx_data);
+ transaction->state = REF_TRANSACTION_CLOSED;
+ return 0;
+}
+
+static int transaction_update_cmp(const void *a, const void *b)
+{
+ return strcmp(((struct reftable_transaction_update *)a)->update->refname,
+ ((struct reftable_transaction_update *)b)->update->refname);
+}
+
+static int write_transaction_table(struct reftable_writer *writer, void *cb_data)
+{
+ struct write_transaction_table_arg *arg = cb_data;
+ struct reftable_merged_table *mt =
+ reftable_stack_merged_table(arg->stack);
+ uint64_t ts = reftable_stack_next_update_index(arg->stack);
+ struct reftable_log_record *logs = NULL;
+ size_t logs_nr = 0, logs_alloc = 0, i;
+ int ret = 0;
+
+ QSORT(arg->updates, arg->updates_nr, transaction_update_cmp);
+
+ reftable_writer_set_limits(writer, ts, ts);
+
+ for (i = 0; i < arg->updates_nr; i++) {
+ struct reftable_transaction_update *tx_update = &arg->updates[i];
+ struct ref_update *u = tx_update->update;
+
+ /*
+ * Write a reflog entry when updating a ref to point to
+ * something new in either of the following cases:
+ *
+ * - The reference is about to be deleted. We always want to
+ * delete the reflog in that case.
+ * - REF_FORCE_CREATE_REFLOG is set, asking us to always create
+ * the reflog entry.
+ * - `core.logAllRefUpdates` tells us to create the reflog for
+ * the given ref.
+ */
+ if (u->flags & REF_HAVE_NEW && !(u->type & REF_ISSYMREF) && is_null_oid(&u->new_oid)) {
+ struct reftable_log_record log = {0};
+ struct reftable_iterator it = {0};
+
+ /*
+ * When deleting refs we also delete all reflog entries
+ * with them. While it is not strictly required to
+ * delete reflogs together with their refs, this
+ * matches the behaviour of the files backend.
+ *
+ * Unfortunately, we have no better way than to delete
+ * all reflog entries one by one.
+ */
+ ret = reftable_merged_table_seek_log(mt, &it, u->refname);
+ while (ret == 0) {
+ struct reftable_log_record *tombstone;
+
+ ret = reftable_iterator_next_log(&it, &log);
+ if (ret < 0)
+ break;
+ if (ret > 0 || strcmp(log.refname, u->refname)) {
+ ret = 0;
+ break;
+ }
+
+ ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
+ tombstone = &logs[logs_nr++];
+ tombstone->refname = xstrdup(u->refname);
+ tombstone->value_type = REFTABLE_LOG_DELETION;
+ tombstone->update_index = log.update_index;
+ }
+
+ reftable_log_record_release(&log);
+ reftable_iterator_destroy(&it);
+
+ if (ret)
+ goto done;
+ } else if (u->flags & REF_HAVE_NEW &&
+ (u->flags & REF_FORCE_CREATE_REFLOG ||
+ should_write_log(&arg->refs->base, u->refname))) {
+ struct reftable_log_record *log;
+
+ ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
+ log = &logs[logs_nr++];
+ memset(log, 0, sizeof(*log));
+
+ fill_reftable_log_record(log);
+ log->update_index = ts;
+ log->refname = xstrdup(u->refname);
+ log->value.update.new_hash = u->new_oid.hash;
+ log->value.update.old_hash = tx_update->current_oid.hash;
+ log->value.update.message =
+ xstrndup(u->msg, arg->refs->write_options.block_size / 2);
+ }
+
+ if (u->flags & REF_LOG_ONLY)
+ continue;
+
+ if (u->flags & REF_HAVE_NEW && is_null_oid(&u->new_oid)) {
+ struct reftable_ref_record ref = {
+ .refname = (char *)u->refname,
+ .update_index = ts,
+ .value_type = REFTABLE_REF_DELETION,
+ };
+
+ ret = reftable_writer_add_ref(writer, &ref);
+ if (ret < 0)
+ goto done;
+ } else if (u->flags & REF_HAVE_NEW) {
+ struct reftable_ref_record ref = {0};
+ struct object_id peeled;
+ int peel_error;
+
+ ref.refname = (char *)u->refname;
+ ref.update_index = ts;
+
+ peel_error = peel_object(&u->new_oid, &peeled);
+ if (!peel_error) {
+ ref.value_type = REFTABLE_REF_VAL2;
+ memcpy(ref.value.val2.target_value, peeled.hash, GIT_MAX_RAWSZ);
+ memcpy(ref.value.val2.value, u->new_oid.hash, GIT_MAX_RAWSZ);
+ } else if (!is_null_oid(&u->new_oid)) {
+ ref.value_type = REFTABLE_REF_VAL1;
+ memcpy(ref.value.val1, u->new_oid.hash, GIT_MAX_RAWSZ);
+ }
+
+ ret = reftable_writer_add_ref(writer, &ref);
+ if (ret < 0)
+ goto done;
+ }
+ }
+
+ /*
+ * Logs are written at the end so that we do not have intermixed ref
+ * and log blocks.
+ */
+ if (logs) {
+ ret = reftable_writer_add_logs(writer, logs, logs_nr);
+ if (ret < 0)
+ goto done;
+ }
+
+done:
+ assert(ret != REFTABLE_API_ERROR);
+ for (i = 0; i < logs_nr; i++)
+ clear_reftable_log_record(&logs[i]);
+ free(logs);
+ return ret;
+}
+
+static int reftable_be_transaction_finish(struct ref_store *ref_store,
+ struct ref_transaction *transaction,
+ struct strbuf *err)
+{
+ struct reftable_transaction_data *tx_data = transaction->backend_data;
+ int ret = 0;
+
+ for (size_t i = 0; i < tx_data->args_nr; i++) {
+ ret = reftable_addition_add(tx_data->args[i].addition,
+ write_transaction_table, &tx_data->args[i]);
+ if (ret < 0)
+ goto done;
+
+ ret = reftable_addition_commit(tx_data->args[i].addition);
+ if (ret < 0)
+ goto done;
+ }
+
+done:
+ assert(ret != REFTABLE_API_ERROR);
+ free_transaction_data(tx_data);
+ transaction->state = REF_TRANSACTION_CLOSED;
+
+ if (ret) {
+ strbuf_addf(err, _("reftable: transaction failure: %s"),
+ reftable_error_str(ret));
+ return -1;
+ }
+ return ret;
+}
+
+static int reftable_be_initial_transaction_commit(struct ref_store *ref_store UNUSED,
+ struct ref_transaction *transaction,
+ struct strbuf *err)
+{
+ return ref_transaction_commit(transaction, err);
+}
+
+static int reftable_be_pack_refs(struct ref_store *ref_store,
+ struct pack_refs_opts *opts)
+{
+ struct reftable_ref_store *refs =
+ reftable_be_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB, "pack_refs");
+ struct reftable_stack *stack;
+ int ret;
+
+ if (refs->err)
+ return refs->err;
+
+ stack = refs->worktree_stack;
+ if (!stack)
+ stack = refs->main_stack;
+
+ ret = reftable_stack_compact_all(stack, NULL);
+ if (ret)
+ goto out;
+ ret = reftable_stack_clean(stack);
+ if (ret)
+ goto out;
+
+out:
+ return ret;
+}
+
+struct write_create_symref_arg {
+ struct reftable_ref_store *refs;
+ struct reftable_stack *stack;
+ const char *refname;
+ const char *target;
+ const char *logmsg;
+};
+
+static int write_create_symref_table(struct reftable_writer *writer, void *cb_data)
+{
+ struct write_create_symref_arg *create = cb_data;
+ uint64_t ts = reftable_stack_next_update_index(create->stack);
+ struct reftable_ref_record ref = {
+ .refname = (char *)create->refname,
+ .value_type = REFTABLE_REF_SYMREF,
+ .value.symref = (char *)create->target,
+ .update_index = ts,
+ };
+ struct reftable_log_record log = {0};
+ struct object_id new_oid;
+ struct object_id old_oid;
+ int ret;
+
+ reftable_writer_set_limits(writer, ts, ts);
+
+ ret = reftable_writer_add_ref(writer, &ref);
+ if (ret)
+ return ret;
+
+ /*
+ * Note that it is important to try and resolve the reference before we
+ * write the log entry. This is because `should_write_log()` will munge
+ * `core.logAllRefUpdates`, which is undesirable when we create a new
+ * repository because it would be written into the config. As HEAD will
+ * not resolve for new repositories this ordering will ensure that this
+ * never happens.
+ */
+ if (!create->logmsg ||
+ !refs_resolve_ref_unsafe(&create->refs->base, create->target,
+ RESOLVE_REF_READING, &new_oid, NULL) ||
+ !should_write_log(&create->refs->base, create->refname))
+ return 0;
+
+ fill_reftable_log_record(&log);
+ log.refname = xstrdup(create->refname);
+ log.update_index = ts;
+ log.value.update.message = xstrndup(create->logmsg,
+ create->refs->write_options.block_size / 2);
+ log.value.update.new_hash = new_oid.hash;
+ if (refs_resolve_ref_unsafe(&create->refs->base, create->refname,
+ RESOLVE_REF_READING, &old_oid, NULL))
+ log.value.update.old_hash = old_oid.hash;
+
+ ret = reftable_writer_add_log(writer, &log);
+ clear_reftable_log_record(&log);
+ return ret;
+}
+
+static int reftable_be_create_symref(struct ref_store *ref_store,
+ const char *refname,
+ const char *target,
+ const char *logmsg)
+{
+ struct reftable_ref_store *refs =
+ reftable_be_downcast(ref_store, REF_STORE_WRITE, "create_symref");
+ struct reftable_stack *stack = stack_for(refs, refname, &refname);
+ struct write_create_symref_arg arg = {
+ .refs = refs,
+ .stack = stack,
+ .refname = refname,
+ .target = target,
+ .logmsg = logmsg,
+ };
+ int ret;
+
+ ret = refs->err;
+ if (ret < 0)
+ goto done;
+
+ ret = reftable_stack_reload(stack);
+ if (ret)
+ goto done;
+
+ ret = reftable_stack_add(stack, &write_create_symref_table, &arg);
+
+done:
+ assert(ret != REFTABLE_API_ERROR);
+ if (ret)
+ error("unable to write symref for %s: %s", refname,
+ reftable_error_str(ret));
+ return ret;
+}
+
+struct write_copy_arg {
+ struct reftable_ref_store *refs;
+ struct reftable_stack *stack;
+ const char *oldname;
+ const char *newname;
+ const char *logmsg;
+ int delete_old;
+};
+
+static int write_copy_table(struct reftable_writer *writer, void *cb_data)
+{
+ struct write_copy_arg *arg = cb_data;
+ uint64_t deletion_ts, creation_ts;
+ struct reftable_merged_table *mt = reftable_stack_merged_table(arg->stack);
+ struct reftable_ref_record old_ref = {0}, refs[2] = {0};
+ struct reftable_log_record old_log = {0}, *logs = NULL;
+ struct reftable_iterator it = {0};
+ struct string_list skip = STRING_LIST_INIT_NODUP;
+ struct strbuf errbuf = STRBUF_INIT;
+ size_t logs_nr = 0, logs_alloc = 0, i;
+ int ret;
+
+ if (reftable_stack_read_ref(arg->stack, arg->oldname, &old_ref)) {
+ ret = error(_("refname %s not found"), arg->oldname);
+ goto done;
+ }
+ if (old_ref.value_type == REFTABLE_REF_SYMREF) {
+ ret = error(_("refname %s is a symbolic ref, copying it is not supported"),
+ arg->oldname);
+ goto done;
+ }
+
+ /*
+ * There's nothing to do in case the old and new name are the same, so
+ * we exit early in that case.
+ */
+ if (!strcmp(arg->oldname, arg->newname)) {
+ ret = 0;
+ goto done;
+ }
+
+ /*
+ * Verify that the new refname is available.
+ */
+ string_list_insert(&skip, arg->oldname);
+ ret = refs_verify_refname_available(&arg->refs->base, arg->newname,
+ NULL, &skip, &errbuf);
+ if (ret < 0) {
+ error("%s", errbuf.buf);
+ goto done;
+ }
+
+ /*
+ * When deleting the old reference we have to use two update indices:
+ * once to delete the old ref and its reflog, and once to create the
+ * new ref and its reflog. They need to be staged with two separate
+ * indices because the new reflog needs to encode both the deletion of
+ * the old branch and the creation of the new branch, and we cannot do
+ * two changes to a reflog in a single update.
+ */
+ deletion_ts = creation_ts = reftable_stack_next_update_index(arg->stack);
+ if (arg->delete_old)
+ creation_ts++;
+ reftable_writer_set_limits(writer, deletion_ts, creation_ts);
+
+ /*
+ * Add the new reference. If this is a rename then we also delete the
+ * old reference.
+ */
+ refs[0] = old_ref;
+ refs[0].refname = (char *)arg->newname;
+ refs[0].update_index = creation_ts;
+ if (arg->delete_old) {
+ refs[1].refname = (char *)arg->oldname;
+ refs[1].value_type = REFTABLE_REF_DELETION;
+ refs[1].update_index = deletion_ts;
+ }
+ ret = reftable_writer_add_refs(writer, refs, arg->delete_old ? 2 : 1);
+ if (ret < 0)
+ goto done;
+
+ /*
+ * When deleting the old branch we need to create a reflog entry on the
+ * new branch name that indicates that the old branch has been deleted
+ * and then recreated. This is a tad weird, but matches what the files
+ * backend does.
+ */
+ if (arg->delete_old) {
+ struct strbuf head_referent = STRBUF_INIT;
+ struct object_id head_oid;
+ int append_head_reflog;
+ unsigned head_type = 0;
+
+ ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
+ memset(&logs[logs_nr], 0, sizeof(logs[logs_nr]));
+ fill_reftable_log_record(&logs[logs_nr]);
+ logs[logs_nr].refname = (char *)arg->newname;
+ logs[logs_nr].update_index = deletion_ts;
+ logs[logs_nr].value.update.message =
+ xstrndup(arg->logmsg, arg->refs->write_options.block_size / 2);
+ logs[logs_nr].value.update.old_hash = old_ref.value.val1;
+ logs_nr++;
+
+ ret = read_ref_without_reload(arg->stack, "HEAD", &head_oid, &head_referent, &head_type);
+ if (ret < 0)
+ goto done;
+ append_head_reflog = (head_type & REF_ISSYMREF) && !strcmp(head_referent.buf, arg->oldname);
+ strbuf_release(&head_referent);
+
+ /*
+ * The files backend uses `refs_delete_ref()` to delete the old
+ * branch name, which will append a reflog entry for HEAD in
+ * case it points to the old branch.
+ */
+ if (append_head_reflog) {
+ ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
+ logs[logs_nr] = logs[logs_nr - 1];
+ logs[logs_nr].refname = "HEAD";
+ logs_nr++;
+ }
+ }
+
+ /*
+ * Create the reflog entry for the newly created branch.
+ */
+ ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
+ memset(&logs[logs_nr], 0, sizeof(logs[logs_nr]));
+ fill_reftable_log_record(&logs[logs_nr]);
+ logs[logs_nr].refname = (char *)arg->newname;
+ logs[logs_nr].update_index = creation_ts;
+ logs[logs_nr].value.update.message =
+ xstrndup(arg->logmsg, arg->refs->write_options.block_size / 2);
+ logs[logs_nr].value.update.new_hash = old_ref.value.val1;
+ logs_nr++;
+
+ /*
+ * In addition to writing the reflog entry for the new branch, we also
+ * copy over all log entries from the old reflog. Last but not least,
+ * when renaming we also have to delete all the old reflog entries.
+ */
+ ret = reftable_merged_table_seek_log(mt, &it, arg->oldname);
+ if (ret < 0)
+ return ret;
+
+ while (1) {
+ ret = reftable_iterator_next_log(&it, &old_log);
+ if (ret < 0)
+ goto done;
+ if (ret > 0 || strcmp(old_log.refname, arg->oldname)) {
+ ret = 0;
+ break;
+ }
+
+ free(old_log.refname);
+
+ /*
+ * Copy over the old reflog entry with the new refname.
+ */
+ ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
+ logs[logs_nr] = old_log;
+ logs[logs_nr].refname = (char *)arg->newname;
+ logs_nr++;
+
+ /*
+ * Delete the old reflog entry in case we are renaming.
+ */
+ if (arg->delete_old) {
+ ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
+ memset(&logs[logs_nr], 0, sizeof(logs[logs_nr]));
+ logs[logs_nr].refname = (char *)arg->oldname;
+ logs[logs_nr].value_type = REFTABLE_LOG_DELETION;
+ logs[logs_nr].update_index = old_log.update_index;
+ logs_nr++;
+ }
+
+ /*
+ * Transfer ownership of the log record we're iterating over to
+ * the array of log records. Otherwise, the pointers would get
+ * free'd or reallocated by the iterator.
+ */
+ memset(&old_log, 0, sizeof(old_log));
+ }
+
+ ret = reftable_writer_add_logs(writer, logs, logs_nr);
+ if (ret < 0)
+ goto done;
+
+done:
+ assert(ret != REFTABLE_API_ERROR);
+ reftable_iterator_destroy(&it);
+ string_list_clear(&skip, 0);
+ strbuf_release(&errbuf);
+ for (i = 0; i < logs_nr; i++) {
+ if (!strcmp(logs[i].refname, "HEAD"))
+ continue;
+ if (logs[i].value.update.old_hash == old_ref.value.val1)
+ logs[i].value.update.old_hash = NULL;
+ if (logs[i].value.update.new_hash == old_ref.value.val1)
+ logs[i].value.update.new_hash = NULL;
+ logs[i].refname = NULL;
+ reftable_log_record_release(&logs[i]);
+ }
+ free(logs);
+ reftable_ref_record_release(&old_ref);
+ reftable_log_record_release(&old_log);
+ return ret;
+}
+
+static int reftable_be_rename_ref(struct ref_store *ref_store,
+ const char *oldrefname,
+ const char *newrefname,
+ const char *logmsg)
+{
+ struct reftable_ref_store *refs =
+ reftable_be_downcast(ref_store, REF_STORE_WRITE, "rename_ref");
+ struct reftable_stack *stack = stack_for(refs, newrefname, &newrefname);
+ struct write_copy_arg arg = {
+ .refs = refs,
+ .stack = stack,
+ .oldname = oldrefname,
+ .newname = newrefname,
+ .logmsg = logmsg,
+ .delete_old = 1,
+ };
+ int ret;
+
+ ret = refs->err;
+ if (ret < 0)
+ goto done;
+
+ ret = reftable_stack_reload(stack);
+ if (ret)
+ goto done;
+ ret = reftable_stack_add(stack, &write_copy_table, &arg);
+
+done:
+ assert(ret != REFTABLE_API_ERROR);
+ return ret;
+}
+
+static int reftable_be_copy_ref(struct ref_store *ref_store,
+ const char *oldrefname,
+ const char *newrefname,
+ const char *logmsg)
+{
+ struct reftable_ref_store *refs =
+ reftable_be_downcast(ref_store, REF_STORE_WRITE, "copy_ref");
+ struct reftable_stack *stack = stack_for(refs, newrefname, &newrefname);
+ struct write_copy_arg arg = {
+ .refs = refs,
+ .stack = stack,
+ .oldname = oldrefname,
+ .newname = newrefname,
+ .logmsg = logmsg,
+ };
+ int ret;
+
+ ret = refs->err;
+ if (ret < 0)
+ goto done;
+
+ ret = reftable_stack_reload(stack);
+ if (ret)
+ goto done;
+ ret = reftable_stack_add(stack, &write_copy_table, &arg);
+
+done:
+ assert(ret != REFTABLE_API_ERROR);
+ return ret;
+}
+
+struct reftable_reflog_iterator {
+ struct ref_iterator base;
+ struct reftable_ref_store *refs;
+ struct reftable_iterator iter;
+ struct reftable_log_record log;
+ struct object_id oid;
+ char *last_name;
+ int err;
+};
+
+static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator)
+{
+ struct reftable_reflog_iterator *iter =
+ (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;
+
+ /*
+ * We want the refnames that we have reflogs for, so we skip if
+ * we've already produced this name. This could be faster by
+ * seeking directly to reflog@update_index==0.
+ */
+ if (iter->last_name && !strcmp(iter->log.refname, iter->last_name))
+ continue;
+
+ if (!refs_resolve_ref_unsafe(&iter->refs->base, iter->log.refname,
+ 0, &iter->oid, &flags)) {
+ error(_("bad ref for %s"), iter->log.refname);
+ continue;
+ }
+
+ 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;
+ }
+
+ if (iter->err > 0) {
+ if (ref_iterator_abort(ref_iterator) != ITER_DONE)
+ return ITER_ERROR;
+ return ITER_DONE;
+ }
+
+ if (iter->err < 0) {
+ ref_iterator_abort(ref_iterator);
+ return ITER_ERROR;
+ }
+
+ return ITER_OK;
+}
+
+static int reftable_reflog_iterator_peel(struct ref_iterator *ref_iterator,
+ struct object_id *peeled)
+{
+ BUG("reftable reflog iterator cannot be peeled");
+ return -1;
+}
+
+static int reftable_reflog_iterator_abort(struct ref_iterator *ref_iterator)
+{
+ struct reftable_reflog_iterator *iter =
+ (struct reftable_reflog_iterator *)ref_iterator;
+ reftable_log_record_release(&iter->log);
+ reftable_iterator_destroy(&iter->iter);
+ free(iter->last_name);
+ free(iter);
+ return ITER_DONE;
+}
+
+static struct ref_iterator_vtable reftable_reflog_iterator_vtable = {
+ .advance = reftable_reflog_iterator_advance,
+ .peel = reftable_reflog_iterator_peel,
+ .abort = reftable_reflog_iterator_abort
+};
+
+static struct reftable_reflog_iterator *reflog_iterator_for_stack(struct reftable_ref_store *refs,
+ struct reftable_stack *stack)
+{
+ struct reftable_merged_table *merged_table;
+ struct reftable_reflog_iterator *iter;
+ int ret;
+
+ 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)
+ goto done;
+
+ ret = reftable_stack_reload(refs->main_stack);
+ if (ret < 0)
+ goto done;
+
+ merged_table = reftable_stack_merged_table(stack);
+
+ ret = reftable_merged_table_seek_log(merged_table, &iter->iter, "");
+ if (ret < 0)
+ goto done;
+
+done:
+ iter->err = ret;
+ return iter;
+}
+
+static struct ref_iterator *reftable_be_reflog_iterator_begin(struct ref_store *ref_store)
+{
+ struct reftable_ref_store *refs =
+ reftable_be_downcast(ref_store, REF_STORE_READ, "reflog_iterator_begin");
+ struct reftable_reflog_iterator *main_iter, *worktree_iter;
+
+ main_iter = reflog_iterator_for_stack(refs, refs->main_stack);
+ if (!refs->worktree_stack)
+ return &main_iter->base;
+
+ worktree_iter = reflog_iterator_for_stack(refs, refs->worktree_stack);
+
+ return merge_ref_iterator_begin(1, &worktree_iter->base, &main_iter->base,
+ iterator_select, NULL);
+}
+
+static int yield_log_record(struct reftable_log_record *log,
+ each_reflog_ent_fn fn,
+ void *cb_data)
+{
+ struct object_id old_oid, new_oid;
+ const char *full_committer;
+
+ oidread(&old_oid, log->value.update.old_hash);
+ oidread(&new_oid, log->value.update.new_hash);
+
+ /*
+ * When both the old object ID and the new object ID are null
+ * then this is the reflog existence marker. The caller must
+ * not be aware of it.
+ */
+ if (is_null_oid(&old_oid) && is_null_oid(&new_oid))
+ return 0;
+
+ full_committer = fmt_ident(log->value.update.name, log->value.update.email,
+ WANT_COMMITTER_IDENT, NULL, IDENT_NO_DATE);
+ return fn(&old_oid, &new_oid, full_committer,
+ log->value.update.time, log->value.update.tz_offset,
+ log->value.update.message, cb_data);
+}
+
+static int reftable_be_for_each_reflog_ent_reverse(struct ref_store *ref_store,
+ const char *refname,
+ each_reflog_ent_fn fn,
+ void *cb_data)
+{
+ struct reftable_ref_store *refs =
+ reftable_be_downcast(ref_store, REF_STORE_READ, "for_each_reflog_ent_reverse");
+ struct reftable_stack *stack = stack_for(refs, refname, &refname);
+ struct reftable_merged_table *mt = NULL;
+ struct reftable_log_record log = {0};
+ struct reftable_iterator it = {0};
+ int ret;
+
+ if (refs->err < 0)
+ return refs->err;
+
+ mt = reftable_stack_merged_table(stack);
+ ret = reftable_merged_table_seek_log(mt, &it, refname);
+ while (!ret) {
+ ret = reftable_iterator_next_log(&it, &log);
+ if (ret < 0)
+ break;
+ if (ret > 0 || strcmp(log.refname, refname)) {
+ ret = 0;
+ break;
+ }
+
+ ret = yield_log_record(&log, fn, cb_data);
+ if (ret)
+ break;
+ }
+
+ reftable_log_record_release(&log);
+ reftable_iterator_destroy(&it);
+ return ret;
+}
+
+static int reftable_be_for_each_reflog_ent(struct ref_store *ref_store,
+ const char *refname,
+ each_reflog_ent_fn fn,
+ void *cb_data)
+{
+ struct reftable_ref_store *refs =
+ reftable_be_downcast(ref_store, REF_STORE_READ, "for_each_reflog_ent");
+ struct reftable_stack *stack = stack_for(refs, refname, &refname);
+ struct reftable_merged_table *mt = NULL;
+ struct reftable_log_record *logs = NULL;
+ struct reftable_iterator it = {0};
+ size_t logs_alloc = 0, logs_nr = 0, i;
+ int ret;
+
+ if (refs->err < 0)
+ return refs->err;
+
+ mt = reftable_stack_merged_table(stack);
+ ret = reftable_merged_table_seek_log(mt, &it, refname);
+ while (!ret) {
+ struct reftable_log_record log = {0};
+
+ ret = reftable_iterator_next_log(&it, &log);
+ if (ret < 0)
+ goto done;
+ if (ret > 0 || strcmp(log.refname, refname)) {
+ reftable_log_record_release(&log);
+ ret = 0;
+ break;
+ }
+
+ ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
+ logs[logs_nr++] = log;
+ }
+
+ for (i = logs_nr; i--;) {
+ ret = yield_log_record(&logs[i], fn, cb_data);
+ if (ret)
+ goto done;
+ }
+
+done:
+ reftable_iterator_destroy(&it);
+ for (i = 0; i < logs_nr; i++)
+ reftable_log_record_release(&logs[i]);
+ free(logs);
+ return ret;
+}
+
+static int reftable_be_reflog_exists(struct ref_store *ref_store,
+ const char *refname)
+{
+ struct reftable_ref_store *refs =
+ reftable_be_downcast(ref_store, REF_STORE_READ, "reflog_exists");
+ struct reftable_stack *stack = stack_for(refs, refname, &refname);
+ struct reftable_merged_table *mt = reftable_stack_merged_table(stack);
+ struct reftable_log_record log = {0};
+ struct reftable_iterator it = {0};
+ int ret;
+
+ ret = refs->err;
+ if (ret < 0)
+ goto done;
+
+ ret = reftable_stack_reload(stack);
+ if (ret < 0)
+ goto done;
+
+ ret = reftable_merged_table_seek_log(mt, &it, refname);
+ if (ret < 0)
+ goto done;
+
+ /*
+ * Check whether we get at least one log record for the given ref name.
+ * If so, the reflog exists, otherwise it doesn't.
+ */
+ ret = reftable_iterator_next_log(&it, &log);
+ if (ret < 0)
+ goto done;
+ if (ret > 0) {
+ ret = 0;
+ goto done;
+ }
+
+ ret = strcmp(log.refname, refname) == 0;
+
+done:
+ reftable_iterator_destroy(&it);
+ reftable_log_record_release(&log);
+ if (ret < 0)
+ ret = 0;
+ return ret;
+}
+
+struct write_reflog_existence_arg {
+ struct reftable_ref_store *refs;
+ const char *refname;
+ struct reftable_stack *stack;
+};
+
+static int write_reflog_existence_table(struct reftable_writer *writer,
+ void *cb_data)
+{
+ struct write_reflog_existence_arg *arg = cb_data;
+ uint64_t ts = reftable_stack_next_update_index(arg->stack);
+ struct reftable_log_record log = {0};
+ int ret;
+
+ ret = reftable_stack_read_log(arg->stack, arg->refname, &log);
+ if (ret <= 0)
+ goto done;
+
+ reftable_writer_set_limits(writer, ts, ts);
+
+ /*
+ * The existence entry has both old and new object ID set to the the
+ * null object ID. Our iterators are aware of this and will not present
+ * them to their callers.
+ */
+ log.refname = xstrdup(arg->refname);
+ log.update_index = ts;
+ log.value_type = REFTABLE_LOG_UPDATE;
+ ret = reftable_writer_add_log(writer, &log);
+
+done:
+ assert(ret != REFTABLE_API_ERROR);
+ reftable_log_record_release(&log);
+ return ret;
+}
+
+static int reftable_be_create_reflog(struct ref_store *ref_store,
+ const char *refname,
+ struct strbuf *errmsg)
+{
+ struct reftable_ref_store *refs =
+ reftable_be_downcast(ref_store, REF_STORE_WRITE, "create_reflog");
+ struct reftable_stack *stack = stack_for(refs, refname, &refname);
+ struct write_reflog_existence_arg arg = {
+ .refs = refs,
+ .stack = stack,
+ .refname = refname,
+ };
+ int ret;
+
+ ret = refs->err;
+ if (ret < 0)
+ goto done;
+
+ ret = reftable_stack_reload(stack);
+ if (ret)
+ goto done;
+
+ ret = reftable_stack_add(stack, &write_reflog_existence_table, &arg);
+
+done:
+ return ret;
+}
+
+struct write_reflog_delete_arg {
+ struct reftable_stack *stack;
+ const char *refname;
+};
+
+static int write_reflog_delete_table(struct reftable_writer *writer, void *cb_data)
+{
+ struct write_reflog_delete_arg *arg = cb_data;
+ struct reftable_merged_table *mt =
+ reftable_stack_merged_table(arg->stack);
+ struct reftable_log_record log = {0}, tombstone = {0};
+ struct reftable_iterator it = {0};
+ uint64_t ts = reftable_stack_next_update_index(arg->stack);
+ int ret;
+
+ reftable_writer_set_limits(writer, ts, ts);
+
+ /*
+ * In order to delete a table we need to delete all reflog entries one
+ * by one. This is inefficient, but the reftable format does not have a
+ * better marker right now.
+ */
+ ret = reftable_merged_table_seek_log(mt, &it, arg->refname);
+ while (ret == 0) {
+ ret = reftable_iterator_next_log(&it, &log);
+ if (ret < 0)
+ break;
+ if (ret > 0 || strcmp(log.refname, arg->refname)) {
+ ret = 0;
+ break;
+ }
+
+ tombstone.refname = (char *)arg->refname;
+ tombstone.value_type = REFTABLE_LOG_DELETION;
+ tombstone.update_index = log.update_index;
+
+ ret = reftable_writer_add_log(writer, &tombstone);
+ }
+
+ reftable_log_record_release(&log);
+ reftable_iterator_destroy(&it);
+ return ret;
+}
+
+static int reftable_be_delete_reflog(struct ref_store *ref_store,
+ const char *refname)
+{
+ struct reftable_ref_store *refs =
+ reftable_be_downcast(ref_store, REF_STORE_WRITE, "delete_reflog");
+ struct reftable_stack *stack = stack_for(refs, refname, &refname);
+ struct write_reflog_delete_arg arg = {
+ .stack = stack,
+ .refname = refname,
+ };
+ int ret;
+
+ ret = reftable_stack_reload(stack);
+ if (ret)
+ return ret;
+ ret = reftable_stack_add(stack, &write_reflog_delete_table, &arg);
+
+ assert(ret != REFTABLE_API_ERROR);
+ return ret;
+}
+
+struct reflog_expiry_arg {
+ struct reftable_stack *stack;
+ struct reftable_log_record *records;
+ struct object_id update_oid;
+ const char *refname;
+ size_t len;
+};
+
+static int write_reflog_expiry_table(struct reftable_writer *writer, void *cb_data)
+{
+ struct reflog_expiry_arg *arg = cb_data;
+ uint64_t ts = reftable_stack_next_update_index(arg->stack);
+ uint64_t live_records = 0;
+ size_t i;
+ int ret;
+
+ for (i = 0; i < arg->len; i++)
+ if (arg->records[i].value_type == REFTABLE_LOG_UPDATE)
+ live_records++;
+
+ reftable_writer_set_limits(writer, ts, ts);
+
+ if (!is_null_oid(&arg->update_oid)) {
+ struct reftable_ref_record ref = {0};
+ struct object_id peeled;
+
+ ref.refname = (char *)arg->refname;
+ ref.update_index = ts;
+
+ if (!peel_object(&arg->update_oid, &peeled)) {
+ ref.value_type = REFTABLE_REF_VAL2;
+ memcpy(ref.value.val2.target_value, peeled.hash, GIT_MAX_RAWSZ);
+ memcpy(ref.value.val2.value, arg->update_oid.hash, GIT_MAX_RAWSZ);
+ } else {
+ ref.value_type = REFTABLE_REF_VAL1;
+ memcpy(ref.value.val1, arg->update_oid.hash, GIT_MAX_RAWSZ);
+ }
+
+ ret = reftable_writer_add_ref(writer, &ref);
+ if (ret < 0)
+ return ret;
+ }
+
+ /*
+ * When there are no more entries left in the reflog we empty it
+ * completely, but write a placeholder reflog entry that indicates that
+ * the reflog still exists.
+ */
+ if (!live_records) {
+ struct reftable_log_record log = {
+ .refname = (char *)arg->refname,
+ .value_type = REFTABLE_LOG_UPDATE,
+ .update_index = ts,
+ };
+
+ ret = reftable_writer_add_log(writer, &log);
+ if (ret)
+ return ret;
+ }
+
+ for (i = 0; i < arg->len; i++) {
+ ret = reftable_writer_add_log(writer, &arg->records[i]);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int reftable_be_reflog_expire(struct ref_store *ref_store,
+ const char *refname,
+ unsigned int flags,
+ reflog_expiry_prepare_fn prepare_fn,
+ reflog_expiry_should_prune_fn should_prune_fn,
+ reflog_expiry_cleanup_fn cleanup_fn,
+ void *policy_cb_data)
+{
+ /*
+ * For log expiry, we write tombstones for every single reflog entry
+ * that is to be expired. This means that the entries are still
+ * retrievable by delving into the stack, and expiring entries
+ * paradoxically takes extra memory. This memory is only reclaimed when
+ * compacting the reftable stack.
+ *
+ * It would be better if the refs backend supported an API that sets a
+ * criterion for all refs, passing the criterion to pack_refs().
+ *
+ * On the plus side, because we do the expiration per ref, we can easily
+ * insert the reflog existence dummies.
+ */
+ struct reftable_ref_store *refs =
+ reftable_be_downcast(ref_store, REF_STORE_WRITE, "reflog_expire");
+ struct reftable_stack *stack = stack_for(refs, refname, &refname);
+ struct reftable_merged_table *mt = reftable_stack_merged_table(stack);
+ struct reftable_log_record *logs = NULL;
+ struct reftable_log_record *rewritten = NULL;
+ struct reftable_ref_record ref_record = {0};
+ struct reftable_iterator it = {0};
+ struct reftable_addition *add = NULL;
+ struct reflog_expiry_arg arg = {0};
+ struct object_id oid = {0};
+ uint8_t *last_hash = NULL;
+ size_t logs_nr = 0, logs_alloc = 0, i;
+ int ret;
+
+ if (refs->err < 0)
+ return refs->err;
+
+ ret = reftable_stack_reload(stack);
+ if (ret < 0)
+ goto done;
+
+ ret = reftable_merged_table_seek_log(mt, &it, refname);
+ if (ret < 0)
+ goto done;
+
+ ret = reftable_stack_new_addition(&add, stack);
+ if (ret < 0)
+ goto done;
+
+ ret = reftable_stack_read_ref(stack, refname, &ref_record);
+ if (ret < 0)
+ goto done;
+ if (reftable_ref_record_val1(&ref_record))
+ oidread(&oid, reftable_ref_record_val1(&ref_record));
+ prepare_fn(refname, &oid, policy_cb_data);
+
+ while (1) {
+ struct reftable_log_record log = {0};
+ struct object_id old_oid, new_oid;
+
+ ret = reftable_iterator_next_log(&it, &log);
+ if (ret < 0)
+ goto done;
+ if (ret > 0 || strcmp(log.refname, refname)) {
+ reftable_log_record_release(&log);
+ break;
+ }
+
+ oidread(&old_oid, log.value.update.old_hash);
+ oidread(&new_oid, log.value.update.new_hash);
+
+ /*
+ * Skip over the reflog existence marker. We will add it back
+ * in when there are no live reflog records.
+ */
+ if (is_null_oid(&old_oid) && is_null_oid(&new_oid)) {
+ reftable_log_record_release(&log);
+ continue;
+ }
+
+ ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
+ logs[logs_nr++] = log;
+ }
+
+ /*
+ * We need to rewrite all reflog entries according to the pruning
+ * callback function:
+ *
+ * - If a reflog entry shall be pruned we mark the record for
+ * deletion.
+ *
+ * - Otherwise we may have to rewrite the chain of reflog entries so
+ * that gaps created by just-deleted records get backfilled.
+ */
+ CALLOC_ARRAY(rewritten, logs_nr);
+ for (i = logs_nr; i--;) {
+ struct reftable_log_record *dest = &rewritten[i];
+ struct object_id old_oid, new_oid;
+
+ *dest = logs[i];
+ oidread(&old_oid, logs[i].value.update.old_hash);
+ oidread(&new_oid, logs[i].value.update.new_hash);
+
+ if (should_prune_fn(&old_oid, &new_oid, logs[i].value.update.email,
+ (timestamp_t)logs[i].value.update.time,
+ logs[i].value.update.tz_offset,
+ logs[i].value.update.message,
+ policy_cb_data)) {
+ dest->value_type = REFTABLE_LOG_DELETION;
+ } else {
+ if ((flags & EXPIRE_REFLOGS_REWRITE) && last_hash)
+ dest->value.update.old_hash = last_hash;
+ last_hash = logs[i].value.update.new_hash;
+ }
+ }
+
+ if (flags & EXPIRE_REFLOGS_UPDATE_REF && last_hash &&
+ reftable_ref_record_val1(&ref_record))
+ oidread(&arg.update_oid, last_hash);
+
+ arg.records = rewritten;
+ arg.len = logs_nr;
+ arg.stack = stack,
+ arg.refname = refname,
+
+ ret = reftable_addition_add(add, &write_reflog_expiry_table, &arg);
+ if (ret < 0)
+ goto done;
+
+ /*
+ * Future improvement: we could skip writing records that were
+ * not changed.
+ */
+ if (!(flags & EXPIRE_REFLOGS_DRY_RUN))
+ ret = reftable_addition_commit(add);
+
+done:
+ if (add)
+ cleanup_fn(policy_cb_data);
+ assert(ret != REFTABLE_API_ERROR);
+
+ reftable_ref_record_release(&ref_record);
+ reftable_iterator_destroy(&it);
+ reftable_addition_destroy(add);
+ for (i = 0; i < logs_nr; i++)
+ reftable_log_record_release(&logs[i]);
+ free(logs);
+ free(rewritten);
+ return ret;
+}
+
+struct ref_storage_be refs_be_reftable = {
+ .name = "reftable",
+ .init = reftable_be_init,
+ .init_db = reftable_be_init_db,
+ .transaction_prepare = reftable_be_transaction_prepare,
+ .transaction_finish = reftable_be_transaction_finish,
+ .transaction_abort = reftable_be_transaction_abort,
+ .initial_transaction_commit = reftable_be_initial_transaction_commit,
+
+ .pack_refs = reftable_be_pack_refs,
+ .create_symref = reftable_be_create_symref,
+ .rename_ref = reftable_be_rename_ref,
+ .copy_ref = reftable_be_copy_ref,
+
+ .iterator_begin = reftable_be_iterator_begin,
+ .read_raw_ref = reftable_be_read_raw_ref,
+ .read_symbolic_ref = reftable_be_read_symbolic_ref,
+
+ .reflog_iterator_begin = reftable_be_reflog_iterator_begin,
+ .for_each_reflog_ent = reftable_be_for_each_reflog_ent,
+ .for_each_reflog_ent_reverse = reftable_be_for_each_reflog_ent_reverse,
+ .reflog_exists = reftable_be_reflog_exists,
+ .create_reflog = reftable_be_create_reflog,
+ .delete_reflog = reftable_be_delete_reflog,
+ .reflog_expire = reftable_be_reflog_expire,
+};
diff --git a/repository.h b/repository.h
index 7a250a6605..1645cef518 100644
--- a/repository.h
+++ b/repository.h
@@ -24,8 +24,9 @@ enum fetch_negotiation_setting {
FETCH_NEGOTIATION_NOOP,
};
-#define REF_STORAGE_FORMAT_UNKNOWN 0
-#define REF_STORAGE_FORMAT_FILES 1
+#define REF_STORAGE_FORMAT_UNKNOWN 0
+#define REF_STORAGE_FORMAT_FILES 1
+#define REF_STORAGE_FORMAT_REFTABLE 2
struct repo_settings {
int initialized;
diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
new file mode 100755
index 0000000000..94ee46984a
--- /dev/null
+++ b/t/t0610-reftable-basics.sh
@@ -0,0 +1,887 @@
+#!/bin/sh
+#
+# Copyright (c) 2020 Google LLC
+#
+
+test_description='reftable basics'
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+. ./test-lib.sh
+
+if ! test_have_prereq REFTABLE
+then
+ skip_all='skipping reftable tests; set GIT_TEST_DEFAULT_REF_FORMAT=reftable'
+ test_done
+fi
+
+INVALID_OID=$(test_oid 001)
+
+test_expect_success 'init: creates basic reftable structures' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ test_path_is_dir repo/.git/reftable &&
+ test_path_is_file repo/.git/reftable/tables.list &&
+ echo reftable >expect &&
+ git -C repo rev-parse --show-ref-format >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'init: sha256 object format via environment variable' '
+ test_when_finished "rm -rf repo" &&
+ GIT_DEFAULT_HASH=sha256 git init repo &&
+ cat >expect <<-EOF &&
+ sha256
+ reftable
+ EOF
+ git -C repo rev-parse --show-object-format --show-ref-format >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'init: sha256 object format via option' '
+ test_when_finished "rm -rf repo" &&
+ git init --object-format=sha256 repo &&
+ cat >expect <<-EOF &&
+ sha256
+ reftable
+ EOF
+ git -C repo rev-parse --show-object-format --show-ref-format >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'init: reinitializing reftable backend succeeds' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ test_commit -C repo A &&
+
+ git -C repo for-each-ref >expect &&
+ git init --ref-format=reftable repo &&
+ git -C repo for-each-ref >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'init: reinitializing files with reftable backend fails' '
+ test_when_finished "rm -rf repo" &&
+ git init --ref-format=files repo &&
+ test_commit -C repo file &&
+
+ cp repo/.git/HEAD expect &&
+ test_must_fail git init --ref-format=reftable repo &&
+ test_cmp expect repo/.git/HEAD
+'
+
+test_expect_success 'init: reinitializing reftable with files backend fails' '
+ test_when_finished "rm -rf repo" &&
+ git init --ref-format=reftable repo &&
+ test_commit -C repo file &&
+
+ cp repo/.git/HEAD expect &&
+ test_must_fail git init --ref-format=files repo &&
+ test_cmp expect repo/.git/HEAD
+'
+
+test_expect_perms () {
+ local perms="$1"
+ local file="$2"
+ local actual=$(ls -l "$file") &&
+
+ case "$actual" in
+ $perms*)
+ : happy
+ ;;
+ *)
+ echo "$(basename $2) is not $perms but $actual"
+ false
+ ;;
+ esac
+}
+
+for umask in 002 022
+do
+ test_expect_success POSIXPERM 'init: honors core.sharedRepository' '
+ test_when_finished "rm -rf repo" &&
+ (
+ umask $umask &&
+ git init --shared=true repo &&
+ test 1 = "$(git -C repo config core.sharedrepository)"
+ ) &&
+ test_expect_perms "-rw-rw-r--" repo/.git/reftable/tables.list &&
+ for table in repo/.git/reftable/*.ref
+ do
+ test_expect_perms "-rw-rw-r--" "$table" ||
+ return 1
+ done
+ '
+done
+
+test_expect_success 'clone: can clone reftable repository' '
+ test_when_finished "rm -rf repo clone" &&
+ git init repo &&
+ test_commit -C repo message1 file1 &&
+
+ git clone repo cloned &&
+ echo reftable >expect &&
+ git -C cloned rev-parse --show-ref-format >actual &&
+ test_cmp expect actual &&
+ test_path_is_file cloned/file1
+'
+
+test_expect_success 'clone: can clone reffiles into reftable repository' '
+ test_when_finished "rm -rf reffiles reftable" &&
+ git init --ref-format=files reffiles &&
+ test_commit -C reffiles A &&
+ git clone --ref-format=reftable ./reffiles reftable &&
+
+ git -C reffiles rev-parse HEAD >expect &&
+ git -C reftable rev-parse HEAD >actual &&
+ test_cmp expect actual &&
+
+ git -C reftable rev-parse --show-ref-format >actual &&
+ echo reftable >expect &&
+ test_cmp expect actual &&
+
+ git -C reffiles rev-parse --show-ref-format >actual &&
+ echo files >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'clone: can clone reftable into reffiles repository' '
+ test_when_finished "rm -rf reffiles reftable" &&
+ git init --ref-format=reftable reftable &&
+ test_commit -C reftable A &&
+ git clone --ref-format=files ./reftable reffiles &&
+
+ git -C reftable rev-parse HEAD >expect &&
+ git -C reffiles rev-parse HEAD >actual &&
+ test_cmp expect actual &&
+
+ git -C reftable rev-parse --show-ref-format >actual &&
+ echo reftable >expect &&
+ test_cmp expect actual &&
+
+ git -C reffiles rev-parse --show-ref-format >actual &&
+ echo files >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'ref transaction: corrupted tables cause failure' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit file1 &&
+ for f in .git/reftable/*.ref
+ do
+ : >"$f" || return 1
+ done &&
+ test_must_fail git update-ref refs/heads/main HEAD
+ )
+'
+
+test_expect_success 'ref transaction: corrupted tables.list cause failure' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit file1 &&
+ echo garbage >.git/reftable/tables.list &&
+ test_must_fail git update-ref refs/heads/main HEAD
+ )
+'
+
+test_expect_success 'ref transaction: refuses to write ref causing F/D conflict' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ test_commit -C repo file &&
+ test_must_fail git -C repo update-ref refs/heads/main/forbidden
+'
+
+test_expect_success 'ref transaction: deleting ref with invalid name fails' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ test_commit -C repo file &&
+ test_must_fail git -C repo update-ref -d ../../my-private-file
+'
+
+test_expect_success 'ref transaction: can skip object ID verification' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ test_must_fail test-tool -C repo ref-store main update-ref msg refs/heads/branch $INVALID_OID $ZERO_OID 0 &&
+ test-tool -C repo ref-store main update-ref msg refs/heads/branch $INVALID_OID $ZERO_OID REF_SKIP_OID_VERIFICATION
+'
+
+test_expect_success 'ref transaction: updating same ref multiple times fails' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ test_commit -C repo A &&
+ cat >updates <<-EOF &&
+ update refs/heads/main $A
+ update refs/heads/main $A
+ EOF
+ cat >expect <<-EOF &&
+ fatal: multiple updates for ref ${SQ}refs/heads/main${SQ} not allowed
+ EOF
+ test_must_fail git -C repo update-ref --stdin <updates 2>err &&
+ test_cmp expect err
+'
+
+test_expect_success 'ref transaction: can delete symbolic self-reference with git-symbolic-ref(1)' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ git -C repo symbolic-ref refs/heads/self refs/heads/self &&
+ git -C repo symbolic-ref -d refs/heads/self
+'
+
+test_expect_success 'ref transaction: deleting symbolic self-reference without --no-deref fails' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ git -C repo symbolic-ref refs/heads/self refs/heads/self &&
+ cat >expect <<-EOF &&
+ error: multiple updates for ${SQ}refs/heads/self${SQ} (including one via symref ${SQ}refs/heads/self${SQ}) are not allowed
+ EOF
+ test_must_fail git -C repo update-ref -d refs/heads/self 2>err &&
+ test_cmp expect err
+'
+
+test_expect_success 'ref transaction: deleting symbolic self-reference with --no-deref succeeds' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ git -C repo symbolic-ref refs/heads/self refs/heads/self &&
+ git -C repo update-ref -d --no-deref refs/heads/self
+'
+
+test_expect_success 'ref transaction: creating symbolic ref fails with F/D conflict' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ test_commit -C repo A &&
+ cat >expect <<-EOF &&
+ error: unable to write symref for refs/heads: file/directory conflict
+ EOF
+ test_must_fail git -C repo symbolic-ref refs/heads refs/heads/foo 2>err &&
+ test_cmp expect err
+'
+
+test_expect_success 'ref transaction: ref deletion' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit file &&
+ HEAD_OID=$(git show-ref -s --verify HEAD) &&
+ cat >expect <<-EOF &&
+ $HEAD_OID refs/heads/main
+ $HEAD_OID refs/tags/file
+ EOF
+ git show-ref >actual &&
+ test_cmp expect actual &&
+
+ test_must_fail git update-ref -d refs/tags/file $INVALID_OID &&
+ git show-ref >actual &&
+ test_cmp expect actual &&
+
+ git update-ref -d refs/tags/file $HEAD_OID &&
+ echo "$HEAD_OID refs/heads/main" >expect &&
+ git show-ref >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'ref transaction: writes cause auto-compaction' '
+ test_when_finished "rm -rf repo" &&
+
+ git init repo &&
+ test_line_count = 1 repo/.git/reftable/tables.list &&
+
+ test_commit -C repo --no-tag A &&
+ test_line_count = 2 repo/.git/reftable/tables.list &&
+
+ test_commit -C repo --no-tag B &&
+ test_line_count = 1 repo/.git/reftable/tables.list
+'
+
+check_fsync_events () {
+ local trace="$1" &&
+ shift &&
+
+ cat >expect &&
+ sed -n \
+ -e '/^{"event":"counter",.*"category":"fsync",/ {
+ s/.*"category":"fsync",//;
+ s/}$//;
+ p;
+ }' \
+ <"$trace" >actual &&
+ test_cmp expect actual
+}
+
+# A fix for this is in flight via jc/reftable-core-fsync.
+test_expect_failure 'ref transaction: writes are synced' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ test_commit -C repo initial &&
+
+ GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
+ GIT_TEST_FSYNC=true \
+ git -C repo -c core.fsync=reference update-ref refs/heads/branch HEAD &&
+ check_fsync_events trace2.txt <<-EOF
+ "name":"hardware-flush","count":2
+ EOF
+'
+
+test_expect_success 'pack-refs: compacts tables' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+
+ test_commit -C repo A &&
+ ls -1 repo/.git/reftable >table-files &&
+ test_line_count = 4 table-files &&
+ test_line_count = 3 repo/.git/reftable/tables.list &&
+
+ git -C repo pack-refs &&
+ ls -1 repo/.git/reftable >table-files &&
+ test_line_count = 2 table-files &&
+ test_line_count = 1 repo/.git/reftable/tables.list
+'
+
+test_expect_success 'pack-refs: prunes stale tables' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ touch repo/.git/reftable/stale-table.ref &&
+ git -C repo pack-refs &&
+ test_path_is_missing repo/.git/reftable/stable-ref.ref
+'
+
+test_expect_success 'pack-refs: does not prune non-table files' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ touch repo/.git/reftable/garbage &&
+ git -C repo pack-refs &&
+ test_path_is_file repo/.git/reftable/garbage
+'
+
+for umask in 002 022
+do
+ # A fix for this is in flight via ps/reftable-compacted-tables-permission-fix.
+ test_expect_failure POSIXPERM 'pack-refs: honors core.sharedRepository' '
+ test_when_finished "rm -rf repo" &&
+ (
+ umask $umask &&
+ git init --shared=true repo &&
+ test_commit -C repo A &&
+ test_line_count = 3 repo/.git/reftable/tables.list
+ ) &&
+ git -C repo pack-refs &&
+ test_expect_perms "-rw-rw-r--" repo/.git/reftable/tables.list &&
+ for table in repo/.git/reftable/*.ref
+ do
+ test_expect_perms "-rw-rw-r--" "$table" ||
+ return 1
+ done
+ '
+done
+
+# A fix for this is in flight.
+test_expect_failure 'packed-refs: writes are synced' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ test_commit -C repo initial &&
+ test_line_count = 2 table-files &&
+
+ GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
+ GIT_TEST_FSYNC=true \
+ git -C repo -c core.fsync=reference pack-refs &&
+ check_fsync_events trace2.txt <<-EOF
+ "name":"hardware-flush","count":2
+ EOF
+'
+
+test_expect_success 'ref iterator: bogus names are flagged' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit --no-tag file &&
+ test-tool ref-store main update-ref msg "refs/heads/bogus..name" $(git rev-parse HEAD) $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
+
+ cat >expect <<-EOF &&
+ $ZERO_OID refs/heads/bogus..name 0xc
+ $(git rev-parse HEAD) refs/heads/main 0x0
+ EOF
+ test-tool ref-store main for-each-ref "" >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'ref iterator: missing object IDs are not flagged' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test-tool ref-store main update-ref msg "refs/heads/broken-hash" $INVALID_OID $ZERO_OID REF_SKIP_OID_VERIFICATION &&
+
+ cat >expect <<-EOF &&
+ $INVALID_OID refs/heads/broken-hash 0x0
+ EOF
+ test-tool ref-store main for-each-ref "" >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'basic: commit and list refs' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ test_commit -C repo file &&
+ test_write_lines refs/heads/main refs/tags/file >expect &&
+ git -C repo for-each-ref --format="%(refname)" >actual &&
+ test_cmp actual expect
+'
+
+test_expect_success 'basic: can write large commit message' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ perl -e "
+ print \"this is a long commit message\" x 50000
+ " >commit-msg &&
+ git -C repo commit --allow-empty --file=../commit-msg
+'
+
+test_expect_success 'basic: show-ref fails with empty repository' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ test_must_fail git -C repo show-ref >actual &&
+ test_must_be_empty actual
+'
+
+test_expect_success 'basic: can check out unborn branch' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ git -C repo checkout -b main
+'
+
+test_expect_success 'basic: peeled tags are stored' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ test_commit -C repo file &&
+ git -C repo tag -m "annotated tag" test_tag HEAD &&
+ for ref in refs/heads/main refs/tags/file refs/tags/test_tag refs/tags/test_tag^{}
+ do
+ echo "$(git -C repo rev-parse "$ref") $ref" || return 1
+ done >expect &&
+ git -C repo show-ref -d >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'basic: for-each-ref can print symrefs' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit file &&
+ git branch &&
+ git symbolic-ref refs/heads/sym refs/heads/main &&
+ cat >expected <<-EOF &&
+ refs/heads/main
+ EOF
+ git for-each-ref --format="%(symref)" refs/heads/sym >actual &&
+ test_cmp expected actual
+ )
+'
+
+test_expect_success 'basic: notes' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ write_script fake_editor <<-\EOF &&
+ echo "$MSG" >"$1"
+ echo "$MSG" >&2
+ EOF
+
+ test_commit 1st &&
+ test_commit 2nd &&
+ GIT_EDITOR=./fake_editor MSG=b4 git notes add &&
+ GIT_EDITOR=./fake_editor MSG=b3 git notes edit &&
+ echo b4 >expect &&
+ git notes --ref commits@{1} show >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'basic: stash' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit file &&
+ git stash list >expect &&
+ test_line_count = 0 expect &&
+
+ echo hoi >>file.t &&
+ git stash push -m stashed &&
+ git stash list >expect &&
+ test_line_count = 1 expect &&
+
+ git stash clear &&
+ git stash list >expect &&
+ test_line_count = 0 expect
+ )
+'
+
+test_expect_success 'basic: cherry-pick' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit message1 file1 &&
+ test_commit message2 file2 &&
+ git branch source &&
+ git checkout HEAD^ &&
+ test_commit message3 file3 &&
+ git cherry-pick source &&
+ test_path_is_file file2
+ )
+'
+
+test_expect_success 'basic: rebase' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit message1 file1 &&
+ test_commit message2 file2 &&
+ git branch source &&
+ git checkout HEAD^ &&
+ test_commit message3 file3 &&
+ git rebase source &&
+ test_path_is_file file2
+ )
+'
+
+test_expect_success 'reflog: can delete separate reflog entries' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+
+ test_commit file &&
+ test_commit file2 &&
+ test_commit file3 &&
+ test_commit file4 &&
+ git reflog >actual &&
+ grep file3 actual &&
+
+ git reflog delete HEAD@{1} &&
+ git reflog >actual &&
+ ! grep file3 actual
+ )
+'
+
+test_expect_success 'reflog: can switch to previous branch' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit file1 &&
+ git checkout -b branch1 &&
+ test_commit file2 &&
+ git checkout -b branch2 &&
+ git switch - &&
+ git rev-parse --symbolic-full-name HEAD >actual &&
+ echo refs/heads/branch1 >expect &&
+ test_cmp actual expect
+ )
+'
+
+test_expect_success 'reflog: copying branch writes reflog entry' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit file1 &&
+ test_commit file2 &&
+ oid=$(git rev-parse --short HEAD) &&
+ git branch src &&
+ cat >expect <<-EOF &&
+ ${oid} dst@{0}: Branch: copied refs/heads/src to refs/heads/dst
+ ${oid} dst@{1}: branch: Created from main
+ EOF
+ git branch -c src dst &&
+ git reflog dst >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'reflog: renaming branch writes reflog entry' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ git symbolic-ref HEAD refs/heads/before &&
+ test_commit file &&
+ git show-ref >expected.refs &&
+ sed s/before/after/g <expected.refs >expected &&
+ git branch -M after &&
+ git show-ref >actual &&
+ test_cmp expected actual &&
+ echo refs/heads/after >expected &&
+ git symbolic-ref HEAD >actual &&
+ test_cmp expected actual
+ )
+'
+
+test_expect_success 'reflog: can store empty logs' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+
+ test_must_fail test-tool ref-store main reflog-exists refs/heads/branch &&
+ test-tool ref-store main create-reflog refs/heads/branch &&
+ test-tool ref-store main reflog-exists refs/heads/branch &&
+ test-tool ref-store main for-each-reflog-ent-reverse refs/heads/branch >actual &&
+ test_must_be_empty actual
+ )
+'
+
+test_expect_success 'reflog: expiry empties reflog' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+
+ test_commit initial &&
+ git checkout -b branch &&
+ test_commit fileA &&
+ test_commit fileB &&
+
+ cat >expect <<-EOF &&
+ commit: fileB
+ commit: fileA
+ branch: Created from HEAD
+ EOF
+ git reflog show --format="%gs" refs/heads/branch >actual &&
+ test_cmp expect actual &&
+
+ git reflog expire branch --expire=all &&
+ git reflog show --format="%gs" refs/heads/branch >actual &&
+ test_must_be_empty actual &&
+ test-tool ref-store main reflog-exists refs/heads/branch
+ )
+'
+
+test_expect_success 'reflog: can be deleted' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit initial &&
+ test-tool ref-store main reflog-exists refs/heads/main &&
+ test-tool ref-store main delete-reflog refs/heads/main &&
+ test_must_fail test-tool ref-store main reflog-exists refs/heads/main
+ )
+'
+
+test_expect_success 'reflog: garbage collection deletes reflog entries' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+
+ for count in $(test_seq 1 10)
+ do
+ test_commit "number $count" file.t $count number-$count ||
+ return 1
+ done &&
+ git reflog refs/heads/main >actual &&
+ test_line_count = 10 actual &&
+ grep "commit (initial): number 1" actual &&
+ grep "commit: number 10" actual &&
+
+ git gc &&
+ git reflog refs/heads/main >actual &&
+ test_line_count = 0 actual
+ )
+'
+
+test_expect_success 'reflog: updates via HEAD update HEAD reflog' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit main-one &&
+ git checkout -b new-branch &&
+ test_commit new-one &&
+ test_commit new-two &&
+
+ echo new-one >expect &&
+ git log -1 --format=%s HEAD@{1} >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'worktree: adding worktree creates separate stack' '
+ test_when_finished "rm -rf repo worktree" &&
+ git init repo &&
+ test_commit -C repo A &&
+
+ git -C repo worktree add ../worktree &&
+ test_path_is_file repo/.git/worktrees/worktree/refs/heads &&
+ echo "ref: refs/heads/.invalid" >expect &&
+ test_cmp expect repo/.git/worktrees/worktree/HEAD &&
+ test_path_is_dir repo/.git/worktrees/worktree/reftable &&
+ test_path_is_file repo/.git/worktrees/worktree/reftable/tables.list
+'
+
+test_expect_success 'worktree: pack-refs in main repo packs main refs' '
+ test_when_finished "rm -rf repo worktree" &&
+ git init repo &&
+ test_commit -C repo A &&
+ git -C repo worktree add ../worktree &&
+
+ test_line_count = 3 repo/.git/worktrees/worktree/reftable/tables.list &&
+ test_line_count = 4 repo/.git/reftable/tables.list &&
+ git -C repo pack-refs &&
+ test_line_count = 3 repo/.git/worktrees/worktree/reftable/tables.list &&
+ test_line_count = 1 repo/.git/reftable/tables.list
+'
+
+test_expect_success 'worktree: pack-refs in worktree packs worktree refs' '
+ test_when_finished "rm -rf repo worktree" &&
+ git init repo &&
+ test_commit -C repo A &&
+ git -C repo worktree add ../worktree &&
+
+ test_line_count = 3 repo/.git/worktrees/worktree/reftable/tables.list &&
+ test_line_count = 4 repo/.git/reftable/tables.list &&
+ git -C worktree pack-refs &&
+ test_line_count = 1 repo/.git/worktrees/worktree/reftable/tables.list &&
+ test_line_count = 4 repo/.git/reftable/tables.list
+'
+
+test_expect_success 'worktree: creating shared ref updates main stack' '
+ test_when_finished "rm -rf repo worktree" &&
+ git init repo &&
+ test_commit -C repo A &&
+
+ git -C repo worktree add ../worktree &&
+ git -C repo pack-refs &&
+ git -C worktree pack-refs &&
+ test_line_count = 1 repo/.git/worktrees/worktree/reftable/tables.list &&
+ test_line_count = 1 repo/.git/reftable/tables.list &&
+
+ git -C worktree update-ref refs/heads/shared HEAD &&
+ test_line_count = 1 repo/.git/worktrees/worktree/reftable/tables.list &&
+ test_line_count = 2 repo/.git/reftable/tables.list
+'
+
+test_expect_success 'worktree: creating per-worktree ref updates worktree stack' '
+ test_when_finished "rm -rf repo worktree" &&
+ git init repo &&
+ test_commit -C repo A &&
+
+ git -C repo worktree add ../worktree &&
+ git -C repo pack-refs &&
+ git -C worktree pack-refs &&
+ test_line_count = 1 repo/.git/worktrees/worktree/reftable/tables.list &&
+ test_line_count = 1 repo/.git/reftable/tables.list &&
+
+ git -C worktree update-ref refs/bisect/per-worktree HEAD &&
+ test_line_count = 2 repo/.git/worktrees/worktree/reftable/tables.list &&
+ test_line_count = 1 repo/.git/reftable/tables.list
+'
+
+test_expect_success 'worktree: creating per-worktree ref from main repo' '
+ test_when_finished "rm -rf repo worktree" &&
+ git init repo &&
+ test_commit -C repo A &&
+
+ git -C repo worktree add ../worktree &&
+ git -C repo pack-refs &&
+ git -C worktree pack-refs &&
+ test_line_count = 1 repo/.git/worktrees/worktree/reftable/tables.list &&
+ test_line_count = 1 repo/.git/reftable/tables.list &&
+
+ git -C repo update-ref worktrees/worktree/refs/bisect/per-worktree HEAD &&
+ test_line_count = 2 repo/.git/worktrees/worktree/reftable/tables.list &&
+ test_line_count = 1 repo/.git/reftable/tables.list
+'
+
+test_expect_success 'worktree: creating per-worktree ref from second worktree' '
+ test_when_finished "rm -rf repo wt1 wt2" &&
+ git init repo &&
+ test_commit -C repo A &&
+
+ git -C repo worktree add ../wt1 &&
+ git -C repo worktree add ../wt2 &&
+ git -C repo pack-refs &&
+ git -C wt1 pack-refs &&
+ git -C wt2 pack-refs &&
+ test_line_count = 1 repo/.git/worktrees/wt1/reftable/tables.list &&
+ test_line_count = 1 repo/.git/worktrees/wt2/reftable/tables.list &&
+ test_line_count = 1 repo/.git/reftable/tables.list &&
+
+ git -C wt1 update-ref worktrees/wt2/refs/bisect/per-worktree HEAD &&
+ test_line_count = 1 repo/.git/worktrees/wt1/reftable/tables.list &&
+ test_line_count = 2 repo/.git/worktrees/wt2/reftable/tables.list &&
+ test_line_count = 1 repo/.git/reftable/tables.list
+'
+
+test_expect_success 'worktree: can create shared and per-worktree ref in one transaction' '
+ test_when_finished "rm -rf repo worktree" &&
+ git init repo &&
+ test_commit -C repo A &&
+
+ git -C repo worktree add ../worktree &&
+ git -C repo pack-refs &&
+ git -C worktree pack-refs &&
+ test_line_count = 1 repo/.git/worktrees/worktree/reftable/tables.list &&
+ test_line_count = 1 repo/.git/reftable/tables.list &&
+
+ cat >stdin <<-EOF &&
+ create worktrees/worktree/refs/bisect/per-worktree HEAD
+ create refs/branches/shared HEAD
+ EOF
+ git -C repo update-ref --stdin <stdin &&
+ test_line_count = 2 repo/.git/worktrees/worktree/reftable/tables.list &&
+ test_line_count = 2 repo/.git/reftable/tables.list
+'
+
+test_expect_success 'worktree: can access common refs' '
+ test_when_finished "rm -rf repo worktree" &&
+ git init repo &&
+ test_commit -C repo file1 &&
+ git -C repo branch branch1 &&
+ git -C repo worktree add ../worktree &&
+
+ echo refs/heads/worktree >expect &&
+ git -C worktree symbolic-ref HEAD >actual &&
+ test_cmp expect actual &&
+ git -C worktree checkout branch1
+'
+
+test_expect_success 'worktree: adds worktree with detached HEAD' '
+ test_when_finished "rm -rf repo worktree" &&
+
+ git init repo &&
+ test_commit -C repo A &&
+ git -C repo rev-parse main >expect &&
+
+ git -C repo worktree add --detach ../worktree main &&
+ git -C worktree rev-parse HEAD >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'fetch: accessing FETCH_HEAD special ref works' '
+ test_when_finished "rm -rf repo sub" &&
+
+ git init sub &&
+ test_commit -C sub two &&
+ git -C sub rev-parse HEAD >expect &&
+
+ git init repo &&
+ test_commit -C repo one &&
+ git -C repo fetch ../sub &&
+ git -C repo rev-parse FETCH_HEAD >actual &&
+ test_cmp expect actual
+'
+
+test_done
diff --git a/t/t0611-reftable-httpd.sh b/t/t0611-reftable-httpd.sh
new file mode 100755
index 0000000000..5e05b9c1f2
--- /dev/null
+++ b/t/t0611-reftable-httpd.sh
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+test_description='reftable HTTPD tests'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-httpd.sh
+
+start_httpd
+
+REPO="$HTTPD_DOCUMENT_ROOT_PATH/repo"
+
+test_expect_success 'serving ls-remote' '
+ git init --ref-format=reftable -b main "$REPO" &&
+ cd "$REPO" &&
+ test_commit m1 &&
+ >.git/git-daemon-export-ok &&
+ git ls-remote "http://127.0.0.1:$LIB_HTTPD_PORT/smart/repo" | cut -f 2-2 -d " " >actual &&
+ cat >expect <<-EOF &&
+ HEAD
+ refs/heads/main
+ refs/tags/m1
+ EOF
+ test_cmp actual expect
+'
+
+test_done
diff --git a/t/test-lib.sh b/t/test-lib.sh
index fc93aa57e6..0fb8757b7d 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1750,6 +1750,8 @@ esac
case "$GIT_DEFAULT_REF_FORMAT" in
files)
test_set_prereq REFFILES;;
+reftable)
+ test_set_prereq REFTABLE;;
*)
echo 2>&1 "error: unknown ref format $GIT_DEFAULT_REF_FORMAT"
exit 1
--
2.43.GIT
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v3 2/2] ci: add jobs to test with the reftable backend
From: Patrick Steinhardt @ 2024-02-05 6:02 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Han-Wen Nienhuys, Karthik Nayak
In-Reply-To: <cover.1707109509.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 3294 bytes --]
Add CI jobs for both GitHub Workflows and GitLab CI to run Git with the
new reftable backend.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
.github/workflows/main.yml | 9 +++++++++
.gitlab-ci.yml | 9 +++++++++
ci/lib.sh | 2 +-
ci/run-build-and-tests.sh | 3 +++
4 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 4d97da57ec..1b43e49dad 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -266,6 +266,9 @@ jobs:
- jobname: linux-sha256
cc: clang
pool: ubuntu-latest
+ - jobname: linux-reftable
+ cc: clang
+ pool: ubuntu-latest
- jobname: linux-gcc
cc: gcc
cc_package: gcc-8
@@ -277,6 +280,9 @@ jobs:
- jobname: osx-clang
cc: clang
pool: macos-13
+ - jobname: osx-reftable
+ cc: clang
+ pool: macos-13
- jobname: osx-gcc
cc: gcc
cc_package: gcc-13
@@ -287,6 +293,9 @@ jobs:
- jobname: linux-leaks
cc: gcc
pool: ubuntu-latest
+ - jobname: linux-reftable-leaks
+ cc: gcc
+ pool: ubuntu-latest
- jobname: linux-asan-ubsan
cc: clang
pool: ubuntu-latest
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 43bfbd8834..c0fa2fe90b 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -26,6 +26,9 @@ test:linux:
- jobname: linux-sha256
image: ubuntu:latest
CC: clang
+ - jobname: linux-reftable
+ image: ubuntu:latest
+ CC: clang
- jobname: linux-gcc
image: ubuntu:20.04
CC: gcc
@@ -40,6 +43,9 @@ test:linux:
- jobname: linux-leaks
image: ubuntu:latest
CC: gcc
+ - jobname: linux-reftable-leaks
+ image: ubuntu:latest
+ CC: gcc
- jobname: linux-asan-ubsan
image: ubuntu:latest
CC: clang
@@ -79,6 +85,9 @@ test:osx:
- jobname: osx-clang
image: macos-13-xcode-14
CC: clang
+ - jobname: osx-reftable
+ image: macos-13-xcode-14
+ CC: clang
artifacts:
paths:
- t/failed-test-artifacts
diff --git a/ci/lib.sh b/ci/lib.sh
index d5dd2f2697..0a73fc7bd1 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -367,7 +367,7 @@ linux-musl)
MAKEFLAGS="$MAKEFLAGS NO_REGEX=Yes ICONV_OMITS_BOM=Yes"
MAKEFLAGS="$MAKEFLAGS GIT_TEST_UTF8_LOCALE=C.UTF-8"
;;
-linux-leaks)
+linux-leaks|linux-reftable-leaks)
export SANITIZE=leak
export GIT_TEST_PASSING_SANITIZE_LEAK=true
export GIT_TEST_SANITIZE_LEAK_LOG=true
diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
index 7a1466b868..c192bd613c 100755
--- a/ci/run-build-and-tests.sh
+++ b/ci/run-build-and-tests.sh
@@ -37,6 +37,9 @@ linux-clang)
linux-sha256)
export GIT_TEST_DEFAULT_HASH=sha256
;;
+linux-reftable|linux-reftable-leaks|osx-reftable)
+ export GIT_TEST_DEFAULT_REF_FORMAT=reftable
+ ;;
pedantic)
# Don't run the tests; we only care about whether Git can be
# built.
--
2.43.GIT
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: git bug
From: Patrick Steinhardt @ 2024-02-05 6:47 UTC (permalink / raw)
To: moti sd; +Cc: git
In-Reply-To: <CAPvDF0P7_s-iy_V7FNSHtXXc9v5E3Fm=AdgduDDsd0rM-zNg-g@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1748 bytes --]
On Mon, Feb 05, 2024 at 12:42:03AM +0100, moti sd wrote:
> Dear Git Team,
>
> I am writing to report a potential bug encountered while using the "git
> stash" command. The issue was observed during a troubleshooting. Please
> find the details below:
>
> *Problem Description:*
>
> The "git stash" command is not providing any feedback or indication of
> execution, even though modifications are present and the stash operation is
> attempted.
> *Steps to Reproduce:*
>
> Make modifications to a file.
> Execute "git stash" to stash changes.
> *Expected Behavior:*
>
> The "git stash" command should provide feedback on the stash operation,
> confirming whether it was successful or if there were any errors.
> *Additional Information:*
> The issue was initially observed when executing "git commit -a --amend" and
> pressing Ctrl+Z to exit. Subsequent attempts to use "git stash" resulted in
> an error.
> Testing with a provided ZIP file containing a problematic repository
> confirmed the issue's recurrence.
> *Investigation and Findings:*
>
> Upon investigation, it was discovered that the use of Ctrl+Z after "git
> commit -a --amend" might be the root cause.
> Deleting a lock file left behind by the incomplete git command resolved the
> issue, indicating a potential bug in how "git stash" handles incomplete
> commands.
> *Recommendation:*
This is indeed it. The problem is that the index will be locked while
git-commit(1) is active, and thus git-stash(1) cannot write to it at the
same time. So this is working as designed. What's not though is the fact
that we don't print an error message.
I'll send a patch in reply to this message to fix this bug. Thanks for
your report!
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH] builtin/stash: report failure to write to index
From: Patrick Steinhardt @ 2024-02-05 7:01 UTC (permalink / raw)
To: git; +Cc: moti sd
In-Reply-To: <CAPvDF0P7_s-iy_V7FNSHtXXc9v5E3Fm=AdgduDDsd0rM-zNg-g@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3630 bytes --]
The git-stash(1) command needs to write to the index for many of its
operations. When the index is locked by a concurrent writer it will thus
fail to operate, which is expected. What is not expected though is that
we do not print any error message at all in this case. The user can thus
easily miss the fact that the command didn't do what they expected it to
do and would be left wondering why that is.
Fix this bug and report failures to write to the index. Add tests for
the subcommands which hit the respective code paths.
Note that the chosen error message ("Cannot write to the index") does
not match our guidelines as it starts with a capitalized letter. This is
intentional though and matches the style of all the other messages used
in git-stash(1).
Reported-by: moti sd <motisd8@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/stash.c | 6 +++---
t/t3903-stash.sh | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/builtin/stash.c b/builtin/stash.c
index b2813c614c..9df072b459 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -537,7 +537,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
repo_read_index_preload(the_repository, NULL, 0);
if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
NULL, NULL, NULL))
- return -1;
+ return error(_("Cannot write to the index"));
if (write_index_as_tree(&c_tree, &the_index, get_index_file(), 0,
NULL))
@@ -1364,7 +1364,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
repo_read_index_preload(the_repository, NULL, 0);
if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
NULL, NULL, NULL) < 0) {
- ret = -1;
+ ret = error(_("Cannot write to the index"));
goto done;
}
@@ -1555,7 +1555,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
NULL, NULL, NULL)) {
- ret = -1;
+ ret = error(_("Cannot write to the index"));
goto done;
}
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 3319240515..770881e537 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1516,4 +1516,56 @@ test_expect_success 'restore untracked files even when we hit conflicts' '
)
'
+test_expect_success 'stash create reports a locked index' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit A A.file &&
+ echo change >A.file &&
+ touch .git/index.lock &&
+
+ cat >expect <<-EOF &&
+ error: Cannot write to the index
+ EOF
+ test_must_fail git stash create 2>err &&
+ test_cmp expect err
+ )
+'
+
+test_expect_success 'stash push reports a locked index' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit A A.file &&
+ echo change >A.file &&
+ touch .git/index.lock &&
+
+ cat >expect <<-EOF &&
+ error: Cannot write to the index
+ EOF
+ test_must_fail git stash push 2>err &&
+ test_cmp expect err
+ )
+'
+
+test_expect_success 'stash apply reports a locked index' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit A A.file &&
+ echo change >A.file &&
+ git stash push &&
+ touch .git/index.lock &&
+
+ cat >expect <<-EOF &&
+ error: Cannot write to the index
+ EOF
+ test_must_fail git stash apply 2>err &&
+ test_cmp expect err
+ )
+'
+
test_done
--
2.43.GIT
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH] Add ideas for GSoC 2024
From: Patrick Steinhardt @ 2024-02-05 8:39 UTC (permalink / raw)
To: git
Cc: Christian Couder, Taylor Blau, Junio C Hamano, Victoria Dye,
Karthik Nayak
In-Reply-To: <d4797f27-825b-4e2b-85a6-cc30f33934e3@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 6059 bytes --]
Add project ideas for the GSoC 2024.
---
I came up with four different topics:
- The reftable unit test refactorings. This topic can also be squashed
into the preexisting unit test topics, I wouldn't mind. In that case
I'd be happy to be a possible mentor, too.
- Ref consistency checks for git-fsck(1). This should be rather
straight forward and make for an interesting topic.
- Making git-bisect(1)'s state more self-contained as recently
discussed. This topic is easy to implement, but the backwards
compatibility issues might require a lot of attention.
- Implementing support for reftables in the "dumb" HTTP protocol. It's
quite niche given that the dumb protocol isn't really used much
nowadays anymore. But it could make for an interesting project
regardless.
It's hard to estimate for me whether their scope is either too small or
too big. So please feel free to chime in and share your concerns if you
think that any of those proposals don't make much sense in your opinion.
Patrick
SoC-2024-Ideas.md | 129 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 129 insertions(+)
diff --git a/SoC-2024-Ideas.md b/SoC-2024-Ideas.md
index 3efbcaf..286aea0 100644
--- a/SoC-2024-Ideas.md
+++ b/SoC-2024-Ideas.md
@@ -39,3 +39,132 @@ Languages: C, shell(bash)
Possible mentors:
* Christian Couder < <christian.couder@gmail.com> >
+### Convert reftable unit tests to use the unit testing framework
+
+The "reftable" unit tests in "t0032-reftable-unittest.sh"
+predate the unit testing framework that was recently
+introduced into Git. These tests should be converted to use
+the new framework.
+
+See:
+
+ - this discussion <https://lore.kernel.org/git/cover.1692297001.git.steadmon@google.com/>
+
+Expected Project Size: 175 hours or 350 hours
+
+Difficulty: Low
+
+Languages: C, shell(bash)
+
+Possible mentors:
+* Patrick Steinhardt < <ps@pks.im> >
+* Karthik Nayak < <karthik.188@gmail.com> >
+
+### Implement consistency checks for refs
+
+The git-fsck(1) command is used to check various data
+structures for consistency. Notably missing though are
+consistency checks for the refdb. While git-fsck(1)
+implicitly checks some of the properties of the refdb
+because it uses its refs for a connectivity check, these
+checks aren't sufficient to properly ensure that all refs
+are properly consistent.
+
+The goal of this project would be to introduce consistency
+checks that can be implemented by the ref backend. Initially
+these checks may only apply to the "files" backend. With the
+ongoing efforts to upstream a new "reftable" backend the
+effort may be extended.
+
+See:
+
+ - https://lore.kernel.org/git/6cfee0e4-3285-4f18-91ff-d097da9de737@rd10.de/
+ - https://lore.kernel.org/git/cover.1706601199.git.ps@pks.im/
+
+Expected Project Size: 175 hours or 350 hours
+
+Difficulty: Medium
+
+Languages: C, shell(bash)
+
+Possible mentors:
+* Patrick Steinhardt < <ps@pks.im> >
+* Karthik Nayak < <karthik.188@gmail.com> >
+
+### Refactor git-bisect(1) to make its state self-contained
+
+The git-bisect(1) command is used to find a commit in a
+range of commits that introduced a specific bug. Starting a
+bisection run creates a set of state files into the Git
+repository which record various different parameters like
+".git/BISECT_START". These files look almost like refs
+due to their names being all-uppercase. This has led to
+confusion with the new "reftable" backend because it wasn't
+quite clear whether those files are in fact refs or not.
+
+As it turns out they are not refs and should never be
+treated like one. Overall, it has been concluded that the
+way those files are currently stored is not ideal. Instead
+of having a proliferation of files in the Git directory, it
+was discussed whether the bisect state should be moved into
+its own "bisect-state" subdirectory. This would make it more
+self-contained and thereby avoid future confusion. It is
+also aligned with the sequencer state used by rebases, which
+is neatly contained in the "rebase-apply" and "rebase-merge"
+directories.
+
+The goal of this project would be to realize this change.
+While rearchitecting the layout should be comparatively easy
+to do, the harder part will be to hash out how to handle
+backwards compatibility.
+
+See:
+
+ - https://lore.kernel.org/git/Za-gF_Hp_lXViGWw@tanuki/
+
+Expected Project Size: 175 hours or 350 hours
+
+Difficulty: Medium
+
+Languages: C, shell(bash)
+
+Possible mentors:
+* Patrick Steinhardt < <ps@pks.im> >
+* Karthik Nayak < <karthik.188@gmail.com> >
+
+### Implement support for reftables in "dumb" HTTP transport
+
+Fetching Git repositories uses one of two major protocols:
+
+ - The "dumb" protocol works without requiring any kind of
+ interactive negotiation like a CGI module. It can thus
+ be served by a static web server.
+
+ - The "smart" protocol works by having the client and
+ server exchange multiple messages with each other. It is
+ more efficient, but requires support for Git in the
+ server.
+
+While almost all servers nowadays use the "smart" protocol,
+there are still some that use the "dumb" protocol.
+
+The "dumb" protocol cannot serve repositories which use the
+"reftable" backend though. While there exists a "info/refs"
+file that is supposed to be backend-agnostic, this file does
+not contain information about the default branch. Instead,
+clients are expected to download the "HEAD" file and derive
+the default branch like that. This file is a mere stub in
+the "reftable" backend though, which breaks this protocol.
+
+The goal of this project is to implement "reftable" support
+for "dumb" fetches.
+
+Expected Project Size: 175 hours or 350 hours
+
+Difficulty: Medium
+
+Languages: C, shell(bash)
+
+Possible mentors:
+* Patrick Steinhardt < <ps@pks.im> >
+* Karthik Nayak < <karthik.188@gmail.com> >
--
2.43.GIT
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: [PATCH 2/2] http: prevent redirect from dropping credentials during reauth
From: Robert Coup @ 2024-02-05 9:22 UTC (permalink / raw)
To: Quentin Bouget; +Cc: rsbecker, git
In-Reply-To: <CYWTFH7Z8IY4.313MNLG552V8M@devyard.org>
Hi Quentin,
> I have suggested to switch to implementing the same behaviour as libcurl when it comes to reusing credentials: if the hostname of the redirect is the same as the original URL, reuse the credentials, otherwise drop them.
The protocol & port number also need to match, so it doesn't end up
the same as CVE-2022-27774 [1]
Rob :)
[1] https://curl.se/docs/CVE-2022-27774.html
^ permalink raw reply
* Re: [PATCH] t0091: allow test in a repository without tags
From: Martin Ågren @ 2024-02-05 10:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqv87aabk3.fsf@gitster.g>
On Tue, 30 Jan 2024 at 20:03, Junio C Hamano <gitster@pobox.com> wrote:
>
> The beginning of the [System Info] section, which should match the
> "git version --build-options" output, may not identify our version
> as "git version 2.whatever". When build in a repository cloned
> without tags, for example, "git version unknown.g00000000" can be a
> legit version string.
I've tried reproducing this along the lines of
git clone --no-tags <url>
cd git
make
(cd t && ./t0091-*)
but this works for me on current master, 2a540e432f ("The thirteenth
batch", 2024-02-02), which doesn't have this patch.
GIT-VERSION-GEN seems to be careful to only use tags on the wanted form.
My build generates a git version of "2.43.GIT", no "unknown..." stuff.
I don't doubt that you've hit this, I just wonder which piece of the
puzzle I'm missing.
> - # The beginning should match "git version --build-info" verbatim,
> + # The beginning should match "git version --build-options" verbatim,
Correct, my thinko-typo, thanks for correcting.
> # but rather than checking bit-for-bit equality, just test some basics.
> - grep "git version [0-9]." system &&
> + grep "git version " system &&
This matches the commit message, ok.
Martin
^ permalink raw reply
* Re: [PATCH v3 5/8] reftable/record: store "val1" hashes as static arrays
From: Karthik Nayak @ 2024-02-05 11:39 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Han-Wen Nienhuys, Junio C Hamano
In-Reply-To: <f1d21904892153c74d22e4fc814aedfafcb28f41.1704262787.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1069 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> When reading ref records of type "val1", we store its object ID in an
> allocated array. This results in an additional allocation for every
> single ref record we read, which is rather inefficient especially when
> iterating over refs.
>
> Refactor the code to instead use an embedded array of `GIT_MAX_RAWSZ`
> bytes. While this means that `struct ref_record` is bigger now, we
> typically do not store all refs in an array anyway and instead only
> handle a limited number of records at the same point in time.
>
> Using `git show-ref --quiet` in a repository with ~350k refs this leads
> to a significant drop in allocations. Before:
>
> HEAP SUMMARY:
> in use at exit: 21,098 bytes in 192 blocks
> total heap usage: 2,116,683 allocs, 2,116,491 frees, 76,098,060 bytes allocated
>
> After:
>
> HEAP SUMMARY:
> in use at exit: 21,098 bytes in 192 blocks
> total heap usage: 1,419,031 allocs, 1,418,839 frees, 62,145,036 bytes allocated
Curious, did you also do perf benchmarking on this?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* [PATCH] branch: clarify <oldbranch> and <newbranch> terms further
From: Dragan Simic @ 2024-02-05 12:45 UTC (permalink / raw)
To: git
Clarify further the uses for <oldbranch> and describe the additional use
for <newbranch>. Mentioning both renaming and copying in these places might
seem a bit redundant, but it should actually make understanding these terms
easier to the readers of the git-branch(1) man page.
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
---
Documentation/git-branch.txt | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 0b0844293235..7392c2f0797d 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -312,12 +312,14 @@ superproject's "origin/main", but tracks the submodule's "origin/main".
option is omitted, the current HEAD will be used instead.
<oldbranch>::
- The name of an existing branch. If this option is omitted,
- the name of the current branch will be used instead.
+ The name of an existing branch to be renamed or copied.
+ If this option is omitted, the name of the current branch
+ will be used instead.
<newbranch>::
- The new name for an existing branch. The same restrictions as for
- <branchname> apply.
+ The new name for an existing branch, when renaming a branch,
+ or the name for a new branch, when copying a branch. The same
+ naming restrictions apply as for <branchname>.
--sort=<key>::
Sort based on the key given. Prefix `-` to sort in descending
^ permalink raw reply related
* Re: [PATCH v3 0/8] reftable: fixes and optimizations (pt.2)
From: Karthik Nayak @ 2024-02-05 13:08 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Han-Wen Nienhuys, Junio C Hamano
In-Reply-To: <cover.1704262787.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 245 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
Hello,
> Hi,
>
> this is the third version of my patch series that contains additional
> fixes and optimizations for the reftable library.
>
I have reviewed this version and have nothing to add. Thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH v3 0/2] refs: introduce reftable backend
From: Karthik Nayak @ 2024-02-05 13:10 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Junio C Hamano, Han-Wen Nienhuys
In-Reply-To: <cover.1707109509.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 3234 bytes --]
Hello,
Patrick Steinhardt <ps@pks.im> writes:
> Hi,
>
> this is the third version of my patch series that introduces the
> reftable backend.
>
> The only real change in this version is that I've pulled in
> kn/for-all-refs at 693e807311 (for-each-ref: avoid filtering on empty
> pattern, 2024-01-29) as a dependency. This series introduces a new
> DO_FOR_EACH_INCLUDE_ALL_REFS flag that we need to handle in the reftable
> backend. With this change all tests should pass again.
>
> Patrick
>
> Patrick Steinhardt (2):
> refs: introduce reftable backend
> ci: add jobs to test with the reftable backend
>
> .github/workflows/main.yml | 9 +
> .gitlab-ci.yml | 9 +
> Documentation/ref-storage-format.txt | 2 +
> .../technical/repository-version.txt | 5 +-
> Makefile | 1 +
> ci/lib.sh | 2 +-
> ci/run-build-and-tests.sh | 3 +
> contrib/workdir/git-new-workdir | 2 +-
> path.c | 2 +-
> path.h | 1 +
> refs.c | 1 +
> refs/refs-internal.h | 1 +
> refs/reftable-backend.c | 2297 +++++++++++++++++
> repository.h | 5 +-
> t/t0610-reftable-basics.sh | 887 +++++++
> t/t0611-reftable-httpd.sh | 26 +
> t/test-lib.sh | 2 +
> 17 files changed, 3248 insertions(+), 7 deletions(-)
> create mode 100644 refs/reftable-backend.c
> create mode 100755 t/t0610-reftable-basics.sh
> create mode 100755 t/t0611-reftable-httpd.sh
>
> Range-diff against v2:
> 1: d6548dcfad ! 1: d83e66e980 refs: introduce reftable backend
> @@ refs/reftable-backend.c (new)
> +{
> + switch (log->value_type) {
> + case REFTABLE_LOG_UPDATE:
> -+ /* when we write log records, the hashes are owned by a struct
> -+ * oid */
> ++ /*
> ++ * When we write log records, the hashes are owned by the
> ++ * caller and thus shouldn't be free'd.
> ++ */
> + log->value.update.old_hash = NULL;
> + log->value.update.new_hash = NULL;
> + break;
> @@ refs/reftable-backend.c (new)
> + break;
> +
> + /*
> -+ * The files backend only lists references contained in
> -+ * "refs/". We emulate the same behaviour here and thus skip
> -+ * all references that don't start with this prefix.
> ++ * Unless DO_FOR_EACH_INCLUDE_ALL_REFS is set, we only list
> ++ * refs starting with "refs/" to mimic the "files" backend.
> + */
> -+ if (!starts_with(iter->ref.refname, "refs/"))
> ++ if (!(iter->flags & DO_FOR_EACH_INCLUDE_ALL_REFS) &&
> ++ !starts_with(iter->ref.refname, "refs/"))
> + continue;
> +
> + if (iter->prefix &&
> 2: 63eafc9f5b = 2: 146bb95c03 ci: add jobs to test with the reftable backend
This range-diff looks good to me. I'd already reviewed the previous
version, so overall looks good now.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* [PATCH 2/2] sequencer: unset GIT_CHERRY_PICK_HELP for 'exec' commands
From: Vegard Nossum @ 2024-02-05 14:13 UTC (permalink / raw)
To: gitster; +Cc: git, Jonathan Nieder, Vegard Nossum, Phillip Wood
In-Reply-To: <0adb1068-ef10-44ed-ad1d-e0927a09245d@gmail.com>
Running "git cherry-pick" as an x-command in the rebase plan loses the
original authorship information.
To fix this, unset GIT_CHERRY_PICK_HELP for 'exec' commands.
Link: https://lore.kernel.org/git/0adb1068-ef10-44ed-ad1d-e0927a09245d@gmail.com/
Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
sequencer.c | 1 +
t/t3515-cherry-pick-rebase.sh | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index 91de546b32..f49a871ac0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3641,6 +3641,7 @@ static int do_exec(struct repository *r, const char *command_line)
fprintf(stderr, _("Executing: %s\n"), command_line);
cmd.use_shell = 1;
strvec_push(&cmd.args, command_line);
+ strvec_push(&cmd.env, "GIT_CHERRY_PICK_HELP");
status = run_command(&cmd);
/* force re-reading of the cache */
diff --git a/t/t3515-cherry-pick-rebase.sh b/t/t3515-cherry-pick-rebase.sh
index ffe6f5fe2a..5cb2b96f66 100755
--- a/t/t3515-cherry-pick-rebase.sh
+++ b/t/t3515-cherry-pick-rebase.sh
@@ -23,7 +23,7 @@ test_expect_success 'cherry-pick preserves authorship information' '
test_cmp expected actual
'
-test_expect_failure 'cherry-pick inside rebase preserves authorship information' '
+test_expect_success 'cherry-pick inside rebase preserves authorship information' '
git checkout -B tmp feature &&
echo "x git cherry-pick -x foo" >rebase-plan &&
test_must_fail env GIT_SEQUENCE_EDITOR="cp rebase-plan" git rebase -i feature &&
--
2.34.1
^ 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