From c963407163220b523dfacabd8fc28ef591792c07 Mon Sep 17 00:00:00 2001 From: Zvezdan Petkovic Date: Thu, 18 Apr 2019 22:47:57 -0700 Subject: [PATCH] Treat project wheel as a special case. The project wheel will be built after the editable install for development. This ensures that the file manifest is generated for the project package before the wheel is built. Otherwise, wheel does not include resource files that `include_package_data=True` ensures to be packed into an sdist. This also helps some packages that generate code after the development install and expect that code to be packed into the wheel. --- .../python/tasks/BuildWheelsTask.groovy | 9 ++-- .../python/tasks/action/pip/WheelBuilder.java | 45 +++++++------------ .../tasks/action/pip/WheelBuilderTest.groovy | 18 ++------ 3 files changed, 26 insertions(+), 46 deletions(-) diff --git a/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/BuildWheelsTask.groovy b/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/BuildWheelsTask.groovy index 1d318d16..a2f14fee 100644 --- a/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/BuildWheelsTask.groovy +++ b/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/BuildWheelsTask.groovy @@ -34,7 +34,6 @@ import com.linkedin.gradle.python.util.PackageInfo import com.linkedin.gradle.python.util.PackageSettings import com.linkedin.gradle.python.util.internal.TaskTimer import com.linkedin.gradle.python.wheel.EmptyWheelCache -import com.linkedin.gradle.python.wheel.LayeredWheelCache import com.linkedin.gradle.python.wheel.WheelCache import org.gradle.api.DefaultTask import org.gradle.api.Project @@ -80,8 +79,12 @@ class BuildWheelsTask extends DefaultTask implements SupportsWheelCache, Support @TaskAction void buildWheelsTask() { - // With LayeredWheelCache, wheels are almost always already built where this task would put them. - if (!wheelCache.isWheelsReady()) { + /* + * With LayeredWheelCache, wheels are almost always already built where this task would put them. + * Project wheel is an exception. + */ + boolean isProject = installFileCollection.size() == 1 && installFileCollection.contains(project.getProjectDir()) + if (isProject || !wheelCache.isWheelsReady()) { buildWheels(project, DependencyOrder.getConfigurationFiles(installFileCollection), getPythonDetails()) } } 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 20ef2104..f82ee656 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 @@ -149,23 +149,21 @@ File getPackage(PackageInfo packageInfo, List extraArgs) { return packageFile; } + // Project wheel will be built after the editable install for development. + if (isPackageDirectory(packageInfo) && project.getProjectDir().equals(packageFile)) { + return packageFile; + } + String name = packageInfo.getName(); String version = packageInfo.getVersion(); - 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. - * 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 + * The generated code, such as rest.li can be a path to directory. + * In that case the version will be null and we need to set it to * project's version. */ - if (isDirectory) { - if (isProject) { - name = project.getName(); - } + if (isPackageDirectory(packageInfo)) { version = project.getVersion().toString(); } @@ -188,20 +186,14 @@ File getPackage(PackageInfo packageInfo, List extraArgs) { wheel = wheelCache.findWheel(name, version, pythonDetails, WheelCacheLayer.PROJECT_LAYER); if (wheel.isPresent()) { packageFile = wheel.get(); - if (PythonHelpers.isPlainOrVerbose(project)) { - logger.lifecycle("{} from wheel: {}", - packageInfo.toShortHand(), packageFile.getAbsolutePath()); - } + logLifecycle(packageInfo, packageFile); return packageFile; } else if (!customBuild) { wheel = wheelCache.findWheel(name, version, pythonDetails, WheelCacheLayer.HOST_LAYER); if (wheel.isPresent()) { packageFile = wheel.get(); wheelCache.storeWheel(packageFile, WheelCacheLayer.PROJECT_LAYER); - if (PythonHelpers.isPlainOrVerbose(project)) { - logger.lifecycle("{} from wheel: {}", - packageInfo.toShortHand(), packageFile.getAbsolutePath()); - } + logLifecycle(packageInfo, packageFile); return packageFile; } } @@ -237,16 +229,6 @@ File getPackage(PackageInfo packageInfo, List extraArgs) { } } - /* - * After ensuring we have a wheel built for our project, - * we still return the project directory back to PipInstallAction - * so that it can install it in editable (development) mode - * into virtualenv. - */ - if (isProject) { - return packageInfo.getPackageFile(); - } - return packageFile; } @@ -298,4 +280,11 @@ private boolean isCustomEnvironment(String name) { } return false; } + + private void logLifecycle(PackageInfo packageInfo, File packageFile) { + if (PythonHelpers.isPlainOrVerbose(project)) { + logger.lifecycle("{} from wheel: {}", + packageInfo.toShortHand(), packageFile.getAbsolutePath()); + } + } } 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 2fb4d14b..818ea0ad 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 @@ -359,32 +359,20 @@ class WheelBuilderTest extends Specification { } } - def 'builds project wheel but returns the project itself'() { + def 'does not build project wheel but returns the project itself'() { setup: "do not return wheel from cache layers on first attempt" def execSpec = Mock(ExecSpec) - def fakeWheel = 'fake/project-dir/wheel' - // Cardinality of calls on stubs cannot be asserted, as opposed to mocks, so we keep the counter. - def storeCounter = 0 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() - storeWheel(!null, WheelCacheLayer.HOST_LAYER) >> { storeCounter++ } } def wheelBuilder = createWheelBuilder(execSpec, stubWheelCache) when: "we request project to be built" def pkg = wheelBuilder.getPackage(PackageInfo.fromPath(wheelBuilder.project.getProjectDir()), []) - then: "wheel is built but not stored to host layer and project directory returned for editable install" + then: "wheel is not built and project directory is returned for editable install" pkg.toString() == wheelBuilder.project.getProjectDir().toString() - storeCounter == 0 - 1 * execSpec.commandLine(_) >> { List> args -> - println args - def it = args[0] - assert it[2] == 'wheel' - } + 0 * execSpec._ } def 'builds generated code wheel but getting the project version for it'() {