Allow skipping file paths in recursive submodule directory clean

[scottsumrall]

Fixes #11099 (#11102)

Proposed changes

• Skip certain file paths when cleaning submodules recursively

Screenshots

Before

After

Test methodology

• Added appropriate unit tests
• Verified in test repo

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.

[mstv]

just nits

[mstv]

Suggested change

	List<string> submodulePaths = (List<string>)Module.GetSubmodulesLocalPaths();
	IReadonlyList<string> submodulePaths = Module.GetSubmodulesLocalPaths();

[mstv]

❔

Suggested change

	targetPath = targetPath.Replace(Module.WorkingDir, "").ToPosixPath();
	targetPath = targetPath.ToPosixPath().Replace(Module.WorkingDir, "");

This should be tested, too.

[scottsumrall]

Tested, this doesn't work since WorkingDir is not stored as Posix path

[mstv]

Does the user need to input paths in the native form in order to match WorkingDir?
I would not make assumptions on the type of path separators in WorkingDir either.
I.e. convert both?

[RussKie]

Suggested change

	targetPath = targetPath.Replace(Module.WorkingDir, "").ToPosixPath();
	targetPath = targetPath.ToPosixPath().Replace(Module.WorkingDir.ToPosixPath(), "");

[gerhardol]

Sorry if I was not clear, but it may be clearer to not resolve a relative path here. Regex cannot be resolved anyway.
#11102 (comment)

If replaced, the exclude (and include) path should be changed for each submodule individually. The "cleaned" path could be have an unexpected effect for the other superprojects and submodules. There may be layers of submodules too.

[scottsumrall]

No worries, I uploaded this as draft to get clarification but forgot to comment. From my understanding, I don't see a clear way to avoid weird edge case conflicts without changes to #11101. The exclude parameter requires that the path be relative to the submodule.

For example if the user wants to exclude:
repo/submodule1/somefile.txt
the exclude has to be written as --exclude=somefile.txt When we iterate, this would also exclude a theoretical file:
repo/submodule2/somefile.txt

Since I don't think the include has a similar requirement, I don't necessarily think it needs to be changed for each submodule individually? I believe this is due to the exclude parameter following gitignore rules while the pathspec parameter (includes) just uses general file paths.

[gerhardol]

I believe no manipulation is easier to explain, so that is my preference.
somefile.txt will be ignored in the other repos anyway with this implementation.
include/exclude should be handled the same

[scottsumrall]

Either I didn't explain myself well or I am misunderstanding, apologies.

By no manipulation do you mean using the full path? I am saying that, from my understanding, the --exclude parameter fundamentally requires that the path be written relative to module, as in my previous example. This is because we aren't passing in a path, we are passing in a Glob pattern since the --exclude parameter follows gitignore rules. As we iterate through each submodule, the exclude pattern gets applied relative to the submodule the clean command is being run within.

[gerhardol]

By no manipulation do you mean using the full path?

(missed this, sorry)
Yes

The exclude can be a regex, then the path cannot be resolved. The paths that applies to the superproject will apply to the submodules anyway. There is also a question on how sub-submodules should be handled.

It is just easier to explain if exactly the same set is applied to all clean commands.

Regardless, the handling should be the same for include/exclude.

[pmiossec]

Regardless, the handling should be the same for include/exclude.

In git command line, the behavior is not exactly the same as @scottsumrall said.
Include accept relative or absolute paths whereas exclude follow the same regex pattern as .gitignore and so accept only relative paths.
So, if we want the same behavior, the only possibility is to transform path to relative ones for include.
I think that's something that should be done because I find it easier to read when double checking the path selected.
And that's also the formatting of output path when doing a dry run/preview.

[scottsumrall]

This gets a little messy since the code was designed using the GUI to store the file paths. I think it might be better to separate the code from the UI. Then I could just put whatever path we want displayed in the GUI and store the proper, transformed path in code. This would also get rid of some unnecessary parsing steps. It's a bit out of scope for this PR but I think it would be beneficial.

In terms of what we want to display, this would give a lot of flexibility. Since the excludes need to be relative to the module just appending the parent module might be clear enough. This would be closest to matching the command line output while still having some level of clarity for the user.

E.g.

file1.txt
folder1/folder2/file2.txt

could look like

SuperProject/file1.txt
SubmoduleX/folder1/folder2/file2.txt

Alternatively, we could display the full path from the super project:

file1.txt
folder1/folder2/file2.txt

could look like

SuperProject/file1.txt
SuperProject/SubmoduleY/SubmoduleX/folder1/folder2/file2.txt

Whatever we decide, it should be relatively easy to make the includes that are shown to the user follow the same pattern.

Its important to note though that this is only a surface level change and the edge cases of matching files of the same relative path in different submodules will still be an issue. I am unsure how to address this other than modifying #11101 to iterate through submodules in code rather than via command line. This would allow me to only apply the relevant exclude patterns for each module.