From 815b96c08099b051419ffd0c9f0de45ca88c5703 Mon Sep 17 00:00:00 2001 From: Denis Date: Wed, 11 Dec 2019 16:35:23 +0300 Subject: [PATCH] Exclude pnpm from managing itself (#7125) --- .../flow/server/frontend/FrontendUtils.java | 131 ++++++++++++++---- .../server/frontend/FrontendUtilsTest.java | 30 +++- 2 files changed, 131 insertions(+), 30 deletions(-) diff --git a/flow-server/src/main/java/com/vaadin/flow/server/frontend/FrontendUtils.java b/flow-server/src/main/java/com/vaadin/flow/server/frontend/FrontendUtils.java index 8275822dd85..074bbc7dd42 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/frontend/FrontendUtils.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/frontend/FrontendUtils.java @@ -62,6 +62,11 @@ */ public class FrontendUtils { + private static final String PNMP_INSTALLED_BY_NPM_FOLDER = "node_modules/pnpm/"; + + private static final String PNMP_INSTALLED_BY_NPM = PNMP_INSTALLED_BY_NPM_FOLDER + + "bin/pnpm.js"; + public static final String PROJECT_BASEDIR = "project.basedir"; /** @@ -368,14 +373,13 @@ public static List getPnpmExecutable(String baseDir) { public static List getPnpmExecutable(String baseDir, boolean failOnAbsence) { // First try local pnpm JS script if it exists - File file = new File(baseDir, "node_modules/pnpm/bin/pnpm.js"); List returnCommand = new ArrayList<>(); - if (file.canRead()) { - // We return a two element list with node binary and npm-cli script + Optional localPnpmScript = getLocalPnpmScript(baseDir); + if (localPnpmScript.isPresent()) { returnCommand.add(getNodeExecutable(baseDir)); - returnCommand.add(file.getAbsolutePath()); + returnCommand.add(localPnpmScript.get().getAbsolutePath()); } else { - // Otherwise look for regulag `pnpm` + // Otherwise look for regular `pnpm` String command = isWindows() ? "pnpm.cmd" : "pnpm"; if (failOnAbsence) { returnCommand.add(getExecutable(baseDir, command, null) @@ -770,32 +774,52 @@ public static void validateNodeAndNpmVersion(String baseDir) { */ public static void ensurePnpm(String baseDir, boolean ensure) { if (ensure && getPnpmExecutable(baseDir, false).isEmpty()) { - List npmExecutable = FrontendUtils - .getNpmExecutable(baseDir); - List command = new ArrayList<>(); - command.addAll(npmExecutable); - command.add("install"); - command.add("pnpm"); - - ProcessBuilder builder = createProcessBuilder(command); - builder.environment().put("ADBLOCK", "1"); - builder.directory(new File(baseDir)); - - Process process = null; - try { - process = builder.inheritIO().start(); - int errorCode = process.waitFor(); - if (errorCode != 0) { - getLogger().error("Couldn't install 'pnpm'"); - } else { - getLogger().debug("Pnpm is successfully installed"); + // copy the current content of package.json file to a temporary + // location + File packageJson = new File(baseDir, "package.json"); + File tempFile = null; + boolean packageJsonExists = packageJson.canRead(); + if (packageJsonExists) { + try { + tempFile = File.createTempFile("package", "json"); + FileUtils.copyFile(packageJson, tempFile); + } catch (IOException exception) { + throw new IllegalStateException( + "Couldn't make a copy of package.json file", + exception); } - } catch (InterruptedException | IOException e) { - getLogger().error("Error when running `npm install`", e); - } finally { - if (process != null) { - process.destroyForcibly(); + packageJson.delete(); + } + // install pnpm locally using npm + installPnpm(baseDir, getNpmExecutable(baseDir)); + + /* + * pnpm is not able to manage itself. To be able to deal with it + * properly let's install it now via itself + */ + installPnpm(baseDir, getPnpmExecutable(baseDir, true)); + + /* + * pnpm installed by npm should have been moved to another location + * which is out of regular "node_modules" package root. We don't + * need anymore pnpm installed inside "node_modules" and it's even + * dangerous to have it there since it's not able to manage itself + * properly. Let's remove it from "node_modules". + */ + new File(baseDir, PNMP_INSTALLED_BY_NPM_FOLDER).delete(); + // remove package-lock.json which contains pnpm as a dependency. + new File(baseDir, "package-lock.json").delete(); + + if (packageJsonExists && tempFile != null) { + // return back the original package.json + try { + FileUtils.copyFile(tempFile, packageJson); + } catch (IOException exception) { + throw new IllegalStateException( + "Couldn't restore package.json file back", + exception); } + tempFile.delete(); } } } @@ -809,6 +833,35 @@ static void checkForFaultyNpmVersion(FrontendVersion npmVersion) { } } + private static void installPnpm(String baseDir, + List installCommand) { + List command = new ArrayList<>(); + command.addAll(installCommand); + command.add("install"); + command.add("pnpm"); + + ProcessBuilder builder = createProcessBuilder(command); + builder.environment().put("ADBLOCK", "1"); + builder.directory(new File(baseDir)); + + Process process = null; + try { + process = builder.inheritIO().start(); + int errorCode = process.waitFor(); + if (errorCode != 0) { + getLogger().error("Couldn't install 'pnpm'"); + } else { + getLogger().debug("Pnpm is successfully installed"); + } + } catch (InterruptedException | IOException e) { + getLogger().error("Error when running `npm install`", e); + } finally { + if (process != null) { + process.destroyForcibly(); + } + } + } + private static String buildTooOldString(String tool, String version, int supportedMajor, int supportedMinor) { return String.format(TOO_OLD, tool, version, supportedMajor, @@ -1003,6 +1056,26 @@ private static Logger getLogger() { return LoggerFactory.getLogger(FrontendUtils.class); } + private static Optional getLocalPnpmScript(String baseDir) { + File movedPnpmScript = new File(baseDir, + "node_modules/.ignored_pnpm/bin/pnpm.js"); + if (movedPnpmScript.canRead()) { + return Optional.of(movedPnpmScript); + } + + movedPnpmScript = new File(baseDir, + "node_modules/.ignored/pnpm/bin/pnpm.js"); + if (movedPnpmScript.canRead()) { + return Optional.of(movedPnpmScript); + } + + File npmInstalled = new File(baseDir, PNMP_INSTALLED_BY_NPM); + if (npmInstalled.canRead()) { + return Optional.of(npmInstalled); + } + return Optional.empty(); + } + /** * Container class for caching the external stats.json contents. */ diff --git a/flow-server/src/test/java/com/vaadin/flow/server/frontend/FrontendUtilsTest.java b/flow-server/src/test/java/com/vaadin/flow/server/frontend/FrontendUtilsTest.java index b06dcce2aed..f8b814da9ab 100644 --- a/flow-server/src/test/java/com/vaadin/flow/server/frontend/FrontendUtilsTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/server/frontend/FrontendUtilsTest.java @@ -26,6 +26,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; import org.junit.Assert; import org.junit.Before; @@ -280,14 +281,41 @@ public void ensurePnpm_doNotEnsure() { FrontendUtils.getPnpmExecutable(baseDir, false)); } + /** + * This test doesn't do anything if pnpm is already installed (globally) + * which is true e.g. for or CI servers (TC/bender). + */ @Test - public void ensurePnpm_requestInstall() { + public void ensurePnpm_requestInstall_keepPackageJson_removePackageLock_ignoredPnpmExists_localPnpmIsRemoved() + throws IOException { List executable = FrontendUtils.getPnpmExecutable(baseDir, false); if (executable.isEmpty()) { + File packageJson = new File(baseDir, "package.json"); + FileUtils.writeStringToFile(packageJson, "{}", + StandardCharsets.UTF_8); + + File packageLockJson = new File(baseDir, "package-lock.json"); + FileUtils.writeStringToFile(packageLockJson, "{}", + StandardCharsets.UTF_8); + FrontendUtils.ensurePnpm(baseDir, true); Assert.assertFalse( FrontendUtils.getPnpmExecutable(baseDir, false).isEmpty()); + + // PNPM is moved to ignored location + Assert.assertTrue( + new File(baseDir, "node_modules/.ignored/pnpm/bin/pnpm.js") + .exists() + || new File(baseDir, + "node_modules/.ignored_pnpm/bin/pnpm.js") + .exists()); + // locally installed pnpm (via npm/pnpm) is removed + Assert.assertFalse(new File("node_modules/pnpm").exists()); + + Assert.assertEquals("{}", FileUtils.readFileToString(packageJson, + StandardCharsets.UTF_8)); + Assert.assertFalse(packageLockJson.exists()); } }