Allow process-specific Git config with "-c" arguments.

[derrickstolee]

See git-ecosystem/git-credential-manager#190 for some context.

To solve the problem, we could allow users to specify config the way they could with Git, using scalar [-c config=value] <verb> and having those arguments pass down into all Git commands. The distance between those two steps is a bit tricky, but perhaps there is a way to do it simply.

The other option would be to do one-off options like scalar clone <url> --access-token "***" for this specific credential issue.

At minimum, the root problem that needs to be solved is: allow a user to specify custom credentials during scalar clone.

[mjcheetham]

All calls to git go via the GitProcess class, so if we can hook in to that from our base ScalarVerb then that should be possible. I know, sadly, that a lot of places just new-up a GitProcess class passing in the file path to git(.exe).
Perhaps a factory or context is required here? Or even just some 'global' (even though I hate that) GitOptions or Config object.

[dscho]

In any case, it might make sense to generate the GIT_CONFIG_PARAMETERS environment variable manually in that case. The format is 'var1=value' 'var2=value2' 'var3=value3' (where the single-space-separator and the single-quoting are mandatory). AFAIR only backslash and single quotes are escaped via backslash.

[derrickstolee]

My thought is that we could collect a static List<string> of the key-value pairs, and then insert them into the Git command. The following diff would suffice for ensuring every Git process used has these options:

diff --git a/Scalar.Common/Git/GitProcess.cs b/Scalar.Common/Git/GitProcess.cs
index 3a6191bb..65142c3b 100644
--- a/Scalar.Common/Git/GitProcess.cs
+++ b/Scalar.Common/Git/GitProcess.cs
@@ -38,6 +38,13 @@ public static string ExpireTimeDateString
             }
         }

+        public static void AddCustomConfig(string keyEqualsValue)
+        {
+            customConfig.Add(keyEqualsValue);
+        }
+
+        private static List<string> customConfig = new List<string>();
+
         private static string expireTimeDateString;

         private const int HResultEHANDLE = -2147024890; // 0x80070006 E_HANDLE
@@ -737,6 +744,11 @@ public Result MultiPackIndexRepack(string gitObjectDirectory, string batchSize)
                 processInfo.EnvironmentVariables["GIT_OBJECT_DIRECTORY"] = gitObjectsDirectory;
             }

+            foreach (string keyEqualsValue in customConfig)
+            {
+                command = $"-c {keyEqualsValue} {command}";
+            }
+
             if (!fetchMissingObjects)
             {
                 command = $"-c {ScalarConstants.GitConfig.UseGvfsHelper}=false {command}";

The issue I'm currently struggling with is how to get multi-valued options using the CommandLineParser library.

[dscho]

AFAIR only backslash and single quotes are escaped via backslash.

I was wrong, exclamation marks are also escaped: https://github.com/git/git/blob/v2.29.0/quote.c#L12-L41

EDIT: I was even wronger. Backslashes are indeed escaped via backslashes, but single quotes as well as exclamation marks are not only escaped, but surrounded by single-quotes (which end and re-start the single-quoting, respectively). The comment above sq_quote_buf() says it all:

/* Help to copy the thing properly quoted for the shell safety.
 * any single quote is replaced with '\'', any exclamation point
 * is replaced with '\!', and the whole thing is enclosed in a
 * single quote pair.
 *
 * E.g.
 *  original     sq_quote     result
 *  name     ==> name      ==> 'name'
 *  a b      ==> a b       ==> 'a b'
 *  a'b      ==> a'\''b    ==> 'a'\''b'
 *  a!b      ==> a'\!'b    ==> 'a'\!'b'
 */

EDIT 2: And even wronger. Backslashes are not treated specially inside single-quoted text at all.

[AtOMiCNebula]

Is there a compelling reason to use a -c-like facility over a credential provider, for getting credentials into Scalar? I'm happy to return to the store credential provider, though perhaps -c would be useful in other scenarios I haven't thought of yet.

[derrickstolee]

Is there a compelling reason to use a -c-like facility over a credential provider, for getting credentials into Scalar? I'm happy to return to the store credential provider,

You should use store if that works for you.

though perhaps -c would be useful in other scenarios I haven't thought of yet.

This is not the first time this idea has come up. It's just actually non-trivial (due to how our code is organized AND a bug in the command-line parser library we use). That's why this issue will not be fast-tracked into the release this week.

[mjcheetham]

For what it's worth, I know the System.CommandLine official command line parsing library supports multiple repeat options, like -c. I've been using it for some toy projects and so far it's pretty easy. Not that we should go about replacing everything right away of course (it's also still in a preview).

[derrickstolee]

For what it's worth, I know the System.CommandLine official command line parsing library supports multiple repeat options, like -c. I've been using it for some toy projects and so far it's pretty easy. Not that we should go about replacing everything right away of course (it's also still in a preview).

There is a bug where you can try using a multiple-repeat option, but it must be at the end, after any "positional" arguments because it tries to parse it as -c <config1> <config2> <config3> instead of -c <config> <url> <dir>. I noticed this while testing an implementation and found this unreleased PR that should fix it.

[github-actions]

Labeling this issue as stale. There has been no activity for 30 days. Remove stale label or comment or this issue will be closed in 7 days.