search

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

Detect Git features (the GVFS Protocol) based on the version #407

Closed mjcheetham closed 4 years ago

mjcheetham commented 4 years ago

Add a way to get a list of features that a version of Git supports, such as support for the GVFS protocol. With this we can better handle and support users of Scalar that are not using Azure DevOps or the Microsoft fork of Git.

In the future we can also gate behaviour on other features such as --changed-paths.

Implements #380

TODO:

jeffhostetler commented 4 years ago

I'm not sure, but I think we're missing a few cases. My dev build has a version number of:

$ ./git version
git version 2.27.0.vfs.1.0.66.g098ca17956.dirty.MSVC

My installed version (from one of the CI builds) looks like:

>git version
git version 2.27.0.vfs.1.0.62.gd95c663
jeffhostetler commented 4 years ago

(I'm not sure we can do anything with those formats, but I thought I'd mention they exist.)

mjcheetham commented 4 years ago

I'm not sure, but I think we're missing a few cases. My dev build has a version number of:

$ ./git version
git version 2.27.0.vfs.1.0.66.g098ca17956.dirty.MSVC

My installed version (from one of the CI builds) looks like:

>git version
git version 2.27.0.vfs.1.0.62.gd95c663

(I'm not sure we can do anything with those formats, but I thought I'd mention they exist.)

If the 4th component is a string (Platform) then we will continue to parse the next two integers, so for both 2.27.0.vfs.1.0.62.gd95c663 and git version 2.27.0.vfs.1.0.66.g098ca17956.dirty.MSVC we'd get 2.27.0.vfs.1.0 as the GitVersion object.

We shouldn't be gating a feature of Git by something beyond that version, right?

derrickstolee commented 4 years ago

If the 4th component is a string (Platform) then we will continue to parse the next two integers, so for both 2.27.0.vfs.1.0.62.gd95c663 and git version 2.27.0.vfs.1.0.66.g098ca17956.dirty.MSVC we'd get 2.27.0.vfs.1.0 as the GitVersion object.

We shouldn't be gating a feature of Git by something beyond that version, right?

I agree that we shouldn't care about the details after the first 5 parts. I think @jeffhostetler just wants to add a unit test to ensure that we properly ignore those details. What do you think, @mjcheetham?

mjcheetham commented 4 years ago

I think @jeffhostetler just wants to add a unit test to ensure that we properly ignore those details. What do you think, @mjcheetham?

I can add a test with that, and also a test with garbage after the first 4 components too, for good measure.

mjcheetham commented 4 years ago
  • [ ] TESTS!

@derrickstolee I'm not sure how we can test this E2E, given the inability to mock out the process creation, and that the version of Git installed would need to be different between test cases (meaning multiple new test jobs, with the difference being the installed version of Git)?

derrickstolee commented 4 years ago
  • [ ] TESTS!

@derrickstolee I'm not sure how we can test this E2E, given the inability to mock out the process creation, and that the version of Git installed would need to be different between test cases (meaning multiple new test jobs, with the difference being the installed version of Git)?

The only automated test I can think of would be to set up a functional test build that only runs the "Vanilla Git repo" tests (using the --include category option) and uses a non-VFS-enabled version of Git.

I think for now, the unit tests are sufficient and we can worry about the actual compatibility at the functional level in a follow-up.