Community
Participate
Working Groups
When asking for the scanner info paths, everything is shoved into a Path object, and then toOSString() is called on it. This means you won't necessarily get what you actually see in the Paths & Symbols. Furthermore, it really messes things up for remote projects, as the discovered paths could be coming from a system that has a different path format to the local machine. If you try passing the converted path to the preprocessor, it won't be able to find header files on that path, because the path doesn't exist. You can't really convert them back to the remote machine's format after the fact, because backslash is a valid filename character on non-Windows platforms, and you have no way of knowing what was originally a backslash in the name and what was a backslash introduced by the bogus path conversion. The proposed solution is to have the paths returned exactly as they are represented in the Paths & Symbols dialog. Since they are typically in the OS format already anyway, the effect on existing implementations should be nil.
Created attachment 170996 [details] proposed patch Proposed patch attached.
getValues functions goes some non-trivial path manipulations to calculate "path". Are you saying it is all non needed? If so all the code that calculates path should be removed, because "path" variable was only used in the statement you removed (except for check for null...) Code is here for reference private String[] getValues(ICLanguageSettingPathEntry pathEntries[]){ String values[] = new String[pathEntries.length]; IPath path; int num = 0; for (ICLanguageSettingPathEntry pathEntry : pathEntries) { String p = pathEntry.getValue(); if(p == null) continue; //TODO: obtain location from pathEntries when entries are resolved path = new Path(p);//p.getLocation(); if(pathEntry.isValueWorkspacePath()){ IWorkspaceRoot root = ResourcesPlugin.getWorkspace().getRoot(); IResource rc = root.findMember(path); if(rc != null){ path = rc.getLocation(); } } else if (!path.isAbsolute()) { IPath projLocation = fProject != null ? fProject.getLocation() : null; if(projLocation != null) path = projLocation.append(path); } if(path != null) values[num++] = path.toOSString(); } if(num < pathEntries.length){ String tmp[] = new String[num]; System.arraycopy(values, 0, tmp, 0, num); values = tmp; } return values; }
This is way too risky for 7.0. Retargeted for 8.0.
(In reply to comment #3) > This is way too risky for 7.0. Retargeted for 8.0. I think there should be a discussion about it before you go an retarget a bug that's assigned to someone else.
(In reply to comment #2) > getValues functions goes some non-trivial path manipulations > to calculate "path". Are you saying it is all non needed? > If so all the code that calculates path should be removed, because "path" > variable was only used in the statement you removed (except for check for > null...) Actually, I'm going to alter the patch some... I see that it's not going to behave right for relative paths, which are supposed to evaluate relative to the project location.
(In reply to comment #4) > I think there should be a discussion about it before you go an retarget a bug > that's assigned to someone else. I think there should be a discussion and an unanimous decision before something as drastic as this change is accepted when only RC4 separates us from the release.
(In reply to comment #6) > (In reply to comment #4) > > I think there should be a discussion about it before you go an retarget a bug > > that's assigned to someone else. > I think there should be a discussion and an unanimous decision before something > as drastic as this change is accepted when only RC4 separates us from the > release. I raised the Bugzilla to facilitate discussion... You don't see a commit do you?
(In reply to comment #7) Please consider my attempt at retargeting as a vote against making this risky change in 7.0.
According to ramp down policy commits after RC have to discussed on mailing list. I know we not really being doing it because cvs-commits list is available now, but I think this is the case - not all committers read all bugs. My vote it to hold off as well as I think it is risky change.
(In reply to comment #0) > The proposed solution is to have the paths returned exactly as they are > represented in the Paths & Symbols dialog. Since they are typically in the OS > format already anyway, the effect on existing implementations should be nil. You are not going to keep relative paths exactly like entered in Paths&Symbols due to disconnection between representation in there (relative to project folder) vs in the extension point and Tool Settings (relative to build directory) for non-builtins. Note that representation from Tool Settings is saved in .cproject. I think saving paths as shown in Tool Settings might be a better idea. On the other hand, the consumers of those settings currently assume normalized full path. Apparently the effect of your changes is not nil if you have to change the tests. The existing code is tightly coupled and it is easy to destabilize it. Well, I suppose you can't really call it "stable" (See bug 275779 and subtasks) but I'd rather see changes in there in the new release cycle.
(In reply to comment #10) > You are not going to keep relative paths exactly like entered in Paths&Symbols > due to disconnection between representation in there (relative to project > folder) vs in the extension point and Tool Settings (relative to build > directory) for non-builtins. Right. But for absolute paths, they would stay as entered. Relative paths could be turned into an absolute path like the following: } else if (!path.isAbsolute()) { URI projLocationURI = fProject != null ? fProject.getLocationURI() : null; if(projLocationURI != null) { URI appendedURI = EFSExtensionManager.getDefault().append(projLocationURI, p); pathString = EFSExtensionManager.getDefault().getPathFromURI(appendedURI); } } The problem is knowing when the path is actually absolute. For most cases, a relative path would start with a character that is not a directory separator on any platform, e.g. some/relative/directory, ./some/relative/directory, or ../some/relative/directory. Those are easy to find. But given that you don't know what format the path is in, if the path begins with a backslash character, you don't know if it's meant to be an absolute Windows path, or a relative UNIX path that uses a backslash character in its name. Not sure what to do about that. I suppose it something could get added to EFSExtensionManager to return whether or not a given string representation of a path is relative or not. > Note that representation from Tool Settings is > saved in .cproject. I think saving paths as shown in Tool Settings might be a > better idea. I'll look into what you describe. > The existing code is tightly coupled and it is easy to destabilize it. Well, I > suppose you can't really call it "stable" (See bug 275779 and subtasks) but I'd > rather see changes in there in the new release cycle. Yeah, it has become a bit more involved than I originally thought. I will retarget to 8.0. In the meantime I will have to put a hack into RDT to convert all backslashes in scanner info paths to slashes. This will mean it will break if you have *NIX files with backslashes in their names, or if you use Windows as your remote machine. However unless I do that, the main use case of using *NIX machines for the remote machine will be broken.
This bug was assigned and targeted at a now released milestone. As that milestone has now passed, the milestone field has been cleared. If this bug has been fixed, please set the milestone to the version it was fixed in and marked the bug as resolved.
