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.

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

remote-https: do not call fetch-pack if using gvfs helper #240

Merged

Conversation

derrickstolee
Copy link
Collaborator

@derrickstolee derrickstolee commented Feb 3, 2020

The gvfs-helper is supposed to avoid calling git fetch-pack by downloading objects through the GVFS protocol instead. For some reason, some git fetch calls still end up calling git fetch-pack which gets a complaint from the remote because it does not support that kind of fetch.

Put a hard stop in the fetch_git() method to prevent this process run.

@derrickstolee derrickstolee changed the title connected: do not fetch a pack if using gvfs-helper remote-https: do not call fetch-pack if using gvfs helper Feb 3, 2020
When using the GVFS protocol, we should _never_ call "git fetch-pack"
to attempt downloading a pack-file via the regular Git protocol. It
appears that the mechanism that prevented this in the VFS for Git
world is due to the read-object hook populating the commits at the
new ref tips in a different way than the gvfs-helper does.

By acting as if the fetch-pack succeeds here in remote-curl, we
prevent a failed fetch.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
@jeffhostetler
Copy link

I'm concerned I don't understand the full context here. What's the full process stack here?

Is gvfs-helper itself invoking fetch-pack and/or remote-https ? Such as a hidden implicit reference to a missing object (something it, itself tripped over) -- and not a named/requested object (that the gvfs-helper-client asked for) ??

