mojohaus / license-maven-plugin

Maven plugin to download and collect license files from project dependencies.
https://www.mojohaus.org/license-maven-plugin/
GNU Lesser General Public License v3.0
106 stars 126 forks source link

update-file-header changes file permissions: clears executable bit #166

Closed ninowalker closed 5 years ago

ninowalker commented 5 years ago

I am running license:update-file-header; one of the files is a .sh with mode 755. After running, even though there is no change to the contents, the permissions are changed:

+ mvn ... license:update-file-header
...
[INFO] Scan 14 files header done in 44.889ms.
[INFO] 
 * update header on 14 files.

+ git status
Changes not staged for commit:
...
    modified:   bin/run.sh

+ git diff
diff --git a/bin/run.sh b/bin/run.sh
old mode 100755
new mode 100644

Note the permissions changed, but the contents did not! (Because the file is already correct)

ninowalker commented 5 years ago

For those with automation pipelines, this workflow is a workaround:

mvn ... license:update-file-header
git -c core.fileMode=false diff
git -c core.fileMode=false commit -m "update license headers" -a
git reset --hard HEAD
ppalaga commented 5 years ago

An integration test reproducing the issue in https://github.com/mojohaus/license-maven-plugin/tree/master/src/it would be highly welcome.

avdland commented 5 years ago

I tried to make an integration test but I cannot... Somehow, when the invoker prebuild script runs, it detects that the file is not executable while it should! The file is executable, but Java/groovy is unable to determine that it is.

Furthermore, I only noticed that the file permission is changed only when the contents is changed (a new header is applied).

gevegab commented 5 years ago

As far as I can understand the code, the UpdateFileHeaderMojo works, by making a copy of the original file, adding the header to the copy, and finally deleting the original file and renaming the copy :

https://github.com/mojohaus/license-maven-plugin/blob/e427a5c08f15f6ad7e2e7cf83268ecda19045724/src/main/java/org/codehaus/mojo/license/AbstractFileHeaderMojo.java#L737-L764

So it is not surprising that file permissions get lost in the processing.

After some debugging I think the fix has to be done in this method

https://github.com/mojohaus/license-maven-plugin/blob/e427a5c08f15f6ad7e2e7cf83268ecda19045724/src/main/java/org/codehaus/mojo/license/utils/FileUtil.java#L156-L176

I am not familiar with manipulating file attributes in Java, but after some stackoverflow search I think you could use

java.nio.file.Files.getPosixFilePermissions and java.nio.file.Files.setPosixFilePermissions to copy the permissions.

I develop in Windows and the combination git+windows makes painful for me to provide a test, but I hope this helps

gevegab commented 5 years ago

I just tested java.nio.file.Files.getPosixFilePermissions/setPosixFilePermissions, and it doesn't work on Windows (throws UnsupportedOperation). So my proposed fix will not work in a portable way.

Perhaps the solution could be just to override the content of the original file (without deleting it, so that it keeps the permissions).

Starting from JDK1.7 you can use the convenient method java.nio.file.Files.copy with the option StandardCopyOption.REPLACE_EXISTING