Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use System's path separator for classpaths #575

Merged
merged 1 commit into from
Sep 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public final class BuildParameterBuilder {
private static final Logger LOGGER = Logger.getLogger(BuildParameterBuilder.class.getName());
private static final String SMITHY_TAG_PROPERTY = "Smithy-Tags";
private static final String SOURCE = "source";
private static final String PATH_SEPARATOR = "path.separator";

private String projectionSource = SOURCE;
private Set<String> projectionSourceTags = new LinkedHashSet<>();
Expand Down Expand Up @@ -135,7 +136,7 @@ public BuildParameterBuilder addSourcesIfExists(Collection<String> sources) {
* @return Returns the builder.
*/
public BuildParameterBuilder buildClasspath(String buildClasspath) {
this.buildClasspath.addAll(splitAndFilterString(":", buildClasspath));
this.buildClasspath.addAll(splitAndFilterString(System.getProperty(PATH_SEPARATOR), buildClasspath));
return this;
}

Expand All @@ -157,7 +158,7 @@ private static Set<String> splitAndFilterString(String delimiter, String value)
* @return Returns the builder.
*/
public BuildParameterBuilder libClasspath(String libClasspath) {
this.libClasspath.addAll(splitAndFilterString(":", libClasspath));
this.libClasspath.addAll(splitAndFilterString(System.getProperty(PATH_SEPARATOR), libClasspath));
return this;
}

Expand Down Expand Up @@ -357,7 +358,9 @@ private Result configureSourceProjection() {
Set<String> combined = new LinkedHashSet<>(libClasspath);
combined.addAll(buildClasspath);

return new Result(this, String.join(":", computedDiscovery), String.join(":", combined), sources);
String discoveryClasspath = String.join(System.getProperty(PATH_SEPARATOR), computedDiscovery);
String classpath = String.join(System.getProperty(PATH_SEPARATOR), combined);
return new Result(this, discoveryClasspath, classpath, sources);
}

/**
Expand All @@ -373,7 +376,7 @@ private Result configureProjection() {
LOGGER.warning("No projection source tags were set for the projection `" + projection + "`, so the "
+ "projection will not have any sources in it other than files found in the sources of "
+ "the package being built.");
String buildCp = String.join(":", buildClasspath);
String buildCp = String.join(System.getProperty(PATH_SEPARATOR), buildClasspath);
return new Result(this, buildCp, buildCp, sources);
}

Expand All @@ -390,8 +393,9 @@ private Result configureProjection() {
Set<String> computedDiscovery = new LinkedHashSet<>(buildClasspath);
computedDiscovery.removeAll(computedSources);

return new Result(this, String.join(":", computedDiscovery),
String.join(":", buildClasspath), computedSources);
String discoveryClasspath = String.join(System.getProperty(PATH_SEPARATOR), computedDiscovery);
String classpath = String.join(System.getProperty(PATH_SEPARATOR), buildClasspath);
return new Result(this, discoveryClasspath, classpath, computedSources);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ private static void handleModelDiscovery(Arguments arguments, ModelAssembler ass
private static void discoverModelsWithClasspath(Arguments arguments, ModelAssembler assembler) {
String rawClasspath = arguments.parameter(SmithyCli.DISCOVER_CLASSPATH);
LOGGER.finer("Discovering models with classpath: " + rawClasspath);
String[] classpath = rawClasspath.split(":");

// Use System.getProperty here each time since it allows the value to be changed.
String[] classpath = rawClasspath.split(System.getProperty("path.separator"));
URL[] urls = new URL[classpath.length];

for (int i = 0; i < classpath.length; i++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ public void findsProjectionJarsWithSourceTags() {
String a = getClass().getResource("jars/a/a.jar").getPath();
String b = getClass().getResource("jars/b/b.jar").getPath();
String c = getClass().getResource("jars/c/c.jar").getPath();
String buildCp = a + ":" + b + ":" + c;
String separator = System.getProperty("path.separator");
String buildCp = a + separator + b + separator + c;

BuildParameterBuilder.Result result = new BuildParameterBuilder()
.projectionSource("foo")
Expand All @@ -230,8 +231,35 @@ public void findsProjectionJarsWithSourceTags() {
// The classpath keeps all of the JARs.
assertThat(result.classpath, equalTo(buildCp));
// The discovery classpath removes a because it's JAR matched the source tag.
assertThat(result.discoveryClasspath, equalTo(b + ":" + c));
assertThat(result.discoveryClasspath, equalTo(b + separator + c));
// The sources now contains a because it matched a source tag.
assertThat(result.sources, contains(a));
}

@Test
public void usesCustomSeparator() {
String currentSeparator = System.getProperty("path.separator");

try {
System.setProperty("path.separator", "|");
String a = getClass().getResource("jars/a/a.jar").getPath();
String b = getClass().getResource("jars/b/b.jar").getPath();
String c = getClass().getResource("jars/c/c.jar").getPath();
String buildCp = a + "|" + b + "|" + c;

BuildParameterBuilder.Result result = new BuildParameterBuilder()
.projectionSource("foo")
.projectionSourceTags("X, Blah")
.libClasspath("abc.jar")
.buildClasspath(buildCp)
.discover(true)
.build();

assertThat(result.classpath, equalTo(buildCp));
assertThat(result.discoveryClasspath, equalTo(b + "|" + c));
assertThat(result.sources, contains(a));
} finally {
System.setProperty("path.separator", currentSeparator);
}
}
}