Hi Junio,
On Mon, 8 Aug 2022, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx>
> writes:
>
> > Due to the semantics being substantially different from Unix, the
> > safe.directory feature presents its own set of problems on Windows. One
> > particular issue would have prevented it from working in GitHub Actions'
> > build agents, which we definitely rely on in the Git project itself. This
> > was addressed via the fifth patch, which had made it (in a slightly
> > different form) already into Git for Windows v2.35.2, and they are ready to
> > be applied to core Git, too.
> >
> > The FAT32 patch came in later, and was released as part of Git for Windows
> > v2.37.0, so I also have confidence that it is stable and ready to be
> > integrated into core Git, too.
> >
> > Changes since v1:
> >
> > * Restructured the patch series.
> > * Instead of an environment variable to turn on debugging, we now always
> > show the platform-dependent information together with the error message
> > about the dubious ownership (iff it is shown, that is), based on an idea
> > by Junio.
> > * Rebased onto gc/bare-repo-discovery to avoid a merge conflict.
>
> I actually had to rebase it back so that we could merge it to
> 'maint' for further 2.37.x releases.
I appreciate the effort you put into it, even if it is not your
responsibility to take care of Git for Windows' releases.
The range-diff shows that you snuck in a commit message change that I
would have either not made at all (I think the original was fine) or would
have made differently (because Access Control Lists are not specific to
Windows, even if Windows is the most obvious example for this permission
model):
-- snip --
1: 301d94f18f5 ! 1: d51e1dff980 setup: fix some formatting
@@ Commit message
the indentation before actually modifying the code.
Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
+ Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
## setup.c ##
@@ setup.c: static int safe_directory_cb(const char *key, const char *value, void *d)
2: 8cc45e4922a ! 2: 17d3883fe9c Prepare for more detailed "dubious ownership" messages
@@ Metadata
Author: Johannes Schindelin <Johannes.Schindelin@xxxxxx>
## Commit message ##
- Prepare for more detailed "dubious ownership" messages
+ setup: prepare for more detailed "dubious ownership" messages
When verifying the ownership of the Git directory, we sometimes would
like to say a bit more about it, e.g. when using a platform-dependent
- code path (think: Windows and the permission model that is so different
+ code path (think: Windows has the permission model that is so different
from Unix'), but only when it is a appropriate to actually say
something.
@@ Commit message
Based-on-an-idea-by: Junio C Hamano <gitster@xxxxxxxxx>
Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
+ Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
## compat/mingw.c ##
@@ compat/mingw.c: static PSID get_current_user_sid(void)
@@ setup.c: static enum discovery_result setup_git_directory_gently_1(struct strbuf
ret = GIT_DIR_DISCOVERED;
} else
@@ setup.c: static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
+ }
+
if (is_git_directory(dir->buf)) {
- if (get_allowed_bare_repo() == ALLOWED_BARE_REPO_EXPLICIT)
- return GIT_DIR_DISALLOWED_BARE;
- if (!ensure_valid_ownership(NULL, NULL, dir->buf))
+ if (!ensure_valid_ownership(NULL, NULL, dir->buf, report))
return GIT_DIR_INVALID_OWNERSHIP;
3: 63494818105 ! 3: e883e04b68b mingw: provide details about unsafe directories' ownership
@@ Commit message
Let's help with that by providing more detailed information.
Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
+ Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
## compat/mingw.c ##
@@
4: 7aaa6248dfe ! 4: 7c83470e64e mingw: be more informative when ownership check fails on FAT32
@@ Commit message
This addresses https://github.com/git-for-windows/git/issues/3886
Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
+ Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
## compat/mingw.c ##
@@ compat/mingw.c: static PSID get_current_user_sid(void)
5: fbfaff2ec21 ! 5: 3f7207e2ea9 mingw: handle a file owned by the Administrators group correctly
@@ Commit message
Administrators Group as if it were owned by said user.
Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
+ Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
## compat/mingw.c ##
@@ compat/mingw.c: int is_path_owned_by_current_sid(const char *path, struct strbuf *report)
-- snap --
Retitling the commit message was okay, of course, using the `setup:`
prefix.
> I'll refer to the original patches in this thread when I merge the
> result to 'seen', of course, to make sure the results do match. It
> would have been slightly less convenient if you did not do this rebase,
> but it would have allowed me to have much better confidence in the
> result that may eventually go to 'maint'. After all, mistakes in
> resolving merge conflicts on 'seen' can be corrected before the topic
> hits 'next'.
Yes, mistakes can happen, and the more people work together the easier it
is to avoid or fix them.
> Thanks. I do not know about the API calls mingw.c part of these
> patches make, but the overall structure looks sensible to me.
I do not think that anybody expected you to know about Win32 API calls ;-)
Ciao,
Dscho
