Skip to content

Add missing scalac options to scaladoc task configuration #6499

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 31, 2021
Merged

Add missing scalac options to scaladoc task configuration #6499

merged 2 commits into from
May 31, 2021

Conversation

pikinier20
Copy link
Contributor

Related to: #12298

@lightbend-cla-validator
Copy link

Hi @pikinier20,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

https://www.lightbend.com/contribute/cla

@pikinier20 pikinier20 mentioned this pull request May 7, 2021
Merged
@adpi2
Copy link
Member

adpi2 commented May 7, 2021 •
edited
Loading

Thanks @pikinier20! Can you give us more details about how scaladoc handles unused scalac options?

From what I know, scaladoc 3.0.0 is still printing a lot of warnings because of unused options. So maybe we should wait for 3.0.1 before merging. What do you think?

@pikinier20
Copy link
Contributor Author

pikinier20 commented May 7, 2021 •
edited
Loading

Thanks @pikinier20! Can you give us more details about how scaladoc handles unused scalac options?

From what I know, scaladoc 3.0.0 is still printing a lot of warnings because of unused options. So maybe we should wait for 3.0.1 before merging. What do you think?

Yes, I think it should be released after release with my PR to dotty, because we are gonna have warns.

My PR to dotty just filters scalacOptions and doesn't print warns for standard scala options.

@smarter
Copy link
Contributor

smarter commented May 23, 2021

I think merging and releasing an sbt with this PR in as soon as possible would be helpful since right now many people cannot publish their scala.js project because doc generation fails (scala/scala3#11943), and the only workaround we can give them is to disable doc generation, when I'd rather tell them to try a dotty nightly with the latest sbt to ensure that this setup works correctly.

@smarter smarter mentioned this pull request May 23, 2021
Closed
@adpi2
Copy link
Member

adpi2 commented May 25, 2021

This fix will generate a lot of warnings on all Scala 3.0.0 projects (not only Scala.js):

sbt:root> kernelJVM / doc
[info] compiling 35 Scala sources to /home/piquerez/typelevel/cats-effect/kernel/jvm/target/scala-3.0.0/classes ...
[info] Main Scala API documentation to /home/piquerez/typelevel/cats-effect/kernel/jvm/target/scala-3.0.0/api...
[warn] bad option '-deprecation' was ignored
[warn] bad option '-feature' was ignored
[warn] bad option '-unchecked' was ignored
[warn] bad option '-language:implicitConversions' was ignored
[warn] bad option '-Ykind-projector' was ignored
[warn] bad option '-source:3.0-migration' was ignored
[warn] bad option '-Xsemanticdb' was ignored
[warn] bad option '-semanticdb-target' was ignored
[warn] bad option '-doc-external-doc:/home/piquerez/.cache/coursier/v1/https/repo1.maven.org/maven2/org/typelevel/cats-core_3/2.6.1/cats-core_3-2.6.1.jar#http://typelevel.org/cats/api/,/home/piquerez/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.5/scala-library-2.13.5.jar#https://www.scala-lang.org/api/2.13.5/,/home/piquerez/.cache/coursier/v1/https/repo1.maven.org/maven2/org/typelevel/cats-kernel_3/2.6.1/cats-kernel_3-2.6.1.jar#http://typelevel.org/cats/api/' was ignored
[warn] Setting -bootclasspath is currently not supported.
[warn] Setting -sourcepath is currently not supported.
[warn] 11 warnings found
[info] Main Scala API documentation successful.
[success] Total time: 42 s, completed May 25, 2021 9:35:51 AM

If people want to try a nightly version of scaladoc with Scala.js, they can do:

Compile / doc / scalacOptions ++= (Compile / scalacOptions).value

@pikinier20 maybe you could add a scripted test in this PR to check that this fix works with a nighlty version of Scala 3.0.1?

@smarter
Copy link
Contributor

smarter commented May 25, 2021

This fix will generate a lot of warnings on all Scala 3.0.0 projects (not only Scala.js):

I understand that, but I think that warnings all the time are better than crashes some of the time :).

@russwyte
Copy link

This fix will generate a lot of warnings on all Scala 3.0.0 projects (not only Scala.js):

I understand that, but I think that warnings all the time are better than crashes some of the time :).

Agreed this has been blocking several releases of libraries migrating to Scala 3

@eed3si9n
Copy link
Member

Having unactionable warnings to all JS users would be pretty bad UX. If Scaladoc also respect fatal warnings wouldn't it also result to occasional errors? Is it possible to maybe filter in only the necessary scalacOptions only when Scala version is 3.0.0 or something?

smarter
smarter reviewed May 27, 2021
View reviewed changes
main/src/main/scala/sbt/Defaults.scala
@@ -2107,7 +2107,11 @@ object Defaults extends BuildCommon {
val projectName = name.value
if (ScalaArtifacts.isScala3(sv)) {
val project = if (config == Compile) projectName else s"$projectName-$config"
Seq("-project", project)
if (scalaVersion.value.startsWith("3.0.0")) {
Seq("-project", project)
Copy link
Contributor

Choose a reason for hiding this comment

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

We could still pass -scalajs here to avoid most issues without introducing warnings:

Suggested change
Seq("-project", project)
compileOptions.filter(_ == "-scalajs") ++ Seq("-project", project)

Copy link
Member

Choose a reason for hiding this comment

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

Is scaladoc 3.0.0 able to use -scalajs? I think it was failing in cats-effect repo even with -scalajs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's true. We didn't handle -scalajs in 3.0.0.

adpi2
adpi2 approved these changes May 27, 2021
View reviewed changes
Copy link
Member

@adpi2 adpi2 left a comment

Choose a reason for hiding this comment

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

Thanks a lot @pikinier20!

I think this can be merged in 1.5 and released before Scala 3.0.1 goes out.

wbillingsley added a commit to wbillingsley/veautiful that referenced this pull request May 29, 2021
@wbillingsley
Temporarily disable scaladoc generation due to bug in sbt
c0eaf67 
At the moment, sbt fails to pass a -scalajs compiler option flag to scaladoc, causing an error reading the TASTY file of scala-js-dom. This appears to be related to
sbt/sbt#6499
which will be included in a later sbt release
@eed3si9n eed3si9n merged commit f2f8844 into sbt:develop May 31, 2021
@eed3si9n eed3si9n added this to the 1.5.3 milestone May 31, 2021
@SethTisue SethTisue mentioned this pull request Mar 27, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adpi2 adpi2 adpi2 approved these changes

+1 more reviewer

@smarter smarter smarter approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.5.3
Development

Successfully merging this pull request may close these issues.
6 participants
@pikinier20 @lightbend-cla-validator @adpi2 @smarter @russwyte @eed3si9n