Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apply.c
Original file line number Diff line number Diff line change
Expand Up @@ -3779,7 +3779,7 @@ static int check_preimage(struct apply_state *state,
if (*ce && !(*ce)->ce_mode)
BUG("ce_mode == 0 for path '%s'", old_name);

if (trust_executable_bit)
if (trust_executable_bit || !S_ISREG(st->st_mode))
st_mode = ce_mode_from_stat(*ce, st->st_mode);
else if (*ce)
st_mode = (*ce)->ce_mode;
Expand Down
32 changes: 21 additions & 11 deletions compat/mingw.c
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,7 @@ int mingw_open (const char *filename, int oflags, ...)
int fd, create = (oflags & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL);
wchar_t wfilename[MAX_PATH];
open_fn_t open_fn;
WIN32_FILE_ATTRIBUTE_DATA fdata;

DECLARE_PROC_ADDR(ntdll.dll, NTSTATUS, NTAPI, RtlGetLastNtStatus, void);

Expand All @@ -653,6 +654,19 @@ int mingw_open (const char *filename, int oflags, ...)
else if (xutftowcs_path(wfilename, filename) < 0)
return -1;

/*
* When `symlink` exists and is a symbolic link pointing to a
* non-existing file, `_wopen(symlink, O_CREAT | O_EXCL)` would
* create that file. Not what we want: Linux would say `EEXIST`
* in that instance, which is therefore what Git expects.
*/
if (create &&
GetFileAttributesExW(wfilename, GetFileExInfoStandard, &fdata) &&
(fdata.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT)) {
errno = EEXIST;
return -1;
}

fd = open_fn(wfilename, oflags, mode);

/*
Expand Down Expand Up @@ -1237,18 +1251,16 @@ char *mingw_getcwd(char *pointer, int len)
{
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Patrick Steinhardt wrote (reply to this):

On Tue, Dec 16, 2025 at 03:33:45PM +0000, Johannes Schindelin via GitGitGadget wrote:
> diff --git a/compat/mingw.c b/compat/mingw.c
> index ba1b7b6dd1..7215b127cc 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -1251,18 +1251,16 @@ char *mingw_getcwd(char *pointer, int len)
>  {
>  	wchar_t cwd[MAX_PATH], wpointer[MAX_PATH];
>  	DWORD ret = GetCurrentDirectoryW(ARRAY_SIZE(cwd), cwd);
> +	HANDLE hnd;
>  
>  	if (!ret || ret >= ARRAY_SIZE(cwd)) {
>  		errno = ret ? ENAMETOOLONG : err_win_to_posix(GetLastError());
>  		return NULL;
>  	}
> -	ret = GetLongPathNameW(cwd, wpointer, ARRAY_SIZE(wpointer));
> -	if (!ret && GetLastError() == ERROR_ACCESS_DENIED) {
> -		HANDLE hnd = CreateFileW(cwd, 0,
> -			FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL,
> -			OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
> -		if (hnd == INVALID_HANDLE_VALUE)
> -			return NULL;
> +	hnd = CreateFileW(cwd, 0,
> +			  FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL,
> +			  OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
> +	if (hnd != INVALID_HANDLE_VALUE) {
>  		ret = GetFinalPathNameByHandleW(hnd, wpointer, ARRAY_SIZE(wpointer), 0);
>  		CloseHandle(hnd);
>  		if (!ret || ret >= ARRAY_SIZE(wpointer))

Okay. Due to the change we now also try calling `GetFileAttributesW()`
in case `CreateFileW()` fails, which wasn't the case before. But I'd
consider that to be a win -- if we cannot figure out the final path
name, then we can at least return the unresolved current working
directory.

Patrick

> @@ -1271,13 +1269,11 @@ char *mingw_getcwd(char *pointer, int len)
>  			return NULL;
>  		return pointer;
>  	}
> -	if (!ret || ret >= ARRAY_SIZE(wpointer))
> -		return NULL;
> -	if (GetFileAttributesW(wpointer) == INVALID_FILE_ATTRIBUTES) {
> +	if (GetFileAttributesW(cwd) == INVALID_FILE_ATTRIBUTES) {
>  		errno = ENOENT;
>  		return NULL;
>  	}
> -	if (xwcstoutf(pointer, wpointer, len) < 0)
> +	if (xwcstoutf(pointer, cwd, len) < 0)
>  		return NULL;
>  	convert_slashes(pointer);
>  	return pointer;

wchar_t cwd[MAX_PATH], wpointer[MAX_PATH];
DWORD ret = GetCurrentDirectoryW(ARRAY_SIZE(cwd), cwd);
HANDLE hnd;

if (!ret || ret >= ARRAY_SIZE(cwd)) {
errno = ret ? ENAMETOOLONG : err_win_to_posix(GetLastError());
return NULL;
}
ret = GetLongPathNameW(cwd, wpointer, ARRAY_SIZE(wpointer));
if (!ret && GetLastError() == ERROR_ACCESS_DENIED) {
HANDLE hnd = CreateFileW(cwd, 0,
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL,
OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
if (hnd == INVALID_HANDLE_VALUE)
return NULL;
hnd = CreateFileW(cwd, 0,
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL,
OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
if (hnd != INVALID_HANDLE_VALUE) {
ret = GetFinalPathNameByHandleW(hnd, wpointer, ARRAY_SIZE(wpointer), 0);
CloseHandle(hnd);
if (!ret || ret >= ARRAY_SIZE(wpointer))
Expand All @@ -1257,13 +1269,11 @@ char *mingw_getcwd(char *pointer, int len)
return NULL;
return pointer;
}
if (!ret || ret >= ARRAY_SIZE(wpointer))
return NULL;
if (GetFileAttributesW(wpointer) == INVALID_FILE_ATTRIBUTES) {
if (GetFileAttributesW(cwd) == INVALID_FILE_ATTRIBUTES) {
errno = ENOENT;
return NULL;
}
if (xwcstoutf(pointer, wpointer, len) < 0)
if (xwcstoutf(pointer, cwd, len) < 0)
return NULL;
convert_slashes(pointer);
return pointer;
Expand Down
4 changes: 2 additions & 2 deletions environment.c
Original file line number Diff line number Diff line change
Expand Up @@ -324,8 +324,8 @@ static enum fsync_component parse_fsync_components(const char *var, const char *
return (current & ~negative) | positive;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Patrick Steinhardt wrote (reply to this):

On Tue, Dec 16, 2025 at 03:33:46PM +0000, Johannes Schindelin via GitGitGadget wrote:
> diff --git a/setup.c b/setup.c
> index 7086741e6c..42e4e7a690 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -2611,7 +2611,7 @@ int init_db(const char *git_dir, const char *real_git_dir,
>  	 * have set up the repository format such that we can evaluate
>  	 * includeIf conditions correctly in the case of re-initialization.
>  	 */
> -	repo_config(the_repository, platform_core_config, NULL);
> +	repo_config(the_repository, git_default_core_config, NULL);
>  
>  	safe_create_dir(the_repository, git_dir, 0);

Two lines further down we call `create_default_files()`, and there we
end up calling `repo_config(the_repository, git_default_config, NULL)`
as one of the first things. We do so after copying templates though, so
indeed this comes too late.

We also cannot really merge these two calls: we need to re-parse the
configuration after having copied over the template, as the template may
contain a gitconfig file itself.

Furthermore, `git_default_core_config()` already knows to call
`platform_core_config()`, as well. So we're not losing any of that
information, either.

All to say that this change makes sense to me and should be safe, as we
don't end up parsing _more_ configuration keys, we only parse a subset
of it a bit earlier.

Patrick

}

