Skip to content

Print a better error when workspace path contains the File.pathSeparator #1985

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

Merged
merged 1 commit into from
Mar 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
124 changes: 70 additions & 54 deletions modules/build/src/main/scala/scala/build/input/Inputs.scala
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package scala.build.input

import java.io.ByteArrayInputStream
import java.io.{ByteArrayInputStream, File}
import java.math.BigInteger
import java.nio.charset.StandardCharsets
import java.security.MessageDigest

import scala.annotation.tailrec
import scala.build.Directories
import scala.build.errors.{BuildException, InputsException}
import scala.build.errors.{BuildException, InputsException, WorkspaceError}
import scala.build.input.ElementsUtils.*
import scala.build.internal.Constants
import scala.build.internal.zip.WrappedZipInputStream
Expand Down Expand Up @@ -125,53 +125,14 @@ object Inputs {
private def forValidatedElems(
validElems: Seq[Element],
baseProjectName: String,
forcedWorkspace: Option[os.Path],
workspace: os.Path,
needsHash: Boolean,
workspaceOrigin: WorkspaceOrigin,
enableMarkdown: Boolean,
allowRestrictedFeatures: Boolean,
extraClasspathWasPassed: Boolean
): Inputs = {
assert(extraClasspathWasPassed || validElems.nonEmpty)
val (inferredWorkspace, inferredNeedsHash, workspaceOrigin) = {
val settingsFiles = validElems.projectSettingsFiles
val dirsAndFiles = validElems.collect {
case d: Directory => d
case f: SourceFile => f
}
settingsFiles.headOption.map { s =>
if (settingsFiles.length > 1)
System.err.println(
s"Warning: more than one ${Constants.projectFileName} file has been found. Setting ${s.base} as the project root directory for this run."
)
(s.base, true, WorkspaceOrigin.SourcePaths)
}.orElse {
dirsAndFiles.collectFirst {
case d: Directory =>
if (dirsAndFiles.length > 1)
System.err.println(
s"Warning: setting ${d.path} as the project root directory for this run."
)
(d.path, true, WorkspaceOrigin.SourcePaths)
case f: SourceFile =>
if (dirsAndFiles.length > 1)
System.err.println(
s"Warning: setting ${f.path / os.up} as the project root directory for this run."
)
(f.path / os.up, true, WorkspaceOrigin.SourcePaths)
}
}.orElse {
validElems.collectFirst {
case _: Virtual =>
(os.temp.dir(), true, WorkspaceOrigin.VirtualForced)
}
}.getOrElse((os.pwd, true, WorkspaceOrigin.Forced))
}

val (workspace, needsHash, workspaceOrigin0) = forcedWorkspace match {
case None => (inferredWorkspace, inferredNeedsHash, workspaceOrigin)
case Some(forcedWorkspace0) =>
val needsHash0 = forcedWorkspace0 != inferredWorkspace || inferredNeedsHash
(forcedWorkspace0, needsHash0, WorkspaceOrigin.Forced)
}
val allDirs = validElems.collect { case d: Directory => d.path }
val updatedElems = validElems.filter {
case f: SourceFile =>
Expand All @@ -189,7 +150,7 @@ object Inputs {
workspace,
baseProjectName,
mayAppendHash = needsHash,
workspaceOrigin = Some(workspaceOrigin0),
workspaceOrigin = Some(workspaceOrigin),
enableMarkdown = enableMarkdown,
allowRestrictedFeatures = allowRestrictedFeatures
)
Expand Down Expand Up @@ -365,7 +326,7 @@ object Inputs {
enableMarkdown: Boolean,
allowRestrictedFeatures: Boolean,
extraClasspathWasPassed: Boolean
)(using ScalaCliInvokeData): Either[BuildException, Inputs] = {
)(using invokeData: ScalaCliInvokeData): Either[BuildException, Inputs] = {
val validatedArgs: Seq[Either[String, Seq[Element]]] =
validateArgs(
args,
Expand All @@ -386,15 +347,70 @@ object Inputs {
case Right(elem) => elem
}.flatten
assert(extraClasspathWasPassed || validElems.nonEmpty)
val (inferredWorkspace, inferredNeedsHash, workspaceOrigin) = {
val settingsFiles = validElems.projectSettingsFiles
val dirsAndFiles = validElems.collect {
case d: Directory => d
case f: SourceFile => f
}
settingsFiles.headOption.map { s =>
if (settingsFiles.length > 1)
System.err.println(
s"Warning: more than one ${Constants.projectFileName} file has been found. Setting ${s.base} as the project root directory for this run."
)
(s.base, true, WorkspaceOrigin.SourcePaths)
}.orElse {
dirsAndFiles.collectFirst {
case d: Directory =>
if (dirsAndFiles.length > 1)
System.err.println(
s"Warning: setting ${d.path} as the project root directory for this run."
)
(d.path, true, WorkspaceOrigin.SourcePaths)
case f: SourceFile =>
if (dirsAndFiles.length > 1)
System.err.println(
s"Warning: setting ${f.path / os.up} as the project root directory for this run."
)
(f.path / os.up, true, WorkspaceOrigin.SourcePaths)
}
}.orElse {
validElems.collectFirst {
case _: Virtual =>
(os.temp.dir(), true, WorkspaceOrigin.VirtualForced)
}
}.getOrElse((os.pwd, true, WorkspaceOrigin.Forced))
}

Right(forValidatedElems(
validElems,
baseProjectName,
forcedWorkspace,
enableMarkdown,
allowRestrictedFeatures,
extraClasspathWasPassed
))
val (workspace, needsHash, workspaceOrigin0) = forcedWorkspace match {
case None => (inferredWorkspace, inferredNeedsHash, workspaceOrigin)
case Some(forcedWorkspace0) =>
val needsHash0 = forcedWorkspace0 != inferredWorkspace || inferredNeedsHash
(forcedWorkspace0, needsHash0, WorkspaceOrigin.Forced)
}

if workspace.toString.contains(File.pathSeparator) then
val prog = invokeData.invocationString
val argsString = args.mkString(" ")
Left(new WorkspaceError(
s"""Invalid workspace path: ${Console.BOLD}$workspace${Console.RESET}
|Workspace path cannot contain a ${Console.BOLD}${File.pathSeparator}${Console.RESET}.
|Consider moving your project to a different path.
|Alternatively, you can force your workspace with the '--workspace' option:
| ${Console.BOLD}$prog --workspace <alternative-workspace-path> $argsString${Console.RESET}"""
.stripMargin
))
else
Right(forValidatedElems(
validElems,
baseProjectName,
workspace,
needsHash,
workspaceOrigin0,
enableMarkdown,
allowRestrictedFeatures,
extraClasspathWasPassed
))
}
else
Left(new InputsException(invalid.mkString(System.lineSeparator())))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,14 @@ case class ScalaCliInvokeData(
subCommandName: String,
subCommand: SubCommand,
isShebangCapableShell: Boolean
)
) {

/** [[progName]] with [[subCommandName]] if any */
def invocationString: String =
subCommand match
case SubCommand.Default => progName
case _ => s"$progName $subCommandName"
}

enum SubCommand:
case Default extends SubCommand
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1129,6 +1129,31 @@ abstract class RunTestDefinitions(val scalaVersionOpt: Option[String])
}
}

test(s"print error if workspace path contains a ${File.pathSeparator}") {
val msg = "Hello"
val relPath = os.rel / s"weird${File.pathSeparator}directory" / "Hello.scala"
TestInputs(
relPath ->
s"""object Hello extends App {
| println("$msg")
|}
|""".stripMargin
)
.fromRoot { root =>
val resWithColon =
os.proc(TestUtil.cli, "run", relPath.toString, extraOptions)
.call(cwd = root, check = false, stderr = os.Pipe)
expect(resWithColon.exitCode == 1)
expect(resWithColon.err.trim().contains(
"you can force your workspace with the '--workspace' option:"
))
val resFixedWorkspace = // should run fine for a forced workspace with no classpath separator on path
os.proc(TestUtil.cli, "run", relPath.toString, "--workspace", ".", extraOptions)
.call(cwd = root)
expect(resFixedWorkspace.out.trim() == msg)
}
}

val commands = Seq("", "run", "compile")

for (command <- commands) {
Expand Down