Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fixed another symlinking issue with Docker + staged files from InitialWorkDirRequirement #1708

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Fixed another symlinking issue with Docker + staged files from InitialWorkDirRequirement #1708

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9508896
Fixed another symlinking issue with Docker + staged files from Initia…
jfennick Aug 16, 2022
20e3eab
use os.chmod and add write to existing mode
jfennick Aug 16, 2022
08bb056
blasting CI
jfennick Aug 16, 2022
806d695
fix pylint
jfennick Mar 9, 2023
f5a7412
use ensure_writable
jfennick Mar 9, 2023
83b0882
added PermissionError
jfennick Mar 9, 2023
a699296
try setting write bit separately from deletion
mr-c Dec 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cwltool/docker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ def add_file_or_directory_volume(
"""Append volume a file/dir mapping to the runtime option list."""
if not volume.resolved.startswith("_:"):
_check_docker_machine_path(volume.resolved)
self.append_volume(runtime, volume.resolved, volume.target)
self.append_volume(runtime, volume.resolved, volume.target, writable=volume.staged)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the goal here? This results in many files that should be mounted read-only to not be..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goal overall was to fix a bug I was encountering with a specific CommandLineTool (Yank) which we are no longer using. I still believe there is a bug in cwltool, but I can't seem to simultaneously fix the bug and get the test suite to pass. This is very low priority work for me, so feel free to close if you like.


def add_writable_file_volume(
self,
Expand Down
13 changes: 12 additions & 1 deletion cwltool/job.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,23 @@
continue
host_outdir_tgt = os.path.join(host_outdir, vol.target[len(container_outdir) + 1 :])
if os.path.islink(host_outdir_tgt) or os.path.isfile(host_outdir_tgt):
try:
ensure_writable(host_outdir_tgt, include_root=True)
except PermissionError:
pass
try:
os.remove(host_outdir_tgt)
except PermissionError:
pass
elif os.path.isdir(host_outdir_tgt) and not vol.resolved.startswith("_:"):
shutil.rmtree(host_outdir_tgt)
try:
ensure_writable(host_outdir_tgt, include_root=True)
except PermissionError:
pass
try:
shutil.rmtree(host_outdir_tgt)
except PermissionError:
pass

Check warning on line 119 in cwltool/job.py

View check run for this annotation

Codecov / codecov/patch

cwltool/job.py#L118-L119 

Added lines #L118 - L119 were not covered by tests
if not vol.resolved.startswith("_:"):
try:
os.symlink(vol.resolved, host_outdir_tgt)
Expand Down