From febf536b69a6dc0b1a0deabf406d19397ab18bab Mon Sep 17 00:00:00 2001 From: Alex Nordlund Date: Sun, 4 Jul 2021 11:49:09 +0200 Subject: [PATCH] Move unsupported platform failure from os.name to separate method. Previously we've accidentally depended on the isWindows check to see if we're on a supported platform or not, this had the unfortunate side-effect that even with `download = false` we triggered the error(). See #178 --- .../github/gradle/node/task/NodeSetupTask.kt | 5 ++++ .../github/gradle/node/util/PlatformHelper.kt | 16 +++++++++++-- .../node/util/PlatformHelperTest.groovy | 24 ++++++++++--------- 3 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src/main/kotlin/com/github/gradle/node/task/NodeSetupTask.kt b/src/main/kotlin/com/github/gradle/node/task/NodeSetupTask.kt index 296ea619..a6c58406 100644 --- a/src/main/kotlin/com/github/gradle/node/task/NodeSetupTask.kt +++ b/src/main/kotlin/com/github/gradle/node/task/NodeSetupTask.kt @@ -49,11 +49,16 @@ abstract class NodeSetupTask : DefaultTask() { @TaskAction fun exec() { + failIfUnsupportedPlatform() deleteExistingNode() unpackNodeArchive() setExecutableFlag() } + private fun failIfUnsupportedPlatform() { + PlatformHelper.INSTANCE.failOnUnsupportedOs() + } + private fun deleteExistingNode() { projectHelper.delete { delete(nodeDir.get().dir("../")) diff --git a/src/main/kotlin/com/github/gradle/node/util/PlatformHelper.kt b/src/main/kotlin/com/github/gradle/node/util/PlatformHelper.kt index b4bfdf7c..0640d676 100644 --- a/src/main/kotlin/com/github/gradle/node/util/PlatformHelper.kt +++ b/src/main/kotlin/com/github/gradle/node/util/PlatformHelper.kt @@ -9,9 +9,8 @@ open class PlatformHelper constructor(private val props: Properties = System.get name.contains("windows") -> "win" name.contains("mac") -> "darwin" name.contains("linux") -> "linux" - name.contains("freebsd") -> "linux" name.contains("sunos") -> "sunos" - else -> error("Unsupported OS: $name") + else -> "unsupported" } } @@ -32,6 +31,14 @@ open class PlatformHelper constructor(private val props: Properties = System.get open val isWindows: Boolean by lazy { osName == "win" } + open val isSupported: Boolean by lazy { osName != "unsupported" } + + fun failOnUnsupportedOs() { + if (!isSupported) { + error("Unsupported OS") + } + } + private fun property(name: String): String { val value = props.getProperty(name) return value ?: System.getProperty(name) ?: @@ -48,6 +55,11 @@ open class PlatformHelper constructor(private val props: Properties = System.get fun main(args: Array) { println("Your os.name is: '${System.getProperty("os.name")}' and is parsed as: ${PlatformHelper.INSTANCE.osName}") println("Your os.arch is: '${System.getProperty("os.arch")}' and is parsed as: ${PlatformHelper.INSTANCE.osArch}") + if (!PlatformHelper.INSTANCE.isSupported) { + println("Your platform is \"unsupported\" (isSupported == false)") + println("Your platform does not support 'download = true' as there's no official Node.js binaries" + + " being published for it. You can still use the plugin, but you need to install Node.js manually") + } if (PlatformHelper.INSTANCE.isWindows) { println("You're on windows (isWindows == true)") } else { diff --git a/src/test/groovy/com/github/gradle/node/util/PlatformHelperTest.groovy b/src/test/groovy/com/github/gradle/node/util/PlatformHelperTest.groovy index 03bd8031..e350baee 100644 --- a/src/test/groovy/com/github/gradle/node/util/PlatformHelperTest.groovy +++ b/src/test/groovy/com/github/gradle/node/util/PlatformHelperTest.groovy @@ -22,18 +22,20 @@ class PlatformHelperTest extends Specification { this.helper.getOsName() == osName this.helper.getOsArch() == osArch this.helper.isWindows() == isWindows + this.helper.isSupported() == isSupported where: - osProp | archProp | osName | osArch | isWindows - 'Windows 8' | 'x86' | 'win' | 'x86' | true - 'Windows 8' | 'x86_64' | 'win' | 'x64' | true - 'Mac OS X' | 'x86' | 'darwin' | 'x86' | false - 'Mac OS X' | 'x86_64' | 'darwin' | 'x64' | false - 'Linux' | 'x86' | 'linux' | 'x86' | false - 'Linux' | 'x86_64' | 'linux' | 'x64' | false - 'Linux' | 'ppc64le' | 'linux' | 'ppc64le' | false - 'SunOS' | 'x86' | 'sunos' | 'x86' | false - 'SunOS' | 'x86_64' | 'sunos' | 'x64' | false + osProp | archProp | osName | osArch | isWindows | isSupported + 'Windows 8' | 'x86' | 'win' | 'x86' | true | true + 'Windows 8' | 'x86_64' | 'win' | 'x64' | true | true + 'Mac OS X' | 'x86' | 'darwin' | 'x86' | false | true + 'Mac OS X' | 'x86_64' | 'darwin' | 'x64' | false | true + 'Linux' | 'x86' | 'linux' | 'x86' | false | true + 'Linux' | 'x86_64' | 'linux' | 'x64' | false | true + 'Linux' | 'ppc64le' | 'linux' | 'ppc64le' | false | true + 'SunOS' | 'x86' | 'sunos' | 'x86' | false | true + 'SunOS' | 'x86_64' | 'sunos' | 'x64' | false | true + 'FreeBSD' | 'amd64' | 'unsupported' | 'x64' | false | false } @Unroll @@ -62,7 +64,7 @@ class PlatformHelperTest extends Specification { this.props.setProperty("os.name", 'Nonsense') when: - this.helper.getOsName() + this.helper.failOnUnsupportedOs() then: thrown(IllegalStateException)