raboof commented on code in PR #433:
URL: https://github.com/apache/creadur-rat/pull/433#discussion_r2050739344


##########
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:
   for the global gitignore, we can pass the root of the project instead



##########
apache-rat-core/src/test/java/org/apache/rat/config/exclusion/fileProcessors/GitIgnoreBuilderTest.java:
##########
@@ -125,13 +135,50 @@ 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);
 
-        assertThat(matcher.toString()).isEqualTo("matcherSet(or('included 
dir1/.gitignore', 'included .gitignore'), or('excluded dir1/.gitignore', 
**/.gitignore, 'excluded .gitignore'))");
+        assertThat(matcher.toString()).isEqualTo("matcherSet(or('included 
.gitignore', 'included .gitignore'), or('excluded .gitignore', **/.gitignore, 
'excluded .gitignore'))");
 
         List<String> notMatching = Arrays.asList("README.txt", "dir1/dir1.md", 
"dir2/dir2.txt", "dir3/file3.log", "dir1/file1.log");
 
-        List<String> matching = Arrays.asList(".gitignore", "root.md", 
"dir1/.gitignore", "dir1/dir1.txt",  "dir2/dir2.md", "dir3/dir3.log");
+        List<String> matching = Arrays.asList(".gitignore", "root.md", 
"dir1/.gitignore", "dir1/dir1.txt", "dir2/dir2.md", "dir3/dir3.log");
+
+        assertCorrect(matcherSets, documentName.getBaseDocumentName(), 
matching, notMatching);
+    }
+
+    /**
+     * Test that exclusions from a global gitignore are also applied
+     *
+     * https://issues.apache.org/jira/browse/RAT-473
+     */
+    @Test
+    public void test_global_gitignore() {
+        URL globalGitIgnoreUrl = 
GitIgnoreBuilderTest.class.getClassLoader().getResource("GitIgnoreBuilderTest/global-gitignore");
+        String globalGitIgnore = globalGitIgnoreUrl.getFile();
+
+        GitIgnoreBuilder underTest = new GitIgnoreBuilder() {
+            @Override
+            protected String globalGitIgnore() {
+                return globalGitIgnore;
+            }
+        };
+        URL url = 
GitIgnoreBuilderTest.class.getClassLoader().getResource("GitIgnoreBuilderTest/src/");
+        File file = new File(url.getFile());
+
+        DocumentName documentName = DocumentName.builder(file).build();
+        List<MatcherSet> matcherSets = underTest.build(documentName);
+        DocumentNameMatcher matcher = 
MatcherSet.merge(matcherSets).createMatcher();
+
+        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);
+
+        assertThat(matcher.toString()).isEqualTo("matcherSet(or('included 
.gitignore', 'included .gitignore'), or('excluded .gitignore', **/.gitignore, 
'excluded .gitignore', 'excluded " + globalGitIgnore.substring(1) + "'))");

Review Comment:
   `globalGitIgnore.substring(1)` does not give the right presentation on 
Windows. I don't have easy access to a Windows machine for experimenting - 
could someone help figure out the right incantation here?



##########
apache-rat-core/src/main/java/org/apache/rat/config/exclusion/fileProcessors/AbstractFileProcessorBuilder.java:
##########
@@ -166,18 +176,17 @@ protected MatcherSet process(final Consumer<MatcherSet> 
matcherSetConsumer, fina
     /**
      * Process the directory tree looking for files that match the filter. 
Call {@link #process} on any matching file.
      * @param level the level being precessed
-     * @param root the directory against which names should be resolved.
      * @param directory The name of the directory to process.
      * @param fileFilter the filter to detect processable files with.
      */
-    private void checkDirectory(final int level, final DocumentName root, 
final DocumentName directory, final FileFilter fileFilter) {
+    private void checkDirectory(final int level, final DocumentName directory, 
final FileFilter fileFilter) {
         File dirFile = directory.asFile();
         for (File file : listFiles(dirFile, fileFilter)) {
             LevelBuilder levelBuilder = levelBuilders.computeIfAbsent(level, k 
-> new LevelBuilder());
-            levelBuilder.add(process(levelBuilder::add, root, 
DocumentName.builder(file).build()));
+            levelBuilder.add(process(levelBuilder::add, 
DocumentName.builder(file).build(), DocumentName.builder(file).build()));

Review Comment:
   for the non-global gitignore, this assumption has now moved here



##########
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:
   this used to assume the pattern is relative to the location of the 
`.gitignore` file, which used to be true, but is not the case for the global 
gitignore.



-- 
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

Reply via email to