Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for using globs with --traverse #583

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

<!-- Package versions for package references across all projects -->
<ItemGroup>
<PackageVersion Include="DotNet.Glob" Version="3.1.3" />
<PackageVersion Include="libClang" Version="18.1.3.2" />
<PackageVersion Include="libClangSharp" Version="18.1.3.1" />
<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.9.0" />
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ Options:
-r, --remap <remap> A declaration name to be remapped to another name during binding generation. []
-std <std> Language standard to compile for. []
-to, --test-output <test-output> The output location to write the generated tests to. []
-t, --traverse <traverse> A file name included either directly or indirectly by -f that should be traversed during binding generation. []
-t, --traverse <traverse> A glob used to filter file names included either directly or indirectly by -f that will be traversed during binding generation. []
-v, --version Prints the current version information for the tool and its native dependencies.
-was, --with-access-specifier <with-access-specifier> An access specifier to be used with the given qualified or remapped declaration name during binding generation. Supports wildcards. []
-wa, --with-attribute <with-attribute> An attribute to be added to the given remapped declaration name during binding generation. Supports wildcards. []
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
<NoWarn>$(NoWarn);CA1303</NoWarn>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="DotNet.Glob" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\ClangSharp\ClangSharp.csproj" />
</ItemGroup>
Expand Down
46 changes: 35 additions & 11 deletions sources/ClangSharp.PInvokeGenerator/PInvokeGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using ClangSharp.CSharp;
using ClangSharp.Interop;
using ClangSharp.XML;
using DotNet.Globbing;
using static ClangSharp.Interop.CX_AttrKind;
using static ClangSharp.Interop.CXBinaryOperatorKind;
using static ClangSharp.Interop.CX_CXXAccessSpecifier;
Expand Down Expand Up @@ -64,6 +65,7 @@ public sealed partial class PInvokeGenerator : IDisposable
private readonly HashSet<string> _topLevelClassNames;
private readonly HashSet<string> _usedRemappings;
private readonly string _placeholderMacroType;
private readonly List<Glob> _traversalGlobs;

private string _filePath;
private string[] _clangCommandLineArgs;
Expand Down Expand Up @@ -163,6 +165,20 @@ public PInvokeGenerator(PInvokeGeneratorConfiguration config, Func<string, Strea
_filePath = "";
_clangCommandLineArgs = [];
_placeholderMacroType = GetPlaceholderMacroType();

_traversalGlobs = [];
if (config.TraversalNames.Count > 0)
{
var options = new GlobOptions();
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
options.Evaluation.CaseInsensitive = true;
}
foreach (var item in config.TraversalNames)
{
_traversalGlobs.Add(Glob.Parse(item, options));
}
}
}
else
{
Expand Down Expand Up @@ -4715,7 +4731,7 @@ bool IsExcludedByFile(Cursor cursor)

declLocation.GetExpansionLocation(out var expansionFile, out var expansionLine, out var expansionColumn, out _);

if ((expansionFile == file) && (expansionLine == line) && (expansionColumn == column) && _config.TraversalNames.Count != 0)
if ((expansionFile == file) && (expansionLine == line) && (expansionColumn == column) && _traversalGlobs.Count != 0)
{
// clang_getLocation is a very expensive call, so exit early if the expansion file is the same
// However, if we are not explicitly specifying traversal names, its possible the expansion location
Expand Down Expand Up @@ -4908,26 +4924,34 @@ bool IsExcludedByName(Cursor cursor, ref uint isExcludedValue)

bool IsIncludedFileOrLocation(Cursor cursor, CXFile file, CXSourceLocation location)
{
// Use case insensitive comparison on Windows
var equalityComparer = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal;

// Normalize paths to be '/' for comparison
var fileName = file.Name.ToString().NormalizePath();
var fileName = file.Name.CString;

if (_visitedFiles.Add(fileName) && _config.LogVisitedFiles)
{
AddDiagnostic(DiagnosticLevel.Info, $"Visiting {fileName}");
}

if (_config.TraversalNames.Contains(fileName, equalityComparer))
// If there is no source file, then exclude it.
if (string.IsNullOrEmpty(fileName))
{
return true;
return false;
}
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

{
return true;
if (_traversalGlobs.Any(glob => glob.IsMatch(fileName)))
{
return true;
}
var fullPath = Path.GetFullPath(fileName);
if (_traversalGlobs.Any(glob => glob.IsMatch(fullPath)))
{
return true;
}
}
else if (_config.TraversalNames.Count == 0 && location.IsFromMainFile)
// Otherwise, only include items from the main file.
else if (location.IsFromMainFile)
{
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion sources/ClangSharpPInvokeGenerator/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,7 @@ private static Option<string[]> GetTraverseOption()
{
return new Option<string[]>(
aliases: s_traverseOptionAliases,
description: "A file name included either directly or indirectly by -f that should be traversed during binding generation.",
description: "A glob used to filter file names included either directly or indirectly by -f that will be traversed during binding generation.",
getDefaultValue: Array.Empty<string>
) {
AllowMultipleArgumentsPerToken = true
Expand Down
36 changes: 36 additions & 0 deletions tests/ClangSharp.PInvokeGenerator.UnitTests/OptionsTest.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright (c) .NET Foundation and Contributors. All Rights Reserved. Licensed under the MIT License (MIT). See License.md in the repository root for more information.

using System;
using System.Collections.Generic;
using System.IO;
using System.Threading.Tasks;
using NUnit.Framework;

Expand Down Expand Up @@ -91,4 +93,38 @@ public partial struct StructD

return ValidateGeneratedCSharpLatestWindowsBindingsAsync(inputContents, expectedOutputContents, withAttributes: withAttributes);
}

[Test]
public async Task TraversalTestsAsync()
{
var inputContents = "struct StructA {};";
var expectedOutputContents =
@"namespace ClangSharp.Test
{
public partial struct StructA
{
}
}
";

#pragma warning disable CA2007 // Consider calling ConfigureAwait on the awaited task
// Match everything.
await ValidateGeneratedCSharpLatestWindowsBindingsAsync(inputContents, expectedOutputContents, withTraversalNames: ["**/*"]);

// Non-matching, then matching pattern.
await ValidateGeneratedCSharpLatestWindowsBindingsAsync(inputContents, expectedOutputContents, withTraversalNames: ["nomatch", "*.h"]);

// Windows-style path separators.
await ValidateGeneratedCSharpLatestWindowsBindingsAsync(inputContents, expectedOutputContents, withTraversalNames: ["**\\*"]);

// Full path, with no wildcards.
await ValidateGeneratedCSharpLatestWindowsBindingsAsync(inputContents, expectedOutputContents, withTraversalNames: [Path.GetFullPath(DefaultInputFileName)]);

// Full path to parent, with file wildcard.
await ValidateGeneratedCSharpLatestWindowsBindingsAsync(inputContents, expectedOutputContents, withTraversalNames: [Environment.CurrentDirectory + "/*"]);

// Doesn't match
await ValidateGeneratedCSharpLatestWindowsBindingsAsync(inputContents, string.Empty, withTraversalNames: ["nomatch", "*.cpp"]);
#pragma warning restore CA2007
}
}
Loading
Loading