Or are we seeing a top-level git fetch stumbling into fetch-pack and/or remote-https/curl (and maybe to the cache-server rather than the origin server). If so, is the problem that gvfs-helper wasn't involved when something things it should be?? Is it the case that the fetch is requesting something that doesn't fall into the missing-object category? (Sorry if I'm just asking dumb questions here, but I haven't quite figured out what's going on.)

@@ -1046,6 +1046,9 @@ static int fetch_git(struct discovery *heads,
struct argv_array args = ARGV_ARRAY_INIT;
struct strbuf rpc_result = STRBUF_INIT;

if (core_use_gvfs_helper)
Copy link

@jeffhostetler jeffhostetler Feb 4, 2020

Choose a reason for hiding this comment

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

I'm concerned this will effectively break remote-curl aka remote-https any time that gvfs-helper is configured (and without regard for whether it is in use by the current process ancestry).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As far as I know, when using gvfs-helper we should avoid fetch-pack at all times. We never want to ask for a pack in the traditional way. Am I wrong?

The only thing I can think of is if we are fetching a submodule, but we do not support that case right now.

@derrickstolee
Copy link
Collaborator Author

I'm concerned I don't understand the full context here. What's the full process stack here?

Is gvfs-helper itself invoking fetch-pack and/or remote-https ? Such as a hidden implicit reference to a missing object (something it, itself tripped over) -- and not a named/requested object (that the gvfs-helper-client asked for) ??

No. gvfs-helper is not triggering this part of the stack.

Or are we seeing a top-level git fetch stumbling into fetch-pack and/or remote-https/curl (and maybe to the cache-server rather than the origin server). If so, is the problem that gvfs-helper wasn't involved when something things it should be?? Is it the case that the fetch is requesting something that doesn't fall into the missing-object category? (Sorry if I'm just asking dumb questions here, but I haven't quite figured out what's going on.)

I cannot reproduce this on my machine, but the way we are reproducing it is the following:

  1. From another machine, update a branch with new commits. (They must not be in an existing prefetch pack.)
  2. From the problematic machine, run git fetch origin. This gets a failure because the remote rejects the upload-pack request.

From what I can tell, the old read-object hook way of thinking was avoiding this fetch-pack request because it would download the missing commits immediately upon asking "do we have this object?" For some reason, the gvfs-helper machinery is answering "no" to that question, and that triggers the fetch-pack call. It seems to be due to the fact that the remote-https process is doing the ref update and we may have turned off gvfs-helper requests from that layer. The end result works, as we download the commits while updating the refs in the fetch process.

@derrickstolee
Copy link
Collaborator Author

/azp run microsoft.git

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@derrickstolee
Copy link
Collaborator Author

I'm concerned I don't understand the full context here. What's the full process stack here?

@jeffhostetler: I didn't fully explain the stack in my earlier message. I sent you a trace2 log via email. Here is the process stack:

  1. git fetch origin
  2. git remote-https origin <url>
  3. git-remote-https origin <url> (run dashed version)
  4. git fetch-pack --stateless-rpc --stdin --lock-pack --include-tag --thin <url>

This PR prevents process 3 from calling process 4 by intercepting the construction of the command-line arguments to git fetch-pack.

@derrickstolee derrickstolee merged commit 87453bb into microsoft:vfs-2.25.0 Feb 4, 2020
derrickstolee added a commit to derrickstolee/scalar that referenced this pull request Feb 5, 2020
See microsoft/git#240 and microsoft/git#243.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
derrickstolee added a commit to microsoft/scalar that referenced this pull request Feb 5, 2020
…abled fetch-pack and block 'git update'

See microsoft/git#240 and microsoft/git#243.

Because of #320, we do not need to update the minimum Git version from `v2.25.0.vfs.1.1` even though we are packaging with `v2.25.0.vfs.1.3` now.
derrickstolee added a commit that referenced this pull request Feb 21, 2020
…g gvfs helper

The `gvfs-helper` is supposed to avoid calling `git fetch-pack` by downloading objects through the GVFS protocol instead. For some reason, some `git fetch` calls still end up calling `git fetch-pack` which gets a complaint from the remote because it does not support that kind of fetch.

Put a hard stop in the `fetch_git()` method to prevent this process run.
derrickstolee added a commit that referenced this pull request Mar 17, 2020
…g gvfs helper

The `gvfs-helper` is supposed to avoid calling `git fetch-pack` by downloading objects through the GVFS protocol instead. For some reason, some `git fetch` calls still end up calling `git fetch-pack` which gets a complaint from the remote because it does not support that kind of fetch.

Put a hard stop in the `fetch_git()` method to prevent this process run.
derrickstolee added a commit that referenced this pull request Mar 23, 2020
…g gvfs helper

The `gvfs-helper` is supposed to avoid calling `git fetch-pack` by downloading objects through the GVFS protocol instead. For some reason, some `git fetch` calls still end up calling `git fetch-pack` which gets a complaint from the remote because it does not support that kind of fetch.

Put a hard stop in the `fetch_git()` method to prevent this process run.
dscho pushed a commit that referenced this pull request May 20, 2020
…g gvfs helper

The `gvfs-helper` is supposed to avoid calling `git fetch-pack` by downloading objects through the GVFS protocol instead. For some reason, some `git fetch` calls still end up calling `git fetch-pack` which gets a complaint from the remote because it does not support that kind of fetch.

Put a hard stop in the `fetch_git()` method to prevent this process run.
dscho pushed a commit that referenced this pull request May 20, 2020
…g gvfs helper

The `gvfs-helper` is supposed to avoid calling `git fetch-pack` by downloading objects through the GVFS protocol instead. For some reason, some `git fetch` calls still end up calling `git fetch-pack` which gets a complaint from the remote because it does not support that kind of fetch.

Put a hard stop in the `fetch_git()` method to prevent this process run.
derrickstolee added a commit that referenced this pull request Jun 1, 2020
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
derrickstolee added a commit that referenced this pull request Jul 20, 2020
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
vdye pushed a commit that referenced this pull request Jul 19, 2023
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Aug 8, 2023
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Aug 8, 2023
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Aug 11, 2023
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
jeffhostetler pushed a commit that referenced this pull request Aug 23, 2023
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Nov 3, 2023
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Nov 3, 2023
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Nov 3, 2023
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Nov 8, 2023
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Nov 14, 2023
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Nov 20, 2023
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
vdye pushed a commit that referenced this pull request Feb 27, 2024
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Apr 23, 2024
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Apr 23, 2024
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Apr 23, 2024
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Apr 24, 2024
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Apr 29, 2024
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request May 14, 2024
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request May 14, 2024
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Jun 3, 2024
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Jul 17, 2024
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Jul 17, 2024
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Jul 17, 2024
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Jul 18, 2024
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
mjcheetham pushed a commit that referenced this pull request Jul 23, 2024
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Jul 25, 2024
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
mjcheetham pushed a commit that referenced this pull request Jul 29, 2024
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Sep 18, 2024
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Sep 24, 2024
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
dscho pushed a commit that referenced this pull request Oct 8, 2024
Includes these pull requests:

 #227
 #228
 #229
 #231
 #240

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants