Skip to content

Conversation

@LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Nov 2, 2022

What changes were proposed in this pull request?

This pr aims upgrade sbt-checkstyle-plugin to 4.0.0 to resolve dev/sbt-checkstyle run failed with sbt 1.7.3, the new version will check the generated source code, so some new suppression rules have been added to dev/checkstyle-suppressions.xml

Why are the changes needed?

#38476 revert sbt 1.7.3 upgrade due to run dev/sbt-checkstyle failed:

[error] org.xml.sax.SAXParseException; lineNumber: 18; columnNumber: 10; DOCTYPE is disallowed when the feature "http://apache.org/xml/features/disallow-doctype-decl" set to true.
[error]         at com.sun.org.apache.xerces.internal.util.ErrorHandlerWrapper.createSAXParseException(ErrorHandlerWrapper.java:203)
[error]         at com.sun.org.apache.xerces.internal.util.ErrorHandlerWrapper.fatalError(ErrorHandlerWrapper.java:177)
[error]         at com.sun.org.apache.xerces.internal.impl.XMLErrorReporter.reportError(XMLErrorReporter.java:400)
[error]         at com.sun.org.apache.xerces.internal.impl.XMLErrorReporter.reportError(XMLErrorReporter.java:327)
[error]         at com.sun.org.apache.xerces.internal.impl.XMLScanner.reportFatalError(XMLScanner.java:1473)
[error]         at com.sun.org.apache.xerces.internal.impl.XMLDocumentScannerImpl$PrologDriver.next(XMLDocumentScannerImpl.java:914)
[error]         at com.sun.org.apache.xerces.internal.impl.XMLDocumentScannerImpl.next(XMLDocumentScannerImpl.java:602)
[error]         at com.sun.org.apache.xerces.internal.impl.XMLDocumentFragmentScannerImpl.scanDocument(XMLDocumentFragmentScannerImpl.java:505)
[error]         at com.sun.org.apache.xerces.internal.parsers.XML11Configuration.parse(XML11Configuration.java:842)
[error]         at com.sun.org.apache.xerces.internal.parsers.XML11Configuration.parse(XML11Configuration.java:771)
[error]         at com.sun.org.apache.xerces.internal.parsers.XMLParser.parse(XMLParser.java:141)
[error]         at com.sun.org.apache.xerces.internal.parsers.AbstractSAXParser.parse(AbstractSAXParser.java:1213)
[error]         at com.sun.org.apache.xerces.internal.jaxp.SAXParserImpl$JAXPSAXParser.parse(SAXParserImpl.java:643)
[error]         at com.sun.org.apache.xerces.internal.jaxp.SAXParserImpl.parse(SAXParserImpl.java:327)
[error]         at scala.xml.factory.XMLLoader.parse(XMLLoader.scala:73)
[error]         at scala.xml.factory.XMLLoader.loadXML(XMLLoader.scala:54)
[error]         at scala.xml.factory.XMLLoader.loadXML$(XMLLoader.scala:53)
[error]         at scala.xml.XML$.loadXML(XML.scala:62)
[error]         at scala.xml.factory.XMLLoader.loadString(XMLLoader.scala:92)
[error]         at scala.xml.factory.XMLLoader.loadString$(XMLLoader.scala:92)
[error]         at scala.xml.XML$.loadString(XML.scala:62)
[error]         at com.etsy.sbt.checkstyle.Checkstyle$.checkstyle(Checkstyle.scala:35)
[error]         at com.etsy.sbt.checkstyle.CheckstylePlugin$autoImport$.$anonfun$checkstyleTask$1(CheckstylePlugin.scala:36)
[error]         at com.etsy.sbt.checkstyle.CheckstylePlugin$autoImport$.$anonfun$checkstyleTask$1$adapted(CheckstylePlugin.scala:34)
[error]         at scala.Function1.$anonfun$compose$1(Function1.scala:49)
[error]         at sbt.internal.util.$tilde$greater.$anonfun$$u2219$1(TypeFunctions.scala:62)
[error]         at sbt.std.Transform$$anon$4.work(Transform.scala:68)
[error]         at sbt.Execute.$anonfun$submit$2(Execute.scala:282)
[error]         at sbt.internal.util.ErrorHandling$.wideConvert(ErrorHandling.scala:23)
[error]         at sbt.Execute.work(Execute.scala:291)
[error]         at sbt.Execute.$anonfun$submit$1(Execute.scala:282)
[error]         at sbt.ConcurrentRestrictions$$anon$4.$anonfun$submitValid$1(ConcurrentRestrictions.scala:265)
[error]         at sbt.CompletionService$$anon$2.call(CompletionService.scala:64)
[error]         at java.util.concurrent.FutureTask.run(FutureTask.java:266)
[error]         at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
[error]         at java.util.concurrent.FutureTask.run(FutureTask.java:266)
[error]         at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
[error]         at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
[error]         at java.lang.Thread.run(Thread.java:748)

