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.

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

flagging a input Directory as writable still symlinks all files, not copy #282

Open
gijzelaerr opened this issue Jan 30, 2017 · 9 comments

Comments

@gijzelaerr
Copy link
Member

gijzelaerr commented Jan 30, 2017

hi!

When I define an input folder as writable it is still being symlinked. Example:

cwlVersion: v1.0
class: CommandLineTool

requirements:
  - class: DockerRequirement
    dockerFile: |
       FROM kernsuite/base:1
       RUN docker-apt-install aoflagger
    dockerImageId: vermeerkat/aoflagger
  - class: InlineJavascriptRequirement
  - class: InitialWorkDirRequirement
    listing:
      - entry: $(inputs.ms)
        entryname: flagged.ms
        writable: true

baseCommand: "aoflagger"
arguments: ["flagged.ms"]

inputs:
  ms: Directory

outputs:
  ms:
    type: Directory
    outputBinding:
      glob: flagged.ms

But if I then look inside the intermediate cached results, the recursive directory structure is created but all files are symlinked:

gijs@sherlock:~/vermeerkat-cwl/cache/085d5148d1d46592125b34efa141b372/flagged.ms (master)
λ  ls -al
total 132
drwxr-xr-x 15 gijs gijs 4096 Jan 30 11:30 .
drwxrwxr-x  3 gijs gijs 4096 Jan 30 11:30 ..
drwxr-xr-x  2 gijs gijs 4096 Jan 30 11:30 ANTENNA
drwxr-xr-x  2 gijs gijs 4096 Jan 30 11:30 DATA_DESCRIPTION
drwxr-xr-x  2 gijs gijs 4096 Jan 30 11:30 FEED
drwxr-xr-x  2 gijs gijs 4096 Jan 30 11:30 STATE
lrwxrwxrwx  1 gijs gijs   84 Jan 30 11:30 table.dat -> /home/gijs/vermeerkat-cwl/cache/ca8d89e0b4b1c166a06924f8a8218b42/result.ms/table.dat
lrwxrwxrwx  1 gijs gijs   83 Jan 30 11:30 table.f0 -> /home/gijs/vermeerkat-cwl/cache/ca8d89e0b4b1c166a06924f8a8218b42/result.ms/table.f0
lrwxrwxrwx  1 gijs gijs   83 Jan 30 11:30 table.f1 -> /home/gijs/vermeerkat-cwl/cache/ca8d89e0b4b1c166a06924f8a8218b42/result.ms/table.f1
lrwxrwxrwx  1 gijs gijs   83 Jan 30 11:30 table.f2 -> /home/gijs/vermeerkat-cwl/cache/ca8d89e0b4b1c166a06924f8a8218b42/result.ms/table.f2
lrwxrwxrwx  1 gijs gijs   85 Jan 30 11:30 table.lock -> /home/gijs/vermeerkat-cwl/cache/ca8d89e0b4b1c166a06924f8a8218b42/result.ms/table.lock
@tetron
Copy link
Member

tetron commented Jan 31, 2017

The writable flag means in this context is that the Directory is writable (you can add/delete/rename files, as opposed to a read-only directory) but that doesn't apply recursively to the file contents.

You should be able to use a Javascript expression in the entry field to return a Directory object with the "writable" flag set for each file.

@gijzelaerr
Copy link
Member Author

ok I'll try. But that seems to be quite a workaround hack to try to accomplish something quite simple? We are going to be using this quite often. Lets chat in the chat channel about extending the standard.

@mr-c
Copy link
Member

mr-c commented Feb 2, 2017 via email

@gijzelaerr
Copy link
Member Author

would adding a recursivewritable: true option make sense? Or fullywritable?

@mr-c
Copy link
Member

mr-c commented Feb 6, 2017

My vote would be for recursiveWritable: true

@mr-c
Copy link
Member

mr-c commented Feb 7, 2017

A couple points:

writable is only a property on Dirent objects in CWL 1.0, and not anywhere else

But the spec says that when the InitialWorkDirRequirement listing is an Expression it can only return Files and/or Directorys, not Dirents.

However, cwltool does accept and process correctly an Expression for listing that returns an array containing a Dirent

But playing with this beyond-spec behavior I am not seeing a way to specify writable: true recursively for an entire directory. You can't put a Dirent object inside a synthetic Directory object, for example.

bonus: even with writable: false, cwltool doesn't make directories read-only, one can delete files from them

To summarize: We have a clear user story for marking an entire Directory tree as writable. One way forward is to issue a clarification that writable: true for a Directory is recursive and then to update the implementations to match.

@mr-c
Copy link
Member

mr-c commented Feb 7, 2017

@tetron while tracing the code, I came across some strangness: https://github.com/common-workflow-language/cwltool/blob/master/cwltool/pathmapper.py#L153

@mr-c
Copy link
Member

mr-c commented Feb 16, 2017

@gijzelaerr wrong button, sorry!

qiukunlong pushed a commit to qiukunlong/cwltool that referenced this issue Mar 25, 2017
2cf0896 Merge pull request common-workflow-language#289 from denis-yuen/patch-1
68f5f90 Merge pull request common-workflow-language#291 from bmeg/master
a82cf0d Merge https://github.com/bmeg/common-workflow-language
e7325dc Adding docker hint for conformance test
70501d5 Merge pull request common-workflow-language#290 from alaindomissy/patch-1
58a9354 Update concepts.md
1b052b5 Fix a typo
4a02bc7 Test that expressionLib requirement of individual tool step overrides expressionLib of workflow.
80e2501 add path to record-job3.yaml
c9df19c Add checksum to cwl.output.json results.
e604399 Add tests that $HOME and $TMPDIR are set correctly according to spec.
a479bbd Add checksums and sizes to secondaryFiles in draft-3 tests.
dfcbfeb Merge pull request common-workflow-language#286 from common-workflow-language/test-nested-array
91bb7b4 Test case for command line generation of array-of-arrays.
5baf924 Merge pull request common-workflow-language#285 from common-workflow-language/checksum-secondary
1c02570 Merge pull request common-workflow-language#282 from common-workflow-language/test-inputbinding-dir
1835540 Add checksums to secondaryFiles and files in Directory objects.
04d9dac Add test for command line inputBinding of Directory input parameter.
8053c4b Merge pull request common-workflow-language#279 from simonovic86/patch-1
168d1a2 Update README.md
e727a46 Merge pull request common-workflow-language#277 from common-workflow-language/add-biostars-link
bfddadc fix typo
9ec9642 add biostars link
b67a4cf Merge pull request common-workflow-language#272 from StarvingMarvin/master
570ae5f Merge pull request common-workflow-language#266 from common-workflow-language/remove-empty-baseCommand
721ffdc Merge remote-tracking branch 'origin/master' into remove-empty-baseCommand
6931eeb minor userguide fixes
a296c5f Remove "baseCommand: []" from tests.

git-subtree-dir: cwltool/schemas
git-subtree-split: 2cf0896
tetron added a commit that referenced this issue Jul 23, 2017
…ng-dir

Add test for command line inputBinding of Directory input parameter.
@gijzelaerr
Copy link
Member Author

I guess this issue can be closed? it looks working now for the new release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants