From 7c4b9aecfe93b6adcbc032cabd3ae253318fbd3f Mon Sep 17 00:00:00 2001 From: Jonah Graham Date: Wed, 21 Jun 2023 14:22:13 -0400 Subject: [PATCH] Rework setting PATH variable to not store in a File temporarily The old code placed multiple entries for adding to PATH in a File temporarily to get the absolute path. This worked generally on non-Windows platforms because often there was only a single entry. However on Windows there are generally two entries. Adding to that is the UNC path names that are sometimes present on Windows and the temporary use of File causes corruption in the settings. Instead collect the new PATH entries in a list of strings, and then before making the final PATH setting, run each individual entry through new File(entry).getAbsolutePath() Fixes #230 --- .../arm/core/EnvironmentVariableSupplier.java | 59 ++++++++---------- .../core/EnvironmentVariableSupplier.java | 61 ++++++++----------- 2 files changed, 53 insertions(+), 67 deletions(-) diff --git a/plugins/org.eclipse.embedcdt.managedbuild.cross.arm.core/src/org/eclipse/embedcdt/managedbuild/cross/arm/core/EnvironmentVariableSupplier.java b/plugins/org.eclipse.embedcdt.managedbuild.cross.arm.core/src/org/eclipse/embedcdt/managedbuild/cross/arm/core/EnvironmentVariableSupplier.java index 222d59f43..070332b63 100644 --- a/plugins/org.eclipse.embedcdt.managedbuild.cross.arm.core/src/org/eclipse/embedcdt/managedbuild/cross/arm/core/EnvironmentVariableSupplier.java +++ b/plugins/org.eclipse.embedcdt.managedbuild.cross.arm.core/src/org/eclipse/embedcdt/managedbuild/cross/arm/core/EnvironmentVariableSupplier.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2009, 2013 Wind River Systems, Inc. and others. + * Copyright (c) 2009, 2023 Wind River Systems, Inc. and others. * * This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 @@ -16,6 +16,11 @@ package org.eclipse.embedcdt.managedbuild.cross.arm.core; import java.io.File; +import java.util.ArrayList; +import java.util.List; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.eclipse.cdt.core.cdtvariables.CdtVariableException; import org.eclipse.cdt.managedbuilder.core.IConfiguration; @@ -29,7 +34,6 @@ import org.eclipse.core.resources.IProject; import org.eclipse.core.runtime.IPath; import org.eclipse.core.runtime.Platform; -import org.eclipse.embedcdt.core.EclipseUtils; import org.eclipse.embedcdt.internal.managedbuild.cross.arm.core.Activator; import org.eclipse.embedcdt.managedbuild.cross.core.preferences.PersistentPreferences; @@ -74,9 +78,9 @@ private static class PathEnvironmentVariable implements IBuildEnvironmentVariabl public static String name = "PATH"; //$NON-NLS-1$ - private File path; + private String path; - private PathEnvironmentVariable(File path) { + private PathEnvironmentVariable(String path) { // System.out.println("cpath=" + path); this.path = path; } @@ -88,12 +92,12 @@ public static PathEnvironmentVariable create(IConfiguration configuration) { Boolean preferXpacksBin = Option.getOptionBooleanValue(configuration, Option.OPTION_PREFER_XPACKS_BIN); - String path = ""; + List path = new ArrayList<>(); if (preferXpacksBin) { IPath projectPath = project.getWorkspace().getRoot().getLocation().append(project.getFullPath()); IPath xpackBinPath = projectPath.append("xpacks").append(".bin"); - path = xpackBinPath.toOSString(); + path.add(xpackBinPath.toOSString()); } // Get the build tools path from the common store. @@ -109,14 +113,8 @@ public static PathEnvironmentVariable create(IConfiguration configuration) { buildToolsPath = deprecatedPersistentPreferences.getBuildToolsPath(project); } - if (path.isEmpty()) { - path = buildToolsPath; - } else { - if (!buildToolsPath.isEmpty()) { - // Concatenate build tools path with toolchain path. - path += EclipseUtils.getPathSeparator(); - path += buildToolsPath; - } + if (!buildToolsPath.isEmpty()) { + path.add(buildToolsPath); } IOption optionId; @@ -139,34 +137,29 @@ public static PathEnvironmentVariable create(IConfiguration configuration) { toolchainPath = deprecatedPersistentPreferences.getToolchainPath(toolchainId, toolchainName, project); } - if (path.isEmpty()) { - path = toolchainPath; - } else { - if (!toolchainPath.isEmpty()) { - // Concatenate build tools path with toolchain path. - path += EclipseUtils.getPathSeparator(); - path += toolchainPath; - } + if (!toolchainPath.isEmpty()) { + path.add(toolchainPath); } if (!path.isEmpty()) { // if present, substitute macros - if (path.indexOf("${") >= 0) { - path = resolveMacros(path, configuration); - } + Stream macrosResolved = path.stream().map(p -> { + if (p.indexOf("${") >= 0) { + p = resolveMacros(p, configuration); + } + return p; + }); + // filter out empty entries and convert string to absolute paths + String pathVar = macrosResolved.filter(Predicate.not(String::isBlank)).map(File::new) + .map(File::getAbsolutePath).collect(Collectors.joining(File.pathSeparator)); - File sysroot = new File(path); - // File bin = new File(sysroot, "bin"); //$NON-NLS-1$ - // if (bin.isDirectory()) { - // sysroot = bin; - // } if (DEBUG_PATH) { if (Activator.getInstance().isDebugging()) { - System.out.println("arm.PathEnvironmentVariable.create() PATH=\"" + sysroot + "\" cfg=" + System.out.println("arm.PathEnvironmentVariable.create() PATH=\"" + pathVar + "\" cfg=" + configuration + " prj=" + configuration.getManagedProject().getOwner().getName()); } } - return new PathEnvironmentVariable(sysroot); + return new PathEnvironmentVariable(pathVar); } if (Activator.getInstance().isDebugging()) { @@ -216,7 +209,7 @@ public int getOperation() { @Override public String getValue() { - return path.getAbsolutePath(); + return path; } } diff --git a/plugins/org.eclipse.embedcdt.managedbuild.cross.riscv.core/src/org/eclipse/embedcdt/managedbuild/cross/riscv/core/EnvironmentVariableSupplier.java b/plugins/org.eclipse.embedcdt.managedbuild.cross.riscv.core/src/org/eclipse/embedcdt/managedbuild/cross/riscv/core/EnvironmentVariableSupplier.java index 46f9bec25..4ab04b002 100644 --- a/plugins/org.eclipse.embedcdt.managedbuild.cross.riscv.core/src/org/eclipse/embedcdt/managedbuild/cross/riscv/core/EnvironmentVariableSupplier.java +++ b/plugins/org.eclipse.embedcdt.managedbuild.cross.riscv.core/src/org/eclipse/embedcdt/managedbuild/cross/riscv/core/EnvironmentVariableSupplier.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2009, 2013 Wind River Systems, Inc. and others. + * Copyright (c) 2009, 2023 Wind River Systems, Inc. and others. * * This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 @@ -17,6 +17,11 @@ package org.eclipse.embedcdt.managedbuild.cross.riscv.core; import java.io.File; +import java.util.ArrayList; +import java.util.List; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.eclipse.cdt.core.cdtvariables.CdtVariableException; import org.eclipse.cdt.managedbuilder.core.IConfiguration; @@ -30,7 +35,6 @@ import org.eclipse.core.resources.IProject; import org.eclipse.core.runtime.IPath; import org.eclipse.core.runtime.Platform; -import org.eclipse.embedcdt.core.EclipseUtils; import org.eclipse.embedcdt.internal.managedbuild.cross.riscv.core.Activator; import org.eclipse.embedcdt.managedbuild.cross.core.preferences.PersistentPreferences; @@ -75,9 +79,9 @@ private static class PathEnvironmentVariable implements IBuildEnvironmentVariabl public static String name = "PATH"; //$NON-NLS-1$ - private File path; + private String path; - private PathEnvironmentVariable(File path) { + private PathEnvironmentVariable(String path) { // System.out.println("cpath=" + path); this.path = path; } @@ -89,28 +93,23 @@ public static PathEnvironmentVariable create(IConfiguration configuration) { Boolean preferXpacksBin = Option.getOptionBooleanValue(configuration, Option.OPTION_PREFER_XPACKS_BIN); - String path = ""; + List path = new ArrayList<>(); if (preferXpacksBin) { IPath projectPath = project.getWorkspace().getRoot().getLocation().append(project.getFullPath()); IPath xpackBinPath = projectPath.append("xpacks").append(".bin"); - path = xpackBinPath.toOSString(); + path.add(xpackBinPath.toOSString()); } // Get the build tools path from the common store. PersistentPreferences commonPersistentPreferences = org.eclipse.embedcdt.internal.managedbuild.cross.core.Activator .getInstance().getPersistentPreferences(); + // Get the build tools path from the common store. String buildToolsPath = commonPersistentPreferences.getBuildToolsPath(project); - if (path.isEmpty()) { - path = buildToolsPath; - } else { - if (!buildToolsPath.isEmpty()) { - // Concatenate build tools path with toolchain path. - path += EclipseUtils.getPathSeparator(); - path += buildToolsPath; - } + if (!buildToolsPath.isEmpty()) { + path.add(buildToolsPath); } IOption optionId; @@ -129,35 +128,29 @@ public static PathEnvironmentVariable create(IConfiguration configuration) { // Eclipse, defaults). toolchainPath = persistentPreferences.getToolchainPath(toolchainId, toolchainName, project); - if (path.isEmpty()) { - path = toolchainPath; - } else { - if (!toolchainPath.isEmpty()) { - // Concatenate build tools path with toolchain path. - path += EclipseUtils.getPathSeparator(); - path += toolchainPath; - } + if (!toolchainPath.isEmpty()) { + path.add(toolchainPath); } if (!path.isEmpty()) { - // if present, substitute macros - if (path.indexOf("${") >= 0) { - path = resolveMacros(path, configuration); - } + Stream macrosResolved = path.stream().map(p -> { + if (p.indexOf("${") >= 0) { + p = resolveMacros(p, configuration); + } + return p; + }); + // filter out empty entries and convert string to absolute paths + String pathVar = macrosResolved.filter(Predicate.not(String::isBlank)).map(File::new) + .map(File::getAbsolutePath).collect(Collectors.joining(File.pathSeparator)); - File sysroot = new File(path); - // File bin = new File(sysroot, "bin"); //$NON-NLS-1$ - // if (bin.isDirectory()) { - // sysroot = bin; - // } if (DEBUG_PATH) { if (Activator.getInstance().isDebugging()) { - System.out.println("riscv.PathEnvironmentVariable.create() PATH=" + sysroot + " cfg=" + System.out.println("riscv.PathEnvironmentVariable.create() PATH=\"" + pathVar + "\" cfg=" + configuration + " prj=" + configuration.getManagedProject().getOwner().getName()); } } - return new PathEnvironmentVariable(sysroot); + return new PathEnvironmentVariable(pathVar); } if (Activator.getInstance().isDebugging()) { @@ -208,7 +201,7 @@ public int getOperation() { @Override public String getValue() { - return path.getAbsolutePath(); + return path; } }