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

Replayed relevant changes from scaladoc3. #611

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Expand Up @@ -399,7 +399,7 @@
<attach>false</attach>
<doclint>-missing</doclint>
<links>
<link>http://java.sun.com/j2se/${maven.compiler.source}/docs/api/</link>
<link>https://docs.oracle.com/javase/8/docs/api/</link>
<link>http://slf4j.org/api/</link>
<link>http://commons.apache.org/lang/api-release/</link>
<link>http://commons.apache.org/io/api-release/</link>
Expand Down
1 change: 1 addition & 0 deletions src/it/test_scaladoc3/invoker.properties
@@ -0,0 +1 @@
invoker.goals=clean package
113 changes: 113 additions & 0 deletions src/it/test_scaladoc3/pom.xml
@@ -0,0 +1,113 @@
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<groupId>it.scala-maven-plugin</groupId>
<artifactId>scaladoc-jar-test</artifactId>
<version>1.0-SNAPSHOT</version>
<name>${project.artifactId}</name>

<properties>
<maven.compiler.source>11</maven.compiler.source>
<maven.compiler.target>11</maven.compiler.target>
<encoding>UTF-8</encoding>
<scala-maven-plugin.version>4.6.3-EXPERIMENTAL</scala-maven-plugin.version>
<displayCmd>false</displayCmd>
</properties>

<build>
<sourceDirectory>src/main/scala</sourceDirectory>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-source-plugin</artifactId>
<version>3.2.1</version>
<executions>
<execution>
<id>attach-sources</id>
<goals>
<goal>jar-no-fork</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>

<dependencies>
<dependency>
<groupId>de.christofreichardt</groupId>
<artifactId>tracelogger</artifactId>
<version>1.9.0</version>
</dependency>
</dependencies>

<profiles>
<profile>
<id>scala3</id>
<activation>
<activeByDefault>true</activeByDefault>
</activation>
<build>
<plugins>
<plugin>
<groupId>@project.groupId@</groupId>
<artifactId>@project.artifactId@</artifactId>
<version>@project.version@</version>
<executions>
<execution>
<id>Scalac</id>
<goals>
<goal>compile</goal>
<goal>testCompile</goal>
<goal>add-source</goal>
</goals>
<configuration>
<args>
<arg>-deprecation</arg>
<arg>-release:11</arg>
<arg>-encoding</arg>
<arg>utf-8</arg>
</args>
</configuration>
</execution>
<execution>
<id>Scaladoc-jar</id>
<goals>
<goal>doc-jar</goal>
</goals>
<phase>package</phase>
<configuration>
<displayCmd>true</displayCmd>
<jvmArgs>
<jvmArg>-Xms64m</jvmArg>
<jvmArg>-Xmx1024m</jvmArg>
</jvmArgs>
<args>
<arg>-doc-footer</arg>
<arg>Scala Maven Plugin - Scaladoc for Scala 3</arg>
<arg>-doc-title</arg>
<arg>scaladoc-jar-test</arg>
<arg>-doc-version</arg>
<arg>${project.version}</arg>
<arg>-author</arg>
</args>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
<dependencies>
<dependency>
<groupId>org.scala-lang</groupId>
<artifactId>scala3-library_3</artifactId>
<version>3.1.2</version>
</dependency>
</dependencies>
</profile>
</profiles>

</project>
@@ -0,0 +1,43 @@
/*
* This is free and unencumbered software released into the public domain.
* See UNLICENSE.
*/
package xyz.diagnosis

import de.christofreichardt.diagnosis.TracerFactory
import de.christofreichardt.diagnosis.AbstractTracer

/**
* Contains some add-ons for the <a href="http://www.christofreichardt.de/Projektstudien/TraceLogger/index.html">TraceLogger</a> library.
*
* @author Christof Reichardt
*/
trait Tracing {
/**
* Custom control structure for tracing of embraced code blocks.
*
* @param resultType denotes the return type
* @param callee the call site
* @param method denotes the method signature
* @param block the embraced code block
* @tparam T the actual type of the embraced code block
* @return returns whatever block returns
*/
def withTracer[T](resultType: String, callee: AnyRef, method: String)(block: => T): T = {
val tracer = getCurrentTracer()
tracer.entry(resultType, callee, method)
try {
block
}
finally {
tracer.wayout()
}
}

/**
* Returns the present tracer for this object.
*
* @return the current tracer, by default the NullTracer
*/
def getCurrentTracer(): AbstractTracer = TracerFactory.getInstance().getDefaultTracer()
}
@@ -0,0 +1,11 @@
/*
* This is free and unencumbered software released into the public domain.
* See UNLICENSE.
*/
package xyz

/**
* Contains an utility useful for debugging and diagnosis.
*/
package object diagnosis {
}
63 changes: 63 additions & 0 deletions src/it/test_scaladoc3/src/main/scala/xyz/math/Polynomial.scala
@@ -0,0 +1,63 @@
/*
* This is free and unencumbered software released into the public domain.
* See UNLICENSE.
*/
package xyz.math

import de.christofreichardt.diagnosis.{AbstractTracer, TracerFactory}
import xyz.diagnosis.Tracing

/**
* Defines a polynomial in its canonical form.
*
* @author Christof Reichardt
*
* @constructor
* @param coefficients the polynoms coefficients
* @param prime the prime modulus
*/
class Polynomial(
val coefficients: IndexedSeq[BigInt],
val prime: BigInt)
extends Tracing {

require(prime.isProbablePrime(100))

/** the remaining coefficients while dropping leading zeros */
val a: Seq[BigInt] = coefficients.dropWhile(c => c == BigInt(0))
/** the degree of the polynomial */
val degree: Int = a.length - 1
/** indicates the zero polynomial */
val isZero: Boolean = degree == -1

/**
* Computes y = P(x).
*
* @param x the x value
* @return the y value
*/
def evaluateAt(x: BigInt): BigInt = {
withTracer("BigInt", this, "evaluateAt(x: BigInt)") {
val tracer = getCurrentTracer()
tracer.out().printfIndentln("x = %s", x)
if (isZero) BigInt(0)
else {
Range.inclusive(0, degree)
.map(i => {
tracer.out().printfIndentln("a(%d) = %s", degree - i: Integer, a(i))
(a(i) * x.modPow(degree - i, prime)).mod(prime)
})
.foldLeft(BigInt(0))((t0, t1) => (t0 + t1).mod(prime))
}
}
}

/**
* Returns a string representation of the Polynomial.
*
* @return the string representation of the Polynomial
*/
override def toString = String.format("Polynomial[a=(%s), degree=%d, isZero=%b, prime=%s]", a.mkString(","), degree: Integer, isZero: java.lang.Boolean, prime)

override def getCurrentTracer(): AbstractTracer = TracerFactory.getInstance().getDefaultTracer
}
slandelle marked this conversation as resolved.
Show resolved Hide resolved
11 changes: 11 additions & 0 deletions src/it/test_scaladoc3/validate.groovy
@@ -0,0 +1,11 @@
try {

slandelle marked this conversation as resolved.
Show resolved Hide resolved
def file = new File(basedir, 'target/scaladoc-jar-test-1.0-SNAPSHOT-javadoc.jar')
assert file.exists()
slandelle marked this conversation as resolved.
Show resolved Hide resolved

return true

} catch(Throwable e) {
e.printStackTrace()
return false
}
30 changes: 26 additions & 4 deletions src/main/java/scala_maven/ScalaDocMojo.java
Expand Up @@ -6,6 +6,7 @@

import java.io.File;
import java.util.*;
import org.apache.maven.artifact.Artifact;
import org.apache.maven.plugins.annotations.Execute;
import org.apache.maven.plugins.annotations.LifecyclePhase;
import org.apache.maven.plugins.annotations.Mojo;
Expand All @@ -17,6 +18,7 @@
import org.codehaus.plexus.util.StringUtils;
import scala_maven_dependency.Context;
import scala_maven_executions.JavaMainCaller;
import scala_maven_executions.ScalaDoc3Caller;
import util.FileUtils;

/** Produces Scala API documentation. */
Expand Down Expand Up @@ -149,11 +151,25 @@ public void doExecute() throws Exception {
generate(null, Locale.getDefault());
}

