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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 7 commits into from
Jul 23, 2020
Merged

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

merged 7 commits into from
Jul 23, 2020

Conversation

mjcheetham
Copy link
Member

@mjcheetham mjcheetham commented Jul 15, 2020 •
edited
Loading

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:

  • Modify the CloneVerb to respond to a lack of GVFS protocol support
  • TESTS!

@mjcheetham mjcheetham changed the title Detect Git features (the GVFS Protocol) based on the version [Draft] Detect Git features (the GVFS Protocol) based on the version Jul 15, 2020
@mjcheetham mjcheetham changed the title [Draft] Detect Git features (the GVFS Protocol) based on the version [WIP] Detect Git features (the GVFS Protocol) based on the version Jul 15, 2020
@mjcheetham
git: add git feature flag detection
514df8e 
Add ability to determine Git features from the GitVersion. Currently
only supports detecting GVFS Protocol support.
@mjcheetham
git: support parsing normal git/git version strings
a4dc043 
Support parsing normal Git version numbers (major.minor.build) as well
as the existing Git for Windows and Microsoft Git style formats
(major.minor.build.platform.revision.minorRevision).
@mjcheetham
clone: skip GVFS protocol check if no GVFS support
d2a818e 
Skip the GVFS protocol authentication check if the installed version of
Git does not support the GVFS protocol (via the GVFS helper).
@mjcheetham
git: support release candidate and local git builds
1c8bbab 
Add support for parsing Git builds that include release candidate
components.

Also support partially parsing local builds of Git that may have other
version formats such as "2.28.0.456.gabcdefg.dirty".
@mjcheetham mjcheetham marked this pull request as ready for review July 21, 2020 09:23
@mjcheetham mjcheetham changed the title [WIP] Detect Git features (the GVFS Protocol) based on the version Detect Git features (the GVFS Protocol) based on the version Jul 21, 2020
@jeffhostetler
Copy link
Contributor

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
Copy link
Contributor

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

@mjcheetham mjcheetham linked an issue Jul 22, 2020 that may be closed by this pull request
Detect Git feature set in Scalar #380
Closed
@mjcheetham
Copy link
Member Author

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
Copy link
Contributor

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
Copy link
Member Author

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
git-version: add another test of version parsing
89a0c82 
Add another test of GitVersion parsing that includes nonsense version
information, and Unicode (Emoji) characters - ensure this is parsed OK
too.
@mjcheetham
Copy link
Member Author

  • 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
Copy link
Contributor

  • 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.

@mjcheetham mjcheetham merged commit 78c5d1b into microsoft:main Jul 23, 2020
@mjcheetham mjcheetham deleted the gitver branch July 23, 2020 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeffhostetler jeffhostetler jeffhostetler left review comments

@derrickstolee derrickstolee derrickstolee approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Detect Git feature set in Scalar
3 participants
@mjcheetham @jeffhostetler @derrickstolee