patch-filters doesn't work on Windows OS

[rnveach]

Identified at https://github.com/checkstyle/eclipse-cs/pull/395#issuecomment-1312743019 ,

My local:

> git status
HEAD detached at z_Bananeweizen/patch_filter_not_working
nothing to commit, working tree clean

> git log -n 1
commit dfc3852f151313cc84ec10e04181084eb4178afc (HEAD, z_Bananeweizen/patch_filter_not_working)
Author: Michael Keppler <Michael.Keppler@gmx.de>
Date:   Sun Nov 13 08:58:40 2022 +0100

    reproduce patch filter bug

    Auditor.java line 78 is the first violation reported by "mvn verify" if
    the patch filters are removed in the checkstyle_sevntu_checks.xml.

    So by adding a very long comment there we should _still_ see that same
    error, even if patch-filter is NOT removed.

> mvn clean verify
....
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for eclipse-cs Maven Parent 10.3.3-SNAPSHOT:
[INFO]
[INFO] eclipse-cs Maven Parent ............................ SUCCESS [  6.464 s]
[INFO] eclipse-cs Target Definition ....................... SUCCESS [  0.723 s]
[INFO] eclipse-cs Branding Plugin ......................... SUCCESS [  4.164 s]
[INFO] Checkstyle Core Library Plugin ..................... SUCCESS [ 20.956 s]
[INFO] eclipse-cs Core Plugin ............................. SUCCESS [  8.231 s]
[INFO] eclipse-cs Documentation Plugin .................... SUCCESS [  2.841 s]
[INFO] eclipse-cs UI Plugin ............................... SUCCESS [ 12.519 s]
[INFO] eclipse-cs Sample Plugin ........................... SUCCESS [  1.117 s]
[INFO] eclipse-cs Feature ................................. SUCCESS [  0.631 s]
[INFO] eclipse-cs Update Site ............................. SUCCESS [  2.995 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  01:23 min
[INFO] Finished at: 2022-11-13T09:20:57-05:00
[INFO] ------------------------------------------------------------------------

> type .\show.patch
diff --git a/net.sf.eclipsecs.core/src/net/sf/eclipsecs/core/builder/Auditor.java b/net.sf.eclipsecs.core/src/net/sf/eclipsecs/core/builder/Auditor.java
index cc6a8495..e23d4329 100644
--- a/net.sf.eclipsecs.core/src/net/sf/eclipsecs/core/builder/Auditor.java
+++ b/net.sf.eclipsecs.core/src/net/sf/eclipsecs/core/builder/Auditor.java
@@ -75,7 +75,7 @@ public class Auditor {
   private IProgressMonitor mMonitor;

   /** Map containing the file resources to audit. */
-  private final Map<String, IFile> mFiles = new HashMap<>();
+  private final Map<String, IFile> mFiles = new HashMap<>(); // CustomDeclarationOrder is raised here when the patch-filter is removed from checkstyle_sevntu_checks.xml

   /** Add the check rule name to the message. */
   private boolean mAddRuleName = false;

Passed with no errors.

Travis: https://app.travis-ci.com/github/checkstyle/eclipse-cs/jobs/588314590#L625

[ERROR] src/net/sf/eclipsecs/core/builder/Auditor.java:[78,3] (extension) CustomDeclarationOrder: Field definition in wrong order. Expected 'Field(private final)' then 'Field(private.*)'. [INFO] eclipse-cs Core Plugin ............................. FAILURE [ 15.526 s]

Reasoning for issue can be found in https://github.com/checkstyle/patch-filters/issues/113#issuecomment-687199615

[rnveach]

@Bananeweizen I have not fully confirmed eclipse-cs yet, but #113 is on the point to being resolved which is this repo's unit tests for Windows. Since eclipse-cs didn't get any exceptions, I am thinking the main culprit for Windows support was https://github.com/checkstyle/patch-filters/issues/113#issuecomment-1312808670 and the slash handling of Audit events.

Please also be careful to note https://github.com/checkstyle/patch-filters/issues/363 as the current behavior of patch-filter in your repo is it won't include unstaged changes.

I will provide my report on eclipse-cs in another post, but I wanted to lay out the plan for even eclipse-cs to get this fix if it has been resolved. A plan is needed because patch-filters has already moved to CS 10.4 and I highly recommend sticking to the required version by sevntu which is still 10 otherwise we can't guarantee what will happen, as we have seen before.

1) eclipse-cs needs to finish upgrading to CS 10.4 or the latest checkstyle version if more time is needed. 2) sevntu will upgrade eclipse-cs version and CS version all to 10.4. 3) sevntu will perform a release and at the same time patch-filters can release its Windows fixes as well. Patch-filters has no reliance on sevntu. 4) eclipse-cs can upgrade all it's versions.

[rnveach]

Patch Filters:

patch-filters> git log -n 1
commit f204f4d343833e786ead877169ccd5808ca65d27 (HEAD -> 113_last, pull/issue_113_last)
Author: rnveach
Date:   Mon Nov 14 13:38:05 2022 -0500

    Issue #113: resolves audit event matching on windows

patch-filters> git status
On branch 113_last
Your branch is up to date with 'pull/issue_113_last'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

        modified:   pom.xml
        modified:   src/test/java/com/puppycrawl/tools/checkstyle/filters/SuppressionJavaPatchFilterTest.java

no changes added to commit (use "git add" and/or "git commit -a")