void addScalaDocToClasspath(Set<File> classpath) throws Exception {
Context sc = findScalaContext();
for (Artifact dep : sc.findScalaDocAndDependencies()) {
classpath.add(dep.getFile());
}
}

protected JavaMainCaller getScalaCommand() throws Exception {
// This ensures we have a valid scala version...
checkScalaVersion();
Context sc = findScalaContext();
JavaMainCaller jcmd = getEmptyScalaCommand(sc.apidocMainClassName(scaladocClassName));
String apidocMainClassName = sc.apidocMainClassName(scaladocClassName);
JavaMainCaller jcmd;
if (sc.version().major < 3) {
jcmd = getEmptyScalaCommand(apidocMainClassName);
} else {
String targetClassesDir = project.getModel().getBuild().getOutputDirectory();
jcmd = new ScalaDoc3Caller(this, apidocMainClassName, targetClassesDir);
}
Comment on lines +165 to +172
Copy link
Owner

Choose a reason for hiding this comment

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

The original purpose of Context (and findScalaContext) is to avoid / to remove every if scalaVersion ... over the code. Can you try to keep this approach ?

Copy link
Author

Choose a reason for hiding this comment

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

I tried to used getEmptyScalaCommand() but I couldn't get it working. In this case, the invocation of dotty.tools.scaladoc.Main raises a NullPointerException:

java.lang.NullPointerException: Cannot invoke "java.lang.ClassLoader.getResourceAsStream(String)" because the return value of "java.lang.Class.getClassLoader()" is null
        at dotty.tools.scaladoc.renderers.Resources.renderResource(Resources.scala:213)
        at dotty.tools.scaladoc.renderers.Resources.renderResource$(Resources.scala:26)
        at dotty.tools.scaladoc.renderers.Renderer.renderResource(Renderer.scala:27)
        at dotty.tools.scaladoc.renderers.HtmlRenderer.renderResources$$anonfun$1(HtmlRenderer.scala:60)
        at scala.collection.immutable.List.flatMap(List.scala:293)
        at scala.collection.immutable.List.flatMap(List.scala:79)
        at dotty.tools.scaladoc.renderers.HtmlRenderer.renderResources(HtmlRenderer.scala:60)
        at dotty.tools.scaladoc.renderers.HtmlRenderer.render(HtmlRenderer.scala:31)
        at dotty.tools.scaladoc.Scaladoc$.run(Scaladoc.scala:249)
        at dotty.tools.scaladoc.Scaladoc$.run$$anonfun$1(Scaladoc.scala:84)
        at scala.runtime.function.JProcedure1.apply(JProcedure1.java:15)
        at scala.runtime.function.JProcedure1.apply(JProcedure1.java:10)
        at scala.Option.map(Option.scala:242)
        at dotty.tools.scaladoc.Scaladoc$.run(Scaladoc.scala:88)
        at dotty.tools.scaladoc.Main.run(Main.scala:18)
        at dotty.tools.scaladoc.Main$.main(Main.scala:24)
        at dotty.tools.scaladoc.Main.main(Main.scala)
        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
        at java.base/java.lang.reflect.Method.invoke(Method.java:577)
        at scala_maven_executions.MainHelper.runMain(MainHelper.java:157)
        at scala_maven_executions.MainWithArgsInFile.main(MainWithArgsInFile.java:30)

I tried to shift things from the bootclasspath to the normal classpath and some other things but without success. IMHO you should consider the removal of the Apache Commons Exec library in favor of ProcessBuilder and related classes from the JDK in the medium term. The Exec library hasn't seen any updates in years whereas ProcessBuilder and related classes have seen some improvements. I might be wrong about this but if someone upstream (Scala compiler etc.) decides that the usage of java.lang.Class.getClassLoader() might come in handy you might run into a similar issue as shown above.

The error above can be reproduced with the branch classloader-issue of the forked repo. This branch isn't merged but git diff master..classloader-issue gives you the relevant changes.

Since we are raising external processes the outcome depends very much on how well the environment has been prepared. But, I don't think the classloader issue has something to do with the environment since the ProcessBuilder works well enough.

I have used the interface JavaMainCaller to hide the internal details of the process invocation.

jcmd.addArgs(args);
jcmd.addJvmArgs(jvmArgs);
addCompilerPluginOptions(jcmd);
Expand All @@ -171,7 +187,11 @@ protected JavaMainCaller getScalaCommand() throws Exception {
.getOutputDirectory())); // remove output to avoid "error for" : error: XXX is
// already defined as package XXX ... object XXX {
addAdditionalDependencies(paths);
if (!paths.isEmpty()) jcmd.addOption("-classpath", FileUtils.toMultiPath(paths));
addScalaDocToClasspath(paths);

if (!paths.isEmpty()) {
jcmd.addOption("-classpath", FileUtils.toMultiPath(paths));
}
// jcmd.addOption("-sourcepath", sourceDir.getAbsolutePath());

jcmd.addArgs("-doc-format:html");
Expand All @@ -196,8 +216,10 @@ public void generate(Sink sink, Locale locale) throws MavenReportException {
if (sources.size() > 0) {
JavaMainCaller jcmd = getScalaCommand();
jcmd.addOption("-d", reportOutputDir.getAbsolutePath());
for (File x : sources) {
jcmd.addArgs(FileUtils.pathOf(x, useCanonicalPath));
if (this.scalaContext.version().major < 3) {
for (File x : sources) {
jcmd.addArgs(FileUtils.pathOf(x, useCanonicalPath));
}
}
jcmd.run(displayCmd);
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/scala_maven/ScalaMojoSupport.java
Expand Up @@ -219,7 +219,7 @@ public MavenArtifactResolver findMavenArtifactResolver() {
return mavenArtifactResolver;
}

private Context scalaContext;
protected Context scalaContext;

public Context findScalaContext() throws Exception {
// reuse/lazy scalaContext creation (doesn't need to be Thread safe, scalaContext should be
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/scala_maven_dependency/ArtifactIds.java
Expand Up @@ -16,6 +16,8 @@ public interface ArtifactIds {

String scalaCompilerArtifactId() throws Exception;

String scalaDocArtifactId() throws Exception;

String compilerMainClassName(boolean useFsc) throws Exception;

String consoleMainClassName() throws Exception;
Expand Down
Expand Up @@ -34,6 +34,11 @@ public String scalaCompilerArtifactId() throws Exception {
return SCALA_COMPILER_ARTIFACTID;
}

@Override
public String scalaDocArtifactId() throws Exception {
return null;
}

public String compilerMainClassName(boolean useFsc) throws Exception {
return useFsc ? "scala.tools.nsc.CompileClient" : "scala.tools.nsc.Main";
}
Expand Down
Expand Up @@ -11,6 +11,7 @@
public class ArtifactIds4Scala3 implements ArtifactIds {
protected static final String SCALA_LIBRARY_ARTIFACTID = "scala3-library";
protected static final String SCALA_COMPILER_ARTIFACTID = "scala3-compiler";
protected static final String SCALA_DOC_ARTIFACTID = "scaladoc";
static final List<String> SCALA_DISTRO_ARTIFACTS =
Arrays.asList(
SCALA_LIBRARY_ARTIFACTID, "scala3-swing", "scala3-dbc", SCALA_COMPILER_ARTIFACTID);
Expand Down Expand Up @@ -49,6 +50,11 @@ public String scalaCompilerArtifactId() throws Exception {
return getScala3ArtifactId(SCALA_COMPILER_ARTIFACTID);
}

@Override
public String scalaDocArtifactId() throws Exception {
return getScala3ArtifactId(SCALA_DOC_ARTIFACTID);
}

public String compilerMainClassName(boolean useFsc) throws Exception {
return "dotty.tools.dotc.Main";
}
Expand All @@ -59,6 +65,6 @@ public String consoleMainClassName() throws Exception {
}

public String apidocMainClassName() throws Exception {
return "dotty.tools.dotc.Main";
return "dotty.tools.scaladoc.Main";
}
}
2 changes: 2 additions & 0 deletions src/main/java/scala_maven_dependency/Context.java
Expand Up @@ -19,6 +19,8 @@ public interface Context {

Set<Artifact> findCompilerAndDependencies() throws Exception;

Set<Artifact> findScalaDocAndDependencies() throws Exception;

String compilerMainClassName(String override, boolean useFsc) throws Exception;

String consoleMainClassName(String override) throws Exception;
Expand Down