Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ignoreErrorForPath does not work, perhaps related to build folder #194

Closed
magneticflux- opened this issue Feb 1, 2018 · 17 comments
Closed

Comments

@magneticflux-
Copy link
Contributor

I have supplied a path src/main/org/x/y/z/ExcludedFile.java to the ignoreErrorForPath method and the file still causes the build to fail. I've tried both Unix and Windows style paths.

Spotless 3.8.0
Gradle 4.5
Eclipse formatter

@nedtwigg nedtwigg added the bug label Feb 1, 2018
@nedtwigg
Copy link
Member

nedtwigg commented Feb 1, 2018

Can you copy-paste the stacktrace of the error?

@magneticflux-
Copy link
Contributor Author

magneticflux- commented Feb 1, 2018

Sorry, I wasn't quite clear in my original post. There are no exceptions thrown, but the file that should be ignored still triggers a build failure because of formatting issues.

Edit: This is where I'm looking for information: link

@magneticflux-
Copy link
Contributor Author

This are my ignore statements:

ignoreErrorForPath 'build/generated/source/proto/test/java/de/javakaffee/kryoserializers/protobuf/SampleProtoAOuterClass.java'
ignoreErrorForPath 'build/generated/source/proto/test/java/de/javakaffee/kryoserializers/protobuf/SampleProtoBOuterClass.java'
ignoreErrorForPath 'build\\generated\\source\\proto\\test\\java\\de\\javakaffee\\kryoserializers\\protobuf\\SampleProtoAOuterClass.java'
ignoreErrorForPath 'build\\generated\\source\\proto\\test\\java\\de\\javakaffee\\kryoserializers\\protobuf\\SampleProtoBOuterClass.java'

@nedtwigg
Copy link
Member

nedtwigg commented Feb 1, 2018

Run gradlew spotlessCheck --stacktrace and copy-paste the error here.

@magneticflux-
Copy link
Contributor Author

It's a standard format violation exception.

