eclipse-plugins icon indicating copy to clipboard operation
eclipse-plugins copied to clipboard

Multiple leading backslashes of Windows network path incorrectly removed from preference for toolchain folder

Open moritzbeermann opened this issue 7 years ago • 2 comments

Description

Global tools path preference for toolchain folder does not handle Windows network paths (with two leading backslashes) correctly.

My GNU ARM Eclipse tools are installed on a network location, let's call it \MyServer\gnu\cross-tools. When adding this as toolchain folder under Preferences -> C/C++ -> Build -> Global Tools Path -> Toolchain folder (or setting ilg.gnuarmeclipse.managedbuild.cross/toolchain.path.1871385609) I end up with this error message: Error: Program "aarch64-none-elf-g++" not found in PATH PATH=[[...some entries...];\MyServer\gnu\cross-tools;[...more entries...]]

It seems that the raw value of the preference is modified in some way before the PATH variable is extended causing the two leading backslashes to be merged into one, which results in an incorrect entry in the PATH variable.

I tried a few things to work around this issue but was unlucky so far: \\MyServer\gnu\cross-tools \\MyServer\gnu\cross-tools \\\\MyServer\gnu\cross-tools //MyServer\gnu\cross-tools /MyServer\gnu\cross-tools /\MyServer\gnu\cross-tools All of the above result in "\MyServer\gnu\cross-tools" to end up in the PATH variable.

Steps to Reproduce

  1. Go to Preferences -> C/C++ -> Build -> Global Tools Path -> Toolchain folder and set Toolchain path to \MyServer\gnu\cross-tools
  2. Try to build a CDT project
  3. The build fails because the toolchain is not found in the PATH

Expected behaviour: The plugin should not merge two leading backslashes before extending the PATH variable.

Actual behaviour: The plugin incorrectly merges all leading backslashes of the Toolchain path value before extending the PATH variable.

Versions

  • [plug-in version] ilg.gnuarmeclipse.managedbuild.cross 2.2.1.201606210758
  • [Eclipse version] Eclipse 4.6.1
  • [Java version] Java 1.8
  • [operating system] Windows Server 2012 R2 Standard
  • [toolchain version] gcc-linaro-aarch64-none-elf-4.9-2014.05

moritzbeermann avatar Sep 01 '17 06:09 moritzbeermann

thank you for the bug report.

windows path management is particularly difficult.

as a temporary workaround I would suggest, if possible, to install the toolchain locally and use POSIX paths.

ilg-ul avatar Sep 01 '17 06:09 ilg-ul

Thanks for confirming the issue promptly.

I agree Windows paths are always a hassle. Unfortunately, I cannot apply the suggested workaround as we are running a multi user setup and some users will need to access the toolchain from a network drive.

moritzbeermann avatar Sep 01 '17 14:09 moritzbeermann

Hello again :-)

This issue is still hitting us today. I think I've located the problematic section in the code base:

https://github.com/eclipse-embed-cdt/eclipse-plugins/blob/752ae80fe82e14770728ebd18626de9d183ca625/plugins/org.eclipse.embedcdt.managedbuild.cross.arm.core/src/org/eclipse/embedcdt/managedbuild/cross/arm/core/EnvironmentVariableSupplier.java#L158

Here, the path string is composed by joining multiple paths using the OS's path separator. Then, the string is passed to the new File(...) constructor.

When Windows UNC paths are involved, this will result in a calls like new File("\\filer\path1;\\filer\path2") When later converting this File object back to a string, the double backslash in the middle will have been replaced by a single backslash \\filer\path1;\\filer\path2 -> \\filer\path1;\filer\path2 which causes the problems that we are observing.

I think the fix would be to either not use File but just use a String to pass to the PathEnvironmentVariable constructor or - if the path sanitizing is important for some flows - the path string would need to be split on the path separator character and each segment path be wrapped in a java.io.File object.

moritzbeermann avatar Jun 21 '23 14:06 moritzbeermann

If you haven't already found a workaround maybe net use to map the UNC path to a drive letter might help?

  • https://learn.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2012-r2-and-2012/gg651155(v=ws.11)

TommyMurphyTM1234 avatar Jun 21 '23 14:06 TommyMurphyTM1234

Hallo Moritz, thank you for your detailed report.

Unfortunately, I don't know how this code should behave on Windows. :-(

@jonahgraham, any suggestions on how to proceed?

ilg-ul avatar Jun 21 '23 14:06 ilg-ul

@jonahgraham, any suggestions on how to proceed?

Is this of any relevance?

  • https://wiki.eclipse.org/Eclipse/UNC_Paths

TommyMurphyTM1234 avatar Jun 21 '23 14:06 TommyMurphyTM1234

@TommyMurphyTM1234, yes mapping the path to a drive letter can work around the problem. I can do this for my own interactive development flow, but I cannot control the setup for all users that use our tools together with the Embedded CDT plugin.

@ilg-ul to clarify again, I can provide an example of what is happening in our case. We are adding two settings in the plugin preferences to point to the following two locations \\somepath\gnu-arm-eclipse-build-tools\2.6-201507152002\bin \\somepath\aarch64-none-elf-11.3.rel1\bin The code I was pointing to above is concatenating those two strings to effectively form path = "\\somepath\gnu-arm-eclipse-build-tools\2.6-201507152002\bin;\\somepath\aarch64-none-elf-11.3.rel1\bin"; At this point, everything is still fine. Then, a java.io.File object is created from it File sysroot = new File(path); This is where things go wrong, as the windows implementation of java.io.File will "optimize out" the inner duplicate backslashes in the string representation. Note that it does correctly keep the two leading backslashes.

So the variable path actually contains a semicolon-separated list of file system paths, whereas the java.io.File class is meant to represent a single file system path.

With my (limited) understanding of the code base, I would propose the following fix.

  1. The class PathEnvironmentVariable has a member private File path;. The only method that uses this member is the getValue() method, which returns a String by doing path.getAbsolutePath(). This is causing the original input string (with the semicolon-separated list of paths) to become corrupted.
  2. This class can be changed to have a member private String pathList; instead. This member would be returned from the getValue() method.
  3. The constructor would be changed to take a List<File> as argument. The pathList member can then be initialized by calling getAbsolutePath() on each element from the input list and joining all values with the OS's path separator.
  4. The static factory method PathEnvironmentVariable.create would then be changed from constructing one long string to constructing a list of File objects from all the settings and pass that one to the constructor. Of course the logic described above could also be moved from the constructor to the static factory method instead.

moritzbeermann avatar Jun 21 '23 15:06 moritzbeermann

I guess I start to understand your problem. You are trying to enter a path list in a place where, by design, it is expected to enter only a single folder path.

You cannot do this. If you have two toolchains, you have to tell Eclipse, for each toolchain separately, the exact folder where the binaries are.

ilg-ul avatar Jun 21 '23 16:06 ilg-ul

You cannot do this. If you have two toolchains, you have to tell Eclipse, for each toolchain separately, the exact folder where the binaries are.

the code does this, not the user :-) e.g.:

https://github.com/eclipse-embed-cdt/eclipse-plugins/blob/752ae80fe82e14770728ebd18626de9d183ca625/plugins/org.eclipse.embedcdt.managedbuild.cross.arm.core/src/org/eclipse/embedcdt/managedbuild/cross/arm/core/EnvironmentVariableSupplier.java#L147-L148

I'll look into this a little, the fact that it works in all other cases (Linux, Windows with drive letters) is probably somewhat undefined behaviour as having a "File" store a non-single file/dir name is an error.

jonahgraham avatar Jun 21 '23 17:06 jonahgraham

Yes, the resulting PATH should use both the toolchain and the build tools paths, but each should be a single folder path.

Normally the preferences pages should validate the input and do not accept non-valid folder paths. If currently it is possible to enter multiple paths, this is a bug that should be fixed.

ilg-ul avatar Jun 21 '23 18:06 ilg-ul

There may be a UI bug, but the current issue is this, the code in EnvironmentVariableSupplier (quoted above) does this (this is the simplified code):

String buildToolsPath = "\\somepath\gnu-arm-eclipse-build-tools\2.6-201507152002\bin";
String toolchainPath = "\\somepath\aarch64-none-elf-11.3.rel1\bin";

String path = buildToolsPath;
path += EclipseUtils.getPathSeparator(); 
path += toolchainPath; 

// at this point path="\\somepath\gnu-arm-eclipse-build-tools\2.6-201507152002\bin;\\somepath\aarch64-none-elf-11.3.rel1\bin`
File sysroot = new File(path);

// at this point File sysroot, despite being of type File does not refer to a single File.

// later in PathEnvironmentVariable.getValue()
return sysroot.getAbsolutePath();

On Linux/mac and windows with drive letters we are getting away with it, the getAbsolutePath it not changing the value. But on Windows the \\ are being simplified (the doubled up \ are being simplified to a single \)

jonahgraham avatar Jun 21 '23 18:06 jonahgraham

I have put my thoughts to code in #576 - but I haven't exercised it yet, so it may not work. But hopefully it shows basically what is going wrong here.

jonahgraham avatar Jun 21 '23 18:06 jonahgraham

Ah, right, so the problem is not entering multiple folders in the Toolchain Folder field, but a bug when composing the the PATH.

I did not check the documentation, but using File() to validate a composed path is most probably an error which must be fixed.

If your PR does this, and is portable to also work on macOS and Linux, it should be fine.

ilg-ul avatar Jun 21 '23 18:06 ilg-ul

@moritzbeermann - can you test the change in #576? It resolves the main issue here, although the GCC (specifically ld) I use does not like the toolchain being located in UNC path and when gcc constructs -L arguments to pass to collect2 (aka ld) it uses the UNC paths, but ld does not understand the UNC paths so I get errors about not found libraries.

jonahgraham avatar Jun 27 '23 18:06 jonahgraham