Bugzilla – Full Text Bug Listing
| Summary: | Default working directory path separator wrong | ||
|---|---|---|---|
| Product: | [Tools] PTP | Reporter: | Roland Schulz <roland> |
| Component: | RM | Assignee: | Project Inbox <ptp-inbox> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | arossi, g.watson, roland |
| Version: | 4.0 | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Windows All | ||
| Whiteboard: | |||
|
Description
Roland Schulz
Just to clarify: this is not PBS specific. (Tested with PBS and SLURM) The problem is that toOSString() returns the path with the separator correct on the local machine. But toOSString is used in many places where the path can be remote and thus the local separator different than the remote separator. Am I missing something or is every call to toOSString for remote paths (there are a lot) a bug? And how is it supposed to be done correctly? The two locations which are important specific for this bug org.eclipse.ptp.remote.ui.dialogs.RemoteResourceBrowser:231 org.eclipse.ptp.rm.pbs.ui.data.PBSBatchScriptTemplate:514 The PBSBatchTemplate code you point to was borrowed from elsewhere, actually. I assumed that method did what it was supposed to. The problem of defining the remote separators is foreseen (but not solved) by the org.eclipse.ptp.rm.pbs.ui.utils.ConfigUtils static methods: 	/* 	 * TODO we need some way of determining this dynamically, though I would 	 * imagine in the vast majority of cases PBS will be running on a UNIX-type 	 * system, so it will be "\n". 	 * 	 * @param rEMOTE_LINE_SEP 	 */ 	public static void setREMOTE_LINE_SEP(String rEMOTE_LINE_SEP) { 		REMOTE_LINE_SEP = rEMOTE_LINE_SEP; 	} 	/* 	 * TODO we need some way of determining this dynamically, though I would 	 * imagine in the vast majority of cases PBS will be running on a UNIX-type 	 * system, so it will be "/". 	 * 	 * @param rEMOTE_PATH_SEP 	 */ 	public static void setREMOTE_PATH_SEP(String rEMOTE_PATH_SEP) { 		REMOTE_PATH_SEP = rEMOTE_PATH_SEP; 	} Certainly these have to be set in a different way. The code I placed here was simply a placeholder/reminder. I agree with Roland that this is a bug of "platform-dependent" means "local platform".
I'm sorry about the garbled mess of the last message. My browser keeps reverting to MacRoman. (In reply to comment #3) The problem is Windows on the client. This should work. And currently doesn't. toOSString returns "\" (backslash) if the *client* is Windows. Thus under the normal case, unix on the server, it breaks. One way to do it, would be to replace all calls to toOSString by calls to toString in remote code. This way the normal case (unix on server) works. But it wouldn't work for cases where the remote code can also be executed locally. Because then it would break for Windows. I guess the correct way would be to have a toString call which takes the current File manager as argument and calls org.eclipse.ptp.remote.core.IRemoteFileManager.getDirectorySeparator() to get the correct separator. What should we do? It is something which should be fixed but it affects a lot of places. Greg? I agree with Roland that this should be connected to the FileManager for the target resource, whether remote or local. Sigh. The whole path thing is a pain. IPath is not designed for dealing with local/remote paths so causes all sorts of problems. I think what we need is an IRemotePath that provides toLocalString() and toRemoteString() along with all the other IPath methods. Annoyingly neither IPath nor Path can be extended, however. We should probably add the interface, but anything else would require fairly extensive changes. Too risky at this stage. I suggest you just replace toOSString in your code to make it work. What if we went through a URL or URI? That is, we take the toOSString(), turn it into a URI-path (with '/'), with the understanding that the proxy on the other end will consider this a URI path, prepend "file:" to it, then create the correct path by doing new File( new URI(wdir) ).getAbsolutePath()? This would also mean a slight change to our attribute handling. 1. cd $workingdir would not get replaced by the client. 2. The client sends the workingdir path as a separate attribute 3. the proxy does this single replacement on the script after it has converted the path to the local file-system representation. Let me try that again. This would also mean a slight change to our attribute handling. 1. cd $workingdir would not get replaced by the client. 2. The client sends the workingdir path as a separate attribute 3. the proxy does this single replacement on the script after it has converted the path to the local file-system representation. (In reply to comment #10) > Let me try that again. > > This would also mean a slight change to our attribute > handling. > > 1. cd $workingdir would not get replaced by the client. > > 2. The client sends the workingdir path as a separate attribute > > 3. the proxy does this single replacement on the script after it has converted > the path to the local file-system representation. Let's not change it again where the attributes get substituted. Let's just replace toOSString by toString. And then when the IRemotePath is in place we use that. OK, I'm fine with that. We can certainly change the PBSBatchTemplate method. Shall we also change the RemoteBrowser? (In reply to comment #12) > OK, I'm fine with that. We can certainly change the PBSBatchTemplate method. > Shall we also change the RemoteBrowser? I just committed the change from toOSString to toString for PBS and RemoteBrowser. I leave it open so we can address it fully (with IRemotePath) after the release. Reducing severity to normal. Superseded by target configurations. |