-
-
Notifications
You must be signed in to change notification settings - Fork 232
Windows Compatibility: ensuring Cwltool works on windows OS #419
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
bee9324
1285c1b
85fa796
9db2091
ffe3aad
9a5221c
fb42e6a
2f9ac55
8d3ed5c
b5882c5
eeee83b
f957ad8
2118bd4
0b601fd
7902707
5ddf49f
367756a
236a217
b546f01
5bca2e0
c311908
bdebbc6
4dc76bd
c63c4dd
9a87223
a93ede0
f235ccf
72fc2e5
70041ce
69fc0fb
77c326b
8a54204
c234e04
51bd3df
5ecba0e
22ace22
b5da59a
c3c9df9
abd647b
3387ae8
b428d0e
82ceb89
0508997
fa7cc8e
6d1e136
75b7f1d
959a2b4
91e2ff7
6740309
aa97c43
5163f59
bcf3bfb
aa1ab81
56c3416
9b2d496
3f8208c
adcfa36
6393295
7c2fe22
fee18d0
2a190da
583b87b
b1854ac
4adfcb7
575f062
386b211
cc2a74b
fd70c8b
681da6b
550c406
5bdc1ef
4954f2c
e9f01e2
cfa1990
1dd8476
9267120
c85ba19
b203750
a0c2dae
85f3299
b63d472
8d3f692
06dd0b2
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 |
---|---|---|
@@ -0,0 +1,28 @@ | ||
version: .{build}-{branch} | ||
|
||
environment: | ||
|
||
matrix: | ||
- PYTHON: "C:\\Python27" | ||
PYTHON_VERSION: "2.7.x" | ||
PYTHON_ARCH: "32" | ||
|
||
- PYTHON: "C:\\Python27-x64" | ||
PYTHON_VERSION: "2.7.x" | ||
PYTHON_ARCH: "64" | ||
|
||
install: | ||
- "SET PATH=%PYTHON%;%PYTHON%\\Scripts;%PATH%" | ||
- "python --version" | ||
- "python -c \"import struct; print(struct.calcsize('P') * 8)\"" | ||
- "pip install --disable-pip-version-check --user --upgrade pip" | ||
- "pip install --upgrade --no-deps --force-reinstall git+git://github.com/kapilkd13/schema_salad@windows#egg=schema_salad-2.4.201706261942" | ||
|
||
build_script: | ||
- "%CMD_IN_ENV% python setup.py install" | ||
|
||
|
||
test_script: | ||
|
||
- "%CMD_IN_ENV% python setup.py test" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ | |
|
||
import shellescape | ||
|
||
from .utils import copytree_with_merge, docker_windows_path_adjust, onWindows | ||
from . import docker | ||
from .builder import Builder | ||
from .docker_uid import docker_vm_uid | ||
|
@@ -106,8 +107,14 @@ def relink_initialworkdir(pathmapper, inplace_update=False): | |
if os.path.islink(vol.target) or os.path.isfile(vol.target): | ||
os.remove(vol.target) | ||
elif os.path.isdir(vol.target): | ||
os.rmdir(vol.target) | ||
os.symlink(vol.resolved, vol.target) | ||
shutil.rmtree(vol.target) | ||
if onWindows(): | ||
if vol.type in ("File", "WritableFile"): | ||
shutil.copy(vol.resolved,vol.target) | ||
elif vol.type in ("Directory", "WritableDirectory"): | ||
copytree_with_merge(vol.resolved, vol.target) | ||
else: | ||
os.symlink(vol.resolved, vol.target) | ||
|
||
class JobBase(object): | ||
def __init__(self): # type: () -> None | ||
|
@@ -278,13 +285,16 @@ def run(self, pull_image=True, rm_container=True, | |
if vars_to_preserve is not None: | ||
for key, value in os.environ.items(): | ||
if key in vars_to_preserve and key not in env: | ||
env[key] = value | ||
env["HOME"] = self.outdir | ||
env["TMPDIR"] = self.tmpdir | ||
|
||
stageFiles(self.pathmapper, os.symlink, ignoreWritable=True) | ||
# On Windows, subprocess env can't handle unicode. | ||
env[key] = str(value) if onWindows() else value | ||
env["HOME"] = str(self.outdir) if onWindows() else self.outdir | ||
env["TMPDIR"] = str(self.tmpdir) if onWindows() else self.tmpdir | ||
if "PATH" not in env: | ||
env["PATH"] = str(os.environ["PATH"]) if onWindows() else os.environ["PATH"] | ||
|
||
stageFiles(self.pathmapper, ignoreWritable=True, symLink=True) | ||
if self.generatemapper: | ||
stageFiles(self.generatemapper, os.symlink, ignoreWritable=self.inplace_update) | ||
stageFiles(self.generatemapper, ignoreWritable=self.inplace_update, symLink=True) | ||
relink_initialworkdir(self.generatemapper, inplace_update=self.inplace_update) | ||
|
||
self._execute([], env, rm_tmpdir=rm_tmpdir, move_outputs=move_outputs) | ||
|
@@ -306,25 +316,26 @@ def add_volumes(self, pathmapper, runtime, stage_output): | |
containertgt = vol.target | ||
if vol.type in ("File", "Directory"): | ||
if not vol.resolved.startswith("_:"): | ||
runtime.append(u"--volume=%s:%s:ro" % (vol.resolved, containertgt)) | ||
runtime.append(u"--volume=%s:%s:ro" % (docker_windows_path_adjust(vol.resolved), docker_windows_path_adjust(containertgt))) | ||
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. Please wrap lines longer than 80 characters for easier review :-) 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 will take care. |
||
elif vol.type == "WritableFile": | ||
if self.inplace_update: | ||
runtime.append(u"--volume=%s:%s:rw" % (vol.resolved, containertgt)) | ||
runtime.append(u"--volume=%s:%s:rw" % (docker_windows_path_adjust(vol.resolved), docker_windows_path_adjust(containertgt))) | ||
else: | ||
shutil.copy(vol.resolved, vol.target) | ||
elif vol.type == "WritableDirectory": | ||
if vol.resolved.startswith("_:"): | ||
os.makedirs(vol.target, 0o0755) | ||
else: | ||
if self.inplace_update: | ||
runtime.append(u"--volume=%s:%s:rw" % (vol.resolved, containertgt)) | ||
runtime.append(u"--volume=%s:%s:rw" % (docker_windows_path_adjust(vol.resolved), docker_windows_path_adjust(containertgt))) | ||
else: | ||
shutil.copytree(vol.resolved, vol.target) | ||
elif vol.type == "CreateFile": | ||
createtmp = os.path.join(host_outdir, os.path.basename(vol.target)) | ||
with open(createtmp, "wb") as f: | ||
f.write(vol.resolved.encode("utf-8")) | ||
runtime.append(u"--volume=%s:%s:ro" % (createtmp, vol.target)) | ||
runtime.append(u"--volume=%s:%s:ro" % (docker_windows_path_adjust(createtmp), docker_windows_path_adjust(vol.target))) | ||
|
||
|
||
def run(self, pull_image=True, rm_container=True, | ||
rm_tmpdir=True, move_outputs="move", **kwargs): | ||
|
@@ -361,14 +372,14 @@ def run(self, pull_image=True, rm_container=True, | |
|
||
runtime = [u"docker", u"run", u"-i"] | ||
|
||
runtime.append(u"--volume=%s:%s:rw" % (os.path.realpath(self.outdir), self.builder.outdir)) | ||
runtime.append(u"--volume=%s:%s:rw" % (os.path.realpath(self.tmpdir), "/tmp")) | ||
runtime.append(u"--volume=%s:%s:rw" % (docker_windows_path_adjust(os.path.realpath(self.outdir)), self.builder.outdir)) | ||
runtime.append(u"--volume=%s:%s:rw" % (docker_windows_path_adjust(os.path.realpath(self.tmpdir)), "/tmp")) | ||
|
||
self.add_volumes(self.pathmapper, runtime, False) | ||
if self.generatemapper: | ||
self.add_volumes(self.generatemapper, runtime, True) | ||
|
||
runtime.append(u"--workdir=%s" % (self.builder.outdir)) | ||
runtime.append(u"--workdir=%s" % (docker_windows_path_adjust(self.builder.outdir))) | ||
runtime.append(u"--read-only=true") | ||
|
||
if kwargs.get("custom_net", None) is not None: | ||
|
@@ -379,9 +390,12 @@ def run(self, pull_image=True, rm_container=True, | |
if self.stdout: | ||
runtime.append("--log-driver=none") | ||
|
||
euid = docker_vm_uid() or os.geteuid() | ||
if onWindows(): # windows os dont have getuid or geteuid functions | ||
euid = docker_vm_uid() | ||
else: | ||
euid = docker_vm_uid() or os.geteuid() | ||
|
||
if kwargs.get("no_match_user", None) is False: | ||
if kwargs.get("no_match_user", None) is False and euid is not None: | ||
runtime.append(u"--user=%s" % (euid)) | ||
|
||
if rm_container: | ||
|
@@ -436,7 +450,7 @@ def _job_popen( | |
|
||
sp = subprocess.Popen(commands, | ||
shell=False, | ||
close_fds=True, | ||
close_fds=not onWindows(), | ||
stdin=stdin, | ||
stdout=stdout, | ||
stderr=stderr, | ||
|
@@ -478,14 +492,14 @@ def _job_popen( | |
stderr_path=stderr_path, | ||
stdin_path=stdin_path, | ||
) | ||
with open(os.path.join(job_dir, "job.json"), "w") as f: | ||
with open(os.path.join(job_dir, "job.json"), "wb") as f: | ||
json.dump(job_description, f) | ||
try: | ||
job_script = os.path.join(job_dir, "run_job.bash") | ||
with open(job_script, "wb") as f: | ||
f.write(job_script_contents.encode('utf-8')) | ||
job_run = os.path.join(job_dir, "run_job.py") | ||
with open(job_run, "w") as f: | ||
with open(job_run, "wb") as f: | ||
f.write(PYTHON_RUN_SCRIPT) | ||
sp = subprocess.Popen( | ||
["bash", job_script.encode("utf-8")], | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,6 +39,8 @@ | |
from .software_requirements import DependenciesConfiguration, get_container_from_software_requirements, SOFTWARE_REQUIREMENTS_ENABLED | ||
from .stdfsaccess import StdFsAccess | ||
from .update import ALLUPDATES, UPDATES | ||
from .utils import onWindows | ||
from ruamel.yaml.comments import Comment, CommentedSeq, CommentedMap | ||
|
||
|
||
_logger = logging.getLogger("cwltool") | ||
|
@@ -695,6 +697,10 @@ def main(argsl=None, # type: List[str] | |
argsl = sys.argv[1:] | ||
args = arg_parser().parse_args(argsl) | ||
|
||
# If On windows platform, A default Docker Container is Used if not explicitely provided by user | ||
if onWindows() and not args.default_container: | ||
args.default_container = "ubuntu" | ||
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 thought we were going with a more basic container? 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 tested conformance tests on Alpine(size 5mb) docker image. 2 types of test are failing 1. test that use 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. Lets add 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. Ok. One problem that we still have is that test 52 and 53 uses 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. wrong means different output. It appears 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. Interesting! Feel free to propose whichever container is needed for those tests |
||
|
||
# If caller provided custom arguments, it may be not every expected | ||
# option is set, so fill in no-op defaults to avoid crashing when | ||
# dereferencing them in args. | ||
|
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.
Then lets only call
str
ifonWindows()
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.
Okay. we can do that