Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mingw_open_existing: handle directories better #5342

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

rimrul
Copy link
Member

@rimrul rimrul commented Jan 5, 2025

CreateFileW() requires FILE_FLAG_BACKUP_SEMANTICS to create a directory handle and errors out with ERROR_ACCESS_DENIED without this flag. Fall back to accessing Directory handles this way.

This fixes #5068

CreateFileW() requires FILE_FLAG_BACKUP_SEMANTICS to create a directory
handle [1] and errors out with ERROR_ACCESS_DENIED without this flag.
Fall back to accessing Directory handles this way.

[1] https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilew#directories

This fixes git-for-windows#5068

Signed-off-by: Matthias Aßhauer <[email protected]>
@@ -810,13 +810,24 @@ static int mingw_open_existing(const wchar_t *filename, int oflags, ...)
&security_attributes, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
if (handle == INVALID_HANDLE_VALUE) {
DWORD err = GetLastError();
if (err == ERROR_ACCESS_DENIED) {
DWORD attrs = GetFileAttributesW(filename);
Copy link
Member Author

Choose a reason for hiding this comment

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

We could probably also check GetFileAttributesW() before the first CreateFileW() and avoid calling CreateFileW() twice.

Copy link
Member

Choose a reason for hiding this comment

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

I think the current version is fine because it optimizes for the common case, avoiding the extra GetFileAttributesW() & CreateFileW() calls for the vast majority of calls.

@@ -810,13 +810,24 @@ static int mingw_open_existing(const wchar_t *filename, int oflags, ...)
&security_attributes, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
if (handle == INVALID_HANDLE_VALUE) {
DWORD err = GetLastError();
if (err == ERROR_ACCESS_DENIED) {
DWORD attrs = GetFileAttributesW(filename);
Copy link
Member

Choose a reason for hiding this comment

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

I think the current version is fine because it optimizes for the common case, avoiding the extra GetFileAttributesW() & CreateFileW() calls for the vast majority of calls.

@dscho
Copy link
Member

dscho commented Jan 7, 2025

It looks like t5616 has a newly-introduced flake now:

  ++ git -c protocol.version=0 -c gc.autoPackLimit=0 -c maintenance.incremental-repack.auto=1234 -C pc1 fetch --refetch origin
 error: unable to open .git/objects/pack/pack-6ac94f9c26c537ad200c52c992a627f57871f8aa.pack: No such file or directory
 fatal: unable to rename temporary '*.pack' file to '.git/objects/pack/pack-6ac94f9c26c537ad200c52c992a627f57871f8aa.pack'
 fatal: index-pack failed

I've restarted the job.

Copy link
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

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

Looks good!

@dscho dscho merged commit f045ed3 into git-for-windows:main Jan 23, 2025
49 checks passed
@dscho
Copy link
Member

dscho commented Jan 23, 2025

/add relnote bug Git for Windows used to issue a misleading warning when .gitignore was a directory, which has been fixed.

The workflow run was started

@dscho dscho added this to the Next release milestone Jan 23, 2025
github-actions bot pushed a commit to git-for-windows/build-extra that referenced this pull request Jan 23, 2025
Git for Windows used [to issue a misleading warning when `.gitignore`
was a directory](git-for-windows/git#5068),
which has been
[fixed](git-for-windows/git#5342).

Signed-off-by: gitforwindowshelper[bot] <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Misleading warning when a directory is named .gitignore
2 participants