Skip to content

Commit

Permalink
Merge pull request #217 from zvezdan/fix-project-dir-handling
Browse files Browse the repository at this point in the history 
Fix handling of project name and directory.
  • Loading branch information
@zvezdan
zvezdan authored Apr 4, 2018
2 parents 8b3f685 + 4bb94cf commit 22b121a
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 34 deletions.
40 changes: 20 additions & 20 deletions .../integTest/groovy/com/linkedin/gradle/python/plugin/PackageSettingsIntegrationTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class PackageSettingsIntegrationTest extends Specification {
| import com.linkedin.gradle.python.util.PackageInfo
|
| class DefaultTestPackageSettings extends DefaultPackageSettings {
| DefaultTestPackageSettings(String projectName) { super(projectName) }
| DefaultTestPackageSettings(File projectDir) { super(projectDir) }
|
| @Override
| Map<String, String> getEnvironment(PackageInfo packageInfo) {
Expand All @@ -58,7 +58,7 @@ class PackageSettingsIntegrationTest extends Specification {
| import com.linkedin.gradle.python.tasks.PipInstallTask
|
| project.tasks.withType(PipInstallTask) { PipInstallTask task ->
| task.packageSettings = new DefaultTestPackageSettings(project.name)
| task.packageSettings = new DefaultTestPackageSettings(project.projectDir)
| }
|
|${PyGradleTestBuilder.createRepoClosure()}
Expand Down Expand Up @@ -100,7 +100,7 @@ class PackageSettingsIntegrationTest extends Specification {
| import com.linkedin.gradle.python.util.PackageInfo
|
| class DefaultTestPackageSettings extends DefaultPackageSettings {
| DefaultTestPackageSettings(String projectName) { super(projectName) }
| DefaultTestPackageSettings(File projectDir) { super(projectDir) }
|
| @Override
| List<String> getGlobalOptions(PackageInfo packageInfo) {
Expand All @@ -111,7 +111,7 @@ class PackageSettingsIntegrationTest extends Specification {
| import com.linkedin.gradle.python.tasks.PipInstallTask
|
| project.tasks.withType(PipInstallTask) { PipInstallTask task ->
| task.packageSettings = new DefaultTestPackageSettings(project.name)
| task.packageSettings = new DefaultTestPackageSettings(project.projectDir)
| }
|
|${PyGradleTestBuilder.createRepoClosure()}
Expand Down Expand Up @@ -152,7 +152,7 @@ class PackageSettingsIntegrationTest extends Specification {
| import com.linkedin.gradle.python.util.PackageInfo
|
| class DefaultTestPackageSettings extends DefaultPackageSettings {
| DefaultTestPackageSettings(String projectName) { super(projectName) }
| DefaultTestPackageSettings(File projectDir) { super(projectDir) }
|
| @Override
| List<String> getInstallOptions(PackageInfo packageInfo) {
Expand All @@ -163,7 +163,7 @@ class PackageSettingsIntegrationTest extends Specification {
| import com.linkedin.gradle.python.tasks.PipInstallTask
|
| project.tasks.withType(PipInstallTask) { PipInstallTask task ->
| task.packageSettings = new DefaultTestPackageSettings(project.name)
| task.packageSettings = new DefaultTestPackageSettings(project.projectDir)
| }
|
|${PyGradleTestBuilder.createRepoClosure()}
Expand Down Expand Up @@ -204,7 +204,7 @@ class PackageSettingsIntegrationTest extends Specification {
| import com.linkedin.gradle.python.util.PackageInfo
|
| class DefaultTestPackageSettings extends DefaultPackageSettings {
| DefaultTestPackageSettings(String projectName) { super(projectName) }
| DefaultTestPackageSettings(File projectDir) { super(projectDir) }
|
| @Override
| List<String> getSupportedLanguageVersions(PackageInfo packageInfo) {
Expand All @@ -215,7 +215,7 @@ class PackageSettingsIntegrationTest extends Specification {
| import com.linkedin.gradle.python.tasks.PipInstallTask
|
| project.tasks.withType(PipInstallTask) { PipInstallTask task ->
| task.packageSettings = new DefaultTestPackageSettings(project.name)
| task.packageSettings = new DefaultTestPackageSettings(project.projectDir)
| }
|
|${PyGradleTestBuilder.createRepoClosure()}
Expand Down Expand Up @@ -262,7 +262,7 @@ class PackageSettingsIntegrationTest extends Specification {
| import com.linkedin.gradle.python.util.PackageInfo
|
| class DefaultTestPackageSettings extends DefaultPackageSettings {
| DefaultTestPackageSettings(String projectName) { super(projectName) }
| DefaultTestPackageSettings(File projectDir) { super(projectDir) }
|
| @Override
| boolean requiresSourceBuild(PackageInfo packageInfo) {
Expand All @@ -273,7 +273,7 @@ class PackageSettingsIntegrationTest extends Specification {
| import com.linkedin.gradle.python.tasks.PipInstallTask
|
| project.tasks.withType(PipInstallTask) { PipInstallTask task ->
| task.packageSettings = new DefaultTestPackageSettings(project.name)
| task.packageSettings = new DefaultTestPackageSettings(project.projectDir)
| }
|
|${PyGradleTestBuilder.createRepoClosure()}
Expand Down Expand Up @@ -317,7 +317,7 @@ class PackageSettingsIntegrationTest extends Specification {
| import com.linkedin.gradle.python.util.PackageInfo
|
| class DefaultTestPackageSettings extends DefaultPackageSettings {
| DefaultTestPackageSettings(String projectName) { super(projectName) }
| DefaultTestPackageSettings(File projectDir) { super(projectDir) }
|
| @Override
| Map<String, String> getEnvironment(PackageInfo packageInfo) {
Expand All @@ -331,7 +331,7 @@ class PackageSettingsIntegrationTest extends Specification {
| import com.linkedin.gradle.python.tasks.BuildWheelsTask
|
| project.tasks.withType(BuildWheelsTask) { BuildWheelsTask task ->
| task.packageSettings = new DefaultTestPackageSettings(project.name)
| task.packageSettings = new DefaultTestPackageSettings(project.projectDir)
| }
|
|${PyGradleTestBuilder.createRepoClosure()}
Expand Down Expand Up @@ -374,7 +374,7 @@ class PackageSettingsIntegrationTest extends Specification {
| import com.linkedin.gradle.python.util.PackageInfo
|
| class DefaultTestPackageSettings extends DefaultPackageSettings {
| DefaultTestPackageSettings(String projectName) { super(projectName) }
| DefaultTestPackageSettings(File projectDir) { super(projectDir) }
|
| @Override
| List<String> getGlobalOptions(PackageInfo packageInfo) {
Expand All @@ -385,7 +385,7 @@ class PackageSettingsIntegrationTest extends Specification {
| import com.linkedin.gradle.python.tasks.BuildWheelsTask
|
| project.tasks.withType(BuildWheelsTask) { BuildWheelsTask task ->
| task.packageSettings = new DefaultTestPackageSettings(project.name)
| task.packageSettings = new DefaultTestPackageSettings(project.projectDir)
| }
|
|${PyGradleTestBuilder.createRepoClosure()}
Expand Down Expand Up @@ -435,7 +435,7 @@ class PackageSettingsIntegrationTest extends Specification {
| import com.linkedin.gradle.python.util.PackageInfo
|
| class DefaultTestPackageSettings extends DefaultPackageSettings {
| DefaultTestPackageSettings(String projectName) { super(projectName) }
| DefaultTestPackageSettings(File projectDir) { super(projectDir) }
|
| @Override
| List<String> getBuildOptions(PackageInfo packageInfo) {
Expand All @@ -446,7 +446,7 @@ class PackageSettingsIntegrationTest extends Specification {
| import com.linkedin.gradle.python.tasks.BuildWheelsTask
|
| project.tasks.withType(BuildWheelsTask) { BuildWheelsTask task ->
| task.packageSettings = new DefaultTestPackageSettings(project.name)
| task.packageSettings = new DefaultTestPackageSettings(project.projectDir)
| }
|
|${PyGradleTestBuilder.createRepoClosure()}
Expand Down Expand Up @@ -496,7 +496,7 @@ class PackageSettingsIntegrationTest extends Specification {
| import com.linkedin.gradle.python.util.PackageInfo
|
| class DefaultTestPackageSettings extends DefaultPackageSettings {
| DefaultTestPackageSettings(String projectName) { super(projectName) }
| DefaultTestPackageSettings(File projectDir) { super(projectDir) }
|
| @Override
| List<String> getSupportedLanguageVersions(PackageInfo packageInfo) {
Expand All @@ -507,7 +507,7 @@ class PackageSettingsIntegrationTest extends Specification {
| import com.linkedin.gradle.python.tasks.BuildWheelsTask
|
| project.tasks.withType(BuildWheelsTask) { BuildWheelsTask task ->
| task.packageSettings = new DefaultTestPackageSettings(project.name)
| task.packageSettings = new DefaultTestPackageSettings(project.projectDir)
| }
|
|${PyGradleTestBuilder.createRepoClosure()}
Expand Down Expand Up @@ -554,7 +554,7 @@ class PackageSettingsIntegrationTest extends Specification {
| import com.linkedin.gradle.python.util.PackageInfo
|
| class DefaultTestPackageSettings extends DefaultPackageSettings {
| DefaultTestPackageSettings(String projectName) { super(projectName) }
| DefaultTestPackageSettings(File projectDir) { super(projectDir) }
|
| @Override
| boolean requiresSourceBuild(PackageInfo packageInfo) {
Expand All @@ -565,7 +565,7 @@ class PackageSettingsIntegrationTest extends Specification {
| import com.linkedin.gradle.python.tasks.BuildWheelsTask
|
| project.tasks.withType(BuildWheelsTask) { BuildWheelsTask task ->
| task.packageSettings = new DefaultTestPackageSettings(project.name)
| task.packageSettings = new DefaultTestPackageSettings(project.projectDir)
| }
|
|${PyGradleTestBuilder.createRepoClosure()}
Expand Down
2 changes: 1 addition & 1 deletion pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/plugin/PythonPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public void apply(final Project project) {
createConfigurations(project);
configureVendedDependencies(project, settings);

DefaultPackageSettings packageSettings = new DefaultPackageSettings(project.getName());
DefaultPackageSettings packageSettings = new DefaultPackageSettings(project.getProjectDir());
project.getTasks().withType(SupportsPackageInfoSettings.class, it -> it.setPackageSettings(packageSettings));

/*
Expand Down
20 changes: 12 additions & 8 deletions pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/util/DefaultPackageSettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.linkedin.gradle.python.util;

import java.io.File;
import java.nio.file.Files;
import java.util.ArrayList;
import java.util.Collections;
Expand All @@ -31,10 +32,10 @@ public class DefaultPackageSettings implements PackageSettings<PackageInfo> {
private static final String PIP_EDITABLE = "--editable";
private static final String PIP_IGNORE_INSTALLED = "--ignore-installed";

private final String projectName;
private final File projectDir;

public DefaultPackageSettings(String projectName) {
this.projectName = projectName;
public DefaultPackageSettings(File projectDir) {
this.projectDir = projectDir;
}

@Override
Expand All @@ -49,9 +50,7 @@ public List<String> getGlobalOptions(PackageInfo packageInfo) {

@Override
public List<String> getInstallOptions(PackageInfo packageInfo) {
String name = packageInfo.getName();
String version = packageInfo.getVersion();
boolean isProjectDir = version == null && Files.isDirectory(packageInfo.getPackageFile().toPath());
List<String> options = new ArrayList<>();

// always reinstall snapshots
Expand All @@ -63,7 +62,7 @@ public List<String> getInstallOptions(PackageInfo packageInfo) {
* The current project is installed editable.
* This option **must be last** because it expects the directory name after it.
*/
if (projectName.equals(name) || isProjectDir) {
if (isProjectDirectory(packageInfo)) {
options.add(PIP_EDITABLE);
}

Expand All @@ -87,14 +86,19 @@ public List<String> getSupportedLanguageVersions(PackageInfo packageInfo) {

@Override
public boolean requiresSourceBuild(PackageInfo packageInfo) {
String name = packageInfo.getName();
String version = packageInfo.getVersion();

// always rebuild the project package itself
if (projectName.equals(name)) {
if (isProjectDirectory(packageInfo)) {
return true;
}
// always rebuild snapshots; otherwise no rebuild required, per semver versions with '-' are pre-release
return (version != null && version.contains("-"));
}

private boolean isProjectDirectory(PackageInfo packageInfo) {
File packageDir = packageInfo.getPackageFile();
String version = packageInfo.getVersion();
return version == null && Files.isDirectory(packageDir.toPath()) && projectDir.equals(packageDir);
}
}
12 changes: 7 additions & 5 deletions pygradle-plugin/src/test/groovy/com/linkedin/gradle/python/util/PackageSettingsTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,17 @@
*/
package com.linkedin.gradle.python.util

import org.gradle.testfixtures.ProjectBuilder
import spock.lang.Specification


/**
* Unit tests for package settings implementers.
*/
class PackageSettingsTest extends Specification {
PackageSettings<PackageInfo> packageSettings = new DefaultPackageSettings('foo')
def project = new ProjectBuilder().build()
def projectPath = project.file(project.getProjectDir())
PackageSettings<PackageInfo> packageSettings = new DefaultPackageSettings(project.projectDir)

def "default package settings environment"() {
expect: "empty environment"
Expand All @@ -47,8 +50,7 @@ class PackageSettingsTest extends Specification {

def "package settings install options for project snapshot()"() {
expect: "project snapshot uses --ignore-installed and --editable"
packageSettings.getInstallOptions(packageInGradleCache('foo-1.2.3-SNAPSHOT.tar.gz')) == [
'--ignore-installed', '--editable']
packageSettings.getInstallOptions(PackageInfo.fromPath(projectPath)) == ['--ignore-installed', '--editable']
}
def "default package settings build options"() {
Expand All @@ -68,7 +70,7 @@ class PackageSettingsTest extends Specification {
def "default package settings supported language versions"() {
expect: "empty supported language versions"
packageSettings.getSupportedLanguageVersions(packageInGradleCache('foo-1.2.3.tar.gz')) == []
packageSettings.getSupportedLanguageVersions(PackageInfo.fromPath(projectPath)) == []
}
def "default package settings requires source build"() {
Expand All @@ -83,7 +85,7 @@ class PackageSettingsTest extends Specification {
def "package settings requires a rebuild for the current project"() {
expect: "project requires a rebuild"
packageSettings.requiresSourceBuild(packageInGradleCache('foo-1.2.3.tar.gz'))
packageSettings.requiresSourceBuild(PackageInfo.fromPath(projectPath))
}
static PackageInfo packageInGradleCache(String name) {
Expand Down

0 comments on commit 22b121a

Please sign in to comment.