diff --git a/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/action/pip/WheelBuilder.java b/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/action/pip/WheelBuilder.java index e591916a..17f1b193 100644 --- a/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/action/pip/WheelBuilder.java +++ b/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/action/pip/WheelBuilder.java @@ -151,18 +151,33 @@ File getPackage(PackageInfo packageInfo, List extraArgs) { String name = packageInfo.getName(); String version = packageInfo.getVersion(); - boolean isProject = isProjectDirectory(packageInfo); + boolean isDirectory = isPackageDirectory(packageInfo); + boolean isProject = isDirectory && project.getProjectDir().equals(packageFile); Optional wheel; /* * Current project is a directory, not a package, so version may be null. - * Compensate for that. + * Set it to project's version. + * The generated code, such as rest.li can also be a path to directory. + * In that case the version will also be null and we need to set it to + * project's version. */ - if (isProject) { - name = project.getName(); + if (isDirectory) { + if (isProject) { + name = project.getName(); + } version = project.getVersion().toString(); } + /* + * Safety belt. + * This should be impossible and prevented by PackageInfo parsing already. + * However, if either of name/version is still null, return the original back. + */ + if (name == null || version == null) { + return packageFile; + } + // set the flag for doPipOperation customBuild = packageSettings.requiresSourceBuild(packageInfo) || packageSettings.isCustomized(packageInfo) @@ -262,10 +277,10 @@ private List cleanupArgs(List args) { return cleanArgs; } - private boolean isProjectDirectory(PackageInfo packageInfo) { + private boolean isPackageDirectory(PackageInfo packageInfo) { File packageDir = packageInfo.getPackageFile(); String version = packageInfo.getVersion(); - return version == null && Files.isDirectory(packageDir.toPath()) && project.getProjectDir().equals(packageDir); + return version == null && Files.isDirectory(packageDir.toPath()); } // Use of pythonEnvironment may hide really customized packages. Catch them! diff --git a/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/wheel/LayeredWheelCache.java b/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/wheel/LayeredWheelCache.java index 16786295..03d441a7 100644 --- a/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/wheel/LayeredWheelCache.java +++ b/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/wheel/LayeredWheelCache.java @@ -16,18 +16,20 @@ package com.linkedin.gradle.python.wheel; import com.linkedin.gradle.python.extension.PythonDetails; +import org.gradle.api.logging.Logger; +import org.gradle.api.logging.Logging; + import java.io.File; import java.io.IOException; import java.io.Serializable; import java.io.UncheckedIOException; import java.nio.file.Files; +import java.nio.file.StandardCopyOption; import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; -import org.gradle.api.logging.Logger; -import org.gradle.api.logging.Logging; public class LayeredWheelCache implements WheelCache, Serializable { @@ -74,8 +76,29 @@ public void storeWheel(File wheel, WheelCacheLayer wheelCacheLayer) { File cacheDir = layeredCacheMap.get(wheelCacheLayer); if (wheel != null && cacheDir != null) { + /* + * We want the attributes of the file preserved. + * Also, we want to overwrite the existing file when present. + * Although it seems unlikely, because we look for the wheel + * before trying to store it, the following scenario is possible + * and observed in testing. + * + * When two sub-projects do not find the wheel in any layer + * of the cache during build, they both proceed with the build + * into their respective project layers. One of them finishes + * first and also stores the wheel into the host layer, if not + * customized. The second one then must not fail trying to + * overwrite the existing file. + * + * Alternatively, we could check for existence of the target file + * and avoid overwriting. Notice that legacy code in PipWheelAction + * used FileUtils.copyFile from org.apache.commons.io.FileUtils + * which overwrites by default. We chose the same. It's simpler + * and perhaps even safer if this happens under any other scenario. + */ try { - Files.copy(wheel.toPath(), new File(cacheDir, wheel.getName()).toPath()); + Files.copy(wheel.toPath(), new File(cacheDir, wheel.getName()).toPath(), + StandardCopyOption.COPY_ATTRIBUTES, StandardCopyOption.REPLACE_EXISTING); } catch (IOException e) { throw new UncheckedIOException(e); } diff --git a/pygradle-plugin/src/test/groovy/com/linkedin/gradle/python/tasks/action/pip/WheelBuilderTest.groovy b/pygradle-plugin/src/test/groovy/com/linkedin/gradle/python/tasks/action/pip/WheelBuilderTest.groovy index eb25e38b..ef2e318a 100644 --- a/pygradle-plugin/src/test/groovy/com/linkedin/gradle/python/tasks/action/pip/WheelBuilderTest.groovy +++ b/pygradle-plugin/src/test/groovy/com/linkedin/gradle/python/tasks/action/pip/WheelBuilderTest.groovy @@ -383,6 +383,31 @@ class WheelBuilderTest extends Specification { } } + def 'builds generated code wheel but getting the project version for it'() { + setup: "do not return wheel from cache layers on first attempt" + def execSpec = Mock(ExecSpec) + def fakeWheel = 'fake/project-dir/wheel' + def stubWheelCache = Stub(WheelCache) { + getTargetDirectory() >> Optional.of(new File('fake/project-dir')) + findWheel(!null, !null, !null, WheelCacheLayer.PROJECT_LAYER) >>> [ + Optional.empty(), Optional.of(new File(fakeWheel))] + findWheel(!null, !null, !null, WheelCacheLayer.HOST_LAYER) >> Optional.empty() + } + def wheelBuilder = createWheelBuilder(execSpec, stubWheelCache) + def generatedDir = temporaryFolder.newFolder('build', 'generated', wheelBuilder.project.getName()) + + when: "we request the generated code directory to be built" + def pkg = wheelBuilder.getPackage(PackageInfo.fromPath(generatedDir), []) + + then: "wheel is built and returned for install" + assert pkg.toString() == fakeWheel + 1 * execSpec.commandLine(_) >> { List> args -> + println args + def it = args[0] + assert it[2] == 'wheel' + } + } + def "wheel with matching custom environment is built"() { setup: "return wheel stub from host layer only" def execSpec = Mock(ExecSpec) diff --git a/pygradle-plugin/src/test/groovy/com/linkedin/gradle/python/wheel/LayeredWheelCacheTest.groovy b/pygradle-plugin/src/test/groovy/com/linkedin/gradle/python/wheel/LayeredWheelCacheTest.groovy index a368ed62..5d17c1d5 100644 --- a/pygradle-plugin/src/test/groovy/com/linkedin/gradle/python/wheel/LayeredWheelCacheTest.groovy +++ b/pygradle-plugin/src/test/groovy/com/linkedin/gradle/python/wheel/LayeredWheelCacheTest.groovy @@ -155,4 +155,18 @@ class LayeredWheelCacheTest extends Specification { cache.findWheel('Sphinx', '1.6.3', pythonDetails, WheelCacheLayer.HOST_LAYER).isPresent() } + def "stores over already present wheel"() { + setup: "put the wheel in both caches" + def wheelFile = new File(otherCache, 'Sphinx-1.6.3-py2.py3-none-any.whl') + wheelFile.createNewFile() + cache.storeWheel(wheelFile) + def wheelInHostLayer = cache.findWheel('Sphinx', '1.6.3', pythonDetails, WheelCacheLayer.HOST_LAYER).get() + + when: "wheel is stored from host to project layer without raising an exception" + cache.storeWheel(wheelInHostLayer, WheelCacheLayer.PROJECT_LAYER) + + then: "wheel is found in both layers" + cache.findWheel('Sphinx', '1.6.3', pythonDetails, WheelCacheLayer.PROJECT_LAYER).isPresent() + cache.findWheel('Sphinx', '1.6.3', pythonDetails, WheelCacheLayer.HOST_LAYER).isPresent() + } }