Skip to content

Commit

Permalink
Fix Windows file separator issues (#1037)
Browse files Browse the repository at this point in the history
* Change conda tests so they can run on Windows

This changes the BWA commandline tool used in the tests to an
other small small command line tool called 'melt'.

 It makes the test a bit slower since the 'melt' tool  also depends
 on Python. But I couldn't find a small cross platform tool on
 bioconda which could be used for the tests.

* Wrap urlparse in a version which also works on Windows

It seems like snakemake uses urlparse to distinguish between URI's and
paths.

That 'undocumented' feature doesn't work with windows paths. So here
we wrap urlparse in snakemake.utils and add some code which adds this
'feature' for windows paths.

* Only use argvquote on Win when cmd.exe is the shell

This changes utils.format function a bit by exposing the  quote_func
arguments of QuotedFormatter.

* Add test for conda when using cmd.exe as shell

* Fix various issues with conda support on Win

* New black version

* Ignore errors in cleanup when testing on CI server

On windows we got :
PermissionError: [WinError 5] Access is denied: 'C:\\...\\vcruntime140.dll

* Use smart_open instead of urllib's urlparse

* Fail gracefully when a scheme is not implemented by smart_open

* Add tests which show file seperator problems on windows

The test show how the input/output with mixed fileseperators cause problems.

The test also show bug #46 where targets in subfolder can't be generated.

* Fixes filesep issues on windows

Input/output file are internally converted to forward
slashes before used by snakemake.

Target files from the command line are also converted internally.

* Add functions to modify subclassed strings

* Enable more tests

Co-authored-by: Johannes Köster <johannes.koester@uni-due.de>
  • Loading branch information
melund and johanneskoester committed Jun 9, 2021
1 parent 2998a0c commit cd5c58d
Show file tree
Hide file tree
Showing 9 changed files with 70 additions and 4 deletions.
15 changes: 15 additions & 0 deletions snakemake/io.py
Expand Up @@ -228,6 +228,15 @@ def __new__(cls, file):

return obj

def new_from(self, new_value):
new = str.__new__(self.__class__, new_value)
new._is_function = self._is_function
new._file = self._file
new.rule = self.rule
if new.is_remote:
new.remote_object._iofile = new
return new

def iocache(func):
@functools.wraps(func)
def wrapper(self, *args, **kwargs):
Expand Down Expand Up @@ -935,6 +944,12 @@ def __init__(self, value):
self.flags = dict()
self.callable = value if is_callable(value) else None

def new_from(self, new_value):
new = str.__new__(self.__class__, new_value)
new.flags = self.flags
new.callable = self.callable
return new


def flag(value, flag_type, flag_value=True):
if isinstance(value, AnnotatedString):
Expand Down
8 changes: 7 additions & 1 deletion snakemake/rules.py
Expand Up @@ -55,7 +55,7 @@
IncompleteCheckpointException,
)
from snakemake.logging import logger
from snakemake.common import Mode, lazy_property, TBDString
from snakemake.common import Mode, ON_WINDOWS, lazy_property, TBDString


class Rule:
Expand Down Expand Up @@ -490,6 +490,12 @@ def _set_inoutput_item(self, item, output=False, name=None):
if isinstance(item, Path):
item = str(item)
if isinstance(item, str):
if ON_WINDOWS:
if isinstance(item, (_IOFile, AnnotatedString)):
item = item.new_from(item.replace(os.sep, os.altsep))
else:
item = item.replace(os.sep, os.altsep)

rule_dependency = None
if isinstance(item, _IOFile) and item.rule and item in item.rule.output:
rule_dependency = item.rule
Expand Down
4 changes: 4 additions & 0 deletions snakemake/workflow.py
Expand Up @@ -644,6 +644,10 @@ def files(items):
)
)
targetfiles = set(chain(files(targets), priorityfiles, forcefiles, untilfiles))

if ON_WINDOWS:
targetfiles = set(tf.replace(os.sep, os.altsep) for tf in targetfiles)

if forcetargets:
forcefiles.update(targetfiles)
forcerules.update(targetrules)
Expand Down
2 changes: 2 additions & 0 deletions tests/common.py
Expand Up @@ -92,6 +92,7 @@ def run(
cleanup=True,
conda_frontend="mamba",
config=dict(),
targets=None,
container_image=os.environ.get("CONTAINER_IMAGE", "snakemake/snakemake:latest"),
**params,
):
Expand Down Expand Up @@ -154,6 +155,7 @@ def run(
stats="stats.txt",
config=config,
verbose=True,
targets=targets,
conda_frontend=conda_frontend,
container_image=container_image,
**params,
Expand Down
2 changes: 2 additions & 0 deletions tests/test_delete_all_output/Snakefile_inner
@@ -1,3 +1,5 @@
shell.executable("bash")

rule all:
input:
"some_dir/final",
Expand Down
25 changes: 25 additions & 0 deletions tests/test_filesep_windows/Snakefile
@@ -0,0 +1,25 @@

#shell.executable("bash")

# This tests that snakemake handles input/output
# defined with both forward and backwards slashes
# on Windows.

rule all:
input:
expand("subfolder\\test{i}.out2", i=range(3))

rule a:
output:
"subfolder\\test{i}.out"
shell:
'echo hello > {output}'


rule b:
input:
"subfolder\\test{i}.out"
output:
"subfolder/test{i}.out2"
shell:
"echo world> {output}"
@@ -0,0 +1 @@
world
Empty file.
17 changes: 14 additions & 3 deletions tests/tests.py
Expand Up @@ -23,9 +23,7 @@ def test_list_untracked():
pytest.mark.xfail(raises=PermissionError) if ON_WINDOWS else lambda x: x
)

# Must fail on Windows with PermissionError since the tempfile.TemporaryDirectory
# can't clean up the protected files generated in the test
@xfail_permissionerror_on_win

def test_delete_all_output():
run(dpath("test_delete_all_output"))

Expand Down Expand Up @@ -1239,5 +1237,18 @@ def test_source_path():
run(dpath("test_source_path"), snakefile="workflow/Snakefile")


@only_on_windows
def test_filesep_windows_targets():
run(
dpath("test_filesep_windows"),
targets=["subfolder/test2.out2", "subfolder/test1.out2"],
)


@only_on_windows
def test_filesep_on_windows():
run(dpath("test_filesep_windows"))


def test_set_resources():
run(dpath("test_set_resources"), overwrite_resources={"a": {"a": 1, "b": "foo"}})

0 comments on commit cd5c58d

Please sign in to comment.