Filter support added by ccp-chargeback · Pull Request #16 · carbonengine/resources

ccp-chargeback · 2026-01-31T15:20:09Z

Allow filtering of included resource files based on rules defined in one or more supplied filter.ini files.
Same changes as in PR/14 (done to try to trigger TC build on new project after latest changes from template project were added)

The change contains:

Reading and parsing of .ini filter files.
Allow overrides of include/exclude rules
Support wildcard filters:
- "*" = current folder
- "..." = any subfolder
Define priorities for include/exclude filter:
- Based on folder/file hierarcy
- Include/Exclude sameName file based on its hierarcy in each
Add CLI integration
Add loads of test coverage

Make use of existing filePath test functions

Next step, add missing tests.

Also fixed clang-tidy suggestions

- Do some function name changes to reduce stuttering.

- Adding 2 more tests as well.

- One for .ini file that does not exist - Another for .ini file with wrong format

CCPCookies

I love how many tests there are and the descriptions are super useful.

Some more comments.

I also think we should take advantage of your knowledge of the filter files and make add a file spec to the documentaiton for the filter ini files.

Here you can see resourceGroup file format https://carbonengine.github.io/resources/DesignDocuments/resourceGroupFileFormat.html

cli/src/CreateResourceGroupCliOperation.cpp

src/ResourceGroupImpl.cpp

CCPCookies · 2026-02-10T13:41:21Z

tests/src/CliTestFixture.cpp

