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

Cannot pass filenames containing spaces through path #79

Open
SamWilsn opened this issue Oct 16, 2024 · 8 comments
Open

Cannot pass filenames containing spaces through path #79

SamWilsn opened this issue Oct 16, 2024 · 8 comments
Labels
question Further information is requested

Comments

@SamWilsn
Copy link

Probably true for a bunch of other weird characters too, like \n.

@DimitriPapadopoulos
Copy link
Collaborator

DimitriPapadopoulos commented Oct 16, 2024

Through which path? Could you help me with an example?

@DimitriPapadopoulos DimitriPapadopoulos added the question Further information is requested label Oct 16, 2024
@SamWilsn
Copy link
Author

Of course! The last line of this snippet:

      - name: Run CodeSpell
        if: steps.changed-files.outputs.any-changed == 'true'
        uses: codespell-project/actions-codespell@v2
        with:
          check_filenames: true
          ignore_words_file: ./codespell-whitelist
          path: ${{ steps.changed-files.outputs.changed-files }}

@DimitriPapadopoulos
Copy link
Collaborator

Understood, thank you for the example. I must confess I don't know much about GitHub actions. I do see parameter path documented, but am not sure how it is processed. Could this be a YAML question more than a question on actions-codespell?

@SamWilsn
Copy link
Author

I've done some very surface level digging, and it looks like the problem occurs here:

res=`{ { codespell --count ${command_args} ${INPUT_PATH}; echo $? 1>&4; } 1>&5; } 4>&1`

INPUT_PATH is the value from the path input in the action, and it is passed into codespell as one-or-more arguments. Unfortunately, that means a file here is a file.md would get parsed by bash as four separate arguments: here, is, a, and file.md.

The safer way to pass an environment variable as an argument would be to enclose it in quotes:

res=`{ { codespell --count ${command_args} "${INPUT_PATH}"; echo $? 1>&4; } 1>&5; } 4>&1` 

But I don't know if that would be an acceptable solution, since using a space-delimited list has been recommended a few times.

@peternewman
Copy link
Collaborator

INPUT_PATH is the value from the path input in the action, and it is passed into codespell as one-or-more arguments. Unfortunately, that means a file here is a file.md would get parsed by bash as four separate arguments: here, is, a, and file.md.

The safer way to pass an environment variable as an argument would be to enclose it in quotes:

res=`{ { codespell --count ${command_args} "${INPUT_PATH}"; echo $? 1>&4; } 1>&5; } 4>&1` 

But I don't know if that would be an acceptable solution, since using a space-delimited list has been recommended a few times.

Yes, we shouldn't quote it within the command @SamWilsn as that would preclude all those options. As well as if people wanted to use single quotes or backslash escapes or...

To quote (if you'll pardon the pun) yourself:

The safer way to pass an environment variable as an argument would be to enclose it in quotes

So if you quote your argument(s) into actions-codespell, it should all work as intended.

Your "changed-files" step implies it could output multiple files already, so that would also break if it was quoted.

Can you not get your existing previous step to quote the arguments, or use xargs?

@SamWilsn
Copy link
Author

SamWilsn commented Oct 17, 2024

I did actually attempt to quote my arguments, but ran into a weirdness with POSIX shells. You can see this effect with:

$ export CHANGED_FILES="'first file' 'second file'"
$ cat $CHANGED_FILES
cat: "'first": No such file or directory
cat: "file'": No such file or directory
cat: "'second": No such file or directory
cat: "file'": No such file or directory

I haven't found a satisfactory way around this. In fact, the only way I've found to interpret an environment variable with quotes is with eval:

$ export CHANGED_FILES="'first file' 'second file'"
$ eval cat $CHANGED_FILES
cat: 'first file': No such file or directory
cat: 'second file': No such file or directory

@peternewman
Copy link
Collaborator

Yeah I'm not sure sorry.

I'd agree that either of these work:

cat "first file" "second file"
cat: 'first file': No such file or directory
cat: 'second file': No such file or directory
cat first\ file second\ file
cat: 'first file': No such file or directory
cat: 'second file': No such file or directory

But yes, from a quick try I can't sort a way to do it in one environment variable. I suspect it really wants to be an array of some sort.

@SamWilsn
Copy link
Author

With other actions, I've been getting by with a hack of splitting on newlines (my "changed files" action will error if it finds a filename with one), but that isn't a general solution.

What would you think about re-implementing your entrypoint in python, and taking JSON as input?

GitHub Actions has great support for JSON (toJSON / fromJSON), you could implement backwards compatible support (keep path as is, put the new functionality in a new input), and requires no new dependencies since you already use a Python base image.

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

No branches or pull requests

3 participants