search

HubSpot / prettier-maven-plugin

Apache License 2.0
116 stars 23 forks source link

Cannot generate diff under windows #10

Closed MSaguer closed 4 years ago

MSaguer commented 4 years ago

Prettier-maven-plugin version 0.7.0

# Environment
# Maven --version
Apache Maven 3.3.9 (bb52d8502b132ec0a5a3f4c09453c07478323dc5; 2015-11-10T17:41:47+01:00)
Maven home: D:\App\Maven
Java version: 1.8.0_45, vendor: Oracle Corporation
Java home: D:\App\Java\jdk1.8.0_45_x64\jre
Default locale: en_US, platform encoding: Cp1252
OS name: "windows server 2012 r2", version: "6.3", arch: "amd64", family: "dos"

# git --version
git version 2.14.1.windows.1

When generateDiff option is set to true, the generate diff crashes with the following error message

[ERROR] Failed to execute goal com.hubspot.maven.plugins:prettier-maven-plugin:0.7:check (default-cli) on project org.project: Error trying to create diff with prettier-java: Can
not run program "/bin/sh" (in directory "D:\temp\project"): CreateProcess error=2, The system cannot find the file specified -> [Help 1]

Using git bash window, sh is defined under "/usr/bin/sh instead of /bin/sh as defined in DefaultDiffGenerator.

What about adding an optional maven option to define the sh path? I could do it if you want.

jhaber commented 4 years ago

Sorry about that, I was aware that diff generation wouldn't work on Windows and tried to document this: https://github.com/HubSpot/prettier-maven-plugin#configuration

We currently pipe the prettier output to the diff command and redirect the output to a file, so my hunch was that fixing the path wouldn't actually fix the issue on Windows where you don't have a diff command at all. I'm not sure the best way to support this on Windows, do you have any thoughts?

MSaguer commented 4 years ago

Oh! I read this page many times and I missed the relevant part.

Under git bash window, both sh and diff commands are available under /usr/bin. That's why I think that replacing "/bin/sh" by "sh" would be enough for me. But removing the absolute path would force the user to add this folder in its path and it may introduce a regression.

Here is a possible solution that let the user configure its env as he wants

MSaguer commented 4 years ago

I proposed a simple fix in the pull request #12 Let me know if it breaks your plugin undex unix

jhaber commented 4 years ago

We run this plugin off of master within HubSpot so #11 and #12 have already been picked up and are working fine within our environment. Can you confirm that things are now working for you on Windows?

MSaguer commented 4 years ago

All works for me on Windows.

jhaber commented 4 years ago

Great, are you alright running from master or would you prefer I make a new release?

MSaguer commented 4 years ago

It would be great if you could release a new version.

jhaber commented 4 years ago

Sorry for the long delay, just pushed the new version to Maven Central: https://github.com/HubSpot/prettier-maven-plugin/releases/tag/prettier-maven-plugin-0.8