Silencing 404s on gvfs/config

[abdurraheemali]

Meant to address #384. Rebase of #504. Work in progress.

---

Thanks for taking the time to contribute to Git!

This fork contains changes specific to monorepo scenarios. If you are an
external contributor, then please detail your reason for submitting to
this fork:

• This is an early version of work already under review upstream.
• [ X] This change only applies to interactions with Azure DevOps and the
  GVFS Protocol.
• This change only applies to the virtualization hook and VFS for Git.

[abdurraheemali]

To do:

identify the call chain where gvfs/config is accessed in scalar clone, and suppress the warning only in that scenario, and keep complaining about 404s in all other instances

So what we need to do is to start a git clone in gdb, set a breakpoint on the code producing the error message, then look at the backtrace (command: bt) to identify the call chain, then add a flag to silence this warning, if called via the clone call chain.

When will I do these? I dunno, both my previous predictions proved too optimistic, so the rational move is to adjust the estimate upwards, but it feels like it shouldn't take too long if I just sit down and do it.

[dscho]

With this diff:

diff --git a/gvfs-helper.c b/gvfs-helper.c
index e8e6fbbdd9f3..bddc99174b4c 100644
--- a/gvfs-helper.c
+++ b/gvfs-helper.c
@@ -3603,7 +3603,9 @@ static enum gh__error_code do_sub_cmd__config(int argc, const char **argv)
 	if (ec == GH__ERROR_CODE__OK)
 		printf("%s\n", config_data.buf);
 	else
+{ open_in_gdb();
 		error("config: %s", status.error_message.buf);
+}
 
 	gh__response_status__release(&status);
 	strbuf_release(&config_data);
@@ -3632,7 +3634,9 @@ static enum gh__error_code do_sub_cmd__endpoint(int argc, const char **argv)
 	if (ec == GH__ERROR_CODE__OK)
 		printf("%s\n", data.buf);
 	else
+{ open_in_gdb();
 		error("config: %s", status.error_message.buf);
+}
 
 	gh__response_status__release(&status);
 	strbuf_release(&data);

I built a local version, then started GIT_TRACE=1 bin-wrappers/scalar clone <some-repository-on-GitHub>, waited until the gdb window popped open, called thread 1 and then bt and got this backtrace:

#0  0x00007ffe89ec4534 in ntdll!ZwDelayExecution ()
   from C:\Windows\SYSTEM32\ntdll.dll
#1  0x00007ffe89e7bb23 in ntdll!RtlDelayExecution ()
   from C:\Windows\SYSTEM32\ntdll.dll
#2  0x00007ffe876cb361 in SleepEx () from C:\Windows\System32\KernelBase.dll
#3  0x00007ff7b8813727 in sleep (seconds=1) at compat/mingw.c:1279
#4  0x00007ff7b881045c in open_in_gdb () at compat/mingw.c:33
#5  0x00007ff7b8726efe in do_sub_cmd__endpoint (argc=2, argv=0x229996506b8)
    at gvfs-helper.c:3637
#6  0x00007ff7b8728133 in do_sub_cmd (argc=2, argv=0x229996506b8)
    at gvfs-helper.c:4139
#7  0x00007ff7b87282ce in cmd_main (argc=2, argv=0x229996506b8)
    at gvfs-helper.c:4213
#8  0x00007ff7b872ed89 in main (argc=5, argv=0x229996506b8)
    at common-main.c:56

I then walked the call stack by issuing up until I was in the do_sub_cmd__endpoint() function and with p argv[1] found out that it was vsts/info.

Then, I let the program resume by calling q, waited until the second gdb window popped up, and this time got this backtrace:

(gdb) bt
#0  0x00007ffe89ec4534 in ntdll!ZwDelayExecution ()
   from C:\Windows\SYSTEM32\ntdll.dll
#1  0x00007ffe89e7bb23 in ntdll!RtlDelayExecution ()
   from C:\Windows\SYSTEM32\ntdll.dll
#2  0x00007ffe876cb361 in SleepEx () from C:\Windows\System32\KernelBase.dll
#3  0x00007ff7b8813727 in sleep (seconds=1) at compat/mingw.c:1279
#4  0x00007ff7b881045c in open_in_gdb () at compat/mingw.c:33
#5  0x00007ff7b8726dd7 in do_sub_cmd__config (argc=1, argv=0x1b21a760688)
    at gvfs-helper.c:3606
#6  0x00007ff7b8728106 in do_sub_cmd (argc=1, argv=0x1b21a760688)
    at gvfs-helper.c:4136
#7  0x00007ff7b87282ce in cmd_main (argc=1, argv=0x1b21a760688)
    at gvfs-helper.c:4213
#8  0x00007ff7b872ed89 in main (argc=4, argv=0x1b21a760688)
    at common-main.c:56

With the config call, it strikes me as natural to simply ignore when it comes back with a 404, but for the generic endpoint, I would only ignore a 404 silently if the endpoint is vsts/info. In other words, I would use a diff like this one:

diff --git a/gvfs-helper.c b/gvfs-helper.c
index e8e6fbbdd9f3..89c18dbfe6af 100644
--- a/gvfs-helper.c
+++ b/gvfs-helper.c
@@ -3602,7 +3602,7 @@ static enum gh__error_code do_sub_cmd__config(int argc, const char **argv)
 
 	if (ec == GH__ERROR_CODE__OK)
 		printf("%s\n", config_data.buf);
-	else
+	else if (ec != GH__ERROR_CODE__HTTP_404)
 		error("config: %s", status.error_message.buf);
 
 	gh__response_status__release(&status);
@@ -3631,7 +3631,7 @@ static enum gh__error_code do_sub_cmd__endpoint(int argc, const char **argv)
 
 	if (ec == GH__ERROR_CODE__OK)
 		printf("%s\n", data.buf);
-	else
+	else if (strcmp(endpoint, "vsts/info") || ec != GH__ERROR_CODE__HTTP_404)
 		error("config: %s", status.error_message.buf);
 
 	gh__response_status__release(&status);

[abdurraheemali]

Thanks, let me try to parse and reproduce these steps...

[abdurraheemali]

Was busy all week last week, and tomorrow will be busy too, but I am planning to push a new commit day after tomorrow

[dscho]

Was busy all week last week, and tomorrow will be busy too, but I am planning to push a new commit day after tomorrow

Okay. Please note that I would like you to continue working in this PR branch and continue to target vfs-2.37.0 until the patch is final, even if I pushed vfs-2.37.1 in the meantime. We need to focus on the patch, and the rebase mechanics can come later (I can do it for you, or guide you through it, but that's just the cherry we'll put on top of the cake in the end, we need to bake the cake first).

[derrickstolee]

This is no longer necessary. scalar clone will default to not use gvfs/config unless the URL looks like Azure DevOps. This can be overridden by --gvfs-protocol (to enable it on other URLs) or --no-gvfs-protocol (to disable it and fall back to Git's partial clone on an ADO URL). See #648 for details.