Skip to content

Use GVFS Helper transport instead of read object hook - #122

Merged
Derrick Stolee (derrickstolee) merged 12 commits into
microsoft:masterfrom
derrickstolee:gvfs-helper
Oct 4, 2019
Merged

Derrick Stolee (derrickstolee) merged 12 commits into
microsoft:masterfrom
derrickstolee:gvfs-helper

Conversation

@derrickstolee

@derrickstolee Derrick Stolee (derrickstolee) commented Sep 20, 2019 •

Copy link
Copy Markdown
Contributor

See microsoft/git#191 for the Git code regarding the GVFS transport layer. This can work in tandem with the read-object hook, but it should make the read-object hook irrelevant.

Resolves #6, #7, #36.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
@derrickstolee

Copy link
Copy Markdown
Contributor Author

After Jeff Hostetler (@jeffhostetler) merges microsoft/git#191, I'll generate a -sc installer and re-push these commits.

In the meantime, the rest is available for review.

cc/ William Baker (@wilbaker), John Briggs (@jrbriggs), Kevin Willford (@kewillford)

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
@derrickstolee Derrick Stolee (derrickstolee) changed the title [PR Build] Use GVFS Helper transport instead of read object hook Use GVFS Helper transport instead of read object hook Oct 3, 2019

@wilbaker William Baker (wilbaker) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just some minor comments and questions, overall looking good!

Comment thread Scalar.Common/Git/GitProcess.cs Resolved
Comment thread Scripts/Mac/BuildScalarForMac.sh Resolved
Comment thread Scripts/Mac/BuildScalarForMac.sh Resolved
Comment thread Scalar/CommandLine/ScalarVerb.cs Resolved
Comment thread Scalar/CommandLine/ScalarVerb.cs Resolved
Comment thread Scalar/CommandLine/PrefetchVerb.cs Resolved
Signed-off-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
@@ -36,6 +36,7 @@ public static class GitConfig
public const string ScalarTelemetryPipe = GitConfig.ScalarPrefix + "telemetry-pipe";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wondering if we should have a comment on the "gvfs.cache-server" and "gvfs.sharedCache" stating that we intended/need to leave the config prefix as "gvfs" and that it wasn't an oversight during the conversion.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Interesting... probably not? I did a pretty thorough find-and-replace, so we should assume from now on that every "gvfs" in the codebase is intended to be there.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

that we intended/need to leave the config prefix as "gvfs"

Why do we need to use "gvfs" for the new settings? Is the long term plan to remove the read-object hook in GVFS as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's because it is related to the "gvfs" protocol (the endpoints have "gvfs" in the name).

There is a possibility that we can move VFS for Git onto the gvfs-helper, too.

@derrickstolee
Derrick Stolee (derrickstolee) merged commit a8b0b08 into microsoft:master Oct 4, 2019
Derrick Stolee (derrickstolee) added a commit that referenced this pull request Oct 10, 2019
Resolves #164.

Rename `PostFetchStep` to `CommitGraphStep` because it only writes the commit-graph files. Convert the step to use the `--reachable` option instead of `--stdin-packs`. This helps in multiple ways:

1. We don't need to communicate a list of downloaded packs.
2. It's faster to start from refs than scanning packs.
3. This will work on vanilla Git repos.
4. We can now run the commit-graph step in the background at a standard cadence instead of running it after a prefetch.

Cleans up some messages that are no longer useful, including the download-object request that was made irrelevant in #122.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove reliance on mount process from read-object hook

4 participants