+	std::cout << "---------------------------" << std::endl;
+
+	TinyProcessLib::Process process1a(
+		arguments, workingDirectory,


Why do you need to change the working directory?

Because the working directory is in the cmake made location for the generated exe where the tests are running.
I need the CLI to know what the working directory is in order to be able to resolve the relative contents of the .ini filter files correctly (as the files it wants to apply filter rules on are all relative to the TEST_DATA_BASE_PATH).
If I don't do that, then when it tries to resolve the relative paths from the .ini files it does so from the WRONG start position (i.e. not the TEST_DATA_BASE_PATH, but from the folder where the resources-cli.exe is located in CARBON_RESOURCES_CLI_EXE_FULLPATH)

This is also one of the reasons why I want to do the comparison on actual absolute lexical paths (see my earlier comment on: #16 (comment))

Wouldn't this base path be better passed in as an argument rather than relying on working directory?

Base path is being passed in now as an argument via the CLI (--filter-file-basepath)
The signature for the RunCLI() function takes in an optional parameter to set the working directory.
None of the tests however call it with an overriding working directory parameter.
All of them just use the default ("" empty string).

tests/src/ResourcesCliTest.cpp

CCPCookies · 2026-02-10T13:51:14Z

tests/src/ResourcesCliTest.cpp

+	arguments.push_back( "--output-file" );
+	arguments.push_back( outputFilePath.lexically_normal().string() );
+
+	int res = RunCli( arguments, output, errorOutput, TEST_DATA_BASE_PATH );


The working directory doesn't need to be changed, there is a helper to resolve paths that the other tests use.

e.g.
std::filesystem::path inputDirectory = GetTestFileFileAbsolutePath( "CreateResourceFiles/ResourceFiles" );

See my other answers on the issue.

Working directory is now not being passed in as a parameter to the RunCli() function.
The filter file basepath can however be set as a command line argument "--filter-file-basepath" which gets passed down as an argument to the tools library.

tools/src/FilterNamedSection.cpp

tools/src/FilterResourceFile.cpp

tools/src/ResourceFilter.cpp

CMakeLists.txt

- Updated tests accordingly.

…hen needed

…of optional)

…er function.

…lResolvedPathMap() function.

- Change the FilePathMatchesIncludeFilterRules() function to use relative paths for comparison on the incoming file vs the resolved path from the filter .ini file. - Make sure that any path sent into the function or resolved from a .ini file are also force converted to their relative version, (from std::filesystem::current_path)

- Using relative paths for comparison can fail on Windows for a valid prefixmap entry of e.g. "rootRes:." where it has trouble interpreting e.g. a path of "./SomeFolder/*"

- Make sure to reset inFilePathAbs and resolvedPathAbs path variables to their normalized versions

- This should work for tests running on Windows ("gold" file comparison) - Hope is that the macOS "gold" files will work as well (else re-run and generate those on a macOS machine and submit those)

- It is no longer needed after reliance on current working directory was removed.

…into filter-changes Merge in correct macOS output files from upstream, and add a test for the new filter-changes functionality.

# Conflicts: # cli/src/CreateResourceGroupCliOperation.cpp # src/ResourceGroupImpl.cpp

CCPCookies

I left some comments but didn't finish completely.

I tried a build and was confused why my ini file was not being found. I pulled the code down to get a better idea. There are some issues with path handling still, confusion with relative/absolute path things.

I've managed to make a few local changes to sort most of it, I'll continue when I'm back on Thurs and push to this branch.

CCPCookies · 2026-02-16T16:39:55Z

cli/src/CreateResourceGroupCliOperation.cpp


 	AddArgument( m_createResourceGroupExportResourcesDestinationPathId, "Represents the base path where the exported resources will be saved. Requires --export-resources", false, false, defaultImportParams.exportResourcesDestinationSettings.basePath.string() );
+
+	AddArgument( m_createResourceGroupIniFilterFilesArgumentId, "Path to INI file(s) for resource filtering.", false, true, "" );


Should remove the (s) as it suggests this accepts a list.
The argument only accepts one path.

The append bool here being true results in docs added so user knows many can be supplied.
Path to INI file for resource filtering. [Accepts multiple] [default: ""]

CCPCookies · 2026-02-16T16:40:26Z

cli/src/CreateResourceGroupCliOperation.cpp

+
+	AddArgument( m_createResourceGroupIniFilterFilesArgumentId, "Path to INI file(s) for resource filtering.", false, true, "" );
+
+	AddArgument( m_createResourceGroupIniFilterFilesBasePathArgumentId, "Base directory for resolving relative paths contained within filter INI file(s).", false, false, "" );


(s) fine here

CCPCookies · 2026-02-16T16:46:55Z

customized_toolchain/toolchains/arm64-osx-carbon.cmake


    set (VCPKG_USE_HOST_TOOLS ON CACHE STRING "")
-    set (CMAKE_CXX_STANDARD 20 CACHE STRING "")
+    set (CMAKE_CXX_STANDARD 17 CACHE STRING "")


What's the story here?

CCPCookies · 2026-02-16T16:51:34Z

src/ResourceGroupImpl.cpp

+						// Check if the file, i.e. entry.path() should be included or excluded based on filtering rules
+						if( !resourceFilter.FilePathMatchesIncludeFilterRules( entry.path() ) )
+						{
+							continue;


Could add in a status update here.

fileProcessingInnerStatusSettings.Update( CarbonResources::StatusProgressType::UNBOUNDED, 0, 0, "Skipping file as it doesn't match filters: " + entry.path().string() );

Probably be very useful later.

Also, if all files are skipped then it will be a long time until any status updates were hit, might look like it's hanging.

CCPCookies · 2026-02-16T16:57:10Z

tests/src/CliTestFixture.cpp

+int CliTestFixture::RunCli( std::vector<std::string>& arguments,
+							std::string* standardOutput /* = nullptr */,
+							std::string* errorOutput /* = nullptr */,
+							const std::string& workingDirectory /* = "" (empty = do not alter it) */ )


working directory change here needs removing.
This way we stop others maybe creating changes that rely on working directory

CCPCookies · 2026-02-16T16:58:38Z

tests/src/CliTestFixture.cpp

+	// Only populate the output and errorOutput if the caller provided non-nullptr for them, otherwise discard them
+	TinyProcessLib::Process process1a(
+		arguments,
+		workingDirectory,


workingDirectory doesn't need setting same reasoning as the previous one

CCPCookies · 2026-02-16T16:59:20Z

tests/src/CliTestFixture.h

 {

-	int RunCli( std::vector<std::string>& arguments, std::string& output );
+	int RunCli( std::vector<std::string>& arguments, std::string* standardOutput = nullptr, std::string* errorOutput = nullptr, const std::string& workingDirectory = "" );


working directory setting removal

CCPCookies · 2026-02-16T17:03:12Z

tests/src/ResourcesCliTest.cpp

+	std::string output;
+	std::string errorOutput;
+	std::vector<std::string> arguments;
+	std::filesystem::path inputDirectoryPath = GetTestFileFileAbsolutePath( "" ); // The base testData directory


Will this give TEST_DATA_BASE_PATH?
Could use that instead

CCPCookies · 2026-02-16T21:21:24Z

cli/src/CreateResourceGroupCliOperation.cpp

-	m_createResourceGroupExportResourcesDestinationPathId( "--export-resources-destination-path" )
+	m_createResourceGroupExportResourcesDestinationPathId( "--export-resources-destination-path" ),
+	m_createResourceGroupIniFilterFilesArgumentId( "--filter-file" ),
+	m_createResourceGroupIniFilterFilesBasePathArgumentId( "--filter-file-basepath" )


The argument name is misleading. This isn't the base path of the filter file.
This is the base path from where the prefix maps are based

CCPCookies · 2026-02-16T21:22:32Z

src/ResourceGroupImpl.cpp

+	{
+		try
+		{
+			resourceFilter.Initialize( params.resourceFilterIniFiles, params.resourceFilterIniFilesBaseDirectory, params.directory );


Why would the filter file require the directory passed in?

I think there is still some confusion with paths happening here

ccp-chargeback added 30 commits December 1, 2025 14:42

Add example1.ini file (for tests)

fbb3830

Add vcpkg inih to project

31479c2

Add first ResourceFilterTest

9ce31fa

Update ExampleIniParsing test

4a0a0d8

Make use of existing filePath test functions

Rename the ResourceFilterTest class

dbbfb83

Add scaffolding for the Filter classes

fd26db0

Include exclude filter implementation without regex

40ec602

Simplify FilterResourceFilter::ParseFilters()

ede9636

Add error tests for FilterResourceFilter::ParseFilters()

b79fe7e

Add tests for Condensed but valid filter strings

a2bd883

Add Doxygen markdown comments to FilterResourceFilter class.

1a406b3

Change scaffolding of FilterResourceFilter

c65e3c4

Move Filter logic into tools

5d151d0

Make filter tool classes part of ResourceTools namespace

c8b8bcd

Fix Filter tests to now use new ResourceTools namespace

5fed2b9

Add test for multiline respaths with empty lines

369e3c8

Refactor of ResourceLing and PrefixMap

f9d4965

Add FilterPrefixMap and Entry classes

8a68f53

Next step, add missing tests.

Add tests for FilterPrefixMap and Entry

e90027b

Include and function signature cleanup

cd967c2

Add tests for class FilterDefaultSection

a84ebd9

Fixup NamedSection and related classes.

12e4d74

Add test for Duplicate Filter in Overrides

3ce25ae

Change m_parentPrefixMap to a const reference

f96197d

Also fixed clang-tidy suggestions

Change m_parentSectionFilter to a const reference

698874b

Remove un-necessary default constructors

74de122

Rename and gorup Resource Filter tests by class tested

022b0cf

Add FilterNamedSection tests

7b31e28

Add FilterResourceFile class

073a220

- Do some function name changes to reduce stuttering.

Add test for FilterResourceFile class (example1.ini)

c885982

ccp-chargeback added 8 commits February 6, 2026 15:26

Cleanup, simplify and comment the FilterResourceFilter class

e365e6b

Cleanup, simplify and comment the FilterResourceFilter class (take 2)

6660d02

Cleanup, simplify and comment the FilterResourcePathFileEntry class

15871dd

Cleanup, simplify and comment the FilterResourcePathFile class

31cf40c

- Adding 2 more tests as well.

Cleanup, simplify and comment the FilterResourceFile class

33cd209

Rename ShouldInclude() to FilePathMatchesIncludeFilterRules()

d4f0a3e

Rename constants in WildcardMatch() as per coding guidelines.

4ac6ebe

Cleanup, simplify and comment the ResourceFilter class

9836626

ccp-chargeback marked this pull request as ready for review February 8, 2026 21:39

ccp-chargeback added 2 commits February 10, 2026 13:48

Use .empty() instead of .size() where applicable

b81076b

Add two CLI tests for error .ini files

399b860

- One for .ini file that does not exist - Another for .ini file with wrong format

CCPCookies reviewed Feb 10, 2026

View reviewed changes

ccp-chargeback and others added 12 commits February 11, 2026 11:12

Remove sending error message to std::cerr in ResourceGroupImpl

5d1efb7

- Updated tests accordingly.

Change RunCli() so that standard and errorOut are only populated if/w…

d369fe1

…hen needed

Change GetResolvedResfileMap() to return a pointer to a map (because …

b26ad54

…of optional)

Optimize dealing with CLI filter files (as per code review suggestion)

93b27fa

Change how NamedSection is populated and its getter function.

4ed3794

Change population of FilterResourceFile members and signature of gett…

828ae10

…er function.

Change initialization of ResourceFilter class and signature of GetFul…

51ecead

…lResolvedPathMap() function.

Reverting back to using absolute paths for comparison

9e6c75f

- Using relative paths for comparison can fail on Windows for a valid prefixmap entry of e.g. "rootRes:." where it has trouble interpreting e.g. a path of "./SomeFolder/*"

Fully reverting back to original absolute paths for comparison

7254135

- Make sure to reset inFilePathAbs and resolvedPathAbs path variables to their normalized versions

Adding CLI filter tests for old-style document version (0.0.0)

6e6ee17

- This should work for tests running on Windows ("gold" file comparison) - Hope is that the macOS "gold" files will work as well (else re-run and generate those on a macOS machine and submit those)

Update macOS gold comparison files for create-group with txt output

18bfb5a

ccp-chargeback requested a review from CCP-Aporia February 13, 2026 11:33

ccp-chargeback added 4 commits February 16, 2026 13:34

Remove reliance on the WorkingDirectory when filter files from the CLI

2daee24

Remove the CurrentWorkingDirectoryChanger helper class.

4ffe5eb

- It is no longer needed after reliance on current working directory was removed.

Merge branch 'filter-changes' of github.com:ccp-chargeback/resources …

7c014c1

…into filter-changes Merge in correct macOS output files from upstream, and add a test for the new filter-changes functionality.

Merge branch 'main' into filter-changes

7345745

# Conflicts: # cli/src/CreateResourceGroupCliOperation.cpp # src/ResourceGroupImpl.cpp

CCPCookies reviewed Feb 16, 2026

View reviewed changes


		AddArgument( m_createResourceGroupExportResourcesDestinationPathId, "Represents the base path where the exported resources will be saved. Requires --export-resources", false, false, defaultImportParams.exportResourcesDestinationSettings.basePath.string() );

		AddArgument( m_createResourceGroupIniFilterFilesArgumentId, "Path to INI file(s) for resource filtering.", false, true, "" );

Conversation

ccp-chargeback commented Jan 31, 2026

Uh oh!

CCPCookies left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CCPCookies left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants