t6042: fix breakage on Windows #109
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Author
|
/submit |
|
Submitted as pull.109.git.gitgitgadget@gmail.com |
Closed
When Git determines whether a file has changed, it looks at the mtime, at the file size, and to detect changes even if the mtime is the same (on Windows, the mtime granularity is 100ns, read: if two files are written within the same 100ns time slot, they have the same mtime) and even if the file size is the same, Git also looks at the inode/device numbers. This design obviously comes from a Linux background, where `lstat()` calls were designed to be cheap. On Windows, there is no `lstat()`. It has to be emulated. And while obtaining the mtime and the file size is not all that expensive (you can get both with a single `GetFileAttributesW()` call), obtaining the equivalent of the inode and device numbers is very expensive (it requires a call to `GetFileInformationByHandle()`, which in turn requires a file handle, which is *a lot* more expensive than one might imagine). As it is very uncommon for developers to modify files within 100ns time slots, Git for Windows chooses not to fill inode/device numbers properly, but simply sets them to 0. However, in t6042 the files file_v1 and file_v2 are typically written within the same 100ns time slot, and they do not differ in file size. So the minor modification is not picked up. Let's work around this issue by avoiding the `git mv` calls in the 'mod6-setup: chains of rename/rename(1to2) and rename/rename(2to1)' test case. The target files are overwritten anyway, so it is not like we really rename those files. This fixes the issue because `git add` will now add the files as new files (as opposed to existing, just renamed files). Functionally, we do not change anything because we replace two `git mv <old> <new>` calls (where `<new>` is completely overwritten and `git add`ed later anyway) by `git rm <old>` calls (removing other files, too, that are also completely overwritten and `git add`ed later). Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Member
Author
|
/submit |
|
Submitted as pull.109.v2.git.gitgitgadget@gmail.com |
|
This branch is now known as |
|
This patch series was integrated into pu via git@7c00b1f. |
|
This patch series was integrated into pu via git@8387ab5. |
|
This patch series was integrated into pu via git@3ecf445. |
|
This patch series was integrated into pu via git@27eceb6. |
|
This patch series was integrated into pu via git@eea38f3. |
|
This patch series was integrated into next via git@9543c96. |
|
This patch series was integrated into pu via git@38f56b1. |
|
This patch series was integrated into pu via git@3009c8b. |
|
This patch series was integrated into next via git@3009c8b. |
|
This patch series was integrated into master via git@3009c8b. |
|
Closed via 3009c8b. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Unfortunately, Travis decided to change some things under the hood that affect the Windows build. As a consequence,
masterhas not been tested in quite a while, even if the test runs pretended that it had been tested :-(So imagine my surprise when
mastersimply would refuse to pass the test suite cleanly outside Travis, always failing at t6042, despite the fact that Travis passed.It turns out that two files are written too quickly in succession, running into the issue where Git for Windows chooses not to populate the inode and device numbers in the
statdata (this is a noticeable performance optimization). As a consequence, Git thinks the file is unchanged, and fails to pick up a modification. And no, we cannot simply undo the performance optimization, it would make things prohibitively slow in particular in large worktrees, and it is not like the bug is likely to be hit in reality: for Git to be fooled into thinking that a file is unchanged, it has to be written with the same file size, and within a 100ns time bucket (it is pretty improbable that there is any real-world scenario that would run into that, except of course our regression test suite).This patch works around this issue by forcing Git to recognize the new file versions as new files (which they really are: the patch simply replaces
by
which is not shorter, but even a performance improvement (an unnoticeable one, of course).
Changes since v1:
Cc: Elijah Newren newren@gmail.com