microsoft / scalar

Scalar: A set of tools and extensions for Git to allow very large monorepos to run on Git without a virtualization layer
MIT License
1.39k stars 63 forks source link

use platform-specific filepath matching and add or revise relevant tests #432

Closed chrisd8088 closed 4 years ago

chrisd8088 commented 4 years ago

Use filesystem-specific case matching when comparing file paths throughout Scalar and the functional test suite.

The primary change in this PR is to introduce a pair of wrappers which we use in place of StringComparison.OrdinalIgnoreCase and StringComparer.OrdinalIgnoreCase throughout the codebase whenever we need to compare or match file names or paths.

These new comparison utilities are named ScalarPlatform.Instance.Constants.Path{Comparison,Comparer} in the main Scalar project, and have equivalents named FileSystemHelpers.Path{Comparison,Comparer} in the functional test program's project.

From microsoft/VFSForGit#1232, microsoft/VFSForGit#1412, and microsoft/VFSForGit#1503, with additional new work and tests.

Given its scope this PR is probably best reviewed commit-by-commit; they should be logically separate and relatively manageable in size.


Note that this PR is not a perfect solution, just a first approximation, because all three platforms support filesystems and/or directories with the opposite sensitivity to their typical mode:

Therefore in commit f712d43cc29ddb1665022c73344525466ec6ef7b we rework the existing Mac-specific case-sensitivity check for all platforms and refactor it into a common method in ScalarPlatform.

This check only runs during the scalar clone step, and then only when the CreateScalarDirectories() method is called, which requires, among other things, that the local Git installation support the GVFS protocol. (This seems reasonable given that the key places where filepath matching is performed in the main Scalar codebase appear to be in maintenance routines handling the prefetch packfiles returned by the GVFS protocol and helper.)


We also add and expand a few of the tests in the EnlistmentPerFixture.CloneTests and EnlistmentPerFixture.FetchStepWithoutSharedCacheTests functional test suites, as these correspond most closely with the key remaining platform-variable file path checks in the main Scalar/CommandLine and Scalar.Common code (now that much of the filepath-matching logic associated with VFSForGit has been removed).

Specifically, the scalar clone command executes a check to determine whether the user is accidentally passing a path with --local-cache-path that is within the Scalar enlistment (i.e., the Scalar working directory), and that check is may be case-sensitive or case-insensitive depending on the platform.

And the scalar run fetch maintenance command, when the GVFS protocol is in use, executes several cleanup steps such as the UpdateKeepPacks() method, which now looks for prefetch-*.pack and *.keep files using our new ScalarPlatform.Instance.Constants.PathComparison path-matching wrapper instead of StringComparison.OrdinalIgnoreCase.

So we extend the functional tests to cover these actions and in particular test that on Linux we see case-sensitive behaviour, while macOS and Windows behave case-insensitively when matching filename patterns and paths.

In addition to the above, we take the opportunity to tidy up both of the relevant functional test classes, removing some now-unused methods, refactoring a number of the CloneTests to use a new common RunCloneCommand() method which also standardizes the test repository URL so that the --repo-to-clone functional test command-line option is fully respected, fixing a hard-coded path which contained Windows-style backslashes as path separators, and adding a CloneInNonEmptyDirectory() test. (The latter unfortunately cannot test the filepath check in TryCreateEnlistment(), though, because that check is used only to determine whether the user-provided path differs from the normalized path [which for our purposes is a Windows-specific concept] when printing the error message, and is not used in the actual check for an empty directory.)

And lastly we make a change to the UpdateKeepPacks() method mentioned above as part the maintenance executed by a scalar run fetch command; we adjust the retrieval of the list of "keep" sentinel files so that it filters out any which do not start with the ScalarConstants.PrefetchPackPrefix string, i.e., the prefix prefetch-. This aligns the behaviour when handling "keep" files to match that of the packfiles themselves earlier in the same method, and it then also allows our new tests to parallel each other for both types of files. It also looks to be in keeping with the case-sensitive filename prefix matching performed in the cb_keep_files() and cb_find_last() functions in gvfs-helper.c in https://github.com/microsoft/git, which I believe is what primarily writes new prefetch-*.{pack,keep,idx} files into the Scalar cache directory in the first place.