patch-filters> mvn install
...
[INFO] --- maven-install-plugin:2.4:install (default-install) @ patch-filters ---
[INFO] Installing M:\checkstyleWorkspaceEclipse\patch-filters\target\patch-filters-1.3.1-SNAPSHOT.jar to C:\Users\rveac\.m2\repository\com\puppycrawl\tools\patch-filters\1.3.1-SNAPSHOT\patch-filters-1.3.1-SNAPSHOT.jar
[INFO] Installing M:\checkstyleWorkspaceEclipse\patch-filters\pom.xml to C:\Users\rveac\.m2\repository\com\puppycrawl\tools\patch-filters\1.3.1-SNAPSHOT\patch-filters-1.3.1-SNAPSHOT.pom
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  20.873 s
[INFO] Finished at: 2022-11-14T14:53:04-05:00
[INFO] ------------------------------------------------------------------------

Change in pom.xml was to temporarily downgrade to CS 10.0 . Change in SuppressionJavaPatchFilterTest was to disable failing test from downgrade.

Eclipse-CS:

eclipse-cs> git log -n 1
commit 0e7c21d75ff024693eb33bd892e783c331e3efc0 (HEAD)
Author: rnveach
Date:   Mon Nov 14 15:05:26 2022 -0500

    MY TEST

eclipse-cs> mvn verify
....
[INFO] --- maven-checkstyle-plugin:3.2.0:check (sevntu-checkstyle-check) @ net.sf.eclipsecs.core ---
[INFO] There is 1 error reported by Checkstyle 10.0 with config/checkstyle_sevntu_checks.xml ruleset.
[ERROR] src\net\sf\eclipsecs\core\builder\Auditor.java:[78,3] (extension) CustomDeclarationOrder: Field definition in wrong order. Expected 'Field(private final)' then 'Field(private.*)'.
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for eclipse-cs Maven Parent 10.3.3-SNAPSHOT:
[INFO]
[INFO] eclipse-cs Maven Parent ............................ SUCCESS [  1.868 s]
[INFO] eclipse-cs Branding Plugin ......................... SUCCESS [  1.791 s]
[INFO] Checkstyle Core Library Plugin ..................... SUCCESS [ 11.986 s]
[INFO] eclipse-cs Core Plugin ............................. FAILURE [  5.212 s]

eclipse-cs> type .\show.patch
diff --git a/net.sf.eclipsecs.core/src/net/sf/eclipsecs/core/builder/Auditor.java b/net.sf.eclipsecs.core/src/net/sf/eclipsecs/core/builder/Auditor.java
index 5086dfe4..06d6a7a9 100644
--- a/net.sf.eclipsecs.core/src/net/sf/eclipsecs/core/builder/Auditor.java
+++ b/net.sf.eclipsecs.core/src/net/sf/eclipsecs/core/builder/Auditor.java
@@ -75,7 +75,7 @@ public class Auditor {
   private IProgressMonitor mMonitor;

   /** Map containing the file resources to audit. */
-  private final Map<String, IFile> mFiles = new HashMap<>();
+  private final Map<String, IFile> mFiles = new HashMap<>(); // CustomDeclarationOrder is raised here when the patch-filter is removed from checkstyle_sevntu_checks.xml

   /** Add the check rule name to the message. */
   private boolean mAddRuleName = false;
diff --git a/pom.xml b/pom.xml
index 6843aeae..b7b01b5b 100644
--- a/pom.xml
+++ b/pom.xml
@@ -26,7 +26,7 @@
         <!-- sevntu and patch-filters need to use a compatible/compiled version with checkstyle -->
         <maven.sevntu.checkstyle.plugin.checkstyle.version>10.0</maven.sevntu.checkstyle.plugin.checkstyle.version>
         <maven.sevntu.checkstyle.plugin.version>1.42.0</maven.sevntu.checkstyle.plugin.version>
-        <maven.patch.filters.plugin.version>1.2.0</maven.patch.filters.plugin.version>
+        <maven.patch.filters.plugin.version>1.3.1-SNAPSHOT</maven.patch.filters.plugin.version>
         <!-- see above -->
     </properties>

I am satisfied that the changes under https://github.com/checkstyle/patch-filters/issues/113 resolve this issue.

[Bananeweizen]

What I can say is that the exceptions in the patch-filters unit tests are definitely gone for me. Beyond that, the Locale issue that we just discussed in the main project stops me from confirming that it works. But I have trust in you solving it fully.

[rnveach]

@romani I added windows run at https://github.com/checkstyle/patch-filters/pull/362 . It is waiting on my other pr to be merged before it's green.

[romani]

@rnveach , is this issue resolved by https://github.com/checkstyle/patch-filters/issues/113 https://github.com/checkstyle/patch-filters/pull/361 ?

[rnveach]

Correct. I provided results at https://github.com/checkstyle/patch-filters/issues/352#issuecomment-1314320650 that it works as reported.

[romani]

Released in 1.4.0

[romani]

@Bananeweizen , are you ok to try https://github.com/checkstyle/patch-filters/issues/363#issuecomment-1334246833 in this repo ?

[Bananeweizen]

Are you asking, whether I would run the command for adding the files from that comment manually on changes? If so, no. I know that I would forget this all the time, since I don't even use the command line for my git operations (I use egit). From my experience with the patch-filters over the previous 2 weeks I'm completely fine with getting the results only for already committed files.