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

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

Closed
ninowalker opened this issue Dec 20, 2018 · 5 comments
Closed

Comments

@ninowalker
Copy link

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
Copy link
Author

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
Copy link
Contributor

ppalaga commented Jan 4, 2019

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

@avdland
Copy link
Contributor

avdland commented Jan 10, 2019

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
Copy link
Contributor

gevegab commented Jan 15, 2019

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 :

File processFile = new File( file.getAbsolutePath() + "_" + timestamp );
boolean doFinalize = false;
try
{
doFinalize = processFile( processor, file, processFile );
}
catch ( Exception e )
{
getLog().warn( "skip failed file : " + e.getMessage()
+ ( e.getCause() == null ? "" : " Cause : " + e.getCause().getMessage() ), e );
FileState.fail.addFile( file, result );
doFinalize = false;
}
finally
{
// whatever was the result, this file is treated.
processedFiles.add( file );
if ( doFinalize )
{
finalizeFile( file, processFile );
}
else
{
FileUtil.deleteFile( processFile );
}
}

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

public static void renameFile( File file, File destination )
throws IOException
{
try
{
try
{
org.apache.commons.io.FileUtils.forceDelete( destination );
}
catch ( FileNotFoundException ex )
{
//Just do nothing
}
org.apache.commons.io.FileUtils.moveFile( file, destination );
}
catch ( IOException ex )
{
throw new IOException( String.format( "could not rename '%s' to '%s'", file, destination ) );
}
}

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
Copy link
Contributor

gevegab commented Jan 15, 2019

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

@ppalaga ppalaga added this to the Backlog milestone Feb 1, 2019
@tchemit tchemit closed this as completed in 0ae6dc7 Feb 3, 2019
tchemit added a commit that referenced this issue Feb 3, 2019
Fix #166 update-file-header changes file permissions: clears executab…
@ppalaga ppalaga modified the milestones: Backlog, 1.18 Feb 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants