From b5e38241c3a922196ed597deb244daa545c9435c Mon Sep 17 00:00:00 2001
From: Li Haoyi <haoyi.sg@gmail.com>
Date: Mon, 21 Oct 2024 10:31:15 +0800
Subject: [PATCH 1/2] .

---
 .../resources/build.mill                      |  5 ++
 .../src/SystemPropertiesEnvTests.scala        | 54 +++++++++++++++++++
 runner/src/mill/runner/MillMain.scala         |  2 +-
 3 files changed, 60 insertions(+), 1 deletion(-)
 create mode 100644 integration/feature/system-properties-env/resources/build.mill
 create mode 100644 integration/feature/system-properties-env/src/SystemPropertiesEnvTests.scala

diff --git a/integration/feature/system-properties-env/resources/build.mill b/integration/feature/system-properties-env/resources/build.mill
new file mode 100644
index 00000000000..2b98b401887
--- /dev/null
+++ b/integration/feature/system-properties-env/resources/build.mill
@@ -0,0 +1,5 @@
+import mill._
+import collection.JavaConverters._
+def properties = Task.Input { System.getProperties.asScala.get("my-prop") }
+
+def environment = Task.Input { Task.env.get("MY_ENV") }
\ No newline at end of file
diff --git a/integration/feature/system-properties-env/src/SystemPropertiesEnvTests.scala b/integration/feature/system-properties-env/src/SystemPropertiesEnvTests.scala
new file mode 100644
index 00000000000..68fb8f98679
--- /dev/null
+++ b/integration/feature/system-properties-env/src/SystemPropertiesEnvTests.scala
@@ -0,0 +1,54 @@
+package mill.integration
+
+import mill.testkit.UtestIntegrationTestSuite
+
+import utest._
+
+// Ensure that properties and environment can be correctly added
+// and removed despite consecutive runs all running in the same JVM
+object SystemPropertiesEnvTests extends UtestIntegrationTestSuite {
+  val tests: Tests = Tests {
+    test("properties") - integrationTest { tester =>
+      val result1 = tester.eval(("show", "properties"))
+      assert(result1.isSuccess == true)
+      result1.out ==> "[]"
+
+      val result2 = tester.eval(("-Dmy-prop=hello", "show", "properties"))
+      assert(result2.isSuccess == true)
+      result2.out ==> "[\n  \"hello\"\n]"
+
+      val result3 = tester.eval(("show", "properties"))
+      assert(result3.isSuccess == true)
+      result3.out ==> "[]"
+
+      val result4 = tester.eval(("-Dmy-prop=hello", "show", "properties"))
+      assert(result4.isSuccess == true)
+      result4.out ==> "[\n  \"hello\"\n]"
+
+      val result5 = tester.eval(("show", "properties"))
+      assert(result5.isSuccess == true)
+      result5.out ==> "[]"
+    }
+    test("environment") - integrationTest { tester =>
+      val result1 = tester.eval(("show", "environment"))
+      assert(result1.isSuccess == true)
+      result1.out ==> "[]"
+
+      val result2 = tester.eval(("show", "environment"), env = Map("MY_ENV" -> "HELLO"))
+      assert(result2.isSuccess == true)
+      result2.out ==> "[\n  \"HELLO\"\n]"
+
+      val result3 = tester.eval(("show", "environment"))
+      assert(result3.isSuccess == true)
+      result3.out ==> "[]"
+
+      val result4 = tester.eval(("show", "environment"), env = Map("MY_ENV" -> "HELLO"))
+      assert(result4.isSuccess == true)
+      result4.out ==> "[\n  \"HELLO\"\n]"
+
+      val result5 = tester.eval(("show", "environment"))
+      assert(result5.isSuccess == true)
+      result5.out ==> "[]"
+    }
+  }
+}
diff --git a/runner/src/mill/runner/MillMain.scala b/runner/src/mill/runner/MillMain.scala
index a43f6a07893..3f42d60247c 100644
--- a/runner/src/mill/runner/MillMain.scala
+++ b/runner/src/mill/runner/MillMain.scala
@@ -414,7 +414,7 @@ object MillMain {
   ): Unit = {
     val currentProps = sys.props
     val desiredProps = initialSystemProperties ++ userSpecifiedProperties
-    val systemPropertiesToUnset = desiredProps.keySet -- currentProps.keySet
+    val systemPropertiesToUnset = currentProps.keySet -- desiredProps.keySet
 
     for (k <- systemPropertiesToUnset) System.clearProperty(k)
     for ((k, v) <- desiredProps) System.setProperty(k, v)

From 711b1cfb695020cdc4d5c7269c811831c374366d Mon Sep 17 00:00:00 2001
From: Li Haoyi <haoyi.sg@gmail.com>
Date: Mon, 21 Oct 2024 10:48:42 +0800
Subject: [PATCH 2/2] .

---
 .../fundamentals/tasks/4-inputs/build.mill    | 61 +++++++++++++++++++
 .../resources/build.mill                      |  5 --
 .../src/SystemPropertiesEnvTests.scala        | 54 ----------------
 3 files changed, 61 insertions(+), 59 deletions(-)
 delete mode 100644 integration/feature/system-properties-env/resources/build.mill
 delete mode 100644 integration/feature/system-properties-env/src/SystemPropertiesEnvTests.scala

diff --git a/example/fundamentals/tasks/4-inputs/build.mill b/example/fundamentals/tasks/4-inputs/build.mill
index 01efc400aa8..307b0206a00 100644
--- a/example/fundamentals/tasks/4-inputs/build.mill
+++ b/example/fundamentals/tasks/4-inputs/build.mill
@@ -79,3 +79,64 @@ def gitStatusTask2 = Task { "version-" + gitStatusInput() }
 // code you put in `Task.Input` runs quickly. Ideally it should just be a simple check
 // "did anything change?" and any heavy-lifting should be delegated to downstream
 // tasks where it can be cached if possible.
+//
+// === System Properties Inputs
+//
+// One major use case of `Input` tasks is to make your build configurable via
+// JVM system properties of environment variables. If you directly access
+// `sys.props` or `sys.env` inside a xref:#_cached_tasks[cached Task{}], the
+// cached value will be used even if the property or environment variable changes
+// in subsequent runs, when you really want it to be re-evaluated. Thus, accessing
+// system properties should be done in a `Task.Input`, and usage of the property
+// should be done downstream in a xref:#_cached_tasks[cached task]:
+
+def myPropertyInput = Task.Input {
+  sys.props("my-property")
+}
+def myPropertyTask = Task {
+  "Hello Prop " + myPropertyInput()
+}
+
+/** Usage
+
+> ./mill show myPropertyTask
+"Hello Prop null"
+
+> ./mill -Dmy-property=world show myPropertyTask # Task is correctly invalidated when prop is added
+"Hello Prop world"
+
+> ./mill show myPropertyTask # Task is correctly invalidated when prop is removed
+"Hello Prop null"
+*/
+
+// Again, `Task.Input` runs every time, and thus you should only do the bare minimum
+// in your `Task.Input` that is necessary to detect changes. Any further processing
+// should be done in downstreak xref:#_cached_tasks[cached tasks] to allow for proper
+// caching and re-use
+//
+// === Environment Variable Inputs
+//
+// Like system properties, environment variables should be referenced in `Task.Input`s. Unlike
+// system properties, you need to use the special API `Task.env` to access the environment,
+// due to JVM limitations:
+
+def myEnvInput = Task.Input {
+  Task.env.getOrElse("MY_ENV", null)
+}
+
+def myEnvTask = Task {
+  "Hello Env " + myEnvInput()
+}
+
+
+/** Usage
+
+> ./mill show myEnvTask
+"Hello Env null"
+
+> MY_ENV=world ./mill show myEnvTask # Task is correctly invalidated when env is added
+"Hello Env world"
+
+> ./mill show myEnvTask # Task is correctly invalidated when env is removed
+"Hello Env null"
+*/
diff --git a/integration/feature/system-properties-env/resources/build.mill b/integration/feature/system-properties-env/resources/build.mill
deleted file mode 100644
index 2b98b401887..00000000000
--- a/integration/feature/system-properties-env/resources/build.mill
+++ /dev/null
@@ -1,5 +0,0 @@
-import mill._
-import collection.JavaConverters._
-def properties = Task.Input { System.getProperties.asScala.get("my-prop") }
-
-def environment = Task.Input { Task.env.get("MY_ENV") }
\ No newline at end of file
diff --git a/integration/feature/system-properties-env/src/SystemPropertiesEnvTests.scala b/integration/feature/system-properties-env/src/SystemPropertiesEnvTests.scala
deleted file mode 100644
index 68fb8f98679..00000000000
--- a/integration/feature/system-properties-env/src/SystemPropertiesEnvTests.scala
+++ /dev/null
@@ -1,54 +0,0 @@
-package mill.integration
-
-import mill.testkit.UtestIntegrationTestSuite
-
-import utest._
-
-// Ensure that properties and environment can be correctly added
-// and removed despite consecutive runs all running in the same JVM
-object SystemPropertiesEnvTests extends UtestIntegrationTestSuite {
-  val tests: Tests = Tests {
-    test("properties") - integrationTest { tester =>
-      val result1 = tester.eval(("show", "properties"))
-      assert(result1.isSuccess == true)
-      result1.out ==> "[]"
-
-      val result2 = tester.eval(("-Dmy-prop=hello", "show", "properties"))
-      assert(result2.isSuccess == true)
-      result2.out ==> "[\n  \"hello\"\n]"
-
-      val result3 = tester.eval(("show", "properties"))
-      assert(result3.isSuccess == true)
-      result3.out ==> "[]"
-
-      val result4 = tester.eval(("-Dmy-prop=hello", "show", "properties"))
-      assert(result4.isSuccess == true)
-      result4.out ==> "[\n  \"hello\"\n]"
-
-      val result5 = tester.eval(("show", "properties"))
-      assert(result5.isSuccess == true)
-      result5.out ==> "[]"
-    }
-    test("environment") - integrationTest { tester =>
-      val result1 = tester.eval(("show", "environment"))
-      assert(result1.isSuccess == true)
-      result1.out ==> "[]"
-
-      val result2 = tester.eval(("show", "environment"), env = Map("MY_ENV" -> "HELLO"))
-      assert(result2.isSuccess == true)
-      result2.out ==> "[\n  \"HELLO\"\n]"
-
-      val result3 = tester.eval(("show", "environment"))
-      assert(result3.isSuccess == true)
-      result3.out ==> "[]"
-
-      val result4 = tester.eval(("show", "environment"), env = Map("MY_ENV" -> "HELLO"))
-      assert(result4.isSuccess == true)
-      result4.out ==> "[\n  \"HELLO\"\n]"
-
-      val result5 = tester.eval(("show", "environment"))
-      assert(result5.isSuccess == true)
-      result5.out ==> "[]"
-    }
-  }
-}