Skip to content

Commit

Permalink
Don't mark inputs (or outputs) executable for no reason (#4728)
Browse files Browse the repository at this point in the history
* Be explicit about executable representation

* Add testing to make sure outputs aren't unexpecteldy executable

* Let js expressions in the scatters take a long time to start Node

---------

Co-authored-by: Lon Blauvelt <[email protected]>
  • Loading branch information
adamnovak and DailyDreaming authored Jan 16, 2024
1 parent f5f2535 commit b2e0516
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/toil/fileStores/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def __init__(self, fileStoreID: str, size: int, executable: bool = False) -> Non

def pack(self) -> str:
"""Pack the FileID into a string so it can be passed through external code."""
return f'{self.size}:{int(self.executable)}:{self}'
return f'{self.size}:{"1" if self.executable else "0"}:{self}'

@classmethod
def forPath(cls, fileStoreID: str, filePath: str) -> 'FileID':
Expand All @@ -54,7 +54,7 @@ def unpack(cls, packedFileStoreID: str) -> 'FileID':
vals = packedFileStoreID.split(':', 2)
# Break up the packed value
size = int(vals[0])
executable = bool(vals[1])
executable = (vals[1] == "1")
value = vals[2]
# Create the FileID
return cls(value, size, executable)
10 changes: 10 additions & 0 deletions src/toil/test/cwl/cwlTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import os
import re
import shutil
import stat
import subprocess
import sys
import unittest
Expand Down Expand Up @@ -136,6 +137,8 @@ def run_conformance_tests(
"--statusWait=10",
"--retryCount=2",
"--relax-path-checks",
# Defaults to 20s but we can't start hundreds of nodejs processes that fast on our CI potatoes
"--eval-timeout=600",
f"--caching={caching}"
]

Expand Down Expand Up @@ -254,6 +257,13 @@ def _tester(self, cwlfile, jobfile, expect, main_args=[], out_name="output", out
out.get(out_name, {}).pop("nameroot", None)
self.assertEqual(out, expect)

for k, v in expect.items():
if isinstance(v, dict) and "class" in v and v["class"] == "File" and "path" in v:
# This is a top-level output file.
# None of our output files should be executable.
self.assertTrue(os.path.exists(v["path"]))
self.assertFalse(os.stat(v["path"]).st_mode & stat.S_IXUSR)

def _debug_worker_tester(self, cwlfile, jobfile, expect):
from toil.cwl import cwltoil

Expand Down

0 comments on commit b2e0516

Please sign in to comment.