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

build issue using Scala-CLI 1.6.1 (Apache Pekko build) #3464

Closed
pjfanning opened this issue Jan 28, 2025 · 11 comments
Closed

build issue using Scala-CLI 1.6.1 (Apache Pekko build) #3464

pjfanning opened this issue Jan 28, 2025 · 11 comments
Assignees
Labels
invalid This doesn't seem right using directives Issues tied with using directives.

Comments

@pjfanning
Copy link

pjfanning commented Jan 28, 2025

Version(s)

Scala CLI version: 1.6.1
Scala version (default): 3.6.3

Describe the bug
Pekko build worked until CI started picking up Scala-CLI 1.6.1 (1.5.x was working for us).

Issue is at apache/pekko#1751

To Reproduce

Expected behaviour

Any help would be appreciated. Is there even a way to pin to the CI job to use an older version of Scala CLI?

@pjfanning pjfanning added the bug Something isn't working label Jan 28, 2025
@pjfanning
Copy link
Author

I'm going to start by trying to get https://github.com/VirtusLab/scala-cli-setup approved by ASF so that we can use that and hopefully try to use an older version of Scala-CLI that doesn't have this issue.

@Gedochao Gedochao added the requires scoping Issue requires a spike to revalidate it and assign an up-to date scope for its requirements. label Jan 29, 2025
@Gedochao Gedochao self-assigned this Jan 29, 2025
@Gedochao
Copy link
Contributor

Is there even a way to pin to the CI job to use an older version of Scala CLI?

As for your CI, the best way would be to use https://github.com/VirtusLab/scala-cli-setup and then set the version param.

If that is not an option, any Scala CLI launcher can switch to an earlier version with the --cli-version launcher arg.
Just keep in mind that with the --cli-version arg it will be much slower.
https://scala-cli.virtuslab.org/docs/commands/basics#scala-cli-version

scala-cli version
# Scala CLI version: 1.6.1
# Scala version (default): 3.6.3
scala-cli --cli-version 1.5.4 version
# Scala CLI version: 1.5.4
# Scala version (default): 3.5.2

@Gedochao
Copy link
Contributor

@pjfanning So your build seems to fail on this script:
https://github.com/apache/pekko/blob/main/.github/workflows/verify-jdk9-classes.sh
It seems to run a bunch of *.sc scripts and from what I understand, fails on one of them when bumped to Scala CLI 1.6.1.

Now, I don't know the content of those .sc scripts, but it seems to me you are running them with the default version of Scala 3, which changed from 3.5.2 to 3.6.3 between the versions you mention.
Looking at the error, my guess is that it's the compiler behaviour that has changed, and not Scala CLI, but that's just a guess.

You can try to pin the Scala version for those scripts and see if that fixes it for now.
https://github.com/apache/pekko/blob/b731dfd87f46efc326dc7e6e1b562fae50f89173/.github/workflows/verify-jdk9-classes.sh#L27
My guess is that you need to change this line to:

if scala-cli -S 3.5.2 "$file" ; then

I suspect that will fix your CI (do get back and let us know if that is the case).

Now, as for the actual change of behaviour, I need a reproduction of the bug to be able to say more.
Can I take a look at the *.sc scripts that are being compiled somewhere? (in particular, the one that's failing)

@Gedochao Gedochao added needs reproduction and removed requires scoping Issue requires a spike to revalidate it and assign an up-to date scope for its requirements. labels Jan 29, 2025
@pjfanning
Copy link
Author

@Gedochao thanks for the quick response.

The script that fails is generated is derived using

https://github.com/apache/pekko/blob/b731dfd87f46efc326dc7e6e1b562fae50f89173/project/VerifyJDK9Classes.scala#L42-L82

The scala version should be 3.3.4 and the pekko version is the version of a local build stored to MavenLocal.

A released version of pekko like 1.1.3 may reproduce this issue too.

The only thing that has change recently was the upgrade of Scala CLI as opposed to the VerifyJDK9Classes job changing.

@Gedochao
Copy link
Contributor

Gedochao commented Jan 29, 2025

The scala version should be 3.3.4 and the pekko version is the version of a local build stored to MavenLocal.

//> using scala 3.3.4
//> using dep "org.apache.pekko::pekko-stream:1.1.3"
object VerifyJDK9Classes {
  def main(args: Array[String]): Unit = {
    import org.apache.pekko.actor.ActorSystem
    import org.apache.pekko.stream.scaladsl.{ JavaFlowSupport, Source }

    import java.lang.System.exit
    import scala.concurrent.Await
    import scala.concurrent.duration.DurationInt
    implicit val system: ActorSystem = ActorSystem.create("test")
    val future = Source(1 to 3).runWith(
      JavaFlowSupport.Sink.asPublisher[Int](fanout = false).mapMaterializedValue { p =>
        JavaFlowSupport.Source.fromPublisher(p).runFold(0)(_ + _)
      })

    val result = Await.result(future, 3.seconds)
    println(s"Result:" + result)
    system.terminate()
    exit(if (result == 6) 0 else 1)
  }
}

@pjfanning so this seems to work fine on my local machine (MacOS M1), and not just for Scala 3.3.4, but for 3.5.2 and 3.6.3, as well... not sure if it's because of the 1.1.3 version.
I checked for --jvm 11, too. No difference.
Can you check with the local build for pekko-stream?

@Gedochao
Copy link
Contributor

Gedochao commented Jan 29, 2025

BTW, looking at the logs from apache/pekko#1751:

Compiling project (Scala 3.6.3, JVM (11))
[error] ./stream/target/scala-cli/VerifyJDK9Classes-2.12.sc:28:26
[error] Cyclic reference involving val <import>
[error] 
[error]  Run with -explain-cyclic for more details.
[error]     implicit val system: ActorSystem = ActorSystem.create("test")
[error]                          ^
[error] ./stream/target/scala-cli/VerifyJDK9Classes-2.12.sc:22:12
[error] value apache is not a member of org
[error]     import org.apache.pekko.actor.ActorSystem
[error]            ^^^^^^^^^^
[error] ./stream/target/scala-cli/VerifyJDK9Classes-2.12.sc:23:12
[error] value apache is not a member of org
[error]     import org.apache.pekko.stream.scaladsl.{ JavaFlowSupport, Source }
[error]            ^^^^^^^^^^
Error compiling project (Scala 3.6.3, JVM (11))
Compilation failed
Error when verifying stream/target/scala-cli/VerifyJDK9Classes-2.12.sc.
Error: Process completed with exit code 1.

@pjfanning So this is becoming increasingly suspicious.
Are you sure the directives are there for the generated script?
It's running with Scala 3.6.3, so the launcher default, as if there was no //> using scala 3.3.4 directive there.
Additionally, the org.apache.pekko.actor.* classes aren't on the classpath, which implies //> using dep "org.apache.pekko::pekko-stream:1.1.3" is not there, either.
Can you log the actual script code on the CI, so that we can confirm with 100% certainty what is being run there?

@pjfanning
Copy link
Author

The script is

/*
 * Licensed to the Apache Software Foundation (ASF) under one or more
 * contributor license agreements. See the NOTICE file distributed with
 * this work for additional information regarding copyright ownership.
 * The ASF licenses this file to You 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.
 */

//> using scala 3
//> using dep "org.apache.pekko::pekko-stream:1.2.0-M0+71-6a42f4e8-SNAPSHOT"
object VerifyJDK9Classes {
  def main(args: Array[String]): Unit = {
    import org.apache.pekko.actor.ActorSystem
    import org.apache.pekko.stream.scaladsl.{ JavaFlowSupport, Source }

    import java.lang.System.exit
    import scala.concurrent.Await
    import scala.concurrent.duration.DurationInt
    implicit val system: ActorSystem = ActorSystem.create("test")
    val future = Source(1 to 3).runWith(
      JavaFlowSupport.Sink.asPublisher[Int](fanout = false).mapMaterializedValue { p =>
        JavaFlowSupport.Source.fromPublisher(p).runFold(0)(_ + _)
      })

    val result = Await.result(future, 3.seconds)
    println(s"Result:" + result)
    system.terminate()
    exit(if (result == 6) 0 else 1)
  }
}

Note the license. The script is also generated with scala 3 as opposed to full version.

@Gedochao
Copy link
Contributor

Note the license.

@pjfanning Ah, right.
I see what is going on here.

As per the using directive specification, using directives need to be on top of a file to be accepted. If they occur after Scala code, they are interpreted as standard comments.
Now, /*..*/ comments used to be accepted as comments for the using directives parser, but they were indeed removed in v1.6.0:

This means the license code has to be moved to after the directives section, like this:

//> using scala 3
//> using dep "org.apache.pekko::pekko-stream:1.2.0-M0+71-6a42f4e8-SNAPSHOT"
/*
 * Licensed to the Apache Software Foundation (ASF) under one or more
 * contributor license agreements. See the NOTICE file distributed with
 * this work for additional information regarding copyright ownership.
 * The ASF licenses this file to You 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.
 */
object VerifyJDK9Classes {
  def main(args: Array[String]): Unit = {
    import org.apache.pekko.actor.ActorSystem
    import org.apache.pekko.stream.scaladsl.{ JavaFlowSupport, Source }

    import java.lang.System.exit
    import scala.concurrent.Await
    import scala.concurrent.duration.DurationInt
    implicit val system: ActorSystem = ActorSystem.create("test")
    val future = Source(1 to 3).runWith(
      JavaFlowSupport.Sink.asPublisher[Int](fanout = false).mapMaterializedValue { p =>
        JavaFlowSupport.Source.fromPublisher(p).runFold(0)(_ + _)
      })

    val result = Await.result(future, 3.seconds)
    println(s"Result:" + result)
    system.terminate()
    exit(if (result == 6) 0 else 1)
  }
}

This is technically not a bug, but we could potentially consider accepting /*..*/ comments before directives, even if that's not matching the current spec... No promises, though. Feel free to raise a feature request, separately.
They used to be accepted within the directives itself, which led to abominations like:

//> using /* comment1 */ key /* comment2 */ value

And that we do not want.

The script is also generated with scala 3 as opposed to full version.

Please note that //> using scala 3 actually leads to the default Scala Next version being used, as opposed to LTS (so it's actually expected for 3.6.3) to be used here.
Also, this used to lead to the newest Scala 3 Next to be used overall, but has been changed to the default in v1.6.0, as per:

So TL;DR, the code would be compiled with Scala 3.6.3, regardless if Scala CLI v1.5.4 (since it's the currently newest Scala 3 Next version) or v1.6.1 (since that's the default there) was used.

Hope this helps.

@Gedochao Gedochao added invalid This doesn't seem right using directives Issues tied with using directives. and removed bug Something isn't working needs reproduction labels Jan 29, 2025
@pjfanning
Copy link
Author

In my local testing, using scala-cli --cli-version 1.5.4 seems to stop me getting the import errors. The scala version doesn't seem to be issue. It appears like the scala-cli version that matters.

@Gedochao
Copy link
Contributor

In my local testing, using scala-cli --cli-version 1.5.4 seems to stop me getting the import errors. The scala version doesn't seem to be issue. It appears like the scala-cli version that matters.

@pjfanning Yep. That's because of the /*..*/ comments, as noted in my previous comment.
For Scala CLI v1.6.0+, using directives have to be placed before any /*..*/ comments, like the license agreement in the reproduction. Otherwise, the directives get ignored.
This is not a bug, but intended behaviour.

@pjfanning
Copy link
Author

I'll close this since @Gedochao has explained what the issue is and how to fix it on my side.

@Gedochao Gedochao closed this as not planned Won't fix, can't repro, duplicate, stale Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right using directives Issues tied with using directives.
Projects
None yet
Development

No branches or pull requests

2 participants