Skip to content

Conversation

dpaoliello
Copy link
Contributor

This adds support for using globs with the --traverse option, via the DotNet.Glob library.

I initially attempted to use the Microsoft.Extension.FileSystemGlobbing library, however it doesn't support globs that are absolute paths (see dotnet/runtime#62333), hence that would break any existing use of --traverse that was using an absolute path.

else if (_config.TraversalNames.Contains(fileName.NormalizeFullPath(), equalityComparer))

// If the user has specified a set of headers to include, then use that.
if (_traversalGlobs.Count > 0)
Copy link
Member

Choose a reason for hiding this comment

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

I'm not really sold on the globbing support here, particularly given how other parameters and matching is being done. There's kind of 3 parts to it here...

  1. ClangSharp itself is being signed and used by tooling where the full trust/validation chain is important. Dotnet.Glob hasn't been touched in 4ish years now, while there are several newer issues opened; so its not clear if it's a good long term thing to depend on for a project like this

  2. It's somewhat intentional that traversal be more explicit to help with determinism, various file system issues (like reparse points or symbolic links) and to avoid accidentally bringing in "everything" which causes conflicts across multiple invocations of the tool. There's also some complexities in how Clang itself surfaces file paths that make it a bit error prone and doesn't always work with file system APIs.

  3. All the other command line parameters are using * very simplistically or with Regex, so this creates a mismatch with the other handling. I imagine the basic support that's needed here could itself be added relatively simply or via an alternative command line option; such as if you want to include all files under a given folder then a --traverseDirectory might be a better option

@dpaoliello
Copy link
Contributor Author

Abandoning: looks like I can do what I need to without having to add glob support directly to ClangSharp.

@dpaoliello dpaoliello closed this Jan 14, 2025
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