From 7c791168bc74e0a8e02a011ba4328ec5685fe936 Mon Sep 17 00:00:00 2001 From: Emma Lautz Date: Tue, 19 Nov 2019 11:57:51 -0800 Subject: [PATCH] Reverted logic that has flake8 run in different venv Modified the Flake8Task to once again inherit from AbstractPythonMainSourceDefaultTask and use the original virtual environment instead of having a dedicated venv just for it's run. Deleted the AbstractPythonInfrastructureDefaultTask as Flake8Task was the only class using it. Modified PythonPlugin to have flake8 be added as a build configuration instead of it's own configuration. Deleted unit and integration tests ensuring that flake8 was run in its own environment. --- .circleci/config.yml | 2 +- .../ParallelWheelsIntegrationTest.groovy | 14 - .../gradle/python/plugin/PythonPlugin.java | 9 +- ...stractPythonInfrastructureDefaultTask.java | 270 ------------------ .../gradle/python/tasks/Flake8Task.java | 40 +-- .../python/util/StandardTextValues.java | 1 - .../gradle/python/tasks/Flake8TaskTest.groovy | 44 --- 7 files changed, 6 insertions(+), 374 deletions(-) delete mode 100644 pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/AbstractPythonInfrastructureDefaultTask.java delete mode 100644 pygradle-plugin/src/test/groovy/com/linkedin/gradle/python/tasks/Flake8TaskTest.groovy diff --git a/.circleci/config.yml b/.circleci/config.yml index c27e7864..83002739 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -4,7 +4,7 @@ jobs: docker: - image: openjdk:8-jdk environment: - _JAVA_OPTIONS: "-Xmx1024m -Xms256m" + _JAVA_OPTIONS: "-Xmx768m -Xms256m" GRADLE_OPTS: -Dorg.gradle.jvmargs=-Xmx1024m -Dorg.gradle.daemon=false steps: - run: diff --git a/pygradle-plugin/src/integTest/groovy/com/linkedin/gradle/python/plugin/ParallelWheelsIntegrationTest.groovy b/pygradle-plugin/src/integTest/groovy/com/linkedin/gradle/python/plugin/ParallelWheelsIntegrationTest.groovy index 7872a961..67a0ac04 100644 --- a/pygradle-plugin/src/integTest/groovy/com/linkedin/gradle/python/plugin/ParallelWheelsIntegrationTest.groovy +++ b/pygradle-plugin/src/integTest/groovy/com/linkedin/gradle/python/plugin/ParallelWheelsIntegrationTest.groovy @@ -92,20 +92,6 @@ class ParallelWheelsIntegrationTest extends Specification { then: out.toString() == "Hello World${ System.getProperty("line.separator") }".toString() - - when: - println "=======================" - result = GradleRunner.create() - .withProjectDir(testProjectDir.root) - - .withArguments('flake8', '--stacktrace', '--info') - .withPluginClasspath() - .withDebug(true) - .build() - println result.output - - then: - result.task(':foo:parallelWheels') == null //the task isn't part of the graph } @IgnoreIf({ OperatingSystem.current() == OperatingSystem.WINDOWS }) diff --git a/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/plugin/PythonPlugin.java b/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/plugin/PythonPlugin.java index ac21fe8f..860801c8 100644 --- a/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/plugin/PythonPlugin.java +++ b/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/plugin/PythonPlugin.java @@ -42,7 +42,6 @@ import static com.linkedin.gradle.python.util.StandardTextValues.CONFIGURATION_BOOTSTRAP_REQS; import static com.linkedin.gradle.python.util.StandardTextValues.CONFIGURATION_BUILD_REQS; -import static com.linkedin.gradle.python.util.StandardTextValues.CONFIGURATION_FLAKE8; import static com.linkedin.gradle.python.util.StandardTextValues.CONFIGURATION_PYDOCS; import static com.linkedin.gradle.python.util.StandardTextValues.CONFIGURATION_PYTHON; import static com.linkedin.gradle.python.util.StandardTextValues.CONFIGURATION_SETUP_REQS; @@ -156,15 +155,11 @@ private static void createConfigurations(Project project) { project.getConfigurations().create(CONFIGURATION_BOOTSTRAP_REQS.getValue()); project.getConfigurations().create(CONFIGURATION_SETUP_REQS.getValue()); - Configuration buildReq = project.getConfigurations().create(CONFIGURATION_BUILD_REQS.getValue()); + project.getConfigurations().create(CONFIGURATION_BUILD_REQS.getValue()); project.getConfigurations().create(CONFIGURATION_PYDOCS.getValue()); project.getConfigurations().create(CONFIGURATION_TEST.getValue()); project.getConfigurations().create(CONFIGURATION_VENV.getValue()); - Configuration flake8 = project.getConfigurations().create(CONFIGURATION_FLAKE8.getValue()); project.getConfigurations().create(CONFIGURATION_WHEEL.getValue()); - - //So flake8 will be installed into the activate-able venv - buildReq.extendsFrom(flake8); } /* @@ -183,7 +178,7 @@ private static void configureVendedDependencies(Project project, PythonExtension project.getDependencies().add(CONFIGURATION_SETUP_REQS.getValue(), settings.forcedVersions.get("pip")); project.getDependencies().add(CONFIGURATION_SETUP_REQS.getValue(), settings.forcedVersions.get("setuptools-git")); - project.getDependencies().add(CONFIGURATION_FLAKE8.getValue(), settings.forcedVersions.get("flake8")); + project.getDependencies().add(CONFIGURATION_BUILD_REQS.getValue(), settings.forcedVersions.get("flake8")); project.getDependencies().add(CONFIGURATION_BUILD_REQS.getValue(), settings.forcedVersions.get("Sphinx")); diff --git a/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/AbstractPythonInfrastructureDefaultTask.java b/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/AbstractPythonInfrastructureDefaultTask.java deleted file mode 100644 index 72743028..00000000 --- a/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/AbstractPythonInfrastructureDefaultTask.java +++ /dev/null @@ -1,270 +0,0 @@ -/* - * Copyright 2016 LinkedIn Corp. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.linkedin.gradle.python.tasks; - -import com.linkedin.gradle.python.PythonExtension; -import com.linkedin.gradle.python.exception.PipExecutionException; -import com.linkedin.gradle.python.extension.PythonDetails; -import com.linkedin.gradle.python.plugin.PythonHelpers; -import com.linkedin.gradle.python.tasks.action.CreateVirtualEnvAction; -import com.linkedin.gradle.python.tasks.action.VirtualEnvCustomizer; -import com.linkedin.gradle.python.tasks.action.pip.PipInstallAction; -import com.linkedin.gradle.python.tasks.exec.ProjectExternalExec; -import com.linkedin.gradle.python.tasks.execution.FailureReasonProvider; -import com.linkedin.gradle.python.tasks.execution.TeeOutputContainer; -import com.linkedin.gradle.python.tasks.provides.ProvidesVenv; -import com.linkedin.gradle.python.tasks.supports.SupportsDistutilsCfg; -import com.linkedin.gradle.python.tasks.supports.SupportsPackageFiltering; -import com.linkedin.gradle.python.tasks.supports.SupportsPackageInfoSettings; -import com.linkedin.gradle.python.tasks.supports.SupportsWheelCache; -import com.linkedin.gradle.python.util.DefaultEnvironmentMerger; -import com.linkedin.gradle.python.util.DependencyOrder; -import com.linkedin.gradle.python.util.EnvironmentMerger; -import com.linkedin.gradle.python.util.PackageInfo; -import com.linkedin.gradle.python.util.PackageSettings; -import com.linkedin.gradle.python.wheel.EditablePythonAbiContainer; -import com.linkedin.gradle.python.wheel.EmptyWheelCache; -import com.linkedin.gradle.python.wheel.WheelCache; -import org.apache.commons.io.FileUtils; -import org.gradle.api.DefaultTask; -import org.gradle.api.artifacts.Configuration; -import org.gradle.api.file.ConfigurableFileTree; -import org.gradle.api.file.FileCollection; -import org.gradle.api.logging.Logger; -import org.gradle.api.logging.Logging; -import org.gradle.api.specs.Spec; -import org.gradle.api.tasks.Input; -import org.gradle.api.tasks.InputFiles; -import org.gradle.api.tasks.Internal; -import org.gradle.api.tasks.Optional; -import org.gradle.api.tasks.TaskAction; -import org.gradle.process.ExecResult; -import org.gradle.process.ExecSpec; - -import javax.annotation.Nullable; -import java.io.File; -import java.io.OutputStream; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.List; - -import static com.linkedin.gradle.python.util.StandardTextValues.CONFIGURATION_SETUP_REQS; - - -/** - * This class is used to make sure that the up-to-date logic works. It also allows for lazy evaluation - * of the sources, which comes from the lazy eval of the getPythonExtension(). It's lazy because its a method call - * and will only get executed right before gradle tries to figure out the inputs/outputs. By making it lazy - * will allow {@link PythonExtension} to be updated by the project and be complete - * when its used in the tasks. - */ -abstract public class AbstractPythonInfrastructureDefaultTask extends DefaultTask implements FailureReasonProvider, - SupportsPackageInfoSettings, SupportsDistutilsCfg, SupportsPackageFiltering, SupportsWheelCache, ProvidesVenv { - - private static final Logger log = Logging.getLogger(AbstractPythonInfrastructureDefaultTask.class); - - private List arguments = new ArrayList<>(); - private PythonExtension pythonExtension; - private String lastMessage; - - @Input - @Optional - private String distutilsCfg; - private PackageSettings packageSettings; - private Spec packageExcludeFilter; - private WheelCache wheelCache = new EmptyWheelCache(); - private EditablePythonAbiContainer editablePythonAbiContainer; - - private EnvironmentMerger environmentMerger = new DefaultEnvironmentMerger(); - - @Input - public boolean ignoreExitValue = false; - - public OutputStream stdOut = System.out; - - public OutputStream errOut = System.err; - - @Input - abstract protected File getVenvPath(); - - @InputFiles - abstract protected Configuration getInstallConfiguration(); - - abstract protected boolean isVenvReady(); - - @InputFiles - public FileCollection getSourceFiles() { - ConfigurableFileTree componentFiles = getProject().fileTree(getPythonExtension().srcDir); - componentFiles.exclude(standardExcludes()); - return componentFiles; - } - - public String[] standardExcludes() { - return new String[]{"**/*.pyc", "**/*.pyo", "**/__pycache__/", "**/*.egg-info/"}; - } - - @Internal - public PythonExtension getPythonExtension() { - if (null == pythonExtension) { - pythonExtension = getProject().getExtensions().getByType(PythonExtension.class); - } - return pythonExtension; - } - - @Internal - abstract public PythonDetails getPythonDetails(); - - public void args(String... args) { - arguments.addAll(Arrays.asList(args)); - } - - public void args(Collection args) { - arguments.addAll(args); - } - - @TaskAction - public void executePythonProcess() { - if (!isVenvReady()) { - prepairVenv(); - } - - preExecution(); - - final TeeOutputContainer container = new TeeOutputContainer(stdOut, errOut); - - ExecResult result = getProject().exec(execSpec -> { - execSpec.environment(getPythonExtension().pythonEnvironment); - execSpec.commandLine(getPythonDetails().getVirtualEnvInterpreter()); - // arguments are passed to the python interpreter - execSpec.args(arguments); - execSpec.setIgnoreExitValue(true); - - container.setOutputs(execSpec); - - configureExecution(execSpec); - }); - - lastMessage = container.getCommandOutput(); - - if (!ignoreExitValue) { - result.assertNormalExitValue(); - } - - processResults(result); - } - - protected void prepairVenv() { - // We don't know what happened to this venv, so lets kill it - PythonDetails pythonDetails = getPythonDetails(); - if (pythonDetails.getVirtualEnv().exists()) { - FileUtils.deleteQuietly(pythonDetails.getVirtualEnv()); - } - - ProjectExternalExec externalExec = new ProjectExternalExec(getProject()); - - CreateVirtualEnvAction action = new CreateVirtualEnvAction(getProject(), pythonDetails, editablePythonAbiContainer); - action.buildVenv(new VirtualEnvCustomizer(distutilsCfg, externalExec, pythonDetails)); - - PipInstallAction pipInstallAction = new PipInstallAction(packageSettings, getProject(), - externalExec, getPythonExtension().pythonEnvironment, - pythonDetails, wheelCache, environmentMerger, packageExcludeFilter); - - installPackages(pipInstallAction, getProject().getConfigurations().getByName(CONFIGURATION_SETUP_REQS.getValue())); - installPackages(pipInstallAction, getInstallConfiguration()); - } - - private void installPackages(PipInstallAction pipInstallAction, Configuration configuration) { - DependencyOrder.configurationPostOrderFiles(configuration).forEach(file -> { - PackageInfo packageInfo = PackageInfo.fromPath(file); - if (packageExcludeFilter != null && packageExcludeFilter.isSatisfiedBy(packageInfo)) { - if (PythonHelpers.isPlainOrVerbose(getProject())) { - log.lifecycle("Skipping {} - Excluded", packageInfo.toShortHand()); - } - } else { - try { - pipInstallAction.execute(packageInfo, Collections.emptyList()); - } catch (PipExecutionException e) { - lastMessage = e.getPipText(); - throw e; - } - } - }); - } - - public void configureExecution(ExecSpec spec) { - // Here for users to override with other impl - } - - public void preExecution() { - // Here for users to override with other impl - } - - public abstract void processResults(ExecResult execResult); - - @Override - public void setPackageSettings(PackageSettings settings) { - packageSettings = settings; - } - - @Override - public PackageSettings getPackageSettings() { - return packageSettings; - } - - @Override - public void setDistutilsCfg(String cfg) { - distutilsCfg = cfg; - } - - @Override - public String getDistutilsCfg() { - return distutilsCfg; - } - - @Nullable - @Override - public Spec getPackageExcludeFilter() { - return packageExcludeFilter; - } - - @Override - public void setPackageExcludeFilter(Spec filter) { - packageExcludeFilter = filter; - } - - @Internal - @Override - public String getReason() { - return lastMessage; - } - - @Override - public WheelCache getWheelCache() { - return wheelCache; - } - - @Override - public void setWheelCache(WheelCache wheelCache) { - this.wheelCache = wheelCache; - } - - @Override - public void setEditablePythonAbiContainer(EditablePythonAbiContainer editablePythonAbiContainer) { - this.editablePythonAbiContainer = editablePythonAbiContainer; - } -} diff --git a/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/Flake8Task.java b/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/Flake8Task.java index 2b951877..9e70b52e 100644 --- a/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/Flake8Task.java +++ b/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/Flake8Task.java @@ -16,14 +16,8 @@ package com.linkedin.gradle.python.tasks; import com.linkedin.gradle.python.PythonExtension; -import com.linkedin.gradle.python.extension.PythonDetails; -import com.linkedin.gradle.python.extension.PythonDetailsFactory; -import com.linkedin.gradle.python.tasks.supports.SupportsDistutilsCfg; -import com.linkedin.gradle.python.tasks.supports.SupportsPackageFiltering; -import com.linkedin.gradle.python.tasks.supports.SupportsPackageInfoSettings; import com.linkedin.gradle.python.util.ExtensionUtils; import org.apache.commons.io.FileUtils; -import org.gradle.api.artifacts.Configuration; import org.gradle.api.logging.Logger; import org.gradle.api.logging.Logging; import org.gradle.process.ExecResult; @@ -31,50 +25,22 @@ import java.io.File; import java.io.IOException; import java.io.UncheckedIOException; -import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.List; -public class Flake8Task extends AbstractPythonInfrastructureDefaultTask implements SupportsPackageInfoSettings, - SupportsDistutilsCfg, SupportsPackageFiltering { +public class Flake8Task extends AbstractPythonMainSourceDefaultTask { private static final Logger log = Logging.getLogger(Flake8Task.class); - @Override - public File getVenvPath() { - return getProject().getBuildDir().toPath().resolve(Paths.get("infra-venv", "flake8")).toFile(); - } - - @Override - public Configuration getInstallConfiguration() { - return getProject().getConfigurations().getByName("flake8"); - } - - @Override - public PythonDetails getPythonDetails() { - PythonDetails projectPythonDetails = ExtensionUtils.getPythonExtension(getProject()).getDetails(); - return PythonDetailsFactory.withNewVenv(getProject(), projectPythonDetails, getVenvPath()); - } - - @Override - protected boolean isVenvReady() { - PythonDetails flake8Python = getPythonDetails(); - File flake8Exec = flake8Python.getVirtualEnvironment().findExecutable("flake8"); - return flake8Exec.exists() && flake8Exec.isFile(); - } - public void preExecution() { - PythonDetails flake8Python = getPythonDetails(); - - File flake8Exec = flake8Python.getVirtualEnvironment().findExecutable("flake8"); + PythonExtension pythonExtension = ExtensionUtils.getPythonExtension(getProject()); + File flake8Exec = pythonExtension.getDetails().getVirtualEnvironment().findExecutable("flake8"); /* Modified to only include folders that exist. if no folders exist, then the task isn't actually run. */ - PythonExtension pythonExtension = getPythonExtension(); - List paths = new ArrayList<>(); if (getProject().file(pythonExtension.srcDir).exists()) { log.info("Flake8: adding {}", pythonExtension.srcDir); diff --git a/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/util/StandardTextValues.java b/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/util/StandardTextValues.java index a39617b3..b048aa8d 100644 --- a/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/util/StandardTextValues.java +++ b/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/util/StandardTextValues.java @@ -30,7 +30,6 @@ public enum StandardTextValues { CONFIGURATION_TEST("test"), CONFIGURATION_VENV("venv"), CONFIGURATION_WHEEL("wheel"), - CONFIGURATION_FLAKE8("flake8"), TASK_BUILD_DOCS("buildDocs"), TASK_CLEAN_SAVE_VENV("cleanSaveVenv"), TASK_CHECK("check"), diff --git a/pygradle-plugin/src/test/groovy/com/linkedin/gradle/python/tasks/Flake8TaskTest.groovy b/pygradle-plugin/src/test/groovy/com/linkedin/gradle/python/tasks/Flake8TaskTest.groovy deleted file mode 100644 index 905ceee7..00000000 --- a/pygradle-plugin/src/test/groovy/com/linkedin/gradle/python/tasks/Flake8TaskTest.groovy +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright 2016 LinkedIn Corp. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.linkedin.gradle.python.tasks - -import com.linkedin.gradle.python.plugin.PythonPlugin -import com.linkedin.gradle.python.util.ExtensionUtils -import org.gradle.testfixtures.ProjectBuilder -import spock.lang.Specification - -class Flake8TaskTest extends Specification { - - def 'python details will use existing details from project'() { - def project = new ProjectBuilder().build() - project.plugins.apply(PythonPlugin) - - def flake8Task = project.tasks.flake8 as Flake8Task - def extension = ExtensionUtils.getPythonExtension(project) - def firstInterpreter = extension.details.systemPythonInterpreter - - expect: - flake8Task.getPythonDetails().getSystemPythonInterpreter() == extension.details.systemPythonInterpreter - - when: - extension.details.setPythonInterpreter(extension.details.getPythonVersion(), new File("/foo/bar")) - - then: - flake8Task.getPythonDetails().getSystemPythonInterpreter() != firstInterpreter - flake8Task.getPythonDetails().getSystemPythonInterpreter() == extension.details.systemPythonInterpreter - - } -}