* 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: brian m. carlson @ 2024-02-04 22:36 UTC (permalink / raw)
To: Quentin Bouget; +Cc: git
In-Reply-To: <20240204185427.39664-3-ypsah@devyard.org>
[-- Attachment #1: Type: text/plain, Size: 1166 bytes --]
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.
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.
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply
* submodules: multiple alternative relative URL schemes?
From: Matthew Ogilvie @ 2024-02-04 22:14 UTC (permalink / raw)
To: git
Background:
At work we are switching to submodules to share various
utility code between mostly-unrelated projects and programs.
When working on any one project, I commonly fetch and push
preliminary changes beween numerous different sandboxes in
various places (same machine, different machines, VM's, etc).
Basically, I make extensive use of git's distributed nature.
Issue:
However, submodules don't seem to streamline fetching/pushing
the whole collection of modules together from varied sources
very well, without manually configuring or manipulating each
submodule and its' "remote"s individually. For example, if
I recursively clone from a local sandbox with local changes
into another local sandbox, it will clone the main superproject
from the local source, but the submodules will be downloaded
from the URLs in the .gitmodules file, not the local copies
(that might have local changes that will be missed).
-----
Relative URLs could maybe help, except there seems to be at least
two common relative URL repository schemes/organizations that
are often needed, and probably more:
1. Subproject bare repositories are immediately adjacent to the
superproject repository, such that relative URLs like
"../submodule1.git" should work.
This is probably common in web-based hosting of personal "forks"
like in github, bitbucket, or similar.
It is also the most obvious way to organize multiple
semi-related repositories on a small self-hosted server.
2. Or you want to fetch changes between sandboxes, perhaps
to test code changes on multiple platforms, or to continue someone
else's incomplete work. In this case, relative links like
"./.git/modules/submodule1" would probably work. But bare/origin/main
repositories would rarely be organized like this.
3. "Official" configured upstream URLs may also be organized in other
ways. For example, in something like github, different upstream URLs
may be "owned" by different owners, so they may URLs like
"BASE/OWNER1/MODULE1.git", "BASE/OWNER2/MODULE2.git",
with different and semi-random "OWNER"s, not amenable to template
strings (see (E) below). Or different submodule upstreams may be on
completely different servers...
-----
Basically, I would like to restore or at least improve git's
ability to transparently/smoothly work in a "distributed" manner
(no "blesssed main upstream repositories") when a project
requires certain submodules.
Some Ideas:
A. Is there something I'm missing? Is there some good solution
I just haven't stumbled over? Any documentation URLs and/or
keywords I should look for?
B. Of course no matter what, you could always set up all your various
remote URLs individually manually, or write special project-specific
helper scripts. But something more automated and built-in
would be nice...
C. future?: Potentially, the .gitmodules file could define multiple
different URLs for each submodule, distinguished by named URL
"schemes" that could be specified on the command line.
"git clone", "git fetch", "git push", "git submodule", etc could
all take an optional "scheme" argument of some kind to indicate
which sets of URLs to use...
D. future?: Potentially hard-code some schemes? 1 and 2 are probably the
most common alternative relative URL schemes, so it might be worth
explicitly supporting them without requiring any explicit configuration
changes. Maybe this would be adequate by itself; don't even support
other "alternative" named schemes (multiple 3's)?
E. future?: In some cases, a "scheme" for a whole set of URLs
could maybe be simplified into a single URL template string.
Substitute in the "base" and the submodule name to get a
specific URL...
F. Potentially add some optional fallback logic? If it fails to
fetch/clone from one URL, try another scheme instead? (But this
might leave things in a confusingly unpredictable/mixed state,
particularly if the network is flakey...)
G. Different approach: Instead of separate repositories, maybe
the submodules could essentially all be in one repository, but
distinguished by using different tags/namespaces in the "refs"
hierarchy. ("refs/heads/SUBMODULE/WHATEVER"?
"refs/SUBMODULE/heads/WHATEVER"?) The "original/main" upstream
might still split up repositories, but clones/local changes/etc
could be more self-contained this way... (Working out all the
nuances to streamline this idea might be a whole separate
discussion...)
H. "git subtree" instead of submodules: If it was up to me, I like
the subtree solution. Most of the time you can ignore the fact that
some code is shared, and only need to worry about it when actually
syncing/merging shared code between different projects, which is
generally much rarer than day-to-day development tasks.
But unfortunately subtrees seems to be somewhat unpopular, as
currently implemented: Isolated in "contrib", doing graph traversal
logic in shell script of all things (very slow, especially on
Windows where many of my coworkers reside), etc. And ultimately
in my case I think I've already lost this argument. So I'm trying
to figure out how to make submodules work better.
I. Any other ideas?
-----
In the short term, I expect I'll have to resort to (B) (custom scripts).
In the longer term, depending on what people think and if I can find
the time, maybe I could actually implement some of these ideas
in git for a future release? What do people think of these ideas?
- Matthew Ogilvie
^ permalink raw reply
* [PATCH 2/2] http: prevent redirect from dropping credentials during reauth
From: Quentin Bouget @ 2024-02-04 18:54 UTC (permalink / raw)
To: git; +Cc: Quentin Bouget
In-Reply-To: <20240204185427.39664-1-ypsah@devyard.org>
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;
}
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] .github/PULL_REQUEST_TEMPLATE.md: add a note about single-commit PRs
From: Junio C Hamano @ 2024-02-04 20:17 UTC (permalink / raw)
To: Philippe Blain via GitGitGadget; +Cc: git, Johannes Schindelin, Philippe Blain
In-Reply-To: <pull.1665.git.git.1707069808205.gitgitgadget@gmail.com>
"Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes:
> diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md
> index 952c7c3a2aa..65fa3a37173 100644
> --- a/.github/PULL_REQUEST_TEMPLATE.md
> +++ b/.github/PULL_REQUEST_TEMPLATE.md
> @@ -4,4 +4,8 @@ a mailing list (git@vger.kernel.org) for code submissions, code reviews, and
> bug reports. Nevertheless, you can use GitGitGadget (https://gitgitgadget.github.io/)
> to conveniently send your Pull Requests commits to our mailing list.
>
> +If you use Gitgitgadget for a single-commit pull request, please *leave the pull
> +request description empty*: your commit message itself should describe your
> +changes.
> +
> Please read the "guidelines for contributing" linked above!
Making it easier for contributors to come up with the right output
is greatly appreciated. I think "If you use Gitgitgadget for" ->
"For" is probably a good change, for two reasons (one, we do not
take pull request except for GGG gateway in the first place, and
two, PR messages being similar to cover letters, you do not want to
have a detailed PR message that (a) takes extra time to write, (b)
can duplicate what the you already have written, and (c) could
contradict what the commit log message says.
I wonder if such a rule can be also enforced at the GGG side? It
apparently knows if it is dealing with a single-patch request or a
multi-patch series (as the types of messages this documentation
update tries to address are the only ones that get duplicates under
the three-dash line), and I've seen GGG complain on the contents of
the commit log message (e.g., "not signed off") so there is enough
support to inspect things in a PR and add instruction to the PR
discussion. Unless the machinery GGG uses lack the ability to read
the PR message (unlike the commit log messages that it can read
apparently), it may be able to enforce that "for a single patch, PR
message should be empty" rule before the /submit instruction. It
might make things even more helpful if it can notice the commit log
message is similar enough or superset of PR message and refrain from
inserting it after the three-dash line, but that might be asking too
much ;-)
Thanks for helping to improve the situation.
Will queue.
^ permalink raw reply
* [PATCH 0/2] Fix gitlab's token-based authentication w/ kerberos
From: Quentin Bouget @ 2024-02-04 18:54 UTC (permalink / raw)
To: git; +Cc: Quentin Bouget
Gitlab supports GSSAPI-based authentication with kerberos but when setup
on port 443, token-based authentication with credentials provided in the
remote's URL no longer works (cf. the note here [1]).
This patch series provides a fix which I tested against such a gitlab
setup.
This is my first ever contribution to git, apologies in advance for any
"faux pas".
[1] https://docs.gitlab.com/ee/integration/kerberos.html#http-git-access-with-kerberos-token-passwordless-authentication
Quentin Bouget (2):
http: only reject basic auth credentials once they have been tried
http: prevent redirect from dropping credentials during reauth
http.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH 1/2] http: only reject basic auth credentials once they have been tried
From: Quentin Bouget @ 2024-02-04 18:54 UTC (permalink / raw)
To: git; +Cc: Quentin Bouget
In-Reply-To: <20240204185427.39664-1-ypsah@devyard.org>
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
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
^ permalink raw reply related
* [PATCH] .github/PULL_REQUEST_TEMPLATE.md: add a note about single-commit PRs
From: Philippe Blain via GitGitGadget @ 2024-02-04 18:03 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Philippe Blain, Philippe Blain
From: Philippe Blain <levraiphilippeblain@gmail.com>
Contributors using Gitgitgadget continue to send single-commit PRs with
their commit message text duplicated below the three-dash line,
increasing the signal-to-noise ratio for reviewers.
This is because Gitgitgadget copies the pull request description as an
in-patch commentary, for single-commit PRs, and _GitHub_ defaults to
prefilling the pull request description with the commit message, for
single-commit PRs (followed by the content of the pull request
template).
Add a note in the pull request template mentioning that for
single-commit PRs, the PR description should thus be kept empty, in the
hope that contributors read it and act on it.
This partly addresses:
https://github.com/gitgitgadget/gitgitgadget/issues/340
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
.github/PULL_REQUEST_TEMPLATE.md: add a note about single-commit PRs
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1665%2Fphil-blain%2Fempty-description-single-commit-prs-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1665/phil-blain/empty-description-single-commit-prs-v1
Pull-Request: https://github.com/git/git/pull/1665
.github/PULL_REQUEST_TEMPLATE.md | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md
index 952c7c3a2aa..65fa3a37173 100644
--- a/.github/PULL_REQUEST_TEMPLATE.md
+++ b/.github/PULL_REQUEST_TEMPLATE.md
@@ -4,4 +4,8 @@ a mailing list (git@vger.kernel.org) for code submissions, code reviews, and
bug reports. Nevertheless, you can use GitGitGadget (https://gitgitgadget.github.io/)
to conveniently send your Pull Requests commits to our mailing list.
+If you use Gitgitgadget for a single-commit pull request, please *leave the pull
+request description empty*: your commit message itself should describe your
+changes.
+
Please read the "guidelines for contributing" linked above!
base-commit: bc7ee2e5e16f0d1e710ef8fab3db59ab11f2bbe7
--
gitgitgadget
^ permalink raw reply related
* Re: Migrate away from vger to GitHub or (on-premise) GitLab?
From: Dragan Simic @ 2024-02-04 15:58 UTC (permalink / raw)
To: Michal Suchánek
Cc: Oswald Buddenhagen, phillip.wood, Patrick Steinhardt,
brian m. carlson, Hans Meiser, Konstantin Ryabitsev, git
In-Reply-To: <20240204155107.GA9696@kitsune.suse.cz>
On 2024-02-04 16:51, Michal Suchánek wrote:
> On Sun, Feb 04, 2024 at 04:28:58PM +0100, Dragan Simic wrote:
>> On 2024-02-04 16:12, Oswald Buddenhagen wrote:
>> > 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.
>>
>> Please, keep in mind that not everyone lives in a web browser and
>> loves to click around. Some people simply prefer to use the CLI
>> utilities and to press the keys on their keyboards, and are very
>> efficient while doing that.
>
> The forge vendors found out, and started to provide CLI tools. That's
> not really a general argument against forge software. Just as people
> living on web is not general argument against e-mail - it's been
> brought
> to the web a long time ago.
Please, don't get me wrong, I'm not against the GUI and web-based
utilities in the sense of telling other people they're bad or shouldn't
be used. I love the variety and the freedom of choice, so everyone
can freely choose the most suitable option for them.
Though, I'd also expect that everyone respect different choices made
by other people. That's how we keep the variety available.
^ permalink raw reply
* Re: Migrate away from vger to GitHub or (on-premise) GitLab?
From: Michal Suchánek @ 2024-02-04 15:51 UTC (permalink / raw)
To: Dragan Simic
Cc: Oswald Buddenhagen, phillip.wood, Patrick Steinhardt,
brian m. carlson, Hans Meiser, Konstantin Ryabitsev, git
In-Reply-To: <6dc25a1ab1531b508e844cee1c970438@manjaro.org>
On Sun, Feb 04, 2024 at 04:28:58PM +0100, Dragan Simic wrote:
> On 2024-02-04 16:12, Oswald Buddenhagen wrote:
> > 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.
>
> Please, keep in mind that not everyone lives in a web browser and
> loves to click around. Some people simply prefer to use the CLI
> utilities and to press the keys on their keyboards, and are very
> efficient while doing that.
The forge vendors found out, and started to provide CLI tools. That's
not really a general argument against forge software. Just as people
living on web is not general argument against e-mail - it's been brought
to the web a long time ago.
Thanks
Micchal
^ permalink raw reply
* Re: Migrate away from vger to GitHub or (on-premise) GitLab?
From: Michal Suchánek @ 2024-02-04 15:47 UTC (permalink / raw)
To: Oswald Buddenhagen
Cc: phillip.wood, Patrick Steinhardt, brian m. carlson, Hans Meiser,
Dragan Simic, Konstantin Ryabitsev, git@vger.kernel.org
In-Reply-To: <Zb+pQk9R3AOouFxF@ugly>
On Sun, Feb 04, 2024 at 04:12:02PM +0100, Oswald Buddenhagen wrote:
> 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.
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.
> 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.
Have you migrated them to gerrit already, and tought all the git
contributors how to use them from gerrit?
Somobody has to do it.
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?
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.
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?
I have seen what hoops Gitea is jumping through to import data from
other forges, and it's not pretty. Understandably, people who have seen
rise and fall of bitkeeper are wary of any tool that keeps your data
locked to itself.
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.
Thanks
Michal
^ permalink raw reply
* Re: Migrate away from vger to GitHub or (on-premise) GitLab?
From: Dragan Simic @ 2024-02-04 15:28 UTC (permalink / raw)
To: Oswald Buddenhagen
Cc: Michal Suchánek, phillip.wood, Patrick Steinhardt,
brian m. carlson, Hans Meiser, Konstantin Ryabitsev, git
In-Reply-To: <Zb+pQk9R3AOouFxF@ugly>
On 2024-02-04 16:12, Oswald Buddenhagen wrote:
> 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.
Please, keep in mind that not everyone lives in a web browser and
loves to click around. Some people simply prefer to use the CLI
utilities and to press the keys on their keyboards, and are very
efficient while doing that.
> 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
* cpkotsoomcsin
From: IPHONE @ 2024-02-04 14:43 UTC (permalink / raw)
To: git
Sent from my iPhone
^ permalink raw reply
* Re: Git in GSoC 2024
From: Kaartic Sivaraam @ 2024-02-04 14:29 UTC (permalink / raw)
To: Patrick Steinhardt, Christian Couder
Cc: git, Taylor Blau, Junio C Hamano, Victoria Dye, Karthik Nayak
In-Reply-To: <d4797f27-825b-4e2b-85a6-cc30f33934e3@gmail.com>
On 03/02/24 17:11, Kaartic Sivaraam wrote:
>
> On 01/02/24 15:08, Patrick Steinhardt wrote:
>>
>> I'll have a the beginning of next week and will think about topics
>> meanwhile.
>>
>
> Thanks, Patrick! It would be great if you could share the same as soon
> as possible. The deadline for applying to GSoC is Feb 6 (18:00 UTC) and
> we need the ideas page to be decent enough before we go ahead with
> applying for this year.
>
> If the elaborate project description could take time, feel free to share
> a paragraph or two that are supplemented with a few references. That
> should be sufficient for applying to GSoC.
>
If at all it's helpful, you could find the earlier idea pages in the
website repo [1]. For instance, the one from 2022 could be found at [2].
[[ References ]]
[1]: https://github.com/git/git.github.io
[2]: https://github.com/git/git.github.io/blob/master/SoC-2022-Ideas.md
--
Sivaraam
^ permalink raw reply
* Re: [PATCH] t/t3515-cherry-pick-rebase.sh: new testcase demonstrating broken behavior
From: Phillip Wood @ 2024-02-04 11:14 UTC (permalink / raw)
To: Vegard Nossum, gitster; +Cc: git, Jonathan Nieder, Harshit Mogalapalli
In-Reply-To: <20240202091850.160203-1-vegard.nossum@oracle.com>
Hi Vegard
On 02/02/2024 09:18, Vegard Nossum wrote:
> Running "git cherry-pick" as an x-command in the rebase plan loses the
> original authorship information.
>
> Write a known-broken test case for this:
>
> $ (cd t && ./t3515-cherry-pick-rebase.sh)
> ok 1 - setup
> ok 2 - cherry-pick preserves authorship information
> not ok 3 - cherry-pick inside rebase preserves authorship information # TODO known breakage
> # still have 1 known breakage(s)
> # passed all remaining 2 test(s)
> 1..3
>
> Running with --verbose we see the diff between expected and actual:
>
> --- expected 2024-02-02 08:54:48.954753285 +0000
> +++ actual 2024-02-02 08:54:48.966753294 +0000
> @@ -1 +1 @@
> -Original Author
> +A U Thor
>
> As far as I can tell, this is due to the check in print_advice()
> which deletes CHERRY_PICK_HEAD when GIT_CHERRY_PICK_HELP is set,
> but I'm not sure what a good fix would be.
Thanks for reporting this and for the test case. I agree with your
diagnosis. I think the simplest fix would be to unset
GIT_CHERRY_PICK_HELP in the child environment in sequencer.c:do_exec().
Long term we should stop setting GIT_CHERRY_PICK_HELP when rebasing and
hard code the rebase conflicts message in sequencer.c as the environment
variable is a vestige of the scripted rebase implementation.
To work around the bug I think you can change the exec lines in the todo
list to
exec unset GIT_CHERRY_PICK_HELP; git cherry-pick ...
Best Wishes
Phillip
> Cc: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> ---
> t/t3515-cherry-pick-rebase.sh | 37 +++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
> create mode 100755 t/t3515-cherry-pick-rebase.sh
>
> diff --git a/t/t3515-cherry-pick-rebase.sh b/t/t3515-cherry-pick-rebase.sh
> new file mode 100755
> index 0000000000..ffe6f5fe2a
> --- /dev/null
> +++ b/t/t3515-cherry-pick-rebase.sh
> @@ -0,0 +1,37 @@
> +#!/bin/sh
> +
> +test_description='test cherry-pick during a rebase'
> +
> +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> +
> +. ./test-lib.sh
> +
> +test_expect_success setup '
> + test_commit --author "Original Author <original.author@example.com>" foo file contents1 &&
> + git checkout -b feature &&
> + test_commit --author "Another Author <another.author@example.com>" bar file contents2
> +'
> +
> +test_expect_success 'cherry-pick preserves authorship information' '
> + git checkout -B tmp feature &&
> + test_must_fail git cherry-pick foo &&
> + git add file &&
> + git commit --no-edit &&
> + git log -1 --format='%an' foo >expected &&
> + git log -1 --format='%an' >actual &&
> + test_cmp expected actual
> +'
> +
> +test_expect_failure '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 &&
> + git add file &&
> + git commit --no-edit &&
> + git log -1 --format='%an' foo >expected &&
> + git log -1 --format='%an' >actual &&
> + test_cmp expected actual
> +'
> +
> +test_done
^ permalink raw reply
* Re: git-users: email list has become spam-drowned
From: Kristoffer Haugsbakk @ 2024-02-04 10:26 UTC (permalink / raw)
To: Konstantin Khomoutov; +Cc: git
In-Reply-To: <20240204084344.jzu2ciulbrdmxz4u@carbon>
On Sun, Feb 4, 2024, at 09:43, Konstantin Khomoutov wrote:
> On Sat, Feb 03, 2024 at 04:02:49PM +0100, Kristoffer Haugsbakk wrote:
>
>> I know that this list has got nothing to do with git-scm (per se). But I
>> was recommended by Dscho to raise the issue here.[1]
>>
>> Maybe someone who happens to be affiliated with git-users happens to
>> read this.
>
> OK, I've posted to the Google Groups help center [2].
> I do not expect this to produce any useful outcome but at least I've tried.
> Please feel free to chime in on the issue to that thread.
>
> 2. https://support.google.com/groups/thread/256971101
Thanks!
https://github.com/git/git-scm.com/pull/1829
--
Kristoffer Haugsbakk
^ permalink raw reply
* Re: git-users: email list has become spam-drowned
From: Konstantin Khomoutov @ 2024-02-04 8:43 UTC (permalink / raw)
To: git
In-Reply-To: <339f17e1-c6a0-4859-aa5b-481dd6e0e91b@app.fastmail.com>
On Sat, Feb 03, 2024 at 04:02:49PM +0100, Kristoffer Haugsbakk wrote:
> I know that this list has got nothing to do with git-scm (per se). But I
> was recommended by Dscho to raise the issue here.[1]
>
> Maybe someone who happens to be affiliated with git-users happens to
> read this.
OK, I've posted to the Google Groups help center [2].
I do not expect this to produce any useful outcome but at least I've tried.
Please feel free to chime in on the issue to that thread.
2. https://support.google.com/groups/thread/256971101
^ permalink raw reply
* Re: git-users: email list has become spam-drowned
From: Konstantin Khomoutov @ 2024-02-04 8:13 UTC (permalink / raw)
To: git
In-Reply-To: <339f17e1-c6a0-4859-aa5b-481dd6e0e91b@app.fastmail.com>
On Sat, Feb 03, 2024 at 04:02:49PM +0100, Kristoffer Haugsbakk wrote:
> I know that this list has got nothing to do with git-scm (per se). But I
> was recommended by Dscho to raise the issue here.[1]
>
> Maybe someone who happens to be affiliated with git-users happens to
> read this.
[...]
> I am not allowed to access it. Just as well because this is apparently
> what it looks like:
> https://www.mail-archive.com/git-users@googlegroups.com/
>
> The last legitimate thread (except for an email saying “bye, this list
> is too spammy”) was 2023-12-29.
(I was one of the active (answering) folks on that list.)
Unfortunately, the sole owner appears to be AWOL long ago, and honestly I do
not think I have ever seen a sole message from them since I have subscribed to
that list (back in 2013 or even earlier). The list appears to be shut down by
Google - possibly due to the fact I and possibly some other folks dutifully
marked each SPAM message as such using the web interface.
I have searched by failed to find any way to contact any kind of "support"
to have the list unblocked. Unfortunately, so far it looks like a lost cause
to me.
This is sad because the group had tractable traffic - as opposed to, say,
[git] tag on StackOverflow which has no SPAM but is literally drowned in silly
questions so that its signal-to-noise ratio is too low to get interested to
follow. Still, there's a couple of knowledgeable folks following it - namely,
Torek and VonC, so you might have luck asking there, when needed.
^ permalink raw reply
* Re: [PATCH v2 0/2] refs: introduce reftable backend
From: Patrick Steinhardt @ 2024-02-04 6:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Han-Wen Nienhuys, Karthik Nayak
In-Reply-To: <xmqq8r41qo0y.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 1115 bytes --]
On Sat, Feb 03, 2024 at 12:41:01PM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > this is the second version of my patch series that introduces the
> > reftable backend.
> >
> > This version addresses the feedback by Karthik. I don't think it really
> > makes sense to spell out every change here -- the range diff should
> > likely be easier to digest.
>
> This, when merged to 'seen' (which also has "for-each-ref now thinks
> an empty string is a signal to show ref-like things outside the
> refs/ hierarchy" topic, kn/for-all-refs), seems to break *-reftable
> CI tests, cf. https://github.com/git/git/actions/runs/7765401528
>
> I'll tentatively eject the topic from 'seen', even though I have a
> suspicion that it byitself would pass all the tests.
>
> Thanks.
Yup, this is known due to the semantic conflict with kn/for-all-refs, as
you point out. Karthik already sent a single-line change in this thread
that fixes the issue. I'll rebase the patch series tomorrow and pull in
the topic as a dependency and adapt accordingly.
Thanks!
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v6 2/2] patch-id: replace `atoi()` with `strtoi_with_tail`
From: Mohit Marathe via GitGitGadget @ 2024-02-04 5:48 UTC (permalink / raw)
To: git; +Cc: Mohit Marathe, Mohit Marathe
In-Reply-To: <pull.1646.v6.git.1707025718.gitgitgadget@gmail.com>
From: Mohit Marathe <mohitmarathe23@gmail.com>
The change is made to improve the error-handling capabilities
during the conversion of string to integers. The
`strtoi_with_tail` function offers a more robust mechanism for
converting strings to integers by providing enhanced error
detection. Unlike `atoi`, `strtoi_with_tail` allows the code to
differentiate between a valid conversion and an invalid one,
offering better resilience against potential issues such as
reading hunk header of a corrupted patch.
Signed-off-by: Mohit Marathe <mohitmarathe@proton.me>
---
builtin/patch-id.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/builtin/patch-id.c b/builtin/patch-id.c
index 3894d2b9706..4e9a301e9fb 100644
--- a/builtin/patch-id.c
+++ b/builtin/patch-id.c
@@ -1,3 +1,4 @@
+#include "git-compat-util.h"
#include "builtin.h"
#include "config.h"
#include "diff.h"
@@ -29,14 +30,16 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)
{
static const char digits[] = "0123456789";
const char *q, *r;
+ char *endp;
int n;
q = p + 4;
n = strspn(q, digits);
if (q[n] == ',') {
q += n + 1;
- *p_before = atoi(q);
- n = strspn(q, digits);
+ if (strtoi_with_tail(q, 10, p_before, &endp) != 0)
+ return 0;
+ n = endp - q;
} else {
*p_before = 1;
}
@@ -48,8 +51,9 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)
n = strspn(r, digits);
if (r[n] == ',') {
r += n + 1;
- *p_after = atoi(r);
- n = strspn(r, digits);
+ if (strtoi_with_tail(r, 10, p_after, &endp) != 0)
+ return 0;
+ n = endp - r;
} else {
*p_after = 1;
}
--
gitgitgadget
^ permalink raw reply related
* [PATCH v6 1/2] git-compat-util: add strtoi_with_tail()
From: Mohit Marathe via GitGitGadget @ 2024-02-04 5:48 UTC (permalink / raw)
To: git; +Cc: Mohit Marathe, Mohit Marathe
In-Reply-To: <pull.1646.v6.git.1707025718.gitgitgadget@gmail.com>
From: Mohit Marathe <mohitmarathe23@gmail.com>
This function is an updated version of strtol_i() function. It will
give more control to handle parsing of the characters after the
numbers and better error handling while parsing numbers.
Signed-off-by: Mohit Marathe <mohitmarathe@proton.me>
---
git-compat-util.h | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index 7c2a6538e5a..59256a441de 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -1296,14 +1296,24 @@ static inline int strtoul_ui(char const *s, int base, unsigned int *result)
return 0;
}
-static inline int strtol_i(char const *s, int base, int *result)
+#define strtol_i(s,b,r) strtoi_with_tail((s), (b), (r), NULL)
+static inline int strtoi_with_tail(char const *s, int base, int *result, char **endp)
{
long ul;
- char *p;
+ char *dummy = NULL;
+ if (!endp)
+ endp = &dummy;
errno = 0;
- ul = strtol(s, &p, base);
- if (errno || *p || p == s || (int) ul != ul)
+ ul = strtol(s, endp, base);
+ if (errno ||
+ /*
+ * if we are told to parse to the end of the string by
+ * passing NULL to endp, it is an error to have any
+ * remaining character after the digits.
+ */
+ (dummy && *dummy) ||
+ *endp == s || (int) ul != ul)
return -1;
*result = ul;
return 0;
--
gitgitgadget
^ permalink raw reply related
* [PATCH v6 0/2] Replace atoi() with strtoi_with_tail()
From: Mohit Marathe via GitGitGadget @ 2024-02-04 5:48 UTC (permalink / raw)
To: git; +Cc: Mohit Marathe
In-Reply-To: <pull.1646.v5.git.1706416952.gitgitgadget@gmail.com>
Hello,
This patch series replaces atoi() with an updated version of strtol_i()
called strtoi_with_tail (Credits: Junio C Hamano). The reasoning behind this
is to improve error handling by not allowing non-numerical characters in the
hunk header (which might happen in case of a corrupt patch, although
rarely).
There is still a change to be made, as Junio says: "A corrupt patch may be
getting a nonsense patch-ID with the current code and hopefully is not
matching other patches that are not corrupt, but with such a change, a
corrupt patch may not be getting any patch-ID and a loop that computes
patch-ID for many files and try to match them up might need to be rewritten
to take the new failure case into account." I'm not sure where this change
needs to me made (maybe get_one_patchid()?). It would be great if anyone
could point me to the correct place.
Thanks, Mohit Marathe
Mohit Marathe (2):
git-compat-util: add strtoi_with_tail()
patch-id: replace `atoi()` with `strtoi_with_tail`
builtin/patch-id.c | 12 ++++++++----
git-compat-util.h | 18 ++++++++++++++----
2 files changed, 22 insertions(+), 8 deletions(-)
base-commit: 2a540e432fe5dff3cfa9d3bf7ca56db2ad12ebb9
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1646%2Fmohit-marathe%2Fupdate-strtol_i-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1646/mohit-marathe/update-strtol_i-v6
Pull-Request: https://github.com/gitgitgadget/git/pull/1646
Range-diff vs v5:
1: f09b0838f04 ! 1: 98e516a7be7 git-compat-util: add strtoi_with_tail()
@@ Commit message
Signed-off-by: Mohit Marathe <mohitmarathe@proton.me>
## git-compat-util.h ##
-@@ git-compat-util.h: static inline int strtol_i(char const *s, int base, int *result)
+@@ git-compat-util.h: static inline int strtoul_ui(char const *s, int base, unsigned int *result)
return 0;
}
+-static inline int strtol_i(char const *s, int base, int *result)
+#define strtol_i(s,b,r) strtoi_with_tail((s), (b), (r), NULL)
+static inline int strtoi_with_tail(char const *s, int base, int *result, char **endp)
-+{
-+ long ul;
+ {
+ long ul;
+- char *p;
+ char *dummy = NULL;
-+
+
+ if (!endp)
+ endp = &dummy;
-+ errno = 0;
+ errno = 0;
+- ul = strtol(s, &p, base);
+- if (errno || *p || p == s || (int) ul != ul)
+ ul = strtol(s, endp, base);
+ if (errno ||
+ /*
@@ git-compat-util.h: static inline int strtol_i(char const *s, int base, int *resu
+ */
+ (dummy && *dummy) ||
+ *endp == s || (int) ul != ul)
-+ return -1;
-+ *result = ul;
-+ return 0;
-+}
-+
- void git_stable_qsort(void *base, size_t nmemb, size_t size,
- int(*compar)(const void *, const void *));
- #ifdef INTERNAL_QSORT
+ return -1;
+ *result = ul;
+ return 0;
2: ee8f4ae991d = 2: 858d6f94e79 patch-id: replace `atoi()` with `strtoi_with_tail`
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH v2 0/3] some unit-test Makefile polishing
From: Jeff King @ 2024-02-04 4:41 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, git, Phillip Wood, SZEDER Gábor,
Adam Dinwoodie, Patrick Steinhardt
In-Reply-To: <xmqqjznmtjr9.fsf@gitster.g>
On Fri, Feb 02, 2024 at 05:32:42PM -0800, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > The line 753 of that file (as can be seen at
> > https://github.com/git/git/blob/38aa6559b0c513d755d6d5ccf32414ed63754726/config.mak.uname#L753)
>
> Ouch. When it is laid out like this it is very obvious why this is
> broken, and what its workaround should be.
>
> Thanks. Let's queue this on top.
>
> ------- >8 ------------- >8 ------------- >8 ------------- >8 -------
> Subject: [PATCH] t/Makefile: say the default target upfront
>
> Similar to how 2731d048 (Makefile: say the default target upfront.,
> 2005-12-01) added the default target to the very beginning of the
> main Makefile to prevent a random rule that happens to be defined
> first in an included makefile fragments from becoming the default
> target, protect this Makefile the same way.
>
> This started to matter as we started to include config.mak.uname
> and that included makefile fragment does more than defining Make
> macros, unfortunately.
>
> Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Thanks both of you for identifying and fixing this. I should have been
able to catch this, as it triggers with a simple "make" in the t/
directory (rather than "make prove" or "make unit-test", which is of
course what I checked).
Sorry I'm slow to chime in; I've been offline all week (and probably
will be for another few days) due to a family emergency. But hopefully
with this fix the topic is OK now.
-Peff
^ permalink raw reply
* Re: [PATCH v3 2/2] add-patch: classify '@' as a synonym for 'HEAD'
From: Junio C Hamano @ 2024-02-03 22:33 UTC (permalink / raw)
To: Ghanshyam Thakkar; +Cc: git, ps
In-Reply-To: <20240203112619.979239-6-shyamthakkar001@gmail.com>
Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:
> Therefore, make a new function user_meant_head() which takes the
> revision string and compares it to 'HEAD' as well as '@'. However, in
> builtin/checkout.c, there is a logic to convert all command line input
> rev to the raw object name for underlying machinery (e.g., diff-index)
> that does not recognize the <a>...<b> notation, but we'd need to leave
> 'HEAD' intact. Now we need to teach that '@' is a synonym to 'HEAD'
> to that code and leave '@' intact, too.
I am not sure what that "However" wants to say.
- Now we have a helper function to see what the end-user said, and
tell if the end-user meant the state that is currently checked
out (aka "HEAD" but some folks like a synonym "@"[*]), or if the
end-user meant some other "concrete" branch.
- In builtin/checkout.c, there is a logic to convert unless what
the end-user meant is the state that is currently checked out.
Isn't the natural conclusion that follows these two stentences
"therefore, the latter is a very good place to use that helper
function, too"?
Side note: the "@" is already problematic not just because
"git branch @" would not refuse to create "refs/heads/@",
but there is no ref "@" (like $GIT_DIR/refs/@ or $GIT_DIR/@)
when it is used as a synonym for "HEAD". There is a check
in builtin/checkout.c:update_refs_for_switch() that runs
strcmp() on a token given by the end-user from the command
line with "HEAD" to notice the no-op case "git checkout
HEAD" but the code does not trigger when "@" is given, and
it happens to work by accident. I really wish we didn't add
that oddball synonym, but that is water under the bridge by
now.
In any case, I think we'd find more places that currently treats the
token "HEAD" given directly by the end-user specially and may want
to teach at least some of them to also accept "@" the same way, and
the helper function you are introducing may become useful in the
future, at which time we may move it to a more public header. If it
needs to be shared already between add-patch.c and builtin/checkout.c
(I am guessing what you meant with "However" as an excuse for open
coding it instead of sharing the code), perhaps we should do so without
waiting for that future, though. I dunno.
If we choose to do so, for now, a squashable patch may look like the
attached, but we'd need to update the log message while squashing it
in.
add-interactive.h | 14 ++++++++++++++
add-patch.c | 11 +++--------
builtin/checkout.c | 3 +--
3 files changed, 18 insertions(+), 10 deletions(-)
diff --git c/add-interactive.h w/add-interactive.h
index 693f125e8e..ca7326336d 100644
--- c/add-interactive.h
+++ w/add-interactive.h
@@ -38,4 +38,18 @@ enum add_p_mode {
int run_add_p(struct repository *r, enum add_p_mode mode,
const char *revision, const struct pathspec *ps);
+/*
+ * When the user gives these tokens from the command line, they mean
+ * the state that the currently checked out state came from. This
+ * single bit of information affects the direction in which the patch
+ * is presented to the end-user: are we showing a patch to go back to
+ * the currently committed state, or are we showing a patch to move
+ * forward to the given commit that may be different from the
+ * committed state we started with?
+ */
+static inline int the_user_meant_head(const char *rev)
+{
+ return !strcmp(rev, "HEAD") || !strcmp(rev, "@");
+}
+
#endif
diff --git c/add-patch.c w/add-patch.c
index 7d565dcb33..5502acebb8 100644
--- c/add-patch.c
+++ w/add-patch.c
@@ -378,11 +378,6 @@ static int parse_hunk_header(struct add_p_state *s, struct hunk *hunk)
return 0;
}
-static inline int user_meant_head(const char *rev)
-{
- return !strcmp(rev, "HEAD") || !strcmp(rev, "@");
-}
-
static int is_octal(const char *p, size_t len)
{
if (!len)
@@ -1734,21 +1729,21 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
if (mode == ADD_P_STASH)
s.mode = &patch_mode_stash;
else if (mode == ADD_P_RESET) {
- if (!revision || user_meant_head(revision))
+ if (!revision || the_user_meant_head(revision))
s.mode = &patch_mode_reset_head;
else
s.mode = &patch_mode_reset_nothead;
} else if (mode == ADD_P_CHECKOUT) {
if (!revision)
s.mode = &patch_mode_checkout_index;
- else if (user_meant_head(revision))
+ else if (the_user_meant_head(revision))
s.mode = &patch_mode_checkout_head;
else
s.mode = &patch_mode_checkout_nothead;
} else if (mode == ADD_P_WORKTREE) {
if (!revision)
s.mode = &patch_mode_checkout_index;
- else if (user_meant_head(revision))
+ else if (the_user_meant_head(revision))
s.mode = &patch_mode_worktree_head;
else
s.mode = &patch_mode_worktree_nothead;
diff --git c/builtin/checkout.c w/builtin/checkout.c
index 79e208ee6d..63c669b157 100644
--- c/builtin/checkout.c
+++ w/builtin/checkout.c
@@ -544,8 +544,7 @@ static int checkout_paths(const struct checkout_opts *opts,
* given a tree-object, new_branch_info->commit would be NULL,
* but we do not have to do any replacement, either.
*/
- if (rev && new_branch_info->commit && strcmp(rev, "HEAD") &&
- strcmp(rev, "@"))
+ if (rev && new_branch_info->commit && !the_user_meant_head(rev))
rev = oid_to_hex_r(rev_oid, &new_branch_info->commit->object.oid);
if (opts->checkout_index && opts->checkout_worktree)
^ permalink raw reply related
* Re: [PATCH v2 0/2] refs: introduce reftable backend
From: Junio C Hamano @ 2024-02-03 20:41 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Han-Wen Nienhuys, Karthik Nayak
In-Reply-To: <cover.1706862705.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> this is the second version of my patch series that introduces the
> reftable backend.
>
> This version addresses the feedback by Karthik. I don't think it really
> makes sense to spell out every change here -- the range diff should
> likely be easier to digest.
This, when merged to 'seen' (which also has "for-each-ref now thinks
an empty string is a signal to show ref-like things outside the
refs/ hierarchy" topic, kn/for-all-refs), seems to break *-reftable
CI tests, cf. https://github.com/git/git/actions/runs/7765401528
I'll tentatively eject the topic from 'seen', even though I have a
suspicion that it byitself would pass all the tests.
Thanks.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox