From febf536b69a6dc0b1a0deabf406d19397ab18bab Mon Sep 17 00:00:00 2001 From: Alex Nordlund Date: Sun, 4 Jul 2021 11:49:09 +0200 Subject: [PATCH 1/3] 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) From f1aed46d5b003fb1c1a497e2c5d35d085288b8d9 Mon Sep 17 00:00:00 2001 From: Alex Nordlund Date: Sun, 4 Jul 2021 12:20:41 +0200 Subject: [PATCH 2/3] Move OS failure to configuration-phase rather than runtime This is not ideal since even if `download = true` there could very well be builds that work fine, especially if you're excluding the tasks from the node plugin. But right now there's harder to diagnose errors appearing further down the line and this should make for a more stable build, at least an overall more stable build. --- src/main/kotlin/com/github/gradle/node/NodePlugin.kt | 4 ++++ src/main/kotlin/com/github/gradle/node/util/PlatformHelper.kt | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/com/github/gradle/node/NodePlugin.kt b/src/main/kotlin/com/github/gradle/node/NodePlugin.kt index 3d94e47c..36309bcf 100644 --- a/src/main/kotlin/com/github/gradle/node/NodePlugin.kt +++ b/src/main/kotlin/com/github/gradle/node/NodePlugin.kt @@ -7,6 +7,7 @@ import com.github.gradle.node.npm.task.NpmTask import com.github.gradle.node.npm.task.NpxTask import com.github.gradle.node.task.NodeSetupTask import com.github.gradle.node.task.NodeTask +import com.github.gradle.node.util.PlatformHelper import com.github.gradle.node.variant.VariantComputer import com.github.gradle.node.yarn.task.YarnInstallTask import com.github.gradle.node.yarn.task.YarnSetupTask @@ -30,6 +31,9 @@ class NodePlugin : Plugin { addYarnRule() project.afterEvaluate { if (nodeExtension.download.get()) { + // Ideally we wouldn't have to do this here, but if we don't + // then we're going to have some interesting failures down the line + PlatformHelper.INSTANCE.failOnUnsupportedOs() nodeExtension.distBaseUrl.orNull?.let { addRepository(it) } configureNodeSetupTask(nodeExtension) } 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 0640d676..1cde2ba3 100644 --- a/src/main/kotlin/com/github/gradle/node/util/PlatformHelper.kt +++ b/src/main/kotlin/com/github/gradle/node/util/PlatformHelper.kt @@ -35,7 +35,7 @@ open class PlatformHelper constructor(private val props: Properties = System.get fun failOnUnsupportedOs() { if (!isSupported) { - error("Unsupported OS") + error("Unsupported OS for `download = true`") } } From f0dd55dc66672bbd77ea066002d5b72be54c9e3e Mon Sep 17 00:00:00 2001 From: Alex Nordlund Date: Sun, 4 Jul 2021 12:30:09 +0200 Subject: [PATCH 3/3] Fix broken test, FreeBSD is not Linux. --- .../com/github/gradle/node/variant/VariantComputerTest.groovy | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/groovy/com/github/gradle/node/variant/VariantComputerTest.groovy b/src/test/groovy/com/github/gradle/node/variant/VariantComputerTest.groovy index c7c9b50f..4c5ceb4e 100644 --- a/src/test/groovy/com/github/gradle/node/variant/VariantComputerTest.groovy +++ b/src/test/groovy/com/github/gradle/node/variant/VariantComputerTest.groovy @@ -105,8 +105,6 @@ class VariantComputerTest extends Specification { 'Linux' | 'ppc64le' | 'node-v5.12.0-linux-ppc64le' | 'org.nodejs:node:5.12.0:linux-ppc64le@tar.gz' 'Mac OS X' | 'x86' | 'node-v5.12.0-darwin-x86' | 'org.nodejs:node:5.12.0:darwin-x86@tar.gz' 'Mac OS X' | 'x86_64' | 'node-v5.12.0-darwin-x64' | 'org.nodejs:node:5.12.0:darwin-x64@tar.gz' - 'FreeBSD' | 'x86' | 'node-v5.12.0-linux-x86' | 'org.nodejs:node:5.12.0:linux-x86@tar.gz' - 'FreeBSD' | 'x86_64' | 'node-v5.12.0-linux-x64' | 'org.nodejs:node:5.12.0:linux-x64@tar.gz' 'SunOS' | 'x86' | 'node-v5.12.0-sunos-x86' | 'org.nodejs:node:5.12.0:sunos-x86@tar.gz' 'SunOS' | 'x86_64' | 'node-v5.12.0-sunos-x64' | 'org.nodejs:node:5.12.0:sunos-x64@tar.gz' }