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.

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

Override CWD in runEngineDistribution to the parent of the project being run to avoid warning #10928

Merged
merged 1 commit into from
Aug 31, 2024
Merged

Override CWD in runEngineDistribution to the parent of the project being run to avoid warning #10928

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
65 changes: 52 additions & 13 deletions project/DistributionPackage.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import scala.sys.process._

import org.enso.build.WithDebugCommand

import scala.util.Try

object DistributionPackage {

/** File extensions. */
Expand Down Expand Up @@ -267,26 +269,36 @@ object DistributionPackage {
): Boolean = {
import scala.collection.JavaConverters._

val enso = distributionRoot / "bin" / batName("enso")
log.info(s"Executing $enso ${args.mkString(" ")}")
val pb = new java.lang.ProcessBuilder()
val all = new java.util.ArrayList[String]()
val disablePrivateCheck = {
val findRun = args.indexOf("--run")
if (findRun >= 0 && findRun + 1 < args.size) {
val whatToRun = args(findRun + 1)
val enso = distributionRoot / "bin" / batName("enso")
val pb = new java.lang.ProcessBuilder()
val all = new java.util.ArrayList[String]()
val runArgumentIndex = locateRunArgument(args)
val runArgument = runArgumentIndex.map(args)
val disablePrivateCheck = runArgument match {
case Some(whatToRun) =>
if (whatToRun.startsWith("test/") && whatToRun.endsWith("_Tests")) {
whatToRun.contains("_Internal_")
} else {
false
}
} else {
false
}
case None => false
}
all.add(enso.getAbsolutePath())

val runArgumentAsFile = runArgument.flatMap(createFileIfValidPath)
val projectDirectory = runArgumentAsFile.flatMap(findProjectRoot)
val cwdOverride: Option[File] =
projectDirectory.flatMap(findParentFile).map(_.getAbsoluteFile)

all.add(enso.getAbsolutePath)
all.addAll(args.asJava)
pb.command(all)
// Override the working directory of new process to be the parent of the project directory.
cwdOverride.foreach { c =>
pb.directory(c)
}
if (cwdOverride.isDefined) {
// If the working directory is changed, we need to translate the path - make it absolute.
all.set(runArgumentIndex.get + 1, runArgumentAsFile.get.getAbsolutePath)
}
Comment on lines +294 to +301
Copy link
Member Author

Choose a reason for hiding this comment

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

My only concern is that because the CWD changes, if there were any other paths given as arguments, they may now become invalid.

But I don't think we often add paths to runEngineDistribution, do we? Maybe I should add a comment to be sure to prefer absolute paths?

I was thinking of a 'heuristic' that will convent relative paths to absolute, but it would be too imprecise - is a test argument a path to the ./test/ directory or a completely different thing? Impossible to tell without context.

Copy link
Member

Choose a reason for hiding this comment

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

I think the only path I really care about is after --run.

if (args.contains("--debug")) {
all.remove("--debug")
pb.environment().put("JAVA_OPTS", "-ea " + WithDebugCommand.DEBUG_OPTION)
Expand All @@ -296,7 +308,9 @@ object DistributionPackage {
if (disablePrivateCheck) {
all.add("--disable-private-check")
}
pb.command(all)
pb.inheritIO()
log.info(s"Executing ${all.asScala.mkString(" ")}")
val p = pb.start()
val exitCode = p.waitFor()
if (exitCode != 0) {
Expand All @@ -305,6 +319,31 @@ object DistributionPackage {
exitCode == 0
}

/** Returns the index of the next argument after `--run`, if it exists. */
private def locateRunArgument(args: Seq[String]): Option[Int] = {
val findRun = args.indexOf("--run")
if (findRun >= 0 && findRun + 1 < args.size) {
Some(findRun + 1)
} else {
None
}
}

/** Returns a file, only if the provided string represented a valid path. */
private def createFileIfValidPath(path: String): Option[File] =
Try(new File(path)).toOption

/** Looks for a parent directory that contains `package.yaml`. */
private def findProjectRoot(file: File): Option[File] =
Copy link
Member

Choose a reason for hiding this comment

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

OK.

if (file.isDirectory && (file / "package.yaml").exists()) {
Some(file)
} else {
findParentFile(file).flatMap(findProjectRoot)
}

private def findParentFile(file: File): Option[File] =
Option(file.getParentFile)

def runProjectManagerPackage(
engineRoot: File,
distributionRoot: File,
Expand Down
Loading