-
Notifications
You must be signed in to change notification settings - Fork 293
Support more bind path options #144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,7 @@ function dockerCommand(options) { | |
throw new Error(ps.stderr); | ||
} | ||
return ps; | ||
} | ||
}; | ||
|
||
/** | ||
* Build the custom Docker image | ||
|
@@ -34,34 +34,67 @@ function buildImage(dockerFile) { | |
return imageName; | ||
}; | ||
|
||
/** | ||
* Test bind path to make sure it's working | ||
* @param {string} bindPath | ||
* @return {boolean} | ||
*/ | ||
function tryBindPath(bindPath) { | ||
const options = ['run', '--rm', '-v', `${bindPath}:/test`, | ||
'alpine', 'ls', '/test/serverless.yml']; | ||
try { | ||
const ps = dockerCommand(options); | ||
return ps.stdout.trim() === '/test/serverless.yml'; | ||
} catch (err) { | ||
return false; | ||
} | ||
}; | ||
|
||
/** | ||
* Get bind path depending on os platform | ||
* @param {string} servicePath | ||
* @return {string} The bind path. | ||
*/ | ||
function getBindPath(servicePath) { | ||
// Determine os platform of docker CLI from 'docker version' | ||
const options = ['version', '--format', '{{with .Client}}{{.Os}}{{end}}']; | ||
const ps = dockerCommand(options); | ||
const cliPlatform = ps.stdout.trim(); | ||
|
||
// Determine bind path | ||
let bindPath; | ||
if (process.platform === 'win32') { | ||
bindPath = servicePath.replace(/\\([^\s])/g, '/$1'); | ||
if (cliPlatform === 'windows') { | ||
bindPath = bindPath.replace(/^\/(\w)\//i, '$1:/'); | ||
} | ||
} else if (isWsl) { | ||
bindPath = servicePath.replace(/^\/mnt\//, '/'); | ||
if (cliPlatform === 'windows') { | ||
bindPath = bindPath.replace(/^\/(\w)\//i, '$1:/'); | ||
} | ||
if (process.platform !== 'win32' && !isWsl) { | ||
return servicePath; | ||
} | ||
|
||
let bindPaths = []; | ||
let baseBindPath = servicePath.replace(/\\([^\s])/g, '/$1'); | ||
let drive; | ||
let path; | ||
|
||
bindPaths.push(baseBindPath); | ||
if (baseBindPath.startsWith('/mnt/')) { // cygwin "/mnt/C/users/..." | ||
baseBindPath = baseBindPath.replace(/^\/mnt\//, '/'); | ||
} | ||
if (baseBindPath[1] == ':') { // normal windows "c:/users/..." | ||
drive = baseBindPath[0]; | ||
path = baseBindPath.substring(3); | ||
} else if (baseBindPath[0] == '/' && baseBindPath[2] == '/') { // gitbash "/c/users/..." | ||
drive = baseBindPath[1]; | ||
path = baseBindPath.substring(3); | ||
} else { | ||
bindPath = servicePath; | ||
throw new Error(`Unknown path format ${baseBindPath.substr(10)}...`); | ||
} | ||
|
||
bindPaths.push(`/${drive.toLowerCase()}/${path}`); | ||
bindPaths.push(`/${drive.toUpperCase()}/${path}`); | ||
bindPaths.push(`/mnt/${drive.toLowerCase()}/${path}`); | ||
bindPaths.push(`/mnt/${drive.toUpperCase()}/${path}`); | ||
bindPaths.push(`${drive.toLowerCase()}:/${path}`); | ||
bindPaths.push(`${drive.toUpperCase()}:/${path}`); | ||
|
||
for (let i = 0; i < bindPaths.length; i++) { | ||
const bindPath = bindPaths[i]; | ||
if (tryBindPath(bindPath)) { | ||
return bindPath; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would the changes from #141 fit better here if we're adding this too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think they would. This fixes the bind path, while the other change fixes the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doh. totally missed that. |
||
} | ||
} | ||
|
||
return bindPath; | ||
throw new Error('Unable to find good bind path format'); | ||
}; | ||
|
||
module.exports = {buildImage, getBindPath}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is trying way too hard. Running a Docker container is not cost free. Here we blindly looping through a set of paths?