Merge pull request #874 from hwine/GH-871_fix_env_option

repo2docker/__main__.py

@@ -39,6 +39,27 @@ def validate_image_name(image_name):
     return image_name
 
 
+# See https://github.com/jupyter/repo2docker/issues/871 for reason
+class MimicDockerEnvHandling(argparse.Action):
+    def __call__(self, parser, namespace, values, option_string=None):
+        # There are 3 cases:
+        #  key=value    pass as is
+        #  key=         pass as is
+        #  key          pass using current value, or don't pass
+        if "=" not in values:
+            try:
+                value_to_append = "{}={}".format(values, os.environ[values])
+            except KeyError:
+                # no local def, so don't pass
+                return
+        else:
+            value_to_append = values
+
+        # destination variable is initially defined as an empty list, so
+        # no special casing of first time is needed.
+        getattr(namespace, self.dest).append(value_to_append)
+
+
 def get_argparser():
     """Get arguments that may be used by repo2docker"""
     argparser = argparse.ArgumentParser(
@@ -158,11 +179,14 @@ def get_argparser():
         "--user-name", help="Username of the primary user in the image"
     )
 
+    # Process the environment options the same way that docker does, as
+    # they are passed directly to docker as the environment to use. This
+    # requires a custom action for argparse.
     argparser.add_argument(
         "--env",
         "-e",
         dest="environment",
-        action="append",
+        action=MimicDockerEnvHandling,
         help="Environment variables to define at container run time",
         default=[],
     )

tests/unit/test_env.py

@@ -11,28 +11,52 @@ from getpass import getuser
 def test_env():
     """
     Validate that you can define environment variables
+
+    See https://gist.github.com/hwine/9f5b02c894427324fafcf12f772b27b7
+    for how docker handles its -e & --env argument values
     """
     ts = str(time.time())
     with tempfile.TemporaryDirectory() as tmpdir:
         username = getuser()
-        subprocess.check_call(
+        os.environ["SPAM"] = "eggs"
+        os.environ["SPAM_2"] = "ham"
+        result = subprocess.run(
             [
                 "repo2docker",
-                "-v",
-                "{}:/home/{}".format(tmpdir, username),
+                # 'key=value' are exported as is in docker
                 "-e",
                 "FOO={}".format(ts),
                 "--env",
                 "BAR=baz",
+                # 'key' is exported with the currently exported value
+                "--env",
+                "SPAM",
+                # 'key' is not exported if it is not exported.
+                "-e",
+                "NO_SPAM",
+                # 'key=' is exported in docker with an empty string as
+                # value
+                "--env",
+                "SPAM_2=",
                 "--",
                 tmpdir,
                 "/bin/bash",
                 "-c",
-                "echo -n $FOO > ts && echo -n $BAR > bar",
-            ]
+                # Docker exports all passed env variables, so we can
+                # just look at exported variables.
+                "export",
+            ],
+            universal_newlines=True,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
         )
+    assert result.returncode == 0
 
-        with open(os.path.join(tmpdir, "ts")) as f:
-            assert f.read().strip() == ts
-        with open(os.path.join(tmpdir, "bar")) as f:
-            assert f.read().strip() == "baz"
+    # all docker output is returned by repo2docker on stderr
+    # extract just the declare for better failure message formatting
+    declares = [x for x in result.stderr.split("\n") if x.startswith("declare")]
+    assert 'declare -x FOO="{}"'.format(ts) in declares
+    assert 'declare -x BAR="baz"' in declares
+    assert 'declare -x SPAM="eggs"' in declares
+    assert "declare -x NO_SPAM" not in declares
+    assert 'declare -x SPAM_2=""' in declares