static int git_default_core_config(const char *var, const char *value,
const struct config_context *ctx, void *cb)
int git_default_core_config(const char *var, const char *value,
const struct config_context *ctx, void *cb)
{
/* This needs a better name */
if (!strcmp(var, "core.filemode")) {
Expand Down
2 changes: 2 additions & 0 deletions environment.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ const char *strip_namespace(const char *namespaced_ref);

int git_default_config(const char *, const char *,
const struct config_context *, void *);
int git_default_core_config(const char *var, const char *value,
const struct config_context *ctx, void *cb);

/*
* TODO: All the below state either explicitly or implicitly relies on
Expand Down
4 changes: 2 additions & 2 deletions lockfile.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ static void trim_last_path_component(struct strbuf *path)
int i = path->len;

/* back up past trailing slashes, if any */
while (i && path->buf[i - 1] == '/')
while (i && is_dir_sep(path->buf[i - 1]))
i--;

/*
* then go backwards until a slash, or the beginning of the
* string
*/
while (i && path->buf[i - 1] != '/')
while (i && !is_dir_sep(path->buf[i - 1]))
i--;

strbuf_setlen(path, i);
Expand Down
2 changes: 1 addition & 1 deletion setup.c
Original file line number Diff line number Diff line change
Expand Up @@ -2611,7 +2611,7 @@ int init_db(const char *git_dir, const char *real_git_dir,
* have set up the repository format such that we can evaluate
* includeIf conditions correctly in the case of re-initialization.
*/
repo_config(the_repository, platform_core_config, NULL);
repo_config(the_repository, git_default_core_config, NULL);

safe_create_dir(the_repository, git_dir, 0);

Expand Down
10 changes: 4 additions & 6 deletions strbuf.c
Original file line number Diff line number Diff line change
Expand Up @@ -566,24 +566,22 @@ ssize_t strbuf_write(struct strbuf *sb, FILE *f)
return sb->len ? fwrite(sb->buf, 1, sb->len, f) : 0;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Patrick Steinhardt wrote (reply to this):

On Tue, Dec 16, 2025 at 03:33:48PM +0000, Karsten Blees via GitGitGadget wrote:
> diff --git a/strbuf.c b/strbuf.c
> index 44a8f6a554..fa4e30f112 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -566,8 +566,6 @@ ssize_t strbuf_write(struct strbuf *sb, FILE *f)
>  	return sb->len ? fwrite(sb->buf, 1, sb->len, f) : 0;
>  }
>  
> -#define STRBUF_MAXLINK (2*PATH_MAX)
> -
>  int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)
>  {
>  	size_t oldalloc = sb->alloc;
> @@ -575,7 +573,7 @@ int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)
>  	if (hint < 32)
>  		hint = 32;
>  
> -	while (hint < STRBUF_MAXLINK) {
> +	for (;;) {
>  		ssize_t len;
>  
>  		strbuf_grow(sb, hint + 1);

This makes me wonder whether we have a better way to figure out the
actual size of the buffer that we ultimately need to allocate. But
reading through readlink(3p) doesn't indicate anything, and I'm not sure
whether we can always rely on lstat(3p) to return the correct size for
symlink contents on all platforms.

One thing that _is_ noted though is that calling the function with a
buffer size larger than SSIZE_MAX is implementation-defined. It does
make me a bit uneasy in that light to grow indefinitely.

Which makes me wonder whether Windows has a limit for the symlink
contents that we could enforce in theory so that we can reasonably turn
this into a bounded loop again?

Patrick

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Karsten Blees via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Karsten Blees <blees@dcon.de>
>
> The `strbuf_readlink()` function refuses to read link targets that
> exceed PATH_MAX (even if a sufficient size was specified by the caller).
>
> As some platforms (*cough* Windows *cough*) support longer paths, remove
> this restriction (similar to `strbuf_getcwd()`).
>
> Signed-off-by: Karsten Blees <blees@dcon.de>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  strbuf.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

We've been bitten before by platforms that sets PATH_MAX too low
(i.e., lower than what they comfortably support), so this is a
welcome change.

> diff --git a/strbuf.c b/strbuf.c
> index 44a8f6a554..fa4e30f112 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -566,8 +566,6 @@ ssize_t strbuf_write(struct strbuf *sb, FILE *f)
>  	return sb->len ? fwrite(sb->buf, 1, sb->len, f) : 0;
>  }
>  
> -#define STRBUF_MAXLINK (2*PATH_MAX)
> -
>  int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)
>  {
>  	size_t oldalloc = sb->alloc;
> @@ -575,7 +573,7 @@ int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)
>  	if (hint < 32)
>  		hint = 32;
>  
> -	while (hint < STRBUF_MAXLINK) {
> +	for (;;) {
>  		ssize_t len;
>  
>  		strbuf_grow(sb, hint + 1);

I briefly wondered if this would cause us loop infinitely on a truly
broken platform, where readlink() somehow keeps returning negative,
but we only retry when we got ERANGE (which can be seen several
lines below the postimage of hte patch), so we should be safe.

Thanks.

}

#define STRBUF_MAXLINK (2*PATH_MAX)

int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)
{
size_t oldalloc = sb->alloc;

if (hint < 32)
hint = 32;

while (hint < STRBUF_MAXLINK) {
for (;;) {
ssize_t len;

strbuf_grow(sb, hint);
len = readlink(path, sb->buf, hint);
strbuf_grow(sb, hint + 1);
len = readlink(path, sb->buf, hint + 1);
if (len < 0) {
if (errno != ERANGE)
break;
} else if (len < hint) {
} else if (len <= hint) {
strbuf_setlen(sb, len);
return 0;
}
Expand Down
6 changes: 5 additions & 1 deletion t/t0001-init.sh
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,11 @@ test_expect_success SYMLINKS 're-init to move gitdir symlink' '
git init --separate-git-dir ../realgitdir
) &&
echo "gitdir: $(pwd)/realgitdir" >expected &&
test_cmp expected newdir/.git &&
case "$GIT_TEST_CMP" in
# `git diff --no-index` does not resolve symlinks
*--no-index*) cmp expected newdir/.git;;
*) test_cmp expected newdir/.git;;
esac &&
test_cmp expected newdir/here &&
test_path_is_dir realgitdir/refs
'
Expand Down
3 changes: 2 additions & 1 deletion t/t0301-credential-cache.sh
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ test_expect_success SYMLINKS 'use user socket if user directory is a symlink to
rmdir \"\$HOME/dir/\" &&
rm \"\$HOME/.git-credential-cache\"
" &&
mkdir -p -m 700 "$HOME/dir/" &&
mkdir -p "$HOME/dir/" &&
chmod 700 "$HOME/dir/" &&
ln -s "$HOME/dir" "$HOME/.git-credential-cache" &&
check approve cache <<-\EOF &&
protocol=https
Expand Down
2 changes: 1 addition & 1 deletion t/t0600-reffiles-backend.sh
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ test_expect_success POSIXPERM 'git reflog expire honors core.sharedRepository' '
esac
'

test_expect_success SYMLINKS 'symref transaction supports symlinks' '
test_expect_success SYMLINKS,!MINGW 'symref transaction supports symlinks' '
test_when_finished "git symbolic-ref -d TEST_SYMREF_HEAD" &&
git update-ref refs/heads/new @ &&
test_config core.prefersymlinkrefs true &&
Expand Down
24 changes: 17 additions & 7 deletions t/t1006-cat-file.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1048,18 +1048,28 @@ test_expect_success 'git cat-file --batch-check --follow-symlinks works for out-
echo .. >>expect &&
echo HEAD:dir/subdir/out-of-repo-link-dir | git cat-file --batch-check --follow-symlinks >actual &&
test_cmp expect actual &&
echo symlink 3 >expect &&
echo ../ >>expect &&
if test_have_prereq MINGW,SYMLINKS
then
test_write_lines "symlink 2" ..
else
test_write_lines "symlink 3" ../
fi >expect &&
echo HEAD:dir/subdir/out-of-repo-link-dir-trailing | git cat-file --batch-check --follow-symlinks >actual &&
test_cmp expect actual
'

test_expect_success 'git cat-file --batch-check --follow-symlinks works for symlinks with internal ..' '
echo HEAD: | git cat-file --batch-check >expect &&
echo HEAD:up-down | git cat-file --batch-check --follow-symlinks >actual &&
test_cmp expect actual &&
echo HEAD:up-down-trailing | git cat-file --batch-check --follow-symlinks >actual &&
test_cmp expect actual &&
if test_have_prereq !MINGW
then
# The `up-down` and `up-down-trailing` symlinks are normalized
# in MSYS in `winsymlinks` mode and are therefore in a
# different shape than Git expects them.
echo HEAD: | git cat-file --batch-check >expect &&
echo HEAD:up-down | git cat-file --batch-check --follow-symlinks >actual &&
test_cmp expect actual &&
echo HEAD:up-down-trailing | git cat-file --batch-check --follow-symlinks >actual &&
test_cmp expect actual
fi &&
echo HEAD:up-down-file | git cat-file --batch-check --follow-symlinks >actual &&
test_cmp found actual &&
echo symlink 7 >expect &&
Expand Down
4 changes: 2 additions & 2 deletions t/t1305-config-include.sh
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ test_expect_success SYMLINKS 'conditional include, relative path with symlinks'
)
'