Starting a Gradle Daemon, 1 incompatible Daemon could not be reused, use --status for details

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':spotlessJava'.
> The following files had format violations:
      build\generated\source\proto\test\java\de\javakaffee\kryoserializers\protobuf\SampleProtoAOuterClass.java
          @@ -1,689 +1,758 @@
          -//╖Generated╖by╖the╖protocol╖buffer╖compiler.╖╖DO╖NOT╖EDIT!
          -//╖source:╖SampleProtoA.proto
          -
          +/*
          +╖*╖Copyright╖2018╖Martin╖Grotzke
          +╖*
          +╖*╖Licensed╖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╖de.javakaffee.kryoserializers.protobuf;

           public╖final╖class╖SampleProtoAOuterClass╖{
          -╖╖private╖SampleProtoAOuterClass()╖{}
          -╖╖public╖static╖void╖registerAllExtensions(
          -╖╖╖╖╖╖com.google.protobuf.ExtensionRegistryLite╖registry)╖{
          -╖╖}
          +\tprivate╖SampleProtoAOuterClass()╖{
          +\t}

          -╖╖public╖static╖void╖registerAllExtensions(
          -╖╖╖╖╖╖com.google.protobuf.ExtensionRegistry╖registry)╖{
          -╖╖╖╖registerAllExtensions(
          -╖╖╖╖╖╖╖╖(com.google.protobuf.ExtensionRegistryLite)╖registry);
          -╖╖}
          -╖╖public╖interface╖SampleProtoAOrBuilder╖extends
          -╖╖╖╖╖╖//╖@@protoc_insertion_point(interface_extends:de.javakaffee.kryoserializers.protobuf.SampleProtoA)
          -╖╖╖╖╖╖com.google.protobuf.MessageOrBuilder╖{
          +\tpublic╖static╖void╖registerAllExtensions(
          +\t\t\tcom.google.protobuf.ExtensionRegistryLite╖registry)╖{
          +\t}

          -╖╖╖╖/**
          -╖╖╖╖╖*╖<code>optional╖string╖name╖=╖1;</code>
          -╖╖╖╖╖*/
          -╖╖╖╖boolean╖hasName();
          -╖╖╖╖/**
          -╖╖╖╖╖*╖<code>optional╖string╖name╖=╖1;</code>
          -╖╖╖╖╖*/
      ... (1351 more lines that didn't fit)
  Violations also present in:
      build\generated\source\proto\test\java\de\javakaffee\kryoserializers\protobuf\SampleProtoBOuterClass.java
  Run 'gradlew spotlessApply' to fix these violations.

* Try:
Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Exception is:
org.gradle.api.tasks.TaskExecutionException: Execution failed for task ':spotlessJava'.
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeActions(ExecuteActionsTaskExecuter.java:103)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.execute(ExecuteActionsTaskExecuter.java:73)
        at org.gradle.api.internal.tasks.execution.OutputDirectoryCreatingTaskExecuter.execute(OutputDirectoryCreatingTaskExecuter.java:51)
        at org.gradle.api.internal.tasks.execution.SkipUpToDateTaskExecuter.execute(SkipUpToDateTaskExecuter.java:59)
        at org.gradle.api.internal.tasks.execution.ResolveTaskOutputCachingStateExecuter.execute(ResolveTaskOutputCachingStateExecuter.java:54)
        at org.gradle.api.internal.tasks.execution.ValidatingTaskExecuter.execute(ValidatingTaskExecuter.java:59)
        at org.gradle.api.internal.tasks.execution.SkipEmptySourceFilesTaskExecuter.execute(SkipEmptySourceFilesTaskExecuter.java:101)
        at org.gradle.api.internal.tasks.execution.FinalizeInputFilePropertiesTaskExecuter.execute(FinalizeInputFilePropertiesTaskExecuter.java:44)
        at org.gradle.api.internal.tasks.execution.CleanupStaleOutputsExecuter.execute(CleanupStaleOutputsExecuter.java:88)
        at org.gradle.api.internal.tasks.execution.ResolveTaskArtifactStateTaskExecuter.execute(ResolveTaskArtifactStateTaskExecuter.java:62)
        at org.gradle.api.internal.tasks.execution.SkipTaskWithNoActionsExecuter.execute(SkipTaskWithNoActionsExecuter.java:52)
        at org.gradle.api.internal.tasks.execution.SkipOnlyIfTaskExecuter.execute(SkipOnlyIfTaskExecuter.java:54)
        at org.gradle.api.internal.tasks.execution.ExecuteAtMostOnceTaskExecuter.execute(ExecuteAtMostOnceTaskExecuter.java:43)
        at org.gradle.api.internal.tasks.execution.CatchExceptionTaskExecuter.execute(CatchExceptionTaskExecuter.java:34)
        at org.gradle.execution.taskgraph.DefaultTaskGraphExecuter$EventFiringTaskWorker$1.run(DefaultTaskGraphExecuter.java:248)
        at org.gradle.internal.progress.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:336)
        at org.gradle.internal.progress.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:328)
        at org.gradle.internal.progress.DefaultBuildOperationExecutor.execute(DefaultBuildOperationExecutor.java:199)
        at org.gradle.internal.progress.DefaultBuildOperationExecutor.run(DefaultBuildOperationExecutor.java:110)
        at org.gradle.execution.taskgraph.DefaultTaskGraphExecuter$EventFiringTaskWorker.execute(DefaultTaskGraphExecuter.java:241)
        at org.gradle.execution.taskgraph.DefaultTaskGraphExecuter$EventFiringTaskWorker.execute(DefaultTaskGraphExecuter.java:230)
        at org.gradle.execution.taskgraph.DefaultTaskPlanExecutor$TaskExecutorWorker.processTask(DefaultTaskPlanExecutor.java:123)
        at org.gradle.execution.taskgraph.DefaultTaskPlanExecutor$TaskExecutorWorker.access$200(DefaultTaskPlanExecutor.java:79)
        at org.gradle.execution.taskgraph.DefaultTaskPlanExecutor$TaskExecutorWorker$1.execute(DefaultTaskPlanExecutor.java:104)
        at org.gradle.execution.taskgraph.DefaultTaskPlanExecutor$TaskExecutorWorker$1.execute(DefaultTaskPlanExecutor.java:98)
        at org.gradle.execution.taskgraph.DefaultTaskExecutionPlan.execute(DefaultTaskExecutionPlan.java:623)
        at org.gradle.execution.taskgraph.DefaultTaskExecutionPlan.executeWithTask(DefaultTaskExecutionPlan.java:578)
        at org.gradle.execution.taskgraph.DefaultTaskPlanExecutor$TaskExecutorWorker.run(DefaultTaskPlanExecutor.java:98)
        at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:63)
        at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:46)
        at org.gradle.internal.concurrent.ThreadFactoryImpl$ManagedThreadRunnable.run(ThreadFactoryImpl.java:55)
