Core review round 3.

build.gradle

@@ -69,40 +69,64 @@ final baseJarName = 'gatk'
 final secondaryBaseJarName = 'hellbender'
 final pythonPackageArchiveName = 'gatkPythonPackageArchive.zip'
 final largeResourcesFolder = "src/main/resources/large"
-final lfsFileSentinel = "version https://git-lfs.github.com/spec/v1"
+// the first line of a git-lfs stub file contains this
+final lfsStubFileHeader = "version https://git-lfs.github.com/spec/v1"
 final buildPrerequisitesMessage = "See https://github.com/broadinstitute/gatk#building for information on how to build GATK."
 
-// Large runtime resource files must be present at build time to be compiled into the jar, so validate
-// that the lfs files have been fetched and that the directory does not contain the git-lfs pointer files
-def validateLargeResourceFile(largeResourceFile, lfsSentinel, buildPrerequisitesMessage) {
+// Large runtime resource files must be present at build time to be compiled into the jar, so force
+// the contents of the large runtime resources to be resolved to real files rather than stub files
+def gitPullLargeResources(largeResourcesFolder) {
+    final String execLFSPull = "git lfs pull --include $largeResourcesFolder";
+    try {
+        println "Fetching large runtime resources via: $execLFSPull"
+        def retCode = execLFSPull.execute().waitFor()
+        if (retCode.intValue() != 0) {
+            throw new GradleException("git-lfs pull execution failed: $retCode");
+        }
+        return retCode
+    } catch (IOException e) {
+        throw new GradleException("git-lfs must be installed to run the GATK build", e)
+    }
+}
+
+// Check to see if the large resource lfs files have been fetched and that the directory does not contain
+// the git-lfs stub files. Returns truy
+def checkLargeResourcesForStubFiles(largeResourcesFolder, lfsStubFileHeader, buildPrerequisitesMessage) {
     def readBytesFromFile = { largeFile, n ->
         final byte[] bytes = new byte[n]
-        largeResourceFile.withInputStream { stream -> stream.read(bytes, 0, bytes.length) }
+        largeFile.withInputStream { stream -> stream.read(bytes, 0, bytes.length) }
         return bytes
     }
-    final byte[] actualBytes = readBytesFromFile(largeResourceFile, lfsSentinel.length());
-    if (new String(actualBytes, "UTF-8") == lfsSentinel) {
-        throw new GradleException(
-                "$largeResourceFile appears to be a git-lfs pointer file." +
-                        " The resource files in src/main/resources/large must be downloaded using git-lfs. " +
-                        " $buildPrerequisitesMessage")
+    def largeResourceFiles = fileTree(dir: largeResourcesFolder)
+    return largeResourceFiles.any() { f ->
+        final byte[] actualBytes = readBytesFromFile(f, lfsStubFileHeader.length());
+        return new String(actualBytes, "UTF-8") == lfsStubFileHeader
     }
 }
 
 // Validate that we have a clone of the repository and that any required git-lfs files have been resolved
-def validateBuildPrerequisites(javaVersion, largeResourcesFolder, lfsSentinel, buildPrerequisitesMessage) {
+def validateBuildPrerequisites(javaVersion, largeResourcesFolder, lfsStubFileHeader, buildPrerequisitesMessage) {
     if (ToolProvider.getSystemToolClassLoader() == null) {
-        throw new GradleException("The SystemToolClassLoader obtained from the ToolProvider is null. A Java $RequiredJavaVersion JDK must be installed. $buildPrerequisitesMessage")
+        throw new GradleException(
+                "The SystemToolClassLoader obtained from the ToolProvider is null. A Java $RequiredJavaVersion JDK must be installed. $buildPrerequisitesMessage")
     }
     if (!file(".git").isDirectory()) {
         throw new GradleException("The GATK Github repository must be cloned using \"git clone\" to run the build. $buildPrerequisitesMessage")
     }
     // Next, ensure that the git-lfs files required to run the build are present
-    def largeResources = fileTree(dir: largeResourcesFolder)
-    largeResources.forEach {f -> validateLargeResourceFile(f, lfsSentinel, buildPrerequisitesMessage)}
+    if (checkLargeResourcesForStubFiles(largeResourcesFolder, lfsStubFileHeader, buildPrerequisitesMessage)) {
+        // try to pull once, then verify again
+        gitPullLargeResources(largeResourcesFolder)
+        if (checkLargeResourcesForStubFiles(largeResourcesFolder, lfsStubFileHeader, buildPrerequisitesMessage)) {
+            throw new GradleException(
+                    "$largeResourcesFolder contains git-lfs stub files." +
+                            " The resource files in $largeResourcesFolder must be downloaded using git-lfs. " +
+                            " $buildPrerequisitesMessage")
+        }
+    }
 }
 
-validateBuildPrerequisites(requiredJavaVersion, largeResourcesFolder, lfsFileSentinel, buildPrerequisitesMessage)
+validateBuildPrerequisites(requiredJavaVersion, largeResourcesFolder, lfsStubFileHeader, buildPrerequisitesMessage)
 
 configurations.all {
     resolutionStrategy {
@@ -297,6 +321,8 @@ processResources {
 processTestResources {
     //Don't waste time packaging unnecessary test data into the test resources:
     include "org/broadinstitute/hellbender/utils/config/*"
+    //Required for IOUtils resource tests
+    include "org/broadinstitute/hellbender/utils/io/*"
 }
 
 sourceCompatibility = 1.8

src/test/java/org/broadinstitute/hellbender/utils/io/IOUtilsUnitTest.java

@@ -269,40 +269,42 @@ public void testUrlEncodeDecode(final String string, final String encoded) {
 
     @DataProvider(name="resourcePaths")
     public Object[][] getResourcePaths() {
+        final String testResourceContents = "this is a test resource file";
         return new Object[][] {
-                { Resource.LARGE_RUNTIME_RESOURCES_PATH + "/largeResourceTest.txt", null }
+                { Resource.LARGE_RUNTIME_RESOURCES_PATH + "/testResourceFile.txt", null , testResourceContents},
+                { "testResourceFile.txt", IOUtils.class , testResourceContents},
         };
     }
 
     @Test(dataProvider = "resourcePaths")
-    public void testLoadLargeRuntimeResource(final String resourcePath, final Class<?> relativeClass) throws IOException {
-        final Resource largeResource = new Resource(resourcePath, null);
+    public void testWriteTempResource(
+            final String resourcePath, final Class<?> relativeClass, final String expectedFirstLine) throws IOException
+    {
+        final Resource largeResource = new Resource(resourcePath, relativeClass);
         final File resourceFile = IOUtils.writeTempResource(largeResource);
-
-        String resourceContents = "";
-        try (final FileReader fr = new FileReader(resourceFile);
-             final BufferedReader br = new BufferedReader(fr)) {
-            resourceContents = br.readLine();
-        }
-        finally {
-            resourceFile.delete();
-        }
-        Assert.assertEquals(resourceContents, "line in large resource file");
+        final String resourceContentsFirstLine = getFirstLineAndDeleteTempFile(resourceFile);
+        Assert.assertEquals(resourceContentsFirstLine, expectedFirstLine);
     }
 
     @Test(dataProvider = "resourcePaths")
-    public void testTempResourceFromPath(final String resourcePath, final Class<?> relativeClass) throws IOException {
+    public void testWriteTempResourceFromPath(
+            final String resourcePath, final Class<?> relativeClass, final String expectedFirstLine) throws IOException
+    {
         final File resourceFile = IOUtils.writeTempResourceFromPath(resourcePath, relativeClass);
+        final String resourceContentsFirstLine = getFirstLineAndDeleteTempFile(resourceFile);
+        Assert.assertEquals(resourceContentsFirstLine, expectedFirstLine);
+    }
 
-        String resourceContents = "";
-        try (final FileReader fr = new FileReader(resourceFile);
+    private String getFirstLineAndDeleteTempFile(final File tempResourceFile) throws IOException {
+        String resourceContentsFirstLine = "";
+        try (final FileReader fr = new FileReader(tempResourceFile);
              final BufferedReader br = new BufferedReader(fr)) {
-            resourceContents = br.readLine();
+            resourceContentsFirstLine = br.readLine();
         }
         finally {
-            resourceFile.delete();
+            tempResourceFile.delete();
         }
-        Assert.assertEquals(resourceContents, "line in large resource file");
+        return resourceContentsFirstLine;
     }
 
 }

src/test/resources/org/broadinstitute/hellbender/utils/io/testResourceFile.txt

@@ -0,0 +1 @@
+this is a test resource file