Skip to content

Commit 71280aa

Browse files
committed
Fix conflation of Builder#name methods
* Builder#name() - the *environment type* the builder produces. * Builder#name(String) - the *desired environment name* to build. These are two different ideas. To reduce confusion and avoid name clashes (especially in appose-python), this commit renames the Builder#name() method to Builder#envType(). The Builder#name(String) method is left as is (instead of renaming to Builder#envName(String)), with the rationale that we want fluid builder syntax to be as succinct as possible, and it can be assumed that the thing we are naming is the environment to be built.
1 parent e91444c commit 71280aa

14 files changed

+95
-81
lines changed

src/main/java/org/apposed/appose/BuildException.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public BuildException(
7878
}
7979

8080
private static String makeMessage(Builder<?> builder, Throwable cause) {
81-
String noun = builder == null ? "build" : builder.name() + " build";
81+
String noun = builder == null ? "build" : builder.envType() + " build";
8282
String verb = cause instanceof InterruptedException ? "interrupted" : "failed";
8383
return noun + " " + verb;
8484
}

src/main/java/org/apposed/appose/Builder.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,11 @@
6464
public interface Builder<T extends Builder<T>> {
6565

6666
/**
67-
* Returns the name of this builder (e.g., "pixi", "mamba", "uv", "system").
67+
* Gets the environment type constructed by this builder (e.g., "pixi", "mamba", "uv").
6868
*
69-
* @return The builder name.
69+
* @return The builder's associated environment type.
7070
*/
71-
String name();
71+
String envType();
7272

7373
/**
7474
* Builds the environment. This is the terminator method for any fluid building chain.

src/main/java/org/apposed/appose/BuilderFactory.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,16 +49,16 @@ public interface BuilderFactory {
4949
* Configuration should be provided via the fluent API
5050
* (e.g., {@link Builder#content}, {@link Builder#scheme}).
5151
*
52-
* @return A new builder instance
52+
* @return A new builder instance.
5353
*/
5454
Builder<?> createBuilder();
5555

5656
/**
57-
* Returns the name of this builder (e.g., "pixi", "mamba", "uv", "system").
57+
* Gets the environment type handled by this builder (e.g., "pixi", "mamba", "uv").
5858
*
59-
* @return The builder name
59+
* @return The builder's associated environment type.
6060
*/
61-
String name();
61+
String envType();
6262

6363
/**
6464
* Checks if this builder supports the given scheme.

src/main/java/org/apposed/appose/Environment.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,21 +58,20 @@ default Map<String, String> envVars() {
5858
}
5959

6060
/**
61-
* Returns the builder that created this environment.
61+
* Gets the builder that created this environment.
6262
* This enables re-configuration and rebuilding of the environment.
6363
*
6464
* @return The builder instance.
6565
*/
6666
Builder<?> builder();
6767

6868
/**
69-
* Returns the type of this environment (e.g., "pixi", "mamba", "uv", "system").
70-
* By default, delegates to the builder's name.
69+
* Gets the type of this environment (e.g., "pixi", "mamba", "uv").
7170
*
72-
* @return The environment type name.
71+
* @return The environment type.
7372
*/
7473
default String type() {
75-
return builder().name();
74+
return builder().envType();
7675
}
7776

7877
/**

src/main/java/org/apposed/appose/builder/BaseBuilder.java

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,13 @@ public abstract class BaseBuilder<T extends BaseBuilder<T>> implements Builder<T
6868
protected String content;
6969

7070
/** Explicit scheme (e.g., "pixi.toml", "environment.yml"). */
71-
protected String scheme;
71+
protected Scheme scheme;
7272

7373
// -- Builder methods --
7474

7575
@Override
7676
public void delete() throws IOException {
77-
File dir = envDir();
77+
File dir = resolveEnvDir();
7878
if (dir.exists()) FilePaths.deleteRecursively(dir);
7979
}
8080

@@ -135,7 +135,7 @@ public T content(String content) {
135135

136136
@Override
137137
public T scheme(String scheme) {
138-
this.scheme = scheme;
138+
this.scheme = Schemes.fromName(scheme);
139139
return typedThis();
140140
}
141141

@@ -164,22 +164,22 @@ private T typedThis() {
164164
return (T) this;
165165
}
166166

167-
protected String envName() {
168-
return envName != null ? envName :
169-
// No explicit environment name set;
170-
// extract name from the source content.
171-
scheme().envName(content);
172-
}
173-
174-
protected File envDir() {
167+
/** Determines the environment directory path. */
168+
protected File resolveEnvDir() {
175169
if (envDir != null) return envDir;
170+
176171
// No explicit environment directory set; fall back to
177172
// a subfolder of the Appose-managed environments directory.
178-
return Paths.get(Environments.apposeEnvsDir(), envName()).toFile();
173+
String dirName = envName != null ? envName :
174+
// No explicit environment name set; extract name from the source content.
175+
resolveScheme().envName(content);
176+
return dirName == null ? null :
177+
Paths.get(Environments.apposeEnvsDir(), dirName).toFile();
179178
}
180179

181-
protected Scheme scheme() {
182-
if (scheme != null) return Schemes.fromName(scheme);
180+
/** Determines the scheme, detecting from content if needed. */
181+
protected Scheme resolveScheme() {
182+
if (scheme != null) return scheme;
183183
if (content != null) return Schemes.fromContent(content);
184184
throw new IllegalStateException("Cannot determine scheme: neither scheme nor content is set");
185185
}

src/main/java/org/apposed/appose/builder/Builders.java

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,14 @@ private Builders() {
5252
(a, b) -> Double.compare(b.priority(), a.priority()));
5353

5454
/**
55-
* Finds a factory by name.
55+
* Finds the first factory capable of building a particular type of environment.
56+
* Factories are checked in priority order.
5657
*
57-
* @param name The builder name to search for.
58-
* @return The factory with matching name, or null if not found.
58+
* @param envType The environment type to target.
59+
* @return A factory supporting that environment type, or null if not found.
5960
*/
60-
public static @Nullable BuilderFactory findFactoryByName(String name) {
61-
return Plugins.find(BUILDERS, factory -> factory.name().equalsIgnoreCase(name));
61+
public static @Nullable BuilderFactory findFactoryByEnvType(String envType) {
62+
return Plugins.find(BUILDERS, factory -> factory.envType().equalsIgnoreCase(envType));
6263
}
6364

6465
/**
@@ -74,7 +75,7 @@ private Builders() {
7475

7576
/**
7677
* Finds the first factory that can wrap the given environment directory.
77-
* Factories are checked in priority order (highest priority first).
78+
* Factories are checked in priority order.
7879
*
7980
* @param envDir The directory to find a factory for.
8081
* @return The first factory that can wrap the directory, or null if none found.
@@ -117,14 +118,25 @@ public static boolean canWrap(File envDir) {
117118
}
118119

119120
/**
120-
* Returns the environment type name for the given directory.
121-
* This is a convenience method equivalent to {@code findFactoryForWrapping(envDir).name()}.
121+
* Returns the given directory's environment type.
122+
* This is a convenience method equivalent to {@code findFactoryForWrapping(envDir).type()}.
123+
*
124+
* @param envDir The directory to check.
125+
* @return The environment type (e.g., "pixi", "mamba", "uv"), or null if not a known environment.
126+
*/
127+
public static @Nullable String envType(String envDir) {
128+
return envType(new File(envDir));
129+
}
130+
131+
/**
132+
* Returns the given directory's environment type.
133+
* This is a convenience method equivalent to {@code findFactoryForWrapping(envDir).type()}.
122134
*
123135
* @param envDir The directory to check.
124-
* @return The environment type name (e.g., "pixi", "mamba", "uv"), or null if not a known environment.
136+
* @return The environment type (e.g., "pixi", "mamba", "uv"), or null if not a known environment.
125137
*/
126138
public static @Nullable String envType(File envDir) {
127139
BuilderFactory factory = findFactoryForWrapping(envDir);
128-
return factory == null ? null : factory.name();
140+
return factory == null ? null : factory.envType();
129141
}
130142
}

src/main/java/org/apposed/appose/builder/DynamicBuilder.java

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.apposed.appose.Builder;
3434
import org.apposed.appose.BuilderFactory;
3535
import org.apposed.appose.Environment;
36+
import org.apposed.appose.Scheme;
3637

3738
/**
3839
* Dynamic builder that auto-detects the appropriate specific builder
@@ -42,38 +43,38 @@
4243
*/
4344
public final class DynamicBuilder extends BaseBuilder<DynamicBuilder> {
4445

45-
private String builderName;
46+
private String envType;
4647

4748
// -- DynamicBuilder methods --
4849

4950
/**
5051
* Specifies the preferred builder to use.
5152
*
52-
* @param builderName The builder name (e.g., "pixi", "mamba", "uv")
53+
* @param envType The builder's environment type (e.g., "pixi", "mamba", "uv")
5354
* @return This builder instance, for fluent-style programming.
5455
*/
55-
public DynamicBuilder builder(String builderName) {
56-
this.builderName = builderName;
56+
public DynamicBuilder builder(String envType) {
57+
this.envType = envType;
5758
return this;
5859
}
5960

6061
// -- Builder methods --
6162

6263
@Override
63-
public String name() {
64-
return "dynamic";
64+
public String envType() {
65+
return envType != null ? envType : "dynamic";
6566
}
6667

6768
@Override
6869
public Environment build() throws BuildException {
69-
Builder<?> delegate = createBuilder(builderName, scheme);
70+
Builder<?> delegate = createBuilder();
7071
copyConfigToDelegate(delegate);
7172
return delegate.build();
7273
}
7374

7475
@Override
7576
public Environment rebuild() throws BuildException {
76-
Builder<?> delegate = createBuilder(builderName, scheme);
77+
Builder<?> delegate = createBuilder();
7778
copyConfigToDelegate(delegate);
7879
return delegate.rebuild();
7980
}
@@ -86,31 +87,31 @@ private void copyConfigToDelegate(Builder<?> delegate) {
8687
if (envName != null) delegate.name(envName);
8788
if (envDir != null) delegate.base(envDir);
8889
if (content != null) delegate.content(content);
89-
if (scheme != null) delegate.scheme(scheme);
90+
if (scheme != null) delegate.scheme(scheme.name());
9091
delegate.channels(channels);
9192
progressSubscribers.forEach(delegate::subscribeProgress);
9293
outputSubscribers.forEach(delegate::subscribeOutput);
9394
errorSubscribers.forEach(delegate::subscribeError);
9495
}
9596

96-
private Builder<?> createBuilder(String name, String scheme) {
97+
private Builder<?> createBuilder() {
9798
// Find the builder matching the specified name, if any.
98-
if (name != null) {
99-
BuilderFactory factory = Builders.findFactoryByName(name);
100-
if (factory == null) throw new IllegalArgumentException("Unknown builder: " + name);
99+
if (envType != null) {
100+
BuilderFactory factory = Builders.findFactoryByEnvType(envType);
101+
if (factory == null) throw new IllegalArgumentException("Unknown builder: " + envType);
101102
return factory.createBuilder();
102103
}
103104

104105
// Detect scheme from content if content is provided but scheme is not.
105-
String effectiveScheme = scheme;
106-
if (effectiveScheme == null && content != null) {
107-
effectiveScheme = scheme().name();
106+
Scheme actualScheme = scheme;
107+
if (actualScheme == null && content != null) {
108+
actualScheme = resolveScheme();
108109
}
109110

110111
// Find the highest-priority builder that supports this scheme.
111-
if (effectiveScheme != null) {
112-
BuilderFactory factory = Builders.findFactoryByScheme(effectiveScheme);
113-
if (factory == null) throw new IllegalArgumentException("No builder supports scheme: " + effectiveScheme);
112+
if (actualScheme != null) {
113+
BuilderFactory factory = Builders.findFactoryByScheme(actualScheme.name());
114+
if (factory == null) throw new IllegalArgumentException("No builder supports scheme: " + actualScheme.name());
114115
return factory.createBuilder();
115116
}
116117

src/main/java/org/apposed/appose/builder/MambaBuilder.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public final class MambaBuilder extends BaseBuilder<MambaBuilder> {
5353
// -- Builder methods --
5454

5555
@Override
56-
public String name() {
56+
public String envType() {
5757
return "mamba";
5858
}
5959

@@ -70,7 +70,7 @@ public MambaBuilder channels(String... channels) {
7070

7171
@Override
7272
public Environment build() throws BuildException {
73-
File envDir = envDir();
73+
File envDir = resolveEnvDir();
7474

7575
// Check for incompatible existing environments.
7676
if (new File(envDir, ".pixi").isDirectory()) {
@@ -93,9 +93,9 @@ public Environment build() throws BuildException {
9393
}
9494

9595
// Infer scheme if not explicitly set.
96-
if (scheme == null) scheme = Schemes.fromContent(content).name();
96+
if (scheme == null) scheme = Schemes.fromContent(content);
9797

98-
if (!"environment.yml".equals(scheme)) {
98+
if (!"environment.yml".equals(scheme.name())) {
9999
throw new IllegalArgumentException("MambaBuilder only supports environment.yml scheme, got: " + scheme);
100100
}
101101

src/main/java/org/apposed/appose/builder/MambaBuilderFactory.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public Builder<?> createBuilder() {
4545
}
4646

4747
@Override
48-
public String name() {
48+
public String envType() {
4949
return "mamba";
5050
}
5151

src/main/java/org/apposed/appose/builder/PixiBuilder.java

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,13 @@ public PixiBuilder pypi(String... packages) {
8181
// -- Builder methods --
8282

8383
@Override
84-
public String name() {
84+
public String envType() {
8585
return "pixi";
8686
}
8787

8888
@Override
8989
public Environment build() throws BuildException {
90-
File envDir = envDir();
90+
File envDir = resolveEnvDir();
9191

9292
// Check for incompatible existing environments.
9393
if (new File(envDir, "conda-meta").exists() && !new File(envDir, ".pixi").exists()) {
@@ -131,21 +131,23 @@ public Environment build() throws BuildException {
131131
}
132132

133133
// Infer scheme if not explicitly set.
134-
if (scheme == null) scheme = Schemes.fromContent(content).name();
134+
if (scheme == null) scheme = Schemes.fromContent(content);
135135

136136
if (!envDir.exists() && !envDir.mkdirs()) {
137137
throw new BuildException(this, "Failed to create environment directory: " + envDir);
138138
}
139139

140-
if ("pixi.toml".equals(scheme)) {
140+
if ("pixi.toml".equals(scheme.name())) {
141141
// Write pixi.toml to envDir.
142142
File pixiTomlFile = new File(envDir, "pixi.toml");
143143
Files.write(pixiTomlFile.toPath(), content.getBytes(StandardCharsets.UTF_8));
144-
} else if ("pyproject.toml".equals(scheme)) {
144+
}
145+
else if ("pyproject.toml".equals(scheme.name())) {
145146
// Write pyproject.toml to envDir (Pixi natively supports it).
146147
File pyprojectTomlFile = new File(envDir, "pyproject.toml");
147148
Files.write(pyprojectTomlFile.toPath(), content.getBytes(StandardCharsets.UTF_8));
148-
} else if ("environment.yml".equals(scheme)) {
149+
}
150+
else if ("environment.yml".equals(scheme.name())) {
149151
// Write environment.yml and import.
150152
File environmentYamlFile = new File(envDir, "environment.yml");
151153
Files.write(environmentYamlFile.toPath(), content.getBytes(StandardCharsets.UTF_8));
@@ -224,14 +226,14 @@ public Environment wrap(File envDir) throws BuildException {
224226
if (pixiToml.exists() && pixiToml.isFile()) {
225227
// Read the content so rebuild() will work even after directory is deleted.
226228
content = new String(Files.readAllBytes(pixiToml.toPath()), StandardCharsets.UTF_8);
227-
scheme = "pixi.toml";
229+
scheme = Schemes.fromName("pixi.toml");
228230
} else {
229231
// Check for pyproject.toml.
230232
File pyprojectToml = new File(envDir, "pyproject.toml");
231233
if (pyprojectToml.exists() && pyprojectToml.isFile()) {
232234
// Read the content so rebuild() will work even after directory is deleted.
233235
content = new String(Files.readAllBytes(pyprojectToml.toPath()), StandardCharsets.UTF_8);
234-
scheme = "pyproject.toml";
236+
scheme = Schemes.fromName("pyproject.toml");
235237
}
236238
}
237239
}

0 commit comments

Comments
 (0)