Skip to content

Commit b58bb1f

Browse files
committed
fix: apply review feedback and clean up implementation
1 parent 7f123dd commit b58bb1f

File tree

3 files changed

+23
-30
lines changed

3 files changed

+23
-30
lines changed

graalpy-maven-plugin/src/main/java/org/graalvm/python/maven/plugin/AbstractGraalPyMojo.java

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ protected void preExec(boolean enableWarnings) throws MojoExecutionException {
216216
}
217217
}
218218

219-
protected Path resolveReqFile() {
219+
protected Path resolveReqFile() throws MojoExecutionException {
220220
if (requirementsFile == null || requirementsFile.isBlank()) {
221221
return null;
222222
}
@@ -226,16 +226,14 @@ protected Path resolveReqFile() {
226226
? path
227227
: project.getBasedir().toPath().resolve(path).normalize();
228228

229-
if (Files.exists(finalPath)) {
230-
return finalPath;
229+
if (!Files.exists(finalPath)) {
230+
throw new MojoExecutionException(
231+
"The configured requirementsFile does not exist: " + finalPath
232+
+ "\nPlease provide a valid path to a pip-compatible requirements file."
233+
);
231234
}
232235

233-
Path defaultReq = project.getBasedir().toPath().resolve("requirements.txt").normalize();
234-
if (Files.exists(defaultReq)) {
235-
return defaultReq;
236-
}
237-
238-
return null;
236+
return finalPath;
239237
}
240238

241239
protected void postExec() throws MojoExecutionException {

graalpy-maven-plugin/src/main/java/org/graalvm/python/maven/plugin/LockPackagesMojo.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,10 @@ private void checkEmptyPackages() throws MojoExecutionException {
139139
Path reqFilePath = resolveReqFile();
140140
boolean emptyPackages = packages == null || packages.isEmpty();
141141
boolean requirementsExists = reqFilePath != null;
142-
if (emptyPackages && !requirementsExists) {
142+
// Disallow lock-packages when no packages OR when requirementsFile is used
143+
if (emptyPackages || requirementsExists) {
144+
getLog().error("");
145+
getLog().error("Missing Python dependency configuration.");
143146
getLog().error("");
144147
getLog().error(MISSING_DEPENDENCY_CONFIGURATION_ERROR);
145148
getLog().error("");

org.graalvm.python.embedding.tools/src/main/java/org/graalvm/python/embedding/tools/vfs/VFSUtils.java

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -560,9 +560,11 @@ static boolean installPackagesFromReqFile(Path venvDirectory, Launcher launcher,
560560
log.warning("Lock file is ignored in <requirements.txt> mode.");
561561
log.warning("The 'lock-packages' goal should not be used together with <requirementsFile>.");
562562

563-
ensureVenv(venvDirectory, graalPyVersion, launcher, log);
563+
VenvContents vc = ensureVenv(venvDirectory, graalPyVersion, launcher, log);
564564

565565
runPip(venvDirectory, "install", log, "--compile", "-r", reqFile.toString());
566+
List<String> reqPackages = requirementsPackages(reqFile);
567+
vc.write(reqPackages);
566568

567569
InstalledPackages installed = InstalledPackages.fromVenv(venvDirectory);
568570
installed.freeze(log);
@@ -578,7 +580,6 @@ static void installPackages(Path venvDirectory, List<String> packages, Path lock
578580
Objects.requireNonNull(packages);
579581
log.info("Using inline <packages> dependency mode.");
580582

581-
validatePackagesOrLockFile(packages, lockFilePath);
582583
logVenvArgs(venvDirectory, packages, lockFilePath, launcher, graalPyVersion, log);
583584

584585
List<String> pluginPackages = trim(packages);
@@ -684,18 +685,6 @@ public static void lockPackages(Path venvDirectory, List<String> packages, Path
684685
}
685686
}
686687

687-
private static void validatePackagesOrLockFile(List<String> packages, Path lockFilePath) {
688-
boolean hasPackages = packages != null && !packages.isEmpty();
689-
boolean hasLockFile = lockFilePath != null;
690-
691-
if (hasPackages == hasLockFile) {
692-
throw new IllegalArgumentException(
693-
"Invalid configuration: <packages> and lock-file cannot be used together. Provide exactly one."
694-
);
695-
}
696-
}
697-
698-
699688
private static void logVenvArgs(Path venvDirectory, List<String> packages, Path lockFile, Launcher launcherArgs,
700689
String graalPyVersion, BuildToolLog log) throws IOException {
701690
if (log.isDebugEnabled()) {
@@ -1019,7 +1008,7 @@ private static void runPip(Path venvDirectory, String command, BuildToolLog log,
10191008
throw new IOException(String.format("failed to execute pip %s", List.of(args)), e);
10201009
}
10211010
}
1022-
1011+
@SuppressWarnings("SameParameterValue")
10231012
private static void runVenvBin(Path venvDirectory, String bin, BuildToolLog log, String... args)
10241013
throws IOException {
10251014
try {
@@ -1029,10 +1018,12 @@ private static void runVenvBin(Path venvDirectory, String bin, BuildToolLog log,
10291018
}
10301019
}
10311020

1032-
private static List<String> trim(List<String> l) {
1021+
private static List<String> trim(List<String> l) {
10331022
Iterator<String> it = l.iterator();
1034-
for (String s : l) {
1035-
if (s == null || s.trim().isEmpty()) {
1023+
// noinspection Java8CollectionRemoveIf
1024+
while (it.hasNext()) {
1025+
String p = it.next();
1026+
if (p == null || p.trim().isEmpty()) {
10361027
it.remove();
10371028
}
10381029
}
@@ -1050,9 +1041,10 @@ private static void info(BuildToolLog log, String txt, Object... args) {
10501041
}
10511042
}
10521043

1053-
private static void lifecycle(BuildToolLog log, Object... args) {
1044+
@SuppressWarnings("SameParameterValue")
1045+
private static void lifecycle(BuildToolLog log, String txt, Object... args) {
10541046
if (log.isLifecycleEnabled()) {
1055-
log.lifecycle(String.format("Created GraalPy lock file: %s", args));
1047+
log.lifecycle(String.format(txt, args));
10561048
}
10571049
}
10581050

0 commit comments

Comments
 (0)