Skip to content

Fix discovery of project file for exclude #3613

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix discovery of project file for exclude #3613

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7b460a8
Fix discovery of project file for exclude
bishabosha Apr 3, 2025
fc032e2
Merge branch 'main' into fix/exclude-nested-project
bishabosha Apr 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion modules/build/src/main/scala/scala/build/CrossSources.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,14 @@ object CrossSources {

val flattenedInputs = inputs.flattened()
val allExclude = { // supports only one exclude directive in one source file, which should be the project file.
val projectScalaFileOpt = flattenedInputs.collectFirst {
val projectScalaFileCandidates = flattenedInputs.collect {
case f: ProjectScalaFile => f
}

// this relies upon the inferred workspace root being the outermost directory.
// which is the common case when you pass a single directory.
val projectScalaFileOpt =
projectScalaFileCandidates.sortBy(_.subPath.segments.size).headOption
Copy link
Contributor

Choose a reason for hiding this comment

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

while defaulting to the outermost directory doesn't sound like a bad fallback (and we can keep it as fallback), I'm thinking we should favor the project.scala which is closest to the designated workspace for the Scala CLI project - and that may not be the shortest path, actually. Especially since the workspace can be overridden with a command line option.

As in, let's say we have:

── partitionA
   └── outerdir
       └── actualWorkspace
           └── project.scala
── partitionB
    └── outerdir
        └── project.scala

partitionA/outerdir/actualWorkspace/project.scala is longer than partitionB/outerdir/project.scala, but it should still be picked.
That being said your change is strictly better than what we have on main, so I'm okay with that being fixed as a follow-up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so closest to the workspace of Inputs?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep. Just be wary that it can be overridden with --workspace.

val excludeFromProjectFile =
value(preprocessSources(projectScalaFileOpt.toSeq))
.flatMap(_.options).flatMap(_.internal.exclude)
Expand Down
36 changes: 36 additions & 0 deletions modules/build/src/test/scala/scala/build/tests/ExcludeTests.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,4 +213,40 @@ class ExcludeTests extends TestUtil.ScalaCliBuildSuite {
}
}

test("exclude nested scala-cli project") {
val testInputs = TestInputs(
os.rel / "Hello.scala" -> "object Hello",
// this project.scala needs to come first so that the inferred workspace is the outermost one.
os.rel / "project.scala" ->
"""//> using exclude */examples/*""",
os.rel / "examples" / "fail.scala" ->
"""val i: Int = "abc";""",
os.rel / "examples" / "project.scala" ->
"""val unused = 23"""
)
testInputs.withInputs { (root, inputs) =>
val (crossSources, _) =
CrossSources.forInputs(
inputs,
preprocessors,
TestLogger(),
SuppressWarningOptions()
)(using ScalaCliInvokeData.dummy).orThrow
val scopedSources = crossSources.scopedSources(BuildOptions())
.orThrow
val sources =
scopedSources.sources(
Scope.Main,
crossSources.sharedOptions(BuildOptions()),
root,
TestLogger()
)
.orThrow

expect(sources.paths.nonEmpty)
expect(sources.paths.length == 2)
expect(sources.paths.map(_._2) == Seq(os.rel / "Hello.scala", os.rel / "project.scala"))
}
}

}