addnab / docker-run-action

MIT License
209 stars 93 forks source link

Remove temporary "semicolon_delimited_script" #17

Closed kohlerdominik closed 3 years ago

kohlerdominik commented 3 years ago

When using this action, the directory will be left with a "semicolon_delimited_script" file.

In my case, this script was then built to a container and distributed to the world, just because it wasn't deleted after usage.

When using use bash Shell Parameter Expansion, we do not need to create this file at all.

addnab commented 3 years ago

Looks good to me although I might have to add a multiline test to make sure it's being delimited with a semicolon right.

Because this is a docker run command, I don't know how it's possible for this file to end up in your built image since the image built before this file was created. Because it's sharing the same volume, I suspect you must be building it again in the same volume after the docker run action. Can you confirm?

kohlerdominik commented 3 years ago

Hi

Yes, that's correct, I have to mount the workdir to run container actions, like:

uses: addnab/docker-run-action@v3
with:
  image: eu.gcr.io/private-image
  options: -v ${{ github.workspace }}:/home/node/app
  run: |
    npm ci
    npm run production

I tested multiline manually (as my script, in fact, uses multi-line).

addnab commented 3 years ago

Thanks for fixing this.

addnab commented 3 years ago

I'll release this as a patch on v3 on Friday after testing it.

heyman commented 2 years ago

Hi!

We're using docker-run-action to run integration tests for https://github.com/heyman/postgresql-backup.

I believe this PR broke our tests where we run commands with multi line SQL queries passed in as arguments.

Here is an example that doesn't work with the main branch: https://github.com/heyman/postgresql-backup/blob/1d38f991cd4bd4d2b7eb2043ac062c9354c53502/.github/workflows/test.yml#L39-L52

I was able to fix the tests by switching to the v3 tag instead of the main branch.

kohlerdominik commented 2 years ago

Hi @heyman

Yes, this change might break some special cases of multiline-script, one might be yours. Your issue is most likely, that you use indents for formatting reason. However, indents in YAML multiline-folding (>) AFAIK does also force a newline.

Other than that, I have to say that I also had a lot of issues with this Action for my purposes. First I wanted to fix them in this repo, but i found that it's very hard to do in a Docker Container Action. I therefore wrote a new Javascript based Action published as kohlerdominik/docker-run-action.