You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@creadur.apache.org by "jbonofre (via GitHub)" <gi...@apache.org> on 2023/11/23 16:56:20 UTC
[PR] RAT-322: be able to scan hidden directories [creadur-rat]
jbonofre opened a new pull request, #166:
URL: https://github.com/apache/creadur-rat/pull/166
This is an alternative to #160
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-322: be able to scan hidden directories [creadur-rat]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on PR #166:
URL: https://github.com/apache/creadur-rat/pull/166#issuecomment-1826227820
In order to be backward compatible (especially to work as before on Windows), I created `NameBasedHiddenFileFilter` that only check if file name contains `. ` (and not the hidden attribute on Windows).
User can still define its own filter if needed.
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-322: be able to scan hidden directories [creadur-rat]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #166:
URL: https://github.com/apache/creadur-rat/pull/166#discussion_r1408111432
##########
apache-rat-core/src/test/java/org/apache/rat/ReportConfigurationTest.java:
##########
@@ -189,6 +194,19 @@ public void inputFileFilterTest() {
assertEquals(filter, underTest.getInputFileFilter());
}
+ @Test
+ public void directoryFilterTest() {
+ assertNotNull("Directory filter should not be null by default", underTest.getDirectoryFilter());
+ assertTrue("Directory filter should be a NameBasedHiddenFileFilter one by default", underTest.getDirectoryFilter() instanceof NameBasedHiddenFileFilter);
Review Comment:
I introduced assertj-core and refactore `ReportConfigurationTest` to use it.
If you want, I can refactore all tests to use assertj.
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-322: be able to scan hidden directories [creadur-rat]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on PR #166:
URL: https://github.com/apache/creadur-rat/pull/166#issuecomment-1830327681
I started to use assertj in `ReportConfigurationTest`.
I'm fixing the build.
@ottlinger do you want me to use assertj everywhere ? Maybe it makes more sense to have another PR dedicated to that. Thought ?
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-322: be able to scan hidden directories [creadur-rat]
Posted by "ottlinger (via GitHub)" <gi...@apache.org>.
ottlinger commented on PR #166:
URL: https://github.com/apache/creadur-rat/pull/166#issuecomment-1826444685
@Claudenw could we just merge this change or would this introduce problems with other/existing PRs? Thanks
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-322: be able to scan hidden directories [creadur-rat]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on PR #166:
URL: https://github.com/apache/creadur-rat/pull/166#issuecomment-1826606268
> Thanks for the contribution @jbonofre
>
> I'm still curious to get to know more details about why you want to scan in hidden directories? Can you provide more info about that.
@ottlinger at ASF, we should have license header in all files included in project release source distributions (which is the minimum a project has to provide for each release). Some projects include hidden directories/iles in their source distributions (for instance, Apache Iceberg includes hidden directories like .palantir, .baseline, etc). Those directories/files can't be excluded else it means that it would not be possible to build the project from the source distribution.
To be "clean", these directories/files have to include ASF/license headers and so they should be considered by RAT.
As workaround, RAT gradle|maven plugins use a "custom" walker to scan hidden directories/files. So I think it makes sense to improve our default walker to support this behavior and so having the same support in CLI.
That's basically the background :)
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-322: be able to scan hidden directories [creadur-rat]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on PR #166:
URL: https://github.com/apache/creadur-rat/pull/166#issuecomment-1832653558
@ottlinger I propose to merge this PR (as it introduces `assertj-core` dependency). I will create another PR to use assertj everywhere.
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-322: be able to scan hidden directories [creadur-rat]
Posted by "ottlinger (via GitHub)" <gi...@apache.org>.
ottlinger commented on code in PR #166:
URL: https://github.com/apache/creadur-rat/pull/166#discussion_r1405263852
##########
apache-rat-core/src/test/java/org/apache/rat/ReportConfigurationTest.java:
##########
@@ -189,6 +194,19 @@ public void inputFileFilterTest() {
assertEquals(filter, underTest.getInputFileFilter());
}
+ @Test
+ public void directoryFilterTest() {
+ assertNotNull("Directory filter should not be null by default", underTest.getDirectoryFilter());
+ assertTrue("Directory filter should be a NameBasedHiddenFileFilter one by default", underTest.getDirectoryFilter() instanceof NameBasedHiddenFileFilter);
Review Comment:
When I read this I thought of adding assertj and use its isExactlyInstanceOf(Foo.class) ... not sure if there are reasons to not add assertj-core ;)
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-322: be able to scan hidden directories [creadur-rat]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #166:
URL: https://github.com/apache/creadur-rat/pull/166#discussion_r1405329282
##########
apache-rat-core/src/test/java/org/apache/rat/ReportConfigurationTest.java:
##########
@@ -189,6 +194,19 @@ public void inputFileFilterTest() {
assertEquals(filter, underTest.getInputFileFilter());
}
+ @Test
+ public void directoryFilterTest() {
+ assertNotNull("Directory filter should not be null by default", underTest.getDirectoryFilter());
+ assertTrue("Directory filter should be a NameBasedHiddenFileFilter one by default", underTest.getDirectoryFilter() instanceof NameBasedHiddenFileFilter);
Review Comment:
I didn't want to introduce a new dependency for the test (as assertj is not in our pom). I don't have problem to add assertj and use it if you prefer.
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-322: be able to scan hidden directories [creadur-rat]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on PR #166:
URL: https://github.com/apache/creadur-rat/pull/166#issuecomment-1825928699
OK, I understand: on Windows, the `HiddenFileFilter` doesn't actually consider file hidden, so the tests are failing on Windows. I'm checking this part.
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-322: be able to scan hidden directories [creadur-rat]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on PR #166:
URL: https://github.com/apache/creadur-rat/pull/166#issuecomment-1825941748
Extracted from `java.io.File#isHidden()` javadoc:
```
Tests whether the file named by this abstract pathname is a hidden file. The exact definition of hidden is system-dependent. On UNIX systems, a file is considered to be hidden if its name begins with a period character ('.'). On Microsoft Windows systems, a file is considered to be hidden if it has been marked as such in the filesystem.
```
So, for the tests, I have to bypass if on Windows. I'm doing that.
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-322: be able to scan hidden directories [creadur-rat]
Posted by "ottlinger (via GitHub)" <gi...@apache.org>.
ottlinger commented on code in PR #166:
URL: https://github.com/apache/creadur-rat/pull/166#discussion_r1405262511
##########
apache-rat-core/src/main/java/org/apache/rat/walker/NameBasedHiddenFileFilter.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ */
+package org.apache.rat.walker;
+
+import org.apache.commons.io.filefilter.AbstractFileFilter;
+import org.apache.commons.io.filefilter.IOFileFilter;
+
+import java.io.File;
+import java.io.Serializable;
+
+/**
+ * This filter accepts Files that are hidden, e.g. file name starts with .
+ *
+ * Example, showing how to print out a list of the current directory's hidden files:
+ *
+ * Using Classic IO
Review Comment:
Would you mind formatting the code via @code as shown here
https://www.baeldung.com/javadoc-multi-line-code
I guess this would improvde readability a lot ....
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-322: be able to scan hidden directories [creadur-rat]
Posted by "ottlinger (via GitHub)" <gi...@apache.org>.
ottlinger commented on code in PR #166:
URL: https://github.com/apache/creadur-rat/pull/166#discussion_r1405264614
##########
apache-rat/src/site/apt/index.apt.vm:
##########
@@ -75,17 +75,17 @@ java -jar apache-rat/target/apache-rat-${project.version}.jar
+------------------------------------------+
-usage: java -jar apache-rat/target/apache-rat-${project.version}.jar
- [options] [DIR|TARBALL]
+usage: java -jar apache-rat/target/apache-rat-CURRENT-VERSION.jar
Review Comment:
Where does the CURRENT-VERSION come from as project.version is standard Maven file filtering? Maybe I do not understand the mechanism properly (sorry for asking)
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-322: be able to scan hidden directories [creadur-rat]
Posted by "ottlinger (via GitHub)" <gi...@apache.org>.
ottlinger commented on PR #166:
URL: https://github.com/apache/creadur-rat/pull/166#issuecomment-1826444662
Thanks for the contribution @jbonofre
I'm still curious to get to know more details about why you want to scan in hidden directories? Can you provide more info about that.
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-322: be able to scan hidden directories [creadur-rat]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on PR #166:
URL: https://github.com/apache/creadur-rat/pull/166#issuecomment-1825927019
@Claudenw I'm checking the build failure on Windows (Jenkins). I tested locally without problem. I'm doing a new pass and I close the "old" PR on the same topic.
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-322: be able to scan hidden directories [creadur-rat]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on PR #166:
URL: https://github.com/apache/creadur-rat/pull/166#issuecomment-1831311452
I pushed the update to define assertj in `dependenciesManagement` and define where it's required.
@ottlinger please let me know if you want me to refactore all tests using assertj in this PR or in separated one. Thanks !
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-322: be able to scan hidden directories [creadur-rat]
Posted by "ottlinger (via GitHub)" <gi...@apache.org>.
ottlinger merged PR #166:
URL: https://github.com/apache/creadur-rat/pull/166
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-322: be able to scan hidden directories [creadur-rat]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on PR #166:
URL: https://github.com/apache/creadur-rat/pull/166#issuecomment-1829513346
I did rebase and added an entry in the changes.
As suggested by @ottlinger I'm updating the tests to use assertj.
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-322: be able to scan hidden directories [creadur-rat]
Posted by "Claudenw (via GitHub)" <gi...@apache.org>.
Claudenw commented on PR #166:
URL: https://github.com/apache/creadur-rat/pull/166#issuecomment-1825917907
This looks good to me, but I have not built and tested it. I trust that the build works and there seems to be a test so I approve this change.
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-322: be able to scan hidden directories [creadur-rat]
Posted by "ottlinger (via GitHub)" <gi...@apache.org>.
ottlinger commented on PR #166:
URL: https://github.com/apache/creadur-rat/pull/166#issuecomment-1826444815
@jbonofre feel free to add a changelog entry (or I can do this upon merging your changes; sorry for my many questions in the code).
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RAT-322: be able to scan hidden directories [creadur-rat]
Posted by "ottlinger (via GitHub)" <gi...@apache.org>.
ottlinger commented on PR #166:
URL: https://github.com/apache/creadur-rat/pull/166#issuecomment-1832612684
@jbonofre go ahead to use assertj more often in a different PR. Thanks for your contribution.
--
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-unsubscribe@creadur.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org