Caused by: org.gradle.api.GradleException: The following files had format violations:
    build\generated\source\proto\test\java\de\javakaffee\kryoserializers\protobuf\SampleProtoAOuterClass.java
        @@ -1,689 +1,758 @@
        -//╖Generated╖by╖the╖protocol╖buffer╖compiler.╖╖DO╖NOT╖EDIT!
        -//╖source:╖SampleProtoA.proto
        -
        +/*
        +╖*╖Copyright╖2018╖Martin╖Grotzke
        +╖*
        +╖*╖Licensed╖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╖&quot;AS╖IS&quot;╖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╖de.javakaffee.kryoserializers.protobuf;

         public╖final╖class╖SampleProtoAOuterClass╖{
        -╖╖private╖SampleProtoAOuterClass()╖{}
        -╖╖public╖static╖void╖registerAllExtensions(
        -╖╖╖╖╖╖com.google.protobuf.ExtensionRegistryLite╖registry)╖{
        -╖╖}
        +\tprivate╖SampleProtoAOuterClass()╖{
        +\t}

        -╖╖public╖static╖void╖registerAllExtensions(
        -╖╖╖╖╖╖com.google.protobuf.ExtensionRegistry╖registry)╖{
        -╖╖╖╖registerAllExtensions(
        -╖╖╖╖╖╖╖╖(com.google.protobuf.ExtensionRegistryLite)╖registry);
        -╖╖}
        -╖╖public╖interface╖SampleProtoAOrBuilder╖extends
        -╖╖╖╖╖╖//╖@@protoc_insertion_point(interface_extends:de.javakaffee.kryoserializers.protobuf.SampleProtoA)
        -╖╖╖╖╖╖com.google.protobuf.MessageOrBuilder╖{
        +\tpublic╖static╖void╖registerAllExtensions(
        +\t\t\tcom.google.protobuf.ExtensionRegistryLite╖registry)╖{
        +\t}

        -╖╖╖╖/**
        -╖╖╖╖╖*╖<code>optional╖string╖name╖=╖1;</code>
        -╖╖╖╖╖*/
        -╖╖╖╖boolean╖hasName();
        -╖╖╖╖/**
        -╖╖╖╖╖*╖<code>optional╖string╖name╖=╖1;</code>
        -╖╖╖╖╖*/
    ... (1351 more lines that didn't fit)
Violations also present in:
    build\generated\source\proto\test\java\de\javakaffee\kryoserializers\protobuf\SampleProtoBOuterClass.java
Run 'gradlew spotlessApply' to fix these violations.
        at com.diffplug.gradle.spotless.SpotlessTask.formatViolationsFor(SpotlessTask.java:283)
        at com.diffplug.gradle.spotless.SpotlessTask.check(SpotlessTask.java:275)
        at com.diffplug.gradle.spotless.SpotlessTask.performAction(SpotlessTask.java:205)
        at org.gradle.internal.reflect.JavaMethod.invoke(JavaMethod.java:73)
        at org.gradle.api.internal.project.taskfactory.IncrementalTaskAction.doExecute(IncrementalTaskAction.java:50)
        at org.gradle.api.internal.project.taskfactory.StandardTaskAction.execute(StandardTaskAction.java:39)
        at org.gradle.api.internal.project.taskfactory.StandardTaskAction.execute(StandardTaskAction.java:26)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter$1.run(ExecuteActionsTaskExecuter.java:124)
        at org.gradle.internal.progress.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:336)
        at org.gradle.internal.progress.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:328)
        at org.gradle.internal.progress.DefaultBuildOperationExecutor.execute(DefaultBuildOperationExecutor.java:199)
        at org.gradle.internal.progress.DefaultBuildOperationExecutor.run(DefaultBuildOperationExecutor.java:110)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeAction(ExecuteActionsTaskExecuter.java:113)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeActions(ExecuteActionsTaskExecuter.java:95)
        ... 30 more


* Get more help at https://help.gradle.org

BUILD FAILED in 8s
3 actionable tasks: 2 executed, 1 up-to-date

@nedtwigg
Copy link
Member

nedtwigg commented Feb 2, 2018

Can you copy paste your full spotless config? I wonder if the ignores are in the wrong place, e.g. in a different format?

@magneticflux-
Copy link
Contributor Author

Here's the commit! I disabled the spotlessCheck task to keep the Travis CI build passing. My goal is to exclude the build/generated/source/proto/test/java/de/javakaffee/kryoserializers/protobuf/SampleProtoAOuterClass.java files from Spotless because they're generated. The only reason they're in java.srcDirs is because the ProtoBuf Gradle plugin doesn't correctly register generated classes.

Just the spotless block:

spotless {
	enforceCheck false
	java {
		it.with {
			licenseHeaderFile 'spotless.license'
			importOrderFile 'spotless.importorder'
			eclipse().configFile 'spotless.eclipseformat.xml'
			trimTrailingWhitespace()
			removeUnusedImports()
			endWithNewline()

			// Eclipse formatter removes whitespace in a for loop without an increment.
			// for(int i = 0; i < 10;)  // what Eclipse does
			// for(int i = 0; i < 10; ) // what I wish Eclipse did
			custom 'For loop fix', { it.replace(';)', '; )') }

			ignoreErrorForPath 'build/generated/source/proto/test/java/de/javakaffee/kryoserializers/protobuf/SampleProtoAOuterClass.java'
			ignoreErrorForPath 'build/generated/source/proto/test/java/de/javakaffee/kryoserializers/protobuf/SampleProtoBOuterClass.java'
			ignoreErrorForPath 'build\\generated\\source\\proto\\test\\java\\de\\javakaffee\\kryoserializers\\protobuf\\SampleProtoAOuterClass.java'
			ignoreErrorForPath 'build\\generated\\source\\proto\\test\\java\\de\\javakaffee\\kryoserializers\\protobuf\\SampleProtoBOuterClass.java'
		}
	}
	groovyGradle {
		it.with {
			greclipse().configFile('spotless.eclipseformat.xml', 'spotless.groovyformat.prefs')
		}
	}
	freshmark {
		it.with {
			target '**/*.md'
			propertiesFile('gradle.properties')
		}
	}
}

The it.with{ ... } blocks are there to allow Intellij to find what the eventual types of the inner blocks are and provide autocomplete and correct highlighting.

@nedtwigg
Copy link
Member

nedtwigg commented Feb 2, 2018

Well this is a very strange bug indeed :) I took a look at the code, and it turns out I'm probably wrong about unix-style relative paths, looks like you do need both the / and the \. We should fix that in a future version.

One workaround you could do is set the target of java manually:

java {
   target 'src/main/**/*.java'

But even if that workaround fixes it, we should do the following:

  • make an integration test for a path with slashes (right now only root files are in the test)
  • confirm that / and \ are needed on Windows, and fix it if so
  • make an integration test that recreates this bug, maybe it's about formatting a file in the build directory?
  • fix it

This isn't gonna make the top of my todo list anytime soon, but it might eventually :) PR's welcome.

@nedtwigg nedtwigg changed the title ignoreErrorForPath does not work correctly ignoreErrorForPath does not work, perhaps related to build folder Feb 2, 2018
@rafalmag
Copy link

rafalmag commented Feb 7, 2018

I have the same issue.
spotless-plugin-gradle-3.8.0.jar
spotless-lib-1.8.0.jar
Gradle 4.1

    spotless {
        format 'misc', {
            target '**/*.gradle', '**/*.md', '**/.gitignore', '**/*.xml', '**/*.conf', '**/*.properties', '**/*.xsd'
            indentWithSpaces()
            trimTrailingWhitespace()
            endWithNewline()
            ignoreErrorForPath('src\\e2eTest\\resources\\myDir\\Sample.xml')
            ignoreErrorForPath('e2e\\src\\e2eTest\\resources\\myDir\\Sample.xml')
            ignoreErrorForPath('e2e/src/e2eTest/resources/myDir/Sample.xml')
            ignoreErrorForPath('src/e2eTest/resources/myDir/Sample.xml')
        }
    }

The error message when invoked gradle spotlessMisc:

The following files had format violations:
      e2e\src\e2eTest\resources\myDir\Sample.xml

@nedtwigg
Copy link
Member

nedtwigg commented Feb 8, 2018

Welp, that's a strike against the "build folder" theory. I'm baffled :) Thanks for the extra evidence! If any other users have evidence to contribute, please do! Best of all would be an opensource repo that reproduces the issue.

@thc202
Copy link
Contributor

thc202 commented Feb 25, 2018

This does not seem to be a bug but a misconception on what ignoreErrorForPath is used to. I thought this was a bug when I tried (many path variations) to ignore one of the files... it was only when I checked out the code that I realised that ignoreErrorForPath does not apply to build failures caused by format violations but actual errors in the formatters. Yes, the README says that... :/

@nedtwigg is this assessment correct? If so, could the README be more explicit and say that the format violations are not included? Could the plugin provide a way to ignore format violations per path/step?

@magneticflux-
Copy link
Contributor Author

@thc202 I believe those exceptions are thrown by the formatters when they detect a format violation (or perhaps that was a legacy system and now more precise return values are used).

@thc202
Copy link
Contributor

thc202 commented Feb 25, 2018

I don't understand that from taking a look at the history, it seems that the Formatter was catching (and initially logging) exceptions thrown by the steps (since v1.0?), so format violations are not exceptions from the steps themselves (which is where the ignoreErrorForPath is checked, through the strict FormatExceptionPolicy).

@magneticflux-
Copy link
Contributor Author

@thc202 You're correct, it appears that formatters currently are intended to throw exceptions under 'exceptional' circumstances, rather than formatting exceptions, and that the check for path inclusion only occurs during an exception. The legacy code that relies on exceptions is located here.

@nedtwigg Can you confirm our hypotheses? Should the path check occur for non-erroring steps as well and just return the original String?

@thc202
Copy link
Contributor

thc202 commented Mar 13, 2018

Bump :)

@nedtwigg
Copy link
Member

Thanks very much for this issue, @thc202 and @Magneticflux.

This does not seem to be a bug but a misconception on what ignoreErrorForPath is used to.

You are correct, and this misconception is so deep that I didn't realize you were right until I finally sat down to try to actually fix the bug. I considered attempting to resolve this by making ignoreErrorForPath do the obvious behavior, which is to ignore formatting errors on that path, but that turned out to be quite hacky. "Which files to format" and "which internal errors to suppress" are different questions, and mushing them together was ugly.

I think this whole issue is actually a symptom of our users didn't have an easy way to exclude a file from formatting. We told people "if you want to ignore a folder, set target to be everything except that one file". But that would be quite onerous for the examples given in this issue. I'm attempting to resolve this by adding a targetExclude parameter in PR #353.

If you've got a file that you'd like to be formatted, but for whatever reason it sometimes causes the third-party formatter to throw exceptions and you don't want them to block your build then use ignoreErrorForPath. If you don't want the file to be formatted in the first place, use targetExclude. I'm gonna close this out, please leave feedback in PR #353.

@nedtwigg
Copy link
Member

Published in 3.18.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants