Skip to content

Commit

Permalink
Rework setting PATH variable to not store in a File temporarily
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jonahgraham committed Jun 27, 2023
1 parent fd62f0b commit 7c4b9ae
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 67 deletions.
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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;
}
Expand All @@ -88,12 +92,12 @@ public static PathEnvironmentVariable create(IConfiguration configuration) {

Boolean preferXpacksBin = Option.getOptionBooleanValue(configuration, Option.OPTION_PREFER_XPACKS_BIN);

String path = "";
List<String> 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.
Expand All @@ -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;
Expand All @@ -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<String> 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()) {
Expand Down Expand Up @@ -216,7 +209,7 @@ public int getOperation() {

@Override
public String getValue() {
return path.getAbsolutePath();
return path;
}

}
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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;
}
Expand All @@ -89,28 +93,23 @@ public static PathEnvironmentVariable create(IConfiguration configuration) {

Boolean preferXpacksBin = Option.getOptionBooleanValue(configuration, Option.OPTION_PREFER_XPACKS_BIN);

String path = "";
List<String> 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;
Expand All @@ -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<String> 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()) {
Expand Down Expand Up @@ -208,7 +201,7 @@ public int getOperation() {

@Override
public String getValue() {
return path.getAbsolutePath();
return path;
}

}
Expand Down

0 comments on commit 7c4b9ae

Please sign in to comment.