test_expect_success SYMLINKS 'conditional include, gitdir matching symlink' '
test_expect_success SYMLINKS,!MINGW 'conditional include, gitdir matching symlink' '
ln -s foo bar &&
(
cd bar &&
Expand All @@ -298,7 +298,7 @@ test_expect_success SYMLINKS 'conditional include, gitdir matching symlink' '
)
'

test_expect_success SYMLINKS 'conditional include, gitdir matching symlink, icase' '
test_expect_success SYMLINKS,!MINGW 'conditional include, gitdir matching symlink, icase' '
(
cd bar &&
echo "[includeIf \"gitdir/i:BAR/\"]path=bar8" >>.git/config &&
Expand Down
9 changes: 7 additions & 2 deletions t/t6423-merge-rename-directories.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5158,13 +5158,18 @@ test_setup_12m () {
git switch B &&
git rm dir/subdir/file &&
mkdir dir &&
ln -s /dev/null dir/subdir &&
if test_have_prereq MINGW
then
cmd //c 'mklink dir\subdir NUL'
else
ln -s /dev/null dir/subdir
fi &&
git add . &&
git commit -m "B"
)
}

test_expect_success '12m: Change parent of renamed-dir to symlink on other side' '
test_expect_success SYMLINKS '12m: Change parent of renamed-dir to symlink on other side' '
test_setup_12m &&
(
cd 12m &&
Expand Down
8 changes: 4 additions & 4 deletions t/t7800-difftool.sh
Original file line number Diff line number Diff line change
Expand Up @@ -752,11 +752,11 @@ test_expect_success SYMLINKS 'difftool --dir-diff handles modified symlinks' '
c
EOF
git difftool --symlinks --dir-diff --extcmd ls >output &&
grep -v ^/ output >actual &&
grep -v ":\$" output >actual &&
test_cmp expect actual &&
git difftool --no-symlinks --dir-diff --extcmd ls >output &&
grep -v ^/ output >actual &&
grep -v ":\$" output >actual &&
test_cmp expect actual &&
# The left side contains symlink "c" that points to "b"
Expand Down Expand Up @@ -786,11 +786,11 @@ test_expect_success SYMLINKS 'difftool --dir-diff handles modified symlinks' '
EOF
git difftool --symlinks --dir-diff --extcmd ls >output &&
grep -v ^/ output >actual &&
grep -v ":\$" output >actual &&
test_cmp expect actual &&
git difftool --no-symlinks --dir-diff --extcmd ls >output &&
grep -v ^/ output >actual &&
grep -v ":\$" output >actual &&
test_cmp expect actual
'

Expand Down
9 changes: 7 additions & 2 deletions t/t9700/test.pl
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,12 @@ sub adjust_dirsep {
unlink $tmpfile;

# paths
is($r->repo_path, $abs_repo_dir . "/.git", "repo_path");
my $abs_git_dir = $abs_repo_dir . "/.git";
if ($^O eq 'msys' or $^O eq 'cygwin') {
$abs_git_dir = `cygpath -am "$abs_repo_dir/.git"`;
$abs_git_dir =~ s/\r?\n?$//;
}
is($r->repo_path, $abs_git_dir, "repo_path");
is($r->wc_path, $abs_repo_dir . "/", "wc_path");
is($r->wc_subdir, "", "wc_subdir initial");
$r->wc_chdir("directory1");
Expand All @@ -127,7 +132,7 @@ sub adjust_dirsep {
# Object generation in sub directory
chdir("directory2");
my $r2 = Git->repository();
is($r2->repo_path, $abs_repo_dir . "/.git", "repo_path (2)");
is($r2->repo_path, $abs_git_dir, "repo_path (2)");
is($r2->wc_path, $abs_repo_dir . "/", "wc_path (2)");
is($r2->wc_subdir, "directory2/", "wc_subdir initial (2)");

Expand Down
Loading