haskell-actions / run-fourmolu

Fourmolu GitHub Action
https://github.com/marketplace/actions/run-fourmolu
Other
9 stars 8 forks source link

Add missing await to chmod execution #7

Closed AlexanderHarley closed 2 years ago

AlexanderHarley commented 2 years ago

When integrating fourmolu-action into one of our workflows I encountered a similar issue to #4, where an exception would cause the fourmolu detected unformatted files error to appear without any output from the fourmolu executable.

I noticed after removing the silent: true parameter from the chmod execution that the call to chmod +x seemed to be happening after the call to execute fourmolu. Logging the error also seemed to indicate that fourmolu didn't have executable mode at the time it was called:

Error: Unable to locate executable file: /opt/hostedtoolcache/fourmolu/0.7.0.1/x64/fourmolu. Please verify either the file path exists or the file can be found within a directory specified by the PATH environment variable. Also check the file mode to verify the file is executable.

This led me to notice the exec call to chmod is missing an await. As exec is an async function, adding the await seems to have resolved the issue where fourmolu is being executed before being given executable mode.

One thing I find strange is I'm not sure why this issue appeared for me but not for other workflow runs, such as in this repo as well as those of other users. I notice the await is also missing in the original ormolu-action (https://github.com/mrkkrp/ormolu-action/blob/4b9de76efeefdf7b0791bec94588d076d917ec64/index.js#L51-L53), so I would have thought more users would have experienced this issue, at least intermittently.

cdepillabout commented 2 years ago

Wow, good catch! Thanks for this.

wraithm commented 2 years ago

Wow! Thanks @AlexanderHarley. I'm surprised we haven't run into this either.

We should prob look to see if there are other execs missing awaits!

cdepillabout commented 2 years ago

@wraithm I did a quick read-through of https://github.com/fourmolu/fourmolu-action/blob/master/index.js, and as far as I can tell we have awaits in all the right places.