Change abbreviation characters #1
Conversation
That is strange. Can you reference some tracked issue/description of the .NET limitation? |
| @@ -54,8 +63,11 @@ public void doNothingIfThereIsEnoughRoom() throws Exception { | |||
| FreeStyleProject p = f.createProject(FreeStyleProject.class, "and_a_project_in_it"); | |||
| p.setAssignedNode(s); | |||
|
|
|||
| // Not enough for anything. | |||
| setMaxPathLength(s, 4096); | |||
There was a problem hiding this comment.
In this test it should be set so it is enough - comment suggests the opposite. Are you suggesting it is not on Windows (and that is why we need this)?
There was a problem hiding this comment.
Sorry, comment should be updated.
There was a problem hiding this comment.
Comment needs to be updated
| FreeStyleBuild b = p.scheduleBuild2(0).get(); | ||
| assertThat(b.getWorkspace().getRemote(), equalTo(s.getRootPath() + "/workspace/" + p.getFullName())); | ||
| assertThat(b.getWorkspace().getRemote(), equalTo(formatPath(s.getRootPath() + "/workspace/" + p.getFullName()))); |
There was a problem hiding this comment.
Why is this needed? If I am not mistaken, this is implemented as File.createTempFile("hudson", "test", base); that should be Windows friendly.
There was a problem hiding this comment.
The tests did not pass before on Windows because slash directions were getting mixed around.
There was a problem hiding this comment.
AHh yeah I remember. it is plenty windows friendly either way. However it puts \ in the Windows paths and / in the unix paths. Which is why formatPath is needed (otherwise the string comparison fails)
There was a problem hiding this comment.
I have fixed the / vs. \ issue differently on master.
| return inputPath.replace("/", "\\"); | ||
| } | ||
| else { | ||
| return inputPath.replace("\\", "/"); |
There was a problem hiding this comment.
Backslash is a valid path character on unix.
There was a problem hiding this comment.
Ahh okay. In that case we probably just want to replace the windows one. It is primarily just for consistency, otherwise you end up with lots of workspace paths like "foo\bar/baz/bop" which is harder to read
|
I work on the .NET team. I'll try to track down the source of this issue. I'm pretty sure it's fixed in .NET Core but has been a long standing issue in the standard .NET Framework. |
mmitche
force-pushed
the
use-net-compat-path
branch
from
1920cb6 to
3fb964e
Compare
|
The issue with the formatPath are two things:
I'm tracking down the original issue of ... in .NET. |
|
Alright, so I tracked it down. Appears it was an msbuild issue where they were using their own path normalization vs. the official ones (which work). However, the issue still stands in most versions of msbuild, and given that msbuild is a pretty common build tool invoked by Jenkins, I think the fix still stands. It was fixed in the .NET Core version of msbuild here: dotnet/msbuild#632 |
|
@mmitche, I do not mind putting it in, though please make the comment more specific so we know what tool is it, in what version it is fixed. |
mmitche
force-pushed
the
use-net-compat-path
branch
from
3fb964e to
0c28784
Compare
While ... is valid in Windows, MSbuild does not recognize this as a valid path. After abbreviation, change ... to --- to avoid issues
mmitche
force-pushed
the
use-net-compat-path
branch
from
0c28784 to
c0c3355
Compare
|
Ahh didn't realize you had fixed the tests. Reverted that bit, updated the comment. Thanks! |
While ... is valid in Windows, .NET does not recognize this as a valid path. As a result, tools like msbuild will fail. After abbreviation, change ... to --- to avoid issues
Also fix tests on Windows
Windows has \ as the path separator character, and a significantly shorter MAX_PATH. Set the cached path length long enough that a test that doesn't want the path to be shortened passes.
Also replace / with \ on Windows (and vice versa for non-Windows platforms) when comparing paths.