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

pathspec is not respected because of bash expansion #239

Closed
alexgarel opened this issue Aug 30, 2022 · 7 comments
Closed

pathspec is not respected because of bash expansion #239

alexgarel opened this issue Aug 30, 2022 · 7 comments
Labels
bug Something isn't working

Comments

@alexgarel
Copy link

Version of the Action
v4.14.1

Describe the bug
I my workflow action,
I am converting jupyter notebook to markdown.
I want to commit all "md" files changed by previous steps in the repository
If I use file_pattern: "*.md", according to pathspec, it should commit every ".md" file it the repository but it does not work.

This is due to the fact that I have a Threshold.md file at the root of my repository, so *.md is expanded to Threshold.md before being send to git command.
I can reproduce it locally, if I use git status *.md, I only get status for Threshold.md,
while if I use git status "*.md", I get status for all ".md" file in the repository.

I tried to use filepattern: "'*.md'" but it did not work. I think quotes are lost during evaluation to pass it in ENV variable ($INPUT_FILE_PATTERN)

To Reproduce

  1. Create git repo with a structure:
  • a.md
  • subdir/b.md
  1. Add a step that modifies those files by eg. appending some value.
  2. Use the action to commit all md file with file_pattern: "*.md".
  3. See that file are not commited.

Expected behavior
I think the script should split $INPUT_FILE_PATTERN on spaces to get the different patterns in an array variable, then use quoting of arguments.

read -a INPUT_FILES <<< "$INPUT_FILE_PATTERN"
... (later on) ...
git status "${INPUT_FILES[@]}"
...
git add "${INPUT_FILES[@]}"

Used Workflow

See link above

@stefanzweifel
Copy link
Owner

Thanks for your feedback and idea for a solution to this problem.

You're correct that quotes are lost in translation between the YAML, the Bash script and Git.
There's a previous discussion about this here: #196 (review)

In February I've opened a PR which tries to fix this; but I'm not sure why I haven't merged or worked more on this.

Will take a closer look on this in the upcoming days. It's time we close this problem for good.

@stefanzweifel stefanzweifel added the bug Something isn't working label Sep 1, 2022
@alexgarel
Copy link
Author

Ok, good to know, I made suggestions on your PR :-)

@stefanzweifel
Copy link
Owner

@alexgarel Oh man. Thank you so much! ♥️
My bash knowledge is still so limited … even after all these years.
Will review your comments in detail when I get the time.

@alexgarel
Copy link
Author

Gentle ping

@stefanzweifel
Copy link
Owner

stefanzweifel commented Sep 14, 2022

@alexgarel It's on my todo list. Currently a lot to do at my day-job and didn't had the energy to invest the time into this issue.
I plan to tackle this tonight.

What I can alrady tell is that <<< didn't seem to work. More later today.

@stefanzweifel
Copy link
Owner

@alexgarel #205 has been updated with your suggestion. Could you give this version a try by updating your workflow to use:

- uses: stefanzweifel/git-auto-commit-action@refactor/expand-file-pattern

If this change resolved your issues, I'm going to merge the PR and tag a new version.

@stefanzweifel
Copy link
Owner

I've tested the changes made in #205 extensivly in a test project of mine. file_pattern should now work more reliably.

The original issue you described here was primarily solved by adding disable_globbing: true to the Workflow. I've updated the README with an example, when someone might run into this:
https://github.com/stefanzweifel/git-auto-commit-action#custom-file_pattern-changed-files-but-seeing-working-tree-clean-nothing-to-commit-in-the-logs
(In a future v5 I would like to permanently fix this issue without resorting to a disable_globbing-option.)

Thanks for helping solving this issue!

alexgarel added a commit to openfoodfacts/off-category-classification that referenced this issue Oct 9, 2022
alexgarel added a commit to openfoodfacts/off-category-classification that referenced this issue Oct 9, 2022
alexgarel added a commit to openfoodfacts/off-category-classification that referenced this issue Oct 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants