* Re: Concurrent fetch commands
From: Dragan Simic @ 2023-12-31 17:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Haller, git
In-Reply-To: <xmqqy1daffk8.fsf@gitster.g>
On 2023-12-31 18:27, Junio C Hamano wrote:
> Stefan Haller <lists@haller-berlin.de> writes:
>
>> I can reliably reproduce this by doing
>>
>> $ git fetch&; sleep 0.1; git pull
>> [1] 42160
>> [1] + done git fetch
>> fatal: Cannot rebase onto multiple branches.
>
> I see a bug here.
>
> How this _ought_ to work is
>
> - The first "git fetch" wants to report what it fetched by writing
> into the $GIT_DIR/FETCH_HEAD file ("git merge FETCH_HEAD" after
> the fetch finishes can consume its contents).
>
> - The second "git pull" runs "git fetch" under the hood. Because
> it also wants to write to $GIT_DIR/FETCH_HEAD, and because there
> is already somebody writing to the file, it should notice and
> barf, saying "fatal: a 'git fetch' is already working" or
> something.
>
> But because there is no "Do not overwrite FETCH_HEAD somebody else
> is using" protection, "git merge" or "git rebase" that is run as the
> second half of the "git pull" ends up working on the contents of
> FETCH_HEAD that is undefined, and GIGO result follows.
>
> The "bug" that the second "git fetch" does not notice an already
> running one (who is in possession of FETCH_HEAD) and refrain from
> starting is not easy to design a fix for---we cannot just abort by
> opening it with O_CREAT|O_EXCL because it is a normal thing for
> $GIT_DIR/FETCH_HEAD to exist after the "last" fetch. We truncate
> its contents before starting to avoid getting affected by contents
> leftover by the last fetch, but when there is a "git fetch" that is
> actively running, and it finishes _after_ the second one starts and
> truncates the file, the second one will end up seeing the contents
> the first one left. We have the "--no-write-fetch-head" option for
> users to explicitly tell which invocation of "git fetch" should not
> write FETCH_HEAD.
>
> Running "background/priming" fetches (the one before "sleep 0.1" you
> have) is not a crime by itself, but it is a crime to run them
> without the "--no-fetch-head" option. Since you have *NO* intention
> of using its contents to feed a "git merge" (or equivalent)
> yourself, you are breaking your "git pull" step in your example
> reproduction yourself.
Thank you very much for this highly detailed explanation.
^ permalink raw reply
* Re: Concurrent fetch commands
From: Junio C Hamano @ 2023-12-31 17:27 UTC (permalink / raw)
To: Stefan Haller; +Cc: git
In-Reply-To: <c11ca0b3-aaf4-4a8d-80a1-3832954aa7aa@haller-berlin.de>
Stefan Haller <lists@haller-berlin.de> writes:
> I can reliably reproduce this by doing
>
> $ git fetch&; sleep 0.1; git pull
> [1] 42160
> [1] + done git fetch
> fatal: Cannot rebase onto multiple branches.
I see a bug here.
How this _ought_ to work is
- The first "git fetch" wants to report what it fetched by writing
into the $GIT_DIR/FETCH_HEAD file ("git merge FETCH_HEAD" after
the fetch finishes can consume its contents).
- The second "git pull" runs "git fetch" under the hood. Because
it also wants to write to $GIT_DIR/FETCH_HEAD, and because there
is already somebody writing to the file, it should notice and
barf, saying "fatal: a 'git fetch' is already working" or
something.
But because there is no "Do not overwrite FETCH_HEAD somebody else
is using" protection, "git merge" or "git rebase" that is run as the
second half of the "git pull" ends up working on the contents of
FETCH_HEAD that is undefined, and GIGO result follows.
The "bug" that the second "git fetch" does not notice an already
running one (who is in possession of FETCH_HEAD) and refrain from
starting is not easy to design a fix for---we cannot just abort by
opening it with O_CREAT|O_EXCL because it is a normal thing for
$GIT_DIR/FETCH_HEAD to exist after the "last" fetch. We truncate
its contents before starting to avoid getting affected by contents
leftover by the last fetch, but when there is a "git fetch" that is
actively running, and it finishes _after_ the second one starts and
truncates the file, the second one will end up seeing the contents
the first one left. We have the "--no-write-fetch-head" option for
users to explicitly tell which invocation of "git fetch" should not
write FETCH_HEAD.
Running "background/priming" fetches (the one before "sleep 0.1" you
have) is not a crime by itself, but it is a crime to run them
without the "--no-fetch-head" option. Since you have *NO* intention
of using its contents to feed a "git merge" (or equivalent)
yourself, you are breaking your "git pull" step in your example
reproduction yourself.
^ permalink raw reply
* Re: Concurrent fetch commands
From: Dragan Simic @ 2023-12-31 14:01 UTC (permalink / raw)
To: Konstantin Tokarev; +Cc: Stefan Haller, git
In-Reply-To: <20231231165042.1d934927@RedEyes>
On 2023-12-31 14:50, Konstantin Tokarev wrote:
> В Sun, 31 Dec 2023 14:30:05 +0100
> Stefan Haller <lists@haller-berlin.de> пишет:
>
>> Currently, git doesn't seem to be very good at handling two concurrent
>> invocations of git fetch (or git fetch and git pull). This is a
>> problem because it is common for git clients to run git fetch
>> periodically in the background.
>
> I think it's a really weird idea which makes a disservice both to human
> git users and git servers. IMO clients doing this should be fixed to
> avoid such behavior, at least by default.
Could you, please, elaborate a bit on the resulting disservice?
Regarding fixing the clients, IDE users often expect such automatic
updating to be performed in the background, so I'm afraid there's not
much wiggle room there.
^ permalink raw reply
* Re: Concurrent fetch commands
From: Konstantin Tokarev @ 2023-12-31 13:50 UTC (permalink / raw)
To: Stefan Haller; +Cc: git
In-Reply-To: <c11ca0b3-aaf4-4a8d-80a1-3832954aa7aa@haller-berlin.de>
В Sun, 31 Dec 2023 14:30:05 +0100
Stefan Haller <lists@haller-berlin.de> пишет:
> Currently, git doesn't seem to be very good at handling two concurrent
> invocations of git fetch (or git fetch and git pull). This is a
> problem because it is common for git clients to run git fetch
> periodically in the background.
I think it's a really weird idea which makes a disservice both to human
git users and git servers. IMO clients doing this should be fixed to
avoid such behavior, at least by default.
^ permalink raw reply
* Re: Concurrent fetch commands
From: Dragan Simic @ 2023-12-31 13:48 UTC (permalink / raw)
To: Stefan Haller; +Cc: git
In-Reply-To: <c11ca0b3-aaf4-4a8d-80a1-3832954aa7aa@haller-berlin.de>
On 2023-12-31 14:30, Stefan Haller wrote:
> Currently, git doesn't seem to be very good at handling two concurrent
> invocations of git fetch (or git fetch and git pull). This is a problem
> because it is common for git clients to run git fetch periodically in
> the background. In that case, when you happen to invoke git pull while
> such a background fetch is running, an error occurs ("Cannot rebase
> onto
> multiple branches").
>
> I can reliably reproduce this by doing
>
> $ git fetch&; sleep 0.1; git pull
> [1] 42160
> [1] + done git fetch
> fatal: Cannot rebase onto multiple branches.
>
> The reason for this failure seems to be that both the first fetch and
> the fetch that runs as part of the pull append their information to
> .git/FETCH_HEAD, so that the information for the current branch ends up
> twice in the file.
>
> Do you think git fetch should be made more robust against scenarios
> like
> this?
I believe a similar issue has been already raised recently, so perhaps
introducing some kind of file-based locking within git itself could be
justified. It would make the things a bit more robust, and would also
improve the overall user experience.
> More context: the git client that I'm contributing to (lazygit) used to
> guard against this for its own background fetch with a global mutex
> that
> allowed only one single fetch, pull, or push at a time. This solved the
> problem nicely for lazygit's own operations (at the expense of some
> lag,
> occasionally); and I'm not aware of any reports about failures because
> some other git client's background fetch got in the way, so maybe we
> don't have to worry about that too much.
>
> However, we now removed that mutex to allow certain parallel fetch
> operations to run at the same time, most notably fetching (and
> updating)
> a branch that is not checked out (by doing "git fetch origin
> branch:branch"). It is useful to be able to trigger this for multiple
> branches concurrently, and actually this works fine.
>
> But now we have the problem described above, where a pull of the
> checked-out branch runs at the same time as a background fetch; this is
> not so unlikely, because lazygit triggers the first background fetch at
> startup, so invoking the pull command right after starting lazygit is
> very likely to fail.
>
> We could re-introduce a mutex and just make it a little less global;
> e.g. protect only pull and parameter-less fetch. But fixing it in git
> itself seems preferable to me.
>
> Sorry for the wall of text, but I figured giving more context could be
> useful.
^ permalink raw reply
* Concurrent fetch commands
From: Stefan Haller @ 2023-12-31 13:30 UTC (permalink / raw)
To: git
Currently, git doesn't seem to be very good at handling two concurrent
invocations of git fetch (or git fetch and git pull). This is a problem
because it is common for git clients to run git fetch periodically in
the background. In that case, when you happen to invoke git pull while
such a background fetch is running, an error occurs ("Cannot rebase onto
multiple branches").
I can reliably reproduce this by doing
$ git fetch&; sleep 0.1; git pull
[1] 42160
[1] + done git fetch
fatal: Cannot rebase onto multiple branches.
The reason for this failure seems to be that both the first fetch and
the fetch that runs as part of the pull append their information to
.git/FETCH_HEAD, so that the information for the current branch ends up
twice in the file.
Do you think git fetch should be made more robust against scenarios like
this?
More context: the git client that I'm contributing to (lazygit) used to
guard against this for its own background fetch with a global mutex that
allowed only one single fetch, pull, or push at a time. This solved the
problem nicely for lazygit's own operations (at the expense of some lag,
occasionally); and I'm not aware of any reports about failures because
some other git client's background fetch got in the way, so maybe we
don't have to worry about that too much.
However, we now removed that mutex to allow certain parallel fetch
operations to run at the same time, most notably fetching (and updating)
a branch that is not checked out (by doing "git fetch origin
branch:branch"). It is useful to be able to trigger this for multiple
branches concurrently, and actually this works fine.
But now we have the problem described above, where a pull of the
checked-out branch runs at the same time as a background fetch; this is
not so unlikely, because lazygit triggers the first background fetch at
startup, so invoking the pull command right after starting lazygit is
very likely to fail.
We could re-introduce a mutex and just make it a little less global;
e.g. protect only pull and parameter-less fetch. But fixing it in git
itself seems preferable to me.
Sorry for the wall of text, but I figured giving more context could be
useful.
Thanks,
Stefan
^ permalink raw reply
* Re: rebase invoking pre-commit
From: Sean Allred @ 2023-12-31 10:52 UTC (permalink / raw)
To: Elijah Newren; +Cc: Sean Allred, Git List, Junio C Hamano
In-Reply-To: <CABPp-BFbvRDCbMp9Gs9PuV7WfgoVNwyOOn1rB7fe_8UvEEdehA@mail.gmail.com>
Elijah Newren <newren@gmail.com> writes:
> On Thu, Dec 21, 2023 at 12:59 PM Sean Allred <allred.sean@gmail.com> wrote:
>> Is there a current reason why pre-commit shouldn't be invoked during
>> rebase, or is this just waiting for a reviewable patch?
>>
>> This was brought up before at [1] in 2015, but that thread so old at
>> this point that it seemed prudent to double-check before investing time
>> in a developing and testing a patch.
>>
>> [1]: https://lore.kernel.org/git/1m55i3m.1fum4zo1fpnhncM%25lists@haller-berlin.de/
>
> I'm very opinionated here. I'm just one person, so definitely take
> this with a grain of salt, but in my view...
>
> Personally, I think implementing any per-commit hook in rebase by
> default is a mistake. It enforces a must-be-in-a-worktree-and-the-
> worktree-must-be-updated-with-every-replayed-commit mindset, which I
> find highly problematic[2], even if that's "what we always used to
> do".
>
> [2] https://lore.kernel.org/git/20231124111044.3426007-1-christian.couder@gmail.com/
I'm not hip with what most pre-commit hooks do, but I'll point out that
a hook like pre-commit assuming there is a worktree is the fault of the
hook implementation, not of the infrastructure that invokes the hook. I
imagine most folks on this list are aware that a worktree is not needed
to create a commit and update a branch to point at it.
FWIW, I would also find such a mindset to be highly problematic :-) I'll
take a moment here to thank you, Christian, and everyone else in that
effort for your interest in and work on git-replay; I've been trying to
watch its activity on-list closely in the hopes that we can adopt it
into our system once it's ready.
> Because of that, I would prefer to see this at most be a command line
> flag. However, we've already got a command line flag that while not
> identical, is essentially equivalent: "--exec $MY_SCRIPT" (it's not
> the same because it's a post-commit check, but you get notification of
> any problematic commits, and an immediate stop of the rebase for you
> to fix up the problematic commit; fixing up the commit shouldn't be
> problematic since you are, after all, already rebasing).
Indeed, and an
--exec 'git hook run pre-commit || git reset --soft HEAD~'
would probably get you farther. I can certainly see an argument for
this, but from the perspective of designing a system for other
developers to use, such a rebase would have to be triggered
automatically (perhaps on pre-push).
> I see Phillip already responded and suggested not running the
> pre-commit hook with every commit, but only upon the first commit
> after a "git rebase --continue". That seems far more reasonable to me
> than running on every commit...though even that worries me in regards
> to what assumptions that entails about what is present in the working
> tree.
It's worth noting the context here is to prevent developers from
committing conflict markers, so this would actually be exactly
sufficient.
Invoking pre-commit at this time would also be consistent with the
behaviors of prepare-commit-msg, commit-msg, and post-commit -- at least
when I reword a commit during a rebase.
However, post-commit is executed after each picked commit during a
rebase, so pre-commit there would also be consistent.
> (For example, what about folks with large repositories, so large that
> a branch switch or full checkout is extremely costly, and which could
> benefit from resolving conflicts in a separate sparse-checkout
> worktree, potentially much more sparse than their main checkout?
As it happens, a single checkout of our source runs upwards of 2GB, so
I'm exactly in the population you're describing :-) The main reason
we're moving to Git from SVN is that an SVN checkout can take upwards of
an hour for us today -- even with some real shenanigans to make them go
faster. On the Git side, we've also looked into (though I don't recall
if we had much success with) narrowing the sparsity patterns to just the
conflicts for conflict resolution workflows -- particularly when moving
feature code between separate trunks. So I guess I'm also glad we
weren't too far off in left field on that one! (As I recall, one of the
main challenges we faced there was ensuring there was enough stuff
'still around' so that both binary and project references could resolve
and folks could use that information to help resolve conflicts.
Hopefully git-replay can be smart enough to allow some customization on
that front. We found some success with feeding the list of conflicted
files into some arbitrary logic that spat out the sparsity pattern to
use.)
> And what if people like that really fast rebase resolution (namely,
> done in a separate very sparse checkout which also has the advantage
> of not polluting your current working tree) so much that they use it
> on smaller repositories as well? Can I not even experiment with this
> idea because of the historical per-commit-at-least-as-full-as-main
> -worktree-checkout assumptions we've baked into rebase?)
I'd be interested in reading more about this baked-in assumption. Are
these mostly laid out in replay-design-notes.txt[3]?
> While at it, I should also mention that I'm not a fan of the broken
> pre-rebase hook; its design ties us to doing a single branch at a
> time. Maybe that hook is not quite as bad, though, since we already
> broke that hook and no one seemed to care (in particular, when
> --update-refs was implemented). But if no one seems to care about
> broken hooks, I think the proper answer is to either get rid of the
> hook or fix it.
If I were to guess, this likely stems either from an inexact definition
of the hook in documentation (ultimately resulting in incomplete tests)
or folks incorrectly assuming what each hook should do based purely on
its name.
Which leads to an interesting point: pre-commit specifically states that
it is invoked by git-commit -- not that it's invoked whenever a commit
is created. So perhaps the correct thing to do here (if a hook is in
fact needed) would be to define a new hook -- but I worry about doing
that in the current state where there doesn't *seem* to be very rigid
coordination of when client hooks are invoked in terms of plumbing
rather than porcelain.
> Anyway, as I mentioned, I'm quite opinionated here. To the point that
> I deemed git-rebase in its current form to possibly be unfixable
> (after first putting a lot of work into improving it over the past
> years) and eventually introduced a new "git-replay" command which I
> hope to make into a competent replacement for it. Given that I do
> have another command to do my experiments, others on the list may
> think it's fine to send rebase further down this route, but I was
> hoping to avoid further drift so that there might be some way of
> re-implementing rebase on top of my "git-replay" ideas/design.
I appreciate your perspective; you've certainly thought a lot about this
space -- and I definitely share your goal of consolidating
implementations for obvious reasons.
So I suppose that leaves me with four possible paths forward:
1. Pursue invoking pre-commit before each commit in `git rebase` (likely
generic in the sequencer) to be consistent with post-commit.
It sounds like this isn't a popular option, but I'm curious to folks'
thoughts on the noted behavior of post-commit here.
2. Pursue invoking pre-commit on `git rebase --continue` (likely on any
--continue in the sequencer). This has the benefit of using existing
configuration on developer machines to purportedly 'do the right
thing' when its likely humans are touching code during conflict
resolution. It's worth noting this isn't the only reason you might
--continue, though, since the naive interpretation of this approach
completely ignores sequencer commands like 'break', though it could
probably just do what commit-msg does.
3. Define and implement a new hook that is called whenever a new commit
is about to be (or has been?) written. Such a hook could be
specifically designed to discourage assuming there's a working copy,
though we're kidding nobody by thinking it won't be used downstream
with that assumption. At least we could be explicit about
expectations, though.
This is *probably* a lot more design work than this little paragraph
lets on, but I've not personally watched the introduction of a new
hook so I don't have context for what to expect.
4. Trigger a rebase --exec in our pre-push. This is certainly the least
work in git.git (i.e., no work at all), but it comes with the
distinct disadvantage of playing whiplash with the developer's focus.
During conflict resolution, they're thinking about conflicts. When
you're ready to push, its likely that you're no longer thinking about
conflicts.
Does the behavior of post-commit here change any minds?
[3]: https://github.com/newren/git/blob/2a621020863c0b867293e020fec0954b43818789/replay-design-notes.txt#L162
--
Sean Allred
^ permalink raw reply
* Re: [PATCH V3 0/1] Replace SID with domain/username on Windows
From: Eric Sunshine @ 2023-12-31 9:18 UTC (permalink / raw)
To: Sören Krecker; +Cc: git
In-Reply-To: <20231231091245.2853-1-soekkle@freenet.de>
On Sun, Dec 31, 2023 at 4:12 AM Sören Krecker <soekkle@freenet.de> wrote:
> I improve the commit message with example of old and new error output.
>
> Thanks to Eric Sunshine.
Thanks for rerolling.
^ permalink raw reply
* [PATCH v3 1/1] Replace SID with domain/username
From: Sören Krecker @ 2023-12-31 9:12 UTC (permalink / raw)
To: git; +Cc: sunshine, Sören Krecker
In-Reply-To: <20231231091245.2853-1-soekkle@freenet.de>
Replace SID with domain/username in error message, if owner of repository
and user are not equal on windows systems.
Old message:
'''
fatal: detected dubious ownership in repository at 'C:/Users/test/source/repos/git'
'C:/Users/test/source/repos/git' is owned by:
'S-1-5-21-571067702-4104414259-3379520149-500'
but the current user is:
'S-1-5-21-571067702-4104414259-3379520149-1001'
To add an exception for this directory, call:
git config --global --add safe.directory C:/Users/test/source/repos/git
'''
New massage:
'''
fatal: detected dubious ownership in repository at 'C:/Users/test/source/repos/git'
'C:/Users/test/source/repos/git' is owned by:
'DESKTOP-L78JVA6/Administrator'
but the current user is:
'DESKTOP-L78JVA6/test'
To add an exception for this directory, call:
git config --global --add safe.directory C:/Users/test/source/repos/git
'''
Signed-off-by: Sören Krecker <soekkle@freenet.de>
---
compat/mingw.c | 28 ++++++++++++++++++++++++----
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 42053c1f65..05aeaaa9ad 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2684,6 +2684,26 @@ static PSID get_current_user_sid(void)
return result;
}
+static BOOL user_sid_to_user_name(PSID sid, LPSTR *str)
+{
+ SID_NAME_USE pe_use;
+ DWORD len_user = 0, len_domain = 0;
+ BOOL translate_sid_to_user;
+
+ /* returns only FALSE, because the string pointers are NULL*/
+ LookupAccountSidA(NULL, sid, NULL, &len_user, NULL, &len_domain,
+ &pe_use);
+ /*Alloc needed space of the strings*/
+ ALLOC_ARRAY((*str), (size_t)len_domain + (size_t)len_user);
+ translate_sid_to_user = LookupAccountSidA(NULL, sid, (*str) + len_domain, &len_user,
+ *str, &len_domain, &pe_use);
+ *(*str + len_domain) = '/';
+ if (translate_sid_to_user == FALSE) {
+ FREE_AND_NULL(*str);
+ }
+ return translate_sid_to_user;
+}
+
static int acls_supported(const char *path)
{
size_t offset = offset_1st_component(path);
@@ -2767,7 +2787,7 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report)
} else if (report) {
LPSTR str1, str2, to_free1 = NULL, to_free2 = NULL;
- if (ConvertSidToStringSidA(sid, &str1))
+ if (user_sid_to_user_name(sid, &str1))
to_free1 = str1;
else
str1 = "(inconvertible)";
@@ -2776,7 +2796,7 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report)
str2 = "(none)";
else if (!IsValidSid(current_user_sid))
str2 = "(invalid)";
- else if (ConvertSidToStringSidA(current_user_sid, &str2))
+ else if (user_sid_to_user_name(current_user_sid, &str2))
to_free2 = str2;
else
str2 = "(inconvertible)";
@@ -2784,8 +2804,8 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report)
"'%s' is owned by:\n"
"\t'%s'\nbut the current user is:\n"
"\t'%s'\n", path, str1, str2);
- LocalFree(to_free1);
- LocalFree(to_free2);
+ free(to_free1);
+ free(to_free2);
}
}
--
2.39.2
^ permalink raw reply related
* [PATCH V3 0/1] Replace SID with domain/username on Windows
From: Sören Krecker @ 2023-12-31 9:12 UTC (permalink / raw)
To: git; +Cc: sunshine, Sören Krecker
In-Reply-To: <CAPig+cT4jy4MkyGxtSOZj6U3vUxLaRa-4wr7PON-EebAjT8pwQ@mail.gmail.com>
I improve the commit message with example of old and new error output.
Thanks to Eric Sunshine.
Sören Krecker (1):
Replace SID with domain/username
compat/mingw.c | 28 ++++++++++++++++++++++++----
1 file changed, 24 insertions(+), 4 deletions(-)
base-commit: e79552d19784ee7f4bbce278fe25f93fbda196fa
--
2.39.2
^ permalink raw reply
* Re: [PATCH 0/1 v2] Replace SID with domain/username on Windows
From: Eric Sunshine @ 2023-12-31 4:08 UTC (permalink / raw)
To: Sören Krecker; +Cc: git
In-Reply-To: <20231229120319.3797-1-soekkle@freenet.de>
On Fri, Dec 29, 2023 at 7:03 AM Sören Krecker <soekkle@freenet.de> wrote:
> Improve error message on windows systems, if owner of reposotory and current user are not equal.
>
> Old Message:
> '''
> fatal: detected dubious ownership in repository at 'C:/Users/test/source/repos/git'
> 'C:/Users/test/source/repos/git' is owned by:
> 'S-1-5-21-571067702-4104414259-3379520149-500'
> but the current user is:
> 'S-1-5-21-571067702-4104414259-3379520149-1001'
> To add an exception for this directory, call:
>
> git config --global --add safe.directory C:/Users/test/source/repos/git
> '''
>
> New Massage:
> '''
> fatal: detected dubious ownership in repository at 'C:/Users/test/source/repos/git'
> 'C:/Users/soren/source/repos/git' is owned by:
> 'DESKTOP-L78JVA6/Administrator'
> but the current user is:
> 'DESKTOP-L78JVA6/test'
> To add an exception for this directory, call:
>
> git config --global --add safe.directory C:/Users/test/source/repos/git
> '''
>
> I hope that I have succeeded in addressing all the points raised.
Thanks, this explanation does an excellent job of helping reviewers
understand why the patch is desirable.
It's also important that people digging through the project history in
the future also understand the reason for this change, so it's very
valuable for the explanation to be part of the commit message of the
patch itself, not just in the cover letter (which doesn't become part
of the permanent project history). Therefore, can you reroll once
again, placing this explanation in the commit message of the patch
itself? Doing so should help the patch get accepted into the project.
That's as much as I can add. Hopefully, one or more Windows folks will
chime in regarding the actual patch content (whether it's acceptable
as-is or needs some tweaks).
Thanks.
^ permalink raw reply
* [PATCH] write-or-die: make GIT_FLUSH a Boolean environment variable
From: Chandra Pratap via GitGitGadget @ 2023-12-30 16:54 UTC (permalink / raw)
To: git; +Cc: Chandra Pratap, Chandra Pratap
From: Chandra Pratap <chandrapratap3519@gmail.com>
Among Git's environment variable, the ones marked as "Boolean"
accept values in a way similar to Boolean configuration variables,
i.e. values like 'yes', 'on', 'true' and positive numbers are
taken as "on" and values like 'no', 'off', 'false' are taken as
"off".
GIT_FLUSH can be used to force Git to use non-buffered I/O when
writing to stdout. It can only accept two values, '1' which causes
Git to flush more often and '0' which makes all output buffered.
Make GIT_FLUSH accept more values besides '0' and '1' by turning it
into a Boolean environment variable, modifying the required logic.
Update the related documentation.
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
---
write-or-die: make GIT_FLUSH a Boolean environment variable
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1628%2FChand-ra%2Fgit_flush-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1628/Chand-ra/git_flush-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1628
Documentation/git.txt | 16 +++++++---------
write-or-die.c | 9 ++++-----
2 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 2535a30194f..83fd60f2d11 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -724,16 +724,14 @@ for further details.
waiting for someone with sufficient permissions to fix it.
`GIT_FLUSH`::
-// NEEDSWORK: make it into a usual Boolean environment variable
- If this environment variable is set to "1", then commands such
+ If this Boolean environment variable is set to true, then commands such
as 'git blame' (in incremental mode), 'git rev-list', 'git log',
- 'git check-attr' and 'git check-ignore' will
- force a flush of the output stream after each record have been
- flushed. If this
- variable is set to "0", the output of these commands will be done
- using completely buffered I/O. If this environment variable is
- not set, Git will choose buffered or record-oriented flushing
- based on whether stdout appears to be redirected to a file or not.
+ 'git check-attr' and 'git check-ignore' will force a flush of the output
+ stream after each record have been flushed. If this variable is set to
+ false, the output of these commands will be done using completely buffered
+ I/O. If this environment variable is not set, Git will choose buffered or
+ record-oriented flushing based on whether stdout appears to be redirected
+ to a file or not.
`GIT_TRACE`::
Enables general trace messages, e.g. alias expansion, built-in
diff --git a/write-or-die.c b/write-or-die.c
index 42a2dc73cd3..f501a6e382a 100644
--- a/write-or-die.c
+++ b/write-or-die.c
@@ -20,14 +20,13 @@ void maybe_flush_or_die(FILE *f, const char *desc)
{
static int skip_stdout_flush = -1;
struct stat st;
- char *cp;
+ int cp;
if (f == stdout) {
if (skip_stdout_flush < 0) {
- /* NEEDSWORK: make this a normal Boolean */
- cp = getenv("GIT_FLUSH");
- if (cp)
- skip_stdout_flush = (atoi(cp) == 0);
+ cp = git_env_bool("GIT_FLUSH", -1);
+ if (cp >= 0)
+ skip_stdout_flush = (cp == 0);
else if ((fstat(fileno(stdout), &st) == 0) &&
S_ISREG(st.st_mode))
skip_stdout_flush = 1;
base-commit: 1a87c842ece327d03d08096395969aca5e0a6996
--
gitgitgadget
^ permalink raw reply related
* RE: You are marked as spam, and therefore cannot authorize a third party application.
From: rsbecker @ 2023-12-30 14:45 UTC (permalink / raw)
To: 'Ramesh', git
In-Reply-To: <7aefb4c3-da1b-429b-a519-044084265994@infohubinnovations.com>
On Saturday, December 30, 2023 7:25 AM, Ramesh wrote:
>Hi GitHub Team,
>
>I am trying to clone my repository from local machine.But i am getting the follow
>exception that "You are marked as spam, and therefore cannot authorize a third
>party application.". Can you please suggest the same.I have deleted all my accounts
>except this account.
This is not a GitHub mailing list. Please use support links at GitHub.com.
^ permalink raw reply
* Draft of Git Rev News edition 106
From: Christian Couder @ 2023-12-30 14:38 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jakub Narebski, Markus Jansen, Kaartic Sivaraam,
Štěpán Němec, Taylor Blau,
Johannes Schindelin, Ævar Arnfjörð Bjarmason,
Josh Soref, Eric Sunshine, Elijah Newren, VonC, Scott Chacon
Hi everyone!
A draft of a new Git Rev News edition is available here:
https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-106.md
Everyone is welcome to contribute in any section either by editing the
above page on GitHub and sending a pull request, or by commenting on
this GitHub issue:
https://github.com/git/git.github.io/issues/675
You can also reply to this email.
In general all kinds of contributions, for example proofreading,
suggestions for articles or links, help on the issues in GitHub,
volunteering for being interviewed and so on, are very much
appreciated.
I tried to Cc everyone who appears in this edition, but maybe I missed
some people, sorry about that.
Jakub, Markus, Kaartic and I plan to publish this edition on
Monday January 1st, 2024.
Thanks,
Christian.
^ permalink raw reply
* Re: You are marked as spam, and therefore cannot authorize a third party application.
From: Christian Couder @ 2023-12-30 14:21 UTC (permalink / raw)
To: Ramesh; +Cc: git
In-Reply-To: <7aefb4c3-da1b-429b-a519-044084265994@infohubinnovations.com>
Hi,
On Sat, Dec 30, 2023 at 1:34 PM Ramesh <ramesh@infohubinnovations.com> wrote:
>
> Hi GitHub Team,
This is a mailing list about Git, the command line tool, not GitHub.
Please don't post here about GitHub specific issues.
> I am trying to clone my repository from local machine.But i am getting
> the follow exception that "You are marked as spam, and therefore cannot
> authorize a third party application.". Can you please suggest the same.I
> have deleted all my accounts except this account.
A quick Google search pointed to this which might help you:
https://github.com/orgs/community/discussions/67196
Anyway, please don't discuss this issue here further. The GitHub
support team is not using this channel to help people having issues
with GitHub. So we cannot help you further.
Best,
Christian.
^ permalink raw reply
* You are marked as spam, and therefore cannot authorize a third party application.
From: Ramesh @ 2023-12-30 12:25 UTC (permalink / raw)
To: git
Hi GitHub Team,
I am trying to clone my repository from local machine.But i am getting
the follow exception that "You are marked as spam, and therefore cannot
authorize a third party application.". Can you please suggest the same.I
have deleted all my accounts except this account.
^ permalink raw reply
* [BUG] Asks for "To" even if "To" already specified in letter
From: Askar Safin @ 2023-12-30 3:20 UTC (permalink / raw)
To: git
Hi. I found a bug. Steps to reproduce:
- Create file /tmp/m with following text:
===
Subject: subj
To: example@example.com
text
===
- Send it using command "git send-email /tmp/m"
You will see that git asks for "To". git says: "To whom should the
emails be sent (if anyone)?"
I don't like this. git should just use "To" from /tmp/m without asking.
Seen with git 2.43.0.
If I execute "git send-email --to-cmd='#' /tmp/m" or
"git send-email --to-cmd=':' /tmp/m" or
"git send-email --to-cmd='true' /tmp/m", then "To" is not asked.
Same happens when I send one single patch without cover letter. Steps
to reproduce:
- Clone some repo
- Make some (single) commit
- Execute in the repo "git send-email --annotate HEAD^"
- Editor will be spawned. Add header "To: example@example.com", then
save and exit from the editor
You will see the question "To whom should the emails be sent (if
anyone)?", too. And again you can workaround using --to-cmd tricks.
--
Askar Safin
^ permalink raw reply
* [Outreachy][PATCH v2] Port helper/test-ctype.c to unit-tests/t-ctype.c
From: Achu Luma @ 2023-12-30 0:09 UTC (permalink / raw)
To: git
Cc: chriscool, christian.couder, l.s.r, gitster, phillip.wood,
steadmon, Achu Luma
In-Reply-To: <20231221231527.8130-1-ach.lumap@gmail.com>
In the recent codebase update(8bf6fbd00d (Merge branch
'js/doc-unit-tests', 2023-12-09)), a new unit testing framework was
merged, providing a standardized approach for testing C code. Prior to
this update, some unit tests relied on the test helper mechanism,
lacking a dedicated unit testing framework. It's more natural to perform
these unit tests using the new unit test framework.
This commit migrates the unit tests for C character classification
functions (isdigit(), isspace(), etc) from the legacy approach
using the test-tool command `test-tool ctype` in t/helper/test-ctype.c
to the new unit testing framework (t/unit-tests/test-lib.h).
The migration involves refactoring the tests to utilize the testing
macros provided by the framework (TEST() and check_*()).
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Achu Luma <ach.lumap@gmail.com>
---
In the revised patch, several improvements were made to the
TEST_CTYPE_FUNC macro. The updated version enhances error handling
by utilizing a direct comparison approach between the func and
is_in(string, i) functions across ASCII characters. Additionally,
the loop control variable i is locally scoped within the loop, ensuring
proper encapsulation. These changes streamline the comparison process
and clarify failure reporting. Special thanks to the invaluable reviews
by Junio, Phillip and René for their insightful feedback and thorough
review of the patch, contributing significantly to these changes.
Makefile | 2 +-
t/helper/test-ctype.c | 70 --------------------------------------
t/helper/test-tool.c | 1 -
t/helper/test-tool.h | 1 -
t/t0070-fundamental.sh | 4 ---
t/unit-tests/t-ctype.c | 77 ++++++++++++++++++++++++++++++++++++++++++
6 files changed, 78 insertions(+), 77 deletions(-)
delete mode 100644 t/helper/test-ctype.c
create mode 100644 t/unit-tests/t-ctype.c
diff --git a/Makefile b/Makefile
index 88ba7a3c51..a4df48ba65 100644
--- a/Makefile
+++ b/Makefile
@@ -792,7 +792,6 @@ TEST_BUILTINS_OBJS += test-chmtime.o
TEST_BUILTINS_OBJS += test-config.o
TEST_BUILTINS_OBJS += test-crontab.o
TEST_BUILTINS_OBJS += test-csprng.o
-TEST_BUILTINS_OBJS += test-ctype.o
TEST_BUILTINS_OBJS += test-date.o
TEST_BUILTINS_OBJS += test-delta.o
TEST_BUILTINS_OBJS += test-dir-iterator.o
@@ -1341,6 +1340,7 @@ THIRD_PARTY_SOURCES += sha1dc/%
UNIT_TEST_PROGRAMS += t-basic
UNIT_TEST_PROGRAMS += t-strbuf
+UNIT_TEST_PROGRAMS += t-ctype
UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))
UNIT_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TEST_PROGRAMS))
UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o
diff --git a/t/helper/test-ctype.c b/t/helper/test-ctype.c
deleted file mode 100644
index e5659df40b..0000000000
--- a/t/helper/test-ctype.c
+++ /dev/null
@@ -1,70 +0,0 @@
-#include "test-tool.h"
-
-static int rc;
-
-static void report_error(const char *class, int ch)
-{
- printf("%s classifies char %d (0x%02x) wrongly\n", class, ch, ch);
- rc = 1;
-}
-
-static int is_in(const char *s, int ch)
-{
- /*
- * We can't find NUL using strchr. Accept it as the first
- * character in the spec -- there are no empty classes.
- */
- if (ch == '\0')
- return ch == *s;
- if (*s == '\0')
- s++;
- return !!strchr(s, ch);
-}
-
-#define TEST_CLASS(t,s) { \
- int i; \
- for (i = 0; i < 256; i++) { \
- if (is_in(s, i) != t(i)) \
- report_error(#t, i); \
- } \
- if (t(EOF)) \
- report_error(#t, EOF); \
-}
-
-#define DIGIT "0123456789"
-#define LOWER "abcdefghijklmnopqrstuvwxyz"
-#define UPPER "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
-#define PUNCT "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~"
-#define ASCII \
- "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f" \
- "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
- "\x20\x21\x22\x23\x24\x25\x26\x27\x28\x29\x2a\x2b\x2c\x2d\x2e\x2f" \
- "\x30\x31\x32\x33\x34\x35\x36\x37\x38\x39\x3a\x3b\x3c\x3d\x3e\x3f" \
- "\x40\x41\x42\x43\x44\x45\x46\x47\x48\x49\x4a\x4b\x4c\x4d\x4e\x4f" \
- "\x50\x51\x52\x53\x54\x55\x56\x57\x58\x59\x5a\x5b\x5c\x5d\x5e\x5f" \
- "\x60\x61\x62\x63\x64\x65\x66\x67\x68\x69\x6a\x6b\x6c\x6d\x6e\x6f" \
- "\x70\x71\x72\x73\x74\x75\x76\x77\x78\x79\x7a\x7b\x7c\x7d\x7e\x7f"
-#define CNTRL \
- "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f" \
- "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
- "\x7f"
-
-int cmd__ctype(int argc UNUSED, const char **argv UNUSED)
-{
- TEST_CLASS(isdigit, DIGIT);
- TEST_CLASS(isspace, " \n\r\t");
- TEST_CLASS(isalpha, LOWER UPPER);
- TEST_CLASS(isalnum, LOWER UPPER DIGIT);
- TEST_CLASS(is_glob_special, "*?[\\");
- TEST_CLASS(is_regex_special, "$()*+.?[\\^{|");
- TEST_CLASS(is_pathspec_magic, "!\"#%&',-/:;<=>@_`~");
- TEST_CLASS(isascii, ASCII);
- TEST_CLASS(islower, LOWER);
- TEST_CLASS(isupper, UPPER);
- TEST_CLASS(iscntrl, CNTRL);
- TEST_CLASS(ispunct, PUNCT);
- TEST_CLASS(isxdigit, DIGIT "abcdefABCDEF");
- TEST_CLASS(isprint, LOWER UPPER DIGIT PUNCT " ");
-
- return rc;
-}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index 37ba996539..33b9501c21 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -19,7 +19,6 @@ static struct test_cmd cmds[] = {
{ "config", cmd__config },
{ "crontab", cmd__crontab },
{ "csprng", cmd__csprng },
- { "ctype", cmd__ctype },
{ "date", cmd__date },
{ "delta", cmd__delta },
{ "dir-iterator", cmd__dir_iterator },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 8a1a7c63da..b72f07ded9 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -12,7 +12,6 @@ int cmd__chmtime(int argc, const char **argv);
int cmd__config(int argc, const char **argv);
int cmd__crontab(int argc, const char **argv);
int cmd__csprng(int argc, const char **argv);
-int cmd__ctype(int argc, const char **argv);
int cmd__date(int argc, const char **argv);
int cmd__delta(int argc, const char **argv);
int cmd__dir_iterator(int argc, const char **argv);
diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh
index 487bc8d905..a4756fbab9 100755
--- a/t/t0070-fundamental.sh
+++ b/t/t0070-fundamental.sh
@@ -9,10 +9,6 @@ Verify wrappers and compatibility functions.
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
-test_expect_success 'character classes (isspace, isalpha etc.)' '
- test-tool ctype
-'
-
test_expect_success 'mktemp to nonexistent directory prints filename' '
test_must_fail test-tool mktemp doesnotexist/testXXXXXX 2>err &&
grep "doesnotexist/test" err
diff --git a/t/unit-tests/t-ctype.c b/t/unit-tests/t-ctype.c
new file mode 100644
index 0000000000..8a215d387a
--- /dev/null
+++ b/t/unit-tests/t-ctype.c
@@ -0,0 +1,77 @@
+#include "test-lib.h"
+
+static int is_in(const char *s, int ch)
+{
+ /*
+ * We can't find NUL using strchr. Accept it as the first
+ * character in the spec -- there are no empty classes.
+ */
+ if (ch == '\0')
+ return ch == *s;
+ if (*s == '\0')
+ s++;
+ return !!strchr(s, ch);
+}
+
+/* Macro to test a character type */
+#define TEST_CTYPE_FUNC(func, string) \
+static void test_ctype_##func(void) \
+{ \
+ for (int i = 0; i < 256; i++) { \
+ if (!check_int(func(i), ==, is_in(string, i))) \
+ test_msg(" i: %02x", i); \
+ } \
+}
+
+#define DIGIT "0123456789"
+#define LOWER "abcdefghijklmnopqrstuvwxyz"
+#define UPPER "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+#define PUNCT "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~"
+#define ASCII \
+ "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f" \
+ "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
+ "\x20\x21\x22\x23\x24\x25\x26\x27\x28\x29\x2a\x2b\x2c\x2d\x2e\x2f" \
+ "\x30\x31\x32\x33\x34\x35\x36\x37\x38\x39\x3a\x3b\x3c\x3d\x3e\x3f" \
+ "\x40\x41\x42\x43\x44\x45\x46\x47\x48\x49\x4a\x4b\x4c\x4d\x4e\x4f" \
+ "\x50\x51\x52\x53\x54\x55\x56\x57\x58\x59\x5a\x5b\x5c\x5d\x5e\x5f" \
+ "\x60\x61\x62\x63\x64\x65\x66\x67\x68\x69\x6a\x6b\x6c\x6d\x6e\x6f" \
+ "\x70\x71\x72\x73\x74\x75\x76\x77\x78\x79\x7a\x7b\x7c\x7d\x7e\x7f"
+#define CNTRL \
+ "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f" \
+ "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
+ "\x7f"
+
+TEST_CTYPE_FUNC(isdigit, DIGIT)
+TEST_CTYPE_FUNC(isspace, " \n\r\t")
+TEST_CTYPE_FUNC(isalpha, LOWER UPPER)
+TEST_CTYPE_FUNC(isalnum, LOWER UPPER DIGIT)
+TEST_CTYPE_FUNC(is_glob_special, "*?[\\")
+TEST_CTYPE_FUNC(is_regex_special, "$()*+.?[\\^{|")
+TEST_CTYPE_FUNC(is_pathspec_magic, "!\"#%&',-/:;<=>@_`~")
+TEST_CTYPE_FUNC(isascii, ASCII)
+TEST_CTYPE_FUNC(islower, LOWER)
+TEST_CTYPE_FUNC(isupper, UPPER)
+TEST_CTYPE_FUNC(iscntrl, CNTRL)
+TEST_CTYPE_FUNC(ispunct, PUNCT)
+TEST_CTYPE_FUNC(isxdigit, DIGIT "abcdefABCDEF")
+TEST_CTYPE_FUNC(isprint, LOWER UPPER DIGIT PUNCT " ")
+
+int cmd_main(int argc, const char **argv) {
+ /* Run all character type tests */
+ TEST(test_ctype_isspace(), "isspace() works as we expect");
+ TEST(test_ctype_isdigit(), "isdigit() works as we expect");
+ TEST(test_ctype_isalpha(), "isalpha() works as we expect");
+ TEST(test_ctype_isalnum(), "isalnum() works as we expect");
+ TEST(test_ctype_is_glob_special(), "is_glob_special() works as we expect");
+ TEST(test_ctype_is_regex_special(), "is_regex_special() works as we expect");
+ TEST(test_ctype_is_pathspec_magic(), "is_pathspec_magic() works as we expect");
+ TEST(test_ctype_isascii(), "isascii() works as we expect");
+ TEST(test_ctype_islower(), "islower() works as we expect");
+ TEST(test_ctype_isupper(), "isupper() works as we expect");
+ TEST(test_ctype_iscntrl(), "iscntrl() works as we expect");
+ TEST(test_ctype_ispunct(), "ispunct() works as we expect");
+ TEST(test_ctype_isxdigit(), "isxdigit() works as we expect");
+ TEST(test_ctype_isprint(), "isprint() works as we expect");
+
+ return test_done();
+}
--
2.42.0.windows.2
^ permalink raw reply related
* Re: [PATCH v5 2/3] trailer: find the end of the log message
From: Linus Arver @ 2023-12-29 21:03 UTC (permalink / raw)
To: Junio C Hamano, Linus Arver via GitGitGadget
Cc: git, Glen Choo, Christian Couder, Phillip Wood, Jonathan Tan
In-Reply-To: <owlyv88hqzlu.fsf@fine.c.googlers.com>
> (1) enrich the trailer API (make trailer.h have simpler data structures
> and practical functions that clients can readily use), and
> (2) make builtin/interpret-trailers.c, and other clients in the Git
> codebase use this new API.
I've done some more thinking/hacking and I'm realizing that changing the
data structures significantly as a first step will be difficult to get
right without being able to unit-test things as we go. As we don't have
unit tests (sorry, I keep forgetting...), I think changing the shape of
data structures right now is a showstopper.
Step (1) will have to be done without changing any of the existing data
structures.
^ permalink raw reply
* [PATCH v2 1/1] Replace SID with domain/username
From: Sören Krecker @ 2023-12-29 12:03 UTC (permalink / raw)
To: git; +Cc: sunshine, Sören Krecker
In-Reply-To: <20231229120319.3797-1-soekkle@freenet.de>
Replace SID with domain/username in erromessage, if owner of repository
and user are not equal on windows systems.
Signed-off-by: Sören Krecker <soekkle@freenet.de>
---
compat/mingw.c | 28 ++++++++++++++++++++++++----
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 42053c1f65..05aeaaa9ad 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2684,6 +2684,26 @@ static PSID get_current_user_sid(void)
return result;
}
+static BOOL user_sid_to_user_name(PSID sid, LPSTR *str)
+{
+ SID_NAME_USE pe_use;
+ DWORD len_user = 0, len_domain = 0;
+ BOOL translate_sid_to_user;
+
+ /* returns only FALSE, because the string pointers are NULL*/
+ LookupAccountSidA(NULL, sid, NULL, &len_user, NULL, &len_domain,
+ &pe_use);
+ /*Alloc needed space of the strings*/
+ ALLOC_ARRAY((*str), (size_t)len_domain + (size_t)len_user);
+ translate_sid_to_user = LookupAccountSidA(NULL, sid, (*str) + len_domain, &len_user,
+ *str, &len_domain, &pe_use);
+ *(*str + len_domain) = '/';
+ if (translate_sid_to_user == FALSE) {
+ FREE_AND_NULL(*str);
+ }
+ return translate_sid_to_user;
+}
+
static int acls_supported(const char *path)
{
size_t offset = offset_1st_component(path);
@@ -2767,7 +2787,7 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report)
} else if (report) {
LPSTR str1, str2, to_free1 = NULL, to_free2 = NULL;
- if (ConvertSidToStringSidA(sid, &str1))
+ if (user_sid_to_user_name(sid, &str1))
to_free1 = str1;
else
str1 = "(inconvertible)";
@@ -2776,7 +2796,7 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report)
str2 = "(none)";
else if (!IsValidSid(current_user_sid))
str2 = "(invalid)";
- else if (ConvertSidToStringSidA(current_user_sid, &str2))
+ else if (user_sid_to_user_name(current_user_sid, &str2))
to_free2 = str2;
else
str2 = "(inconvertible)";
@@ -2784,8 +2804,8 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report)
"'%s' is owned by:\n"
"\t'%s'\nbut the current user is:\n"
"\t'%s'\n", path, str1, str2);
- LocalFree(to_free1);
- LocalFree(to_free2);
+ free(to_free1);
+ free(to_free2);
}
}
--
2.39.2
^ permalink raw reply related
* [PATCH 0/1 v2] Replace SID with domain/username on Windows
From: Sören Krecker @ 2023-12-29 12:03 UTC (permalink / raw)
To: git; +Cc: sunshine, Sören Krecker
Improve error message on windows systems, if owner of reposotory and current user are not equal.
Old Message:
'''
fatal: detected dubious ownership in repository at 'C:/Users/test/source/repos/git'
'C:/Users/test/source/repos/git' is owned by:
'S-1-5-21-571067702-4104414259-3379520149-500'
but the current user is:
'S-1-5-21-571067702-4104414259-3379520149-1001'
To add an exception for this directory, call:
git config --global --add safe.directory C:/Users/test/source/repos/git
'''
New Massage:
'''
fatal: detected dubious ownership in repository at 'C:/Users/test/source/repos/git'
'C:/Users/soren/source/repos/git' is owned by:
'DESKTOP-L78JVA6/Administrator'
but the current user is:
'DESKTOP-L78JVA6/test'
To add an exception for this directory, call:
git config --global --add safe.directory C:/Users/test/source/repos/git
'''
I hope that I have succeeded in addressing all the points raised.
Sören Krecker (1):
Replace SID with domain/username
compat/mingw.c | 28 ++++++++++++++++++++++++----
1 file changed, 24 insertions(+), 4 deletions(-)
base-commit: e79552d19784ee7f4bbce278fe25f93fbda196fa
--
2.39.2
^ permalink raw reply
* Re: [PATCH 3/6] refs/files: skip creation of "refs/{heads,tags}" for worktrees
From: Eric Sunshine @ 2023-12-29 10:35 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <3cf6ceb274d20ce76d0be65e4362a33579627052.1703754513.git.ps@pks.im>
On Fri, Dec 29, 2023 at 5:16 AM Patrick Steinhardt <ps@pks.im> wrote:
> The files ref backend will create both "refs/heads" and "refs/tags" in
> the Git directory. While this logic makes sense for normal repositories,
> it does not fo worktrees because those refs are "common" refs that would
> always be contained in the main repository's ref database.
s/fo/for/
(not worth a reroll)
> Introduce a new flag telling the backend that it is expected to create a
> per-worktree ref database and skip creation of these dirs in the files
> backend when the flag is set. No other backends (currently) need
> worktree-specific logic, so this is the only required change to start
> creating per-worktree ref databases via `refs_init_db()`.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
^ permalink raw reply
* Re: [PATCH v3 00/12] Introduce `refStorage` extension
From: Eric Sunshine @ 2023-12-29 8:44 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Karthik Nayak, Junio C Hamano
In-Reply-To: <cover.1703833818.git.ps@pks.im>
On Fri, Dec 29, 2023 at 2:26 AM Patrick Steinhardt <ps@pks.im> wrote:
> this is the third version of my patch series that introduces the new
> `refStorage` extension. This extension will be used for the upcoming
> reftable backend.
>
> Changes compared to v3:
> - Reworded the commit message in patch 2, proposed by Eric.
> - Added a NEEDSWORK comment to `get_worktrees_internal()`, propose by
> Eric.
Thanks for addressing my minor review comments.
^ permalink raw reply
* [PATCH v3 12/12] t9500: write "extensions.refstorage" into config
From: Patrick Steinhardt @ 2023-12-29 7:27 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Junio C Hamano, Eric Sunshine
In-Reply-To: <cover.1703833818.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1312 bytes --]
In t9500 we're writing a custom configuration that sets up gitweb. This
requires us to manually ensure that the repository format is configured
as required, including both the repository format version and
extensions. With the introduction of the "extensions.refStorage"
extension we need to update the test to also write this new one.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t9500-gitweb-standalone-no-errors.sh | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 0333065d4d..7679780fb8 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -627,6 +627,7 @@ test_expect_success \
test_expect_success 'setup' '
version=$(git config core.repositoryformatversion) &&
algo=$(test_might_fail git config extensions.objectformat) &&
+ refstorage=$(test_might_fail git config extensions.refstorage) &&
cat >.git/config <<-\EOF &&
# testing noval and alternate separator
[gitweb]
@@ -637,6 +638,10 @@ test_expect_success 'setup' '
if test -n "$algo"
then
git config extensions.objectformat "$algo"
+ fi &&
+ if test -n "$refstorage"
+ then
+ git config extensions.refstorage "$refstorage"
fi
'
--
2.43.GIT
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v3 11/12] builtin/clone: introduce `--ref-format=` value flag
From: Patrick Steinhardt @ 2023-12-29 7:27 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Junio C Hamano, Eric Sunshine
In-Reply-To: <cover.1703833818.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 4004 bytes --]
Introduce a new `--ref-format` value flag for git-clone(1) that allows
the user to specify the ref format that is to be used for a newly
initialized repository.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git-clone.txt | 6 ++++++
builtin/clone.c | 12 +++++++++++-
t/t5601-clone.sh | 17 +++++++++++++++++
3 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index c37c4a37f7..6e43eb9c20 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -311,6 +311,12 @@ or `--mirror` is given)
The result is Git repository can be separated from working
tree.
+--ref-format=<ref-format::
+
+Specify the given ref storage format for the repository. The valid values are:
++
+include::ref-storage-format.txt[]
+
-j <n>::
--jobs <n>::
The number of submodules fetched at the same time.
diff --git a/builtin/clone.c b/builtin/clone.c
index 0fb3816d0c..f1635e0e8c 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -72,6 +72,7 @@ static char *remote_name = NULL;
static char *option_branch = NULL;
static struct string_list option_not = STRING_LIST_INIT_NODUP;
static const char *real_git_dir;
+static const char *ref_format;
static char *option_upload_pack = "git-upload-pack";
static int option_verbosity;
static int option_progress = -1;
@@ -157,6 +158,8 @@ static struct option builtin_clone_options[] = {
N_("any cloned submodules will be shallow")),
OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
N_("separate git dir from working tree")),
+ OPT_STRING(0, "ref-format", &ref_format, N_("format"),
+ N_("specify the reference format to use")),
OPT_STRING_LIST('c', "config", &option_config, N_("key=value"),
N_("set config inside the new repository")),
OPT_STRING_LIST(0, "server-option", &server_options,
@@ -932,6 +935,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
int submodule_progress;
int filter_submodules = 0;
int hash_algo;
+ unsigned int ref_storage_format = REF_STORAGE_FORMAT_UNKNOWN;
const int do_not_override_repo_unix_permissions = -1;
struct transport_ls_refs_options transport_ls_refs_options =
@@ -957,6 +961,12 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
if (option_single_branch == -1)
option_single_branch = deepen ? 1 : 0;
+ if (ref_format) {
+ ref_storage_format = ref_storage_format_by_name(ref_format);
+ if (ref_storage_format == REF_STORAGE_FORMAT_UNKNOWN)
+ die(_("unknown ref storage format '%s'"), ref_format);
+ }
+
if (option_mirror)
option_bare = 1;
@@ -1108,7 +1118,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
* their on-disk data structures.
*/
init_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN,
- REF_STORAGE_FORMAT_UNKNOWN, NULL,
+ ref_storage_format, NULL,
do_not_override_repo_unix_permissions, INIT_DB_QUIET | INIT_DB_SKIP_REFDB);
if (real_git_dir) {
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 47eae641f0..fb1b9c686d 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -157,6 +157,23 @@ test_expect_success 'clone --mirror does not repeat tags' '
'
+test_expect_success 'clone with files ref format' '
+ test_when_finished "rm -rf ref-storage" &&
+ git clone --ref-format=files --mirror src ref-storage &&
+ echo files >expect &&
+ git -C ref-storage rev-parse --show-ref-format >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'clone with garbage ref format' '
+ cat >expect <<-EOF &&
+ fatal: unknown ref storage format ${SQ}garbage${SQ}
+ EOF
+ test_must_fail git clone --ref-format=garbage --mirror src ref-storage 2>err &&
+ test_cmp expect err &&
+ test_path_is_missing ref-storage
+'
+
test_expect_success 'clone to destination with trailing /' '
git clone src target-1/ &&
--
2.43.GIT
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox