Claudenw commented on code in PR #433:
URL: https://github.com/apache/creadur-rat/pull/433#discussion_r2081603482
##########
apache-rat-core/src/main/java/org/apache/rat/config/exclusion/fileProcessors/GitIgnoreBuilder.java:
##########
@@ -53,6 +53,16 @@ public GitIgnoreBuilder() {
super(IGNORE_FILE, COMMENT_PREFIX, true);
}
+ @Override
+ void initializeBuilder(final DocumentName root) {
+ File globalGitIgnore = new File(globalGitIgnore());
+ if (globalGitIgnore.exists()) {
+ LevelBuilder levelBuilder = levelBuilders.computeIfAbsent(-1, k ->
new LevelBuilder());
+ DocumentName documentName =
DocumentName.builder(globalGitIgnore).build();
+ levelBuilder.add(process(levelBuilder::add, root, documentName));
Review Comment:
I think this can go away if you do the follwoing:
1. create a `processGlobalIgnore` method based on
AbstractFileProcessorBuilder.process() but calls the
ExclusionUtils.qualifyPattern() with `root` as you did in your chage. As you
ascertained this will create a matcher that will match the patters in your
global file against the root of the source tree.
2. override `process()` and call super.process() as well as
processGlobalIgnore.
3. The MatcherSet returned from processGlobalIgnore will not have the proper
names. You want something like "global GIT ignore" and it will have the name
returned from the file location. You can change the names by creating a
MatcherSet that returns renamed values from the processGlobalIgnore matcher set
by calling `new DocumentNameMatcher("the name you want",
matcherSet.includes())` and `new DocumentNameMatcher("the name you want",
matcherSet.excludes())`
4. When you create the LevelBuilder for the GlobalIgnore assign it to level
`Integer.MAX_VALUE`
If you get this all correct then the DocumentNameMatcher in your test should
return a `toString()` result that looks like
```
matcherSet(or('included dir1/.gitignore', 'included .gitignore'),
or('excluded the name you want', 'excluded dir1/.gitignore', **/.gitignore,
'excluded .gitignore'))
```
Note that the output shows you what files includes and excludes are being
processed from and their order of precedence.
Entries that start with 'included' or 'excluded' are file name processor
results. Everything else is directly entered or entered via a
`StandardCollection`.
##########
apache-rat-core/src/main/java/org/apache/rat/config/exclusion/fileProcessors/AbstractFileProcessorBuilder.java:
##########
@@ -151,7 +161,7 @@ protected MatcherSet process(final Consumer<MatcherSet>
matcherSetConsumer, fina
ExclusionUtils.asIterator(documentName.asFile(), commentFilter)
.map(entry -> modifyEntry(matcherSetConsumer, documentName,
entry).orElse(null))
.filter(Objects::nonNull)
- .map(entry -> ExclusionUtils.qualifyPattern(documentName,
entry))
+ .map(entry -> ExclusionUtils.qualifyPattern(root, entry))
Review Comment:
While the original may be incorrect for the specific case of the global
ignore, it is correct for the rest of the ignore. If root is specified then
all `.gitignore` files operate as though they were in the root of the source
tree.
Changing this will affect all exclusion builders.
The solution here is to implement a custom `process()` method in
GitIgnoreBuilder. (See comments for GitIgnoreBuilder class) Doing the custom
work in the GitIgnoreBuilder will also remove the need for an
`initializeBuilder` method.
see apache-rat-core/src/site/markdown/development/write_file_processor.md
CVSIgnoreBuilder and HGIgnoreBuilder for examples.
##########
apache-rat-core/src/test/java/org/apache/rat/config/exclusion/fileProcessors/GitIgnoreBuilderTest.java:
##########
@@ -125,13 +135,51 @@ public void test_RAT_335() {
DocumentName candidate = DocumentName.builder()
.setName("/home/claude/apache/creadur-rat/apache-rat-core/target/test-classes/GitIgnoreBuilderTest/src/dir1/file1.log")
.setBaseName("home/claude/apache/creadur-rat/apache-rat-core/target/test-classes/GitIgnoreBuilderTest/src/").build();
- System.out.println("Decomposition for "+candidate);
+ System.out.println("Decomposition for " + candidate);
Review Comment:
This is junk I missed removing when I was cleaning the debugging bits from
the code base.
If you replace this block with
```String dirName = file.toString();
DocumentName candidate = DocumentName.builder()
.setName(dirName+"/dir1/file1.log")
.setBaseName(dirName).build();
System.out.println("Decomposition for " + candidate);
matcher.decompose(candidate).forEach(System.out::println);
```
You will see the results of the matcher execution against the candidate. It
starts with the `matcherSet.toString()` and then shows you the result of each
component within the include and exclude branches. The decomposition lists
each path segment as a separate string in an array. So you can see which
patters cause the test to pass or fail.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
