Add config to allow file protocol to be used-Tests

[vbjay]

Fixes #10387 for tests. Will reopen issue and tie to second pull request.
Closes #10394

Proposed changes

• Add config to allow file protocol.

Test methodology

• Run tests and confirm tests succeed

Test environment(s)

• GIT
• Windows

Merge strategy

I agree that the maintainer squash merge this PR (if the commit message is clear).

---

✒️ I contribute this code under The Developer Certificate of Origin.

[vbjay]

Looking into

Modules: repo1 repo1/repo2
Expected is <System.String[3]>, actual is <System.Collections.Generic.List`1[System.String]> with 2 elements
Values differ at index [2]
Missing: < "repo1/repo2/repo3" >

debugged and got temp path

tree /A |clip

\---repo0
    \---repo1
        \---repo2

[vbjay]

Confirmed fixed both in test and adding a submodule in UI

Only thing left is to adjust config to allow the protocol on all submodule commands. Looks like submodule update needs it too.

[vbjay]

Last to check are the SubmoduleStatusProviderTests.

[vbjay]

Please hold up. I am doing this methodically. I just got tests green. I am now going to play with what the different configs do. I did make this Pull request WIP for a reason.

[vbjay]

What user means:
user - protocol is only able to be used when GIT_PROTOCOL_FROM_USER is either unset or has a value of 1. This policy should be used when you want a protocol to be directly usable by the user but don’t want it used by commands which execute clone/fetch/push commands without user input, e.g. recursive submodule initialization.

found at https://git-scm.com/docs/git-config#Documentation/git-config.txt-protocolallow

So user basically breaks recursive submodule update --init and add.

GetSubmodulesLocalPaths test:

git -c protocol.file.allow=user submodule add -f "C:/Users/vbjay/AppData/Local/Temp/ymhgxebg.gsp/repo3/" "repo3"

at GitCommands.ExecutableExtensions.d__9.MoveNext() in C:\git\gitextensions\GitCommands\Git\ExecutableExtensions.cs:line 326

user and never result in

[vbjay]

Ok. now ready for review.

[vbjay]

Even setting default repo config fails the submodule add. They require -c...
git submodule add -f "C:/Users/vbjay/AppData/Local/Temp/piibmzoi.jod/repo3/" "repo3"

---

While

works

[vbjay]

This work will be the basis for another pull request that will define ui popup functionality based on needed ui interaction to allow command.

@RussKie @gerhardol . Review this and let me know if you have feedback. This gets tests green and lays the foundation for a ui user allows x functionality built on the work here.

[gerhardol]

too tired to review...

[RussKie]

Can you please re-enable all tests that were disabled in fe1d5b2?

[gerhardol]

+1
Enable tests as Russkie suggested.
Looks fine, not much else to do I assume.
I would would prefer a string instead of the nullable configs argument in GitCommandHelpers.cs, no strong opinion.

[RussKie]

I'm away. I'll be able to review next week.

[vbjay]

See https://bugs.launchpad.net/ubuntu/+source/git/+bug/1993586 along with https://github.blog/2022-10-18-git-security-vulnerabilities-announced/#fn-67904-1 for details. I have tried with never, and user. Always is required and at least with git for windows even setting config in git config protocol.file.allow always fails this as shown in the AddSubmodule code found at

I left this code in to make sure that adding submodules still works down the road.

Since we are in direct control of what the repo is and what are in the submodules for the tests, we can set always to true for tests. For the program we will need to rethink adding submodules and submodule update --recursive. Git has locked it down HARD. This pull request is ONLY concerned with making tests work again.

The next pull request will be started after this is merged. That work will entail us forgoing recursive submodule work, or asking user if it trusts the repos in submodule chain and if so, we pass in the config to always. Started #10426 to discuss this

[mstv]

Thank you very much for making the tests work again and for all the testing.

[mstv]

I tend to rebase and merge this PR in order to keep the commits (although some are tiny). This merge strategy has the disadvantage that no reference to this PR is added.

[vbjay]

I can rebase and force push if you want.

[vbjay]

Should be able to just merge now. @mstv

[mstv]

I tend to rebase and merge this PR in order to keep the commits (although some are tiny). This merge strategy has the disadvantage that no reference to this PR is added.

I can rebase and force push if you want.
Should be able to just merge now. @mstv

There is no need to manually rebase. This can be done using .

We want to avoid unnecessary merge commits. I want to discuss

• whether we should keep such commits
• how to merge including a reference to the PR

[vbjay]

whether we should keep such commits

I follow the idea of atomic commits. I try to keep changes as small as possible so that reviews are easier. If you squash then it gets hairy understanding work like this. The add config is pretty big but is clear on what it is doing. So I prefer commits stay.

how to merge including a reference to the PR

I can reword last commit and add a reference to pull request if you like.

[mstv]

I can reword last commit and add a reference to pull request if you like.

Yes, please.

We can continue the generic discussion after merging.

[vbjay]

Next step is to get the core team to opine on #10426. This pull request started the plumbing on how to pass in the config when needed. The discussion is there for how do we want the program to act. I don't want to go down a rabbit hole and have to rework. Please discuss there and let's get a direction on the discussion and then from discussion I'll wire up the expected functionality in the gui.

[mstv]

Thank you