This pr upgrade sbt-checkstyle-plugin to 4.0.0 to resolve the above issue, the https://github.com/stringbean/sbt-checkstyle-plugin was forked from etsy/sbt-checkstyle-plugin in 2022 after it became unmaintained, the release notes as follows:

Does this PR introduce any user-facing change?

No

How was this patch tested?

  • Pass GitHub Actions
  • Manual test dev/sbt-checkstyle with sbt 1.7.3 and this pr: Checkstyle checks passed.

@github-actions github-actions bot added the BUILD label Nov 2, 2022
@LuciferYang LuciferYang changed the title [DONT'MERGE] Upgrade sbt-checkstyle-plugin and sbt [SPARK-40996] Upgrade sbt-checkstyle-plugin and sbt Nov 2, 2022
@LuciferYang LuciferYang changed the title [SPARK-40996] Upgrade sbt-checkstyle-plugin and sbt [SPARK-40996][BUILD] Upgrade sbt-checkstyle-plugin to 4.0.0 to resolve dev/sbt-checkstyle run failed with failure Nov 2, 2022
@LuciferYang LuciferYang changed the title [SPARK-40996][BUILD] Upgrade sbt-checkstyle-plugin to 4.0.0 to resolve dev/sbt-checkstyle run failed with failure [SPARK-40996][BUILD] Upgrade sbt-checkstyle-plugin to 4.0.0 to resolve dev/sbt-checkstyle run failed with sbt 1.7.3 Nov 2, 2022
@LuciferYang
Copy link
Contributor Author

The previous version of sbt-checkstyle-plugin seems invalid, run

build/sbt -Pkinesis-asl -Pmesos -Pkubernetes -Pyarn -Phive -Phive-thriftserver checkstyle test:checkstyle

with sbt 1.7.2:

the result as follows:

[warn] note: a setting might still be used by a command; to exclude a key from this `lintUnused` check
[warn] either append it to `Global / excludeLintKeys` or call .withRank(KeyRanks.Invisible) on the key
Files to process must be specified, found 0.
Files to process must be specified, found 0.
Files to process must be specified, found 0.
Files to process must be specified, found 0.
Files to process must be specified, found 0.
Files to process must be specified, found 0.
Files to process must be specified, found 0.
Files to process must be specified, found 0.
Files to process must be specified, found 0.
Files to process must be specified, found 0.
Files to process must be specified, found 0.
Files to process must be specified, found 0.
Files to process must be specified, found 0.
Files to process must be specified, found 0.

@LuciferYang
Copy link
Contributor Author

Run

build/sbt -Pkinesis-asl -Pmesos -Pkubernetes -Pyarn -Phive -Phive-thriftserver checkstyle test:checkstyle

with sbt 1.7.3 and this pr:

[warn] either append it to `Global / excludeLintKeys` or call .withRank(KeyRanks.Invisible) on the key
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[info] Checkstyle complete. No issues found.
[success] Total time: 10 s, completed 2022-11-2 17:50:16

# limitations under the License.
#
# Please update the version in appveyor-install-dependencies.ps1 together.
sbt.version=1.7.2
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the convenience of others manual check, it will be revert later

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fdc2067 revert sbt version to 1.7.2, will another pr for re upgrade. Please manually modify project/build.properties when you double check @linhongliu-db

# ========================== SBT
Push-Location $tools

$sbtVer = "1.7.2"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it will be revert later

@LuciferYang
Copy link
Contributor Author

Could you help check this change @HyukjinKwon @dongjoon-hyun @itholic @linhongliu-db

@LuciferYang
Copy link
Contributor Author

If there is no change in the java code, the java check-style will only be executed through maven. Am I right? @HyukjinKwon , this may be the reason why GA is still successful.

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine. cc @linhongliu-db to double check if you find some time.

@LuciferYang
Copy link
Contributor Author

@linhongliu-db Please help double check.Thanks ~

I will revert the changes of the sbt version after your check. And If we want to complete the sbt upgrade in this one, please also tell me, I need to update the pr description

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for investigating this plugin issue.

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine if it works

@srowen srowen closed this in 67d0dc8 Nov 3, 2022
@srowen
Copy link
Member

srowen commented Nov 3, 2022

Merged to master

@dongjoon-hyun
Copy link
Member

Thank you, @LuciferYang and @srowen , @HyukjinKwon .

Could you make a PR for SBT 1.7.3 again, @LuciferYang ?

@LuciferYang
Copy link
Contributor Author

Thank you, @LuciferYang and @srowen , @HyukjinKwon .

Could you make a PR for SBT 1.7.3 again, @LuciferYang ?

OK ~

SandishKumarHN pushed a commit to SandishKumarHN/spark that referenced this pull request Dec 12, 2022
…lve `dev/sbt-checkstyle` run failed with sbt 1.7.3

### What changes were proposed in this pull request?
This pr aims upgrade `sbt-checkstyle-plugin` to 4.0.0 to resolve `dev/sbt-checkstyle` run failed with sbt 1.7.3, the new version will check the generated source code, so some new suppression rules have been added to `dev/checkstyle-suppressions.xml`

### Why are the changes needed?

apache#38476 revert sbt 1.7.3 upgrade due to run `dev/sbt-checkstyle` failed:

```
[error] org.xml.sax.SAXParseException; lineNumber: 18; columnNumber: 10; DOCTYPE is disallowed when the feature "http://apache.org/xml/features/disallow-doctype-decl" set to true.
[error]         at com.sun.org.apache.xerces.internal.util.ErrorHandlerWrapper.createSAXParseException(ErrorHandlerWrapper.java:203)
[error]         at com.sun.org.apache.xerces.internal.util.ErrorHandlerWrapper.fatalError(ErrorHandlerWrapper.java:177)
[error]         at com.sun.org.apache.xerces.internal.impl.XMLErrorReporter.reportError(XMLErrorReporter.java:400)
[error]         at com.sun.org.apache.xerces.internal.impl.XMLErrorReporter.reportError(XMLErrorReporter.java:327)
[error]         at com.sun.org.apache.xerces.internal.impl.XMLScanner.reportFatalError(XMLScanner.java:1473)
[error]         at com.sun.org.apache.xerces.internal.impl.XMLDocumentScannerImpl$PrologDriver.next(XMLDocumentScannerImpl.java:914)
[error]         at com.sun.org.apache.xerces.internal.impl.XMLDocumentScannerImpl.next(XMLDocumentScannerImpl.java:602)
[error]         at com.sun.org.apache.xerces.internal.impl.XMLDocumentFragmentScannerImpl.scanDocument(XMLDocumentFragmentScannerImpl.java:505)
[error]         at com.sun.org.apache.xerces.internal.parsers.XML11Configuration.parse(XML11Configuration.java:842)
[error]         at com.sun.org.apache.xerces.internal.parsers.XML11Configuration.parse(XML11Configuration.java:771)
[error]         at com.sun.org.apache.xerces.internal.parsers.XMLParser.parse(XMLParser.java:141)
[error]         at com.sun.org.apache.xerces.internal.parsers.AbstractSAXParser.parse(AbstractSAXParser.java:1213)
[error]         at com.sun.org.apache.xerces.internal.jaxp.SAXParserImpl$JAXPSAXParser.parse(SAXParserImpl.java:643)
[error]         at com.sun.org.apache.xerces.internal.jaxp.SAXParserImpl.parse(SAXParserImpl.java:327)
[error]         at scala.xml.factory.XMLLoader.parse(XMLLoader.scala:73)
[error]         at scala.xml.factory.XMLLoader.loadXML(XMLLoader.scala:54)
[error]         at scala.xml.factory.XMLLoader.loadXML$(XMLLoader.scala:53)
[error]         at scala.xml.XML$.loadXML(XML.scala:62)
[error]         at scala.xml.factory.XMLLoader.loadString(XMLLoader.scala:92)
[error]         at scala.xml.factory.XMLLoader.loadString$(XMLLoader.scala:92)
[error]         at scala.xml.XML$.loadString(XML.scala:62)
[error]         at com.etsy.sbt.checkstyle.Checkstyle$.checkstyle(Checkstyle.scala:35)
[error]         at com.etsy.sbt.checkstyle.CheckstylePlugin$autoImport$.$anonfun$checkstyleTask$1(CheckstylePlugin.scala:36)
[error]         at com.etsy.sbt.checkstyle.CheckstylePlugin$autoImport$.$anonfun$checkstyleTask$1$adapted(CheckstylePlugin.scala:34)
[error]         at scala.Function1.$anonfun$compose$1(Function1.scala:49)
[error]         at sbt.internal.util.$tilde$greater.$anonfun$$u2219$1(TypeFunctions.scala:62)
[error]         at sbt.std.Transform$$anon$4.work(Transform.scala:68)
[error]         at sbt.Execute.$anonfun$submit$2(Execute.scala:282)
[error]         at sbt.internal.util.ErrorHandling$.wideConvert(ErrorHandling.scala:23)
[error]         at sbt.Execute.work(Execute.scala:291)
[error]         at sbt.Execute.$anonfun$submit$1(Execute.scala:282)
[error]         at sbt.ConcurrentRestrictions$$anon$4.$anonfun$submitValid$1(ConcurrentRestrictions.scala:265)
[error]         at sbt.CompletionService$$anon$2.call(CompletionService.scala:64)
[error]         at java.util.concurrent.FutureTask.run(FutureTask.java:266)
[error]         at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
[error]         at java.util.concurrent.FutureTask.run(FutureTask.java:266)
[error]         at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
[error]         at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
[error]         at java.lang.Thread.run(Thread.java:748)
```

This pr upgrade `sbt-checkstyle-plugin` to 4.0.0 to resolve the above issue, the `https://github.com/stringbean/sbt-checkstyle-plugin` was forked from etsy/sbt-checkstyle-plugin in 2022 after it became unmaintained, the release notes as follows:

- https://github.com/stringbean/sbt-checkstyle-plugin/releases/tag/3.2.0

- https://github.com/stringbean/sbt-checkstyle-plugin/releases/tag/3.3.0

- https://github.com/stringbean/sbt-checkstyle-plugin/releases/tag/v4.0.0

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
- Pass GitHub Actions
- Manual test `dev/sbt-checkstyle` with sbt 1.7.3 and this pr: `Checkstyle checks passed.`

Closes apache#38481 from LuciferYang/173-pass-checkstyle.

Authored-by: yangjie01 <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
allisonport-db added a commit to delta-io/delta that referenced this pull request May 3, 2024
<!--
Thanks for sending a pull request!  Here are some tips for you:
1. If this is your first time, please read our contributor guidelines:
https://github.com/delta-io/delta/blob/master/CONTRIBUTING.md
2. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]
Your PR title ...'.
  3. Be sure to keep the PR description updated to reflect all changes.
  4. Please write your PR title to summarize what this PR proposes.
5. If possible, provide a concise example to reproduce the issue for a
faster review.
6. If applicable, include the corresponding issue number in the PR title
and link it in the body.
-->

#### Which Delta project/connector is this regarding?
<!--
Please add the component selected below to the beginning of the pull
request title
For example: [Spark] Title of my pull request
-->

- [ ] Spark
- [ ] Standalone
- [ ] Flink
- [ ] Kernel
- [X] Other (fill in here)

## Description

#2828 upgrades the SBT version
from 1.5.5 to 1.9.9 which causes `projectName/checkstyle` to fail with
```
sbt:delta> kernelApi/checkstyle
[error] stack trace is suppressed; run last kernelApi / checkstyle for the full output
[error] (kernelApi / checkstyle) org.xml.sax.SAXParseException; lineNumber: 18; columnNumber: 10; DOCTYPE is disallowed when the feature "http://apache.org/xml/features/disallow-doctype-decl" set to true.
[error] Total time: 0 s, completed May 1, 2024 2:59:48 PM
```

This failure was silent in our CI runs for some reason, if you search
the logs before that commit you can see "checkstyle" in them but no
instances after. This is a little concerning but don't really have time
to figure out why this was silent.

For now, upgrades versions to match Spark's current plugins which fixes
the issue. See the matching Spark PR here
apache/spark#38481.

## How was this patch tested?

Ran `kernelApi/checkstyle` locally.
TODO: verify it's present in the CI runs after as well

## Does this PR introduce _any_ user-facing changes?

No.
allisonport-db added a commit to allisonport-db/delta that referenced this pull request May 4, 2024
…#3019)

<!--
Thanks for sending a pull request!  Here are some tips for you:
1. If this is your first time, please read our contributor guidelines:
https://github.com/delta-io/delta/blob/master/CONTRIBUTING.md
2. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]
Your PR title ...'.
  3. Be sure to keep the PR description updated to reflect all changes.
  4. Please write your PR title to summarize what this PR proposes.
5. If possible, provide a concise example to reproduce the issue for a
faster review.
6. If applicable, include the corresponding issue number in the PR title
and link it in the body.
-->

#### Which Delta project/connector is this regarding?
<!--
Please add the component selected below to the beginning of the pull
request title
For example: [Spark] Title of my pull request
-->

- [ ] Spark
- [ ] Standalone
- [ ] Flink
- [ ] Kernel
- [X] Other (fill in here)

## Description

delta-io#2828 upgrades the SBT version
from 1.5.5 to 1.9.9 which causes `projectName/checkstyle` to fail with
```
sbt:delta> kernelApi/checkstyle
[error] stack trace is suppressed; run last kernelApi / checkstyle for the full output
[error] (kernelApi / checkstyle) org.xml.sax.SAXParseException; lineNumber: 18; columnNumber: 10; DOCTYPE is disallowed when the feature "http://apache.org/xml/features/disallow-doctype-decl" set to true.
[error] Total time: 0 s, completed May 1, 2024 2:59:48 PM
```

This failure was silent in our CI runs for some reason, if you search
the logs before that commit you can see "checkstyle" in them but no
instances after. This is a little concerning but don't really have time
to figure out why this was silent.

For now, upgrades versions to match Spark's current plugins which fixes
the issue. See the matching Spark PR here
apache/spark#38481.

## How was this patch tested?

Ran `kernelApi/checkstyle` locally.
TODO: verify it's present in the CI runs after as well

## Does this PR introduce _any_ user-facing changes?

No.

(cherry picked from commit 12cabb7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants