Detect failure of pre-assembly in R buildpack

If the pre-assembly step succeeds we do not re-run it after copying the
repository contents.

Author: betatim

repo2docker/buildpacks/base.py

@@ -125,7 +125,14 @@
 # of the repository but don't access any files in the repository. By executing
 # them before copying the repository itself we can cache these steps. For
 # example installing APT packages.
-{% for sd in pre_assemble_script_directives -%}
+{% if preassemble_script_files -%}
+# If scripts required during build are present, copy them
+{% for src, dst in preassemble_script_files.items() %}
+COPY src/{{ src }} ${REPO_DIR}/{{ dst }}
+{% endfor -%}
+{% endif -%}
+
+{% for sd in preassemble_script_directives -%}
 {{ sd }}
 {% endfor %}
 
@@ -379,6 +386,19 @@ def get_build_scripts(self):
 
         return []
 
+    def get_preassemble_script_files(self):
+        """
+        Dict of files to be copied to the container image for use in preassembly.
+
+        This is copied before the `build_scripts`, `preassemble_scripts` and
+        `assemble_scripts` are run, so can be executed from either of them.
+
+        It's a dictionary where the key is the source file path in the
+        repository and the value is the destination file path inside the
+        repository in the container.
+        """
+        return {}
+
     def get_preassemble_scripts(self):
         """
         Ordered list of shell snippets to build an image for this repository.
@@ -499,13 +519,13 @@ def render(self):
                 "RUN {}".format(textwrap.dedent(script.strip("\n")))
             )
 
-        pre_assemble_script_directives = []
+        preassemble_script_directives = []
         last_user = "root"
         for user, script in self.get_preassemble_scripts():
             if last_user != user:
-                pre_assemble_script_directives.append("USER {}".format(user))
+                preassemble_script_directives.append("USER {}".format(user))
                 last_user = user
-            pre_assemble_script_directives.append(
+            preassemble_script_directives.append(
                 "RUN {}".format(textwrap.dedent(script.strip("\n")))
             )
 
@@ -516,7 +536,8 @@ def render(self):
             env=self.get_env(),
             labels=self.get_labels(),
             build_script_directives=build_script_directives,
-            pre_assemble_script_directives=pre_assemble_script_directives,
+            preassemble_script_files=self.get_preassemble_script_files(),
+            preassemble_script_directives=preassemble_script_directives,
             assemble_script_directives=assemble_script_directives,
             build_script_files=self.get_build_script_files(),
             base_packages=sorted(self.get_base_packages()),

repo2docker/buildpacks/r.py

@@ -291,39 +291,53 @@ def get_build_scripts(self):
 
         return super().get_build_scripts() + scripts
 
+    def get_preassemble_script_files(self):
+        files = {}
+        installR_path = self.binder_path("install.R")
+        if os.path.exists(installR_path):
+            files[installR_path] = installR_path
+
+        return files
+
     def get_preassemble_scripts(self):
-        """Install contents of install.R"""
+        """Install contents of install.R
+
+        Attempt to execute `install.R` before copying the contents of the
+        repository. We speculate that most of the time we do not need access.
+        In case this fails we re-run it after copying the repository contents.
+
+        The advantage of executing it before copying is that minor edits to the
+        repository content will not trigger a re-install making things faster.
+        """
         scripts = []
 
         installR_path = self.binder_path("install.R")
         if os.path.exists(installR_path):
-            packages = []
-            with open(installR_path) as f:
-                for line in f:
-                    line = line.strip()
-                    # skip commented out lines
-                    if line.startswith("#"):
-                        continue
-                    else:
-                        # XXX This needs a better solution for detecting which
-                        # XXX kind of string quoting the user used/how to
-                        # XXX escape quotes...
-                        # using " as quote
-                        if '"' in line:
-                            packages.append("-e '{}'".format(line))
-                        # using ' as quote or no quotes
-                        else:
-                            packages.append('-e "{}"'.format(line))
-
-            package_expressions = " \\ \n".join(sorted(packages))
-            scripts += [("${NB_USER}", "R --quiet %s" % package_expressions)]
+            scripts += [
+                (
+                    "${NB_USER}",
+                    "Rscript %s && touch /tmp/.preassembled || true" % installR_path,
+                )
+            ]
 
         return super().get_preassemble_scripts() + scripts
 
     def get_assemble_scripts(self):
-        """Install the repository itself as a R package"""
+        """Install the dependencies of or the repository itself"""
         assemble_scripts = super().get_assemble_scripts()
 
+        installR_path = self.binder_path("install.R")
+        if os.path.exists(installR_path):
+            assemble_scripts += [
+                (
+                    "${NB_USER}",
+                    # only run install.R if the pre-assembly failed
+                    "if [ ! -f /tmp/.preassembled ]; then Rscript {}; fi".format(
+                        installR_path
+                    ),
+                )
+            ]
+
         description_R = "DESCRIPTION"
         if not self.binder_dir and os.path.exists(description_R):
             assemble_scripts += [