dynamically assign drive letter to WORKSPACE env var#8398
dynamically assign drive letter to WORKSPACE env var#8398colinsurprenant merged 1 commit intoelastic:masterfrom
Conversation
|
I am +1 on this proposal (using |
ci/unit_tests.bat
Outdated
| setlocal enabledelayedexpansion | ||
|
|
||
| setlocal | ||
| if not exist %WORKSPACE% ( |
There was a problem hiding this comment.
Is there a current working in batch (e.g. pwd) that could be used instead of a the Jenkins environment variable ? (It is a bit kinder to Windows developers )
There was a problem hiding this comment.
yeah, once we know this work we can certainly improve it to not make it Jenkins specific somehow... or have some sensible fallback defaults.
ci/unit_tests.bat
Outdated
|
|
||
| setlocal | ||
| if not exist %WORKSPACE% ( | ||
| echo Error: env var WORKSPACE must be defined |
There was a problem hiding this comment.
hmm...i ran into this error while testing, but didn't get the echo to print in the cmd console.
There was a problem hiding this comment.
oh ok. do you mean testing in Jenkins? If so maybe WORKSPACE is not defined then? but it should be per the Jenkins build env var list page!?
There was a problem hiding this comment.
Sorry, i meant running locally I did not define %WORKSPACE% and had to review the code to understand the issue since the echo didn't print.
I haven't tested in Jenkins yet.
There was a problem hiding this comment.
This seems to fix it. Note - the following code works from cmd, but seems to not work from PS, Jenkins is not PS, and I think .bat files are default executed by cmd.
if "%WORKSPACE%"=="" (
echo Error: env var WORKSPACE must be defined
exit /B 1
)
There was a problem hiding this comment.
fixed this.
colinsurprenant
changed the title
|
I added WIP in the title because there will certainly be some back & forth with this since I did not test it under Jenkins. |
| exit /B 1 | ||
| ) | ||
|
|
||
| :: see if %WORKSPACE% is alread mapped to a drive |
There was a problem hiding this comment.
I commented out
rem %RAKEPATH% test:install-core
rem %RAKEPATH% test:core
and ran this a few times (in cmd) to test this, and the drives kept incrementing every time i ran it. (it did succesfully skip c:\ however)
I am not familiar with the voodoo here, but trying to echo %%G or %%~sfH seemed to echo the literals not what I assume they are expanding to ?
There was a problem hiding this comment.
uh .. this is weird. I tested the "reuse" bits in isolation and it worked in my 2012 box ... but I did not test when included in the user_tests.bat. will do, thanks!
There was a problem hiding this comment.
fixed with latest set of commits
|
I will readily admit that I don't fully understand the line by line details of the batch scripting. But I don't think that is needed here it since it either works or it doesn't. I worked with @colinsurprenant to get this fully tested in on our Windows 2012 and Windows 2016 boxes and all looks good.
LGTM |
colinsurprenant
force-pushed
the
fix/long_path
branch
from
9e5df69 to
6b6dc35
Compare
fix underfined env var check check for defined JRUBYSRCDIR env var check for errorlevel after launching rake tasks cosmetics call rake keep original path cosmetic cosmetic
colinsurprenant
force-pushed
the
fix/long_path
branch
from
6b6dc35 to
e172341
Compare
|
rebased and merged into 5.5, 5.6, 6.0, 6.x and master. |
colinsurprenant
changed the title
3 participants
Try to assign a drive letter to the
WORKSPACEenv var to shorten the current path to avoid hitting the 260 chars limits in Windows.Strategy:
substdrives to see ifWORKSPACEwas already assigned to a drive and reuse itsubstthe first free drive letter toWORKSPACEThe idea is to avoid just assigning new drive letters blindly since this is a long-running server. The number of used drive letters will top at the number of unique
WORKSPACEpaths which shoud avoid depletion.Relates to #7767