tj-actions/changed-files

[BUG] Spaces in file names are not handled correctly

vertxxyz opened this issue · 8 comments

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug?

If you have spaces in your file names and you provide a custom separator (eg. separator: ",") all spaces are replaced by the separator and the results sorted.

To Reproduce

  1. On a repo containing the paths:
  • embeds/general 412874536418279426/read-me-1 899830563651854336.json
  • Test 1/Test 1.json
  1. Running
- id: json_changes
  uses: tj-actions/changed-files@v10.1
  with:
      files: |
        .json$
      separator: ","

- name: List modified files
  run: |
    echo ${{ steps.json_changes.outputs.all_modified_files }}
  1. Sees the result of:
    1.json,1/Test,412874536418279426/read-me-1,899830563651854336.json,Test,embeds/general

What OS are you seeing the problem on?

ubuntu-latest or ubuntu-20.04

Expected behavior?

I expect:
embeds/general 412874536418279426/read-me-1 899830563651854336.json,Test 2/Test 1.json

Relevant log output

No response

Anything else?

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct

Thanks for reporting this issue.

I'll strongly discourage use of spaces in the file names as leads to more errors down the line.

bash loops by default rely on spaces as the Internal field separator and might result in bugs like

for file in embeds/general 412874536418279426/read-me-1 899830563651854336.json Test 1/Test 1.json
do
   echo "$file"
done

Which prints out

embeds/general
412874536418279426/read-me-1
899830563651854336.json
Test
1/Test
1.json

But we should be able to support this use case.

See: https://askubuntu.com/a/344418/385863

I appreciate it, thanks.
I'd note that the test file in this very project also contains a space! (test/test new.txt)

Correct, It's a known limitation and that file was created for testing and future support of file names with spaces.

@vertxxyz This should now be available in the latest release v11

Hi

Is this patch in 11.2? I am getting an error here trying to run some tests against a checkin with 1 or 2 files that each contain spaces:

image

   steps:
    - uses: actions/checkout@master
      with:
        fetch-depth: 0 # or 2?
#        ref: nbval-test-tags
    - id: changed-files
      uses: tj-actions/changed-files@v11.2
      with:
          files: |
            .ipynb$

    - name: test changed files
      run: |
        for file in ${{ steps.changed-files.outputs.all_modified_files }}; do
            py.test  --nbval "$file" || continue
          done
        done
      continue-on-error: true

If I use for file in "${{ steps.changed-files.outputs.all_modified_files }}" then all the files are quoted as one filename. Do I need to use another separator style and then map onto an array?

(Apols for the naive question, I am a bit out of my depth...!)

@psychemedia No worries, you can use IFS=$',' and set the separator input to ,

- id: changed-files
      uses: tj-actions/changed-files@v11.2
      with:
          separator: ","
          files: |
            .ipynb$

For example, you'll need to use

...
      - name: List all modified files
         run: |
           IFS=$',' read -a MODIFIED_FILES_ARRAY <<< "${{ steps.changed-files-comma.outputs.modified_files }}"
           for file in "${MODIFIED_FILES_ARRAY[@]}"; do
             echo $file
           done
         shell:
           bash

See this PR for more information.

It requires discipline but shell scripting does support whitespace in filenames just fine:
https://mywiki.wooledge.org/Quotes
https://mywiki.wooledge.org/ParsingLs
https://mywiki.wooledge.org/BashGuide/Arrays

Shellcheck catches 99.99% missing quotes: https://github.com/koalaman/shellcheck/wiki/SC2046

bash loops by default rely on spaces as the Internal field separator and might result in bugs like

Only loops "buggy by default". Fix:

- for file in embeds/general 412874536418279426/read-me-1 899830563651854336.json Test 1/Test 1.json
+ for file in embeds/general 412874536418279426/read-me-1 899830563651854336.json 'Test 1/Test 1.json'