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

Fix SPARK-1629: Spark should inline use of commons-lang `SystemUtils.IS_... #569

Closed
wants to merge 3 commits into from
Closed

Fix SPARK-1629: Spark should inline use of commons-lang `SystemUtils.IS_... #569

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
49e248e
Fix SPARK-1629: Spark should inline use of commons-lang `SystemUtils.…
witgo Apr 27, 2014
fcaafd7
merge mastet
witgo Apr 27, 2014
31520eb
Merge branch 'master' of https://github.com/apache/spark into SPARK-1629
witgo Apr 30, 2014
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
13 changes: 9 additions & 4 deletions core/src/main/scala/org/apache/spark/util/Utils.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import scala.reflect.ClassTag
import scala.util.Try

import com.google.common.io.Files
import org.apache.commons.lang.SystemUtils
import com.google.common.util.concurrent.ThreadFactoryBuilder
import org.apache.hadoop.fs.{FileSystem, FileUtil, Path}
import org.json4s._
Expand All @@ -50,7 +49,7 @@ private[spark] object Utils extends Logging {
val random = new Random()

def sparkBin(sparkHome: String, which: String): File = {
val suffix = if (SystemUtils.IS_OS_WINDOWS) ".cmd" else ""
val suffix = if (isWindows) ".cmd" else ""
new File(sparkHome + File.separator + "bin", which + suffix)
}

Expand Down Expand Up @@ -614,7 +613,7 @@ private[spark] object Utils extends Logging {
*/
def isSymlink(file: File): Boolean = {
if (file == null) throw new NullPointerException("File must not be null")
if (SystemUtils.IS_OS_WINDOWS) return false
if (isWindows) return false
val fileInCanonicalDir = if (file.getParent() == null) {
file
} else {
Expand Down Expand Up @@ -1018,7 +1017,7 @@ private[spark] object Utils extends Logging {
throw new IOException("Destination must be relative")
}
var cmdSuffix = ""
val linkCmd = if (SystemUtils.IS_OS_WINDOWS) {
val linkCmd = if (isWindows) {
// refer to http://technet.microsoft.com/en-us/library/cc771254.aspx
cmdSuffix = " /s /e /k /h /y /i"
"cmd /c xcopy "
Expand Down Expand Up @@ -1062,6 +1061,12 @@ private[spark] object Utils extends Logging {
getHadoopFileSystem(new URI(path))
}

/**
* return true if this is Windows.
*/
def isWindows = Option(System.getProperty("os.name")).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@srowen

def isWindows(): Boolean = {
  try {
    val osName = System.getProperty("os.name")
    osName != null && osName.startsWith("Windows")
  } catch {
    case e: SecurityException => (log a warning and return false)
  }
}

You think here will be thrown a SecurityException .Why?

Copy link
Member

Choose a reason for hiding this comment

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

Regarding SecurityException, System.getProperty will often be disallowed in an environment where the security manager has been enabled. I strongly suspect that the rest of Spark fails already in such an environment for other reasons, and you could argue that the only reasonable response here is to fail rather than "guess" that the environment is not Windows. But I put it in mostly to mimic SystemUtils. It could go either way.

map(_.startsWith("Windows")).getOrElse(false)

/**
* Indicates whether Spark is currently running unit tests.
*/
Expand Down