Keep permisions of additionalResources on packaging [Linux] #155

Ali-RS · 2022-02-23T06:26:47Z

Hello!

I'm submitting a…

bug report
feature request
other

Short description of the issue/suggestion:
In the packageMyAppForLinux task I have added a file into additionalResources to be included in the package. The original file is marked as executable (chmod +x) but when packaged into the tarball it does not keep that permission.

Can you please help me to solve this issue?

What is the expected behavior?

I am expecting the packaged file to keep the permissions of its original file. (In my case the executable permission)

What is the current behavior?

It does not keep the original file permissions. (In my case the executable permission)

Do you have outputs, screenshots, demos or samples which demonstrate the problem or enhancement?

Here is my packageMyAppForLinux task:

The file blue-cube.desktop is the one I want to preserve its permission when packaging.

task packageMyAppForLinux(type: io.github.fvarrui.javapackager.gradle.PackageTask, dependsOn: build) {
    platform = io.github.fvarrui.javapackager.model.Platform.linux
    createTarball = true
    outputDirectory = file("$buildDir/bundles/linux-x64")
    additionalResources = javapackager.additionalResources + [file('blue-cube.desktop')]
}

Please tell us about your environment:

JavaPackager version: 1.6.3
OS version: Linux Mint 20.1 Ulyssa
JDK version: 14
Build tool:
- Maven
- Gradle

Regards

The text was updated successfully, but these errors were encountered:

fvarrui · 2022-02-23T12:08:07Z

Hi @Ali-RS!

JavaPackager is using Apache Commons FileUtils to copy additional files/folders, which doesn't copy permissions, only content:

JavaPackager/src/main/java/io/github/fvarrui/javapackager/packagers/Packager.java

Lines 191 to 233 in 1420f52

    
           	/** 
        
           	 * Copy a list of resources to a folder 
        
           	 *  
        
           	 * @param resources   List of files and folders to be copied 
        
           	 * @param destination Destination folder. All specified resources will be copied 
        
           	 *                    here 
        
           	 */ 
        
           	protected void copyAdditionalResources(List<File> resources, File destination) { 
        
           		Logger.infoIndent("Copying additional resources"); 
        
           		resources.stream().forEach(r -> { 
        
           			if (!r.exists()) { 
        
           				Logger.warn("Additional resource " + r + " doesn't exist"); 
        
           				return; 
        
           			} 
        
           			try { 
        
           				if (r.isDirectory()) { 
        
           					FileUtils.copyFolderToFolder(r, destination); 
        
           				} else if (r.isFile()) { 
        
           					FileUtils.copyFileToFolder(r, destination); 
        
           				} 
        
           			} catch (Exception e) { 
        
           				Logger.error(e.getMessage(), e); 
        
           			} 
        
           		}); 
        
           		// copy bootstrap script 
        
           		if (FileUtils.exists(getScripts().getBootstrap())) { 
        
           			String scriptExtension = getExtension(getScripts().getBootstrap().getName()); 
        
           			File scriptsFolder = new File(destination, "scripts"); 
        
           			bootstrapFile = new File(scriptsFolder, "bootstrap" + (!scriptExtension.isEmpty() ? "." + scriptExtension : "")); 
        
           			try { 
        
           				FileUtils.copyFileToFile(getScripts().getBootstrap(), bootstrapFile); 
        
           				bootstrapFile.setExecutable(true, false); 
        
           			} catch (Exception e) { 
        
           				Logger.error(e.getMessage(), e); 
        
           			} 
        
           		} 
        
           		Logger.infoUnindent("All additional resources copied!"); 
        
           	}

Then, when creating the tarball, permissions are set only for those files that JavaPackager considers executable: like the app binary, jre/bin/* (if we bundle a JRE) and bootstrap scripts:

JavaPackager/src/main/java/io/github/fvarrui/javapackager/gradle/CreateTarball.java

Lines 56 to 72 in 1420f52

    
           // if zipball is for linux platform 
        
           else if (Platform.linux.equals(platform)) { 
        
           	tarTask.from(appFolder.getParentFile(), copySpec -> { 
        
           		copySpec.include(appFolder.getName() + "/**"); 
        
           		copySpec.exclude(appFolder.getName() + "/" + executable.getName()); 
        
           		copySpec.exclude(appFolder.getName() + "/" + jreDirectoryName + "/bin/*"); 
        
           		copySpec.exclude(appFolder.getName() + "/scripts/*"); 
        
           	}); 
        
           	tarTask.from(appFolder.getParentFile(), copySpec -> { 
        
           		copySpec.include(appFolder.getName() + "/" + executable.getName()); 
        
           		copySpec.include(appFolder.getName() + "/" + jreDirectoryName + "/bin/*"); 
        
           		copySpec.include(appFolder.getName() + "/scripts/*"); 
        
           		copySpec.setFileMode(0755); 
        
           	}); 
        
           }

In the case of Gradle, this behaviour is hardcoded ... in Maven, you could make your own VTL template to create a customized tarball.

JavaPackager/src/main/resources/linux/assembly.xml.vtl

Lines 8 to 34 in 1420f52

    
           	<fileSets> 
        
           		<fileSet> 
        
           			<outputDirectory>${info.name}</outputDirectory> 
        
           			<directory>${info.outputDirectory}/${info.name}</directory> 
        
           			<excludes> 
        
           				<exclude>${info.executable.name}</exclude> 
        
           #if ($info.bundleJre)				 
        
           				<exclude>${info.jreDirectoryName}/bin/*</exclude> 
        
           				<exclude>${info.jreDirectoryName}/lib/jspawnhelper</exclude> 
        
           				<exclude>scripts/*</exclude> 
        
           #end 
        
           			</excludes> 
        
           		</fileSet> 
        
           		<fileSet> 
        
           			<outputDirectory>${info.name}</outputDirectory> 
        
           			<directory>${info.outputDirectory}/${info.name}</directory> 
        
           			<includes> 
        
           				<include>${info.executable.name}</include> 
        
           #if ($info.bundleJre)				 
        
           				<include>${info.jreDirectoryName}/bin/*</include> 
        
           				<include>${info.jreDirectoryName}/lib/jspawnhelper</include> 
        
           				<include>scripts/*</include> 
        
           #end 
        
           			</includes> 
        
           			<fileMode>0755</fileMode> 
        
           		</fileSet> 
        
           	</fileSets>

Options:

Including a new option in JavaPackager to specify which additional resources must have execution permissions (this should work on any platform).
Copying files on Linux/MacOS using an alternative method, which allows to copy the permissions of the original file (this only would work on Linux/MacOS, but not on Windows, as the only way to specify those permissions on Windows is inside the tarball).

Ali-RS · 2022-02-23T13:10:42Z

I see, thanks for explaining. Because in my case it is a single file it is easy to manually change permissions after packaging.

Copying files on Linux/MacOS using an alternative method, which allows to copy the permissions of the original file

Maybe you can use java.nio.file.Files.

Files.copy(source, target, StandardCopyOption.COPY_ATTRIBUTES)

@fvarrui should I keep this issue open?

fvarrui · 2022-02-23T13:44:54Z

I see, thanks for explaining. Because in my case it is a single file it is easy to manually change permissions after packaging.

Copying files on Linux/MacOS using an alternative method, which allows to copy the permissions of the original file

Maybe you can use java.nio.file.Files.

Files.copy(source, target, StandardCopyOption.COPY_ATTRIBUTES)

I've not tested yet, but I read about this method, and it seems not to be working on MacOS. But could be a first and easy approach.

@fvarrui should I keep this issue open?

Yes, please! I think it's an interesting question.

fvarrui · 2022-02-23T14:03:44Z

Option 2 released to issue-155 branch as JavaPackager 1.6.4-SNAPSHOT (you have to install it manually to your local repo). Now it's using java.nio.file.Files.copy(...):

JavaPackager/src/main/java/io/github/fvarrui/javapackager/utils/FileUtils.java

Lines 85 to 94 in 222549f

    
           public static void copyFileToFolder(File source, File destFolder, boolean overwrite) throws Exception { 
        
           	Logger.info("Copying file [" + source + "] to folder [" + destFolder + "]"); 
        
           	if (new File(destFolder, source.getName()).exists() && !overwrite) return; 
        
           	try { 
        
           		//copyFileToDirectory(source, destFolder); 
        
           		Files.copy(source.toPath(), new File(destFolder, source.getName()).toPath(), StandardCopyOption.COPY_ATTRIBUTES); 
        
           	} catch (IOException e) { 
        
           		throw new Exception(e.getMessage(), e); 
        
           	} 
        
           }

I've tested on Linux and Windows, and it seems to be working fine (remember that when packaging from Windows to Linux/MacOS, additional resources will not have the expected permissions). Try it and give me some feedback, please!

Ali-RS · 2022-02-23T14:09:20Z

Cool, will try it soon. Thank you so much!

Ali-RS · 2022-02-23T14:52:08Z

Tested it on Linux and it works like charm! Thanks @fvarrui

fvarrui · 2022-02-23T17:01:52Z

Branch issue-155 merged into devel.

fvarrui · 2022-03-03T09:05:58Z

JavaPackager 1.6.4 released to Maven Central.

Ali-RS · 2022-03-03T09:08:48Z

Thanks!

maths22 · 2022-03-22T01:46:17Z

Sadly Files.copy(source, target, StandardCopyOption.COPY_ATTRIBUTES) seems to not preserve permissions on macos. I'm not sure what the best solution is (maybe option 1, but that feels kinda gross?)

fvarrui · 2022-03-22T09:46:19Z

😞

Ali-RS · 2022-03-22T11:13:22Z

@maths22 what java version are you using?

l found a similar bug report at https://bugs.openjdk.java.net/browse/JDK-8204848 but seems it is already resolved.

maths22 · 2022-03-22T12:01:42Z

I'm on java 17. It doesn't throw an error; it just also doesn't preserve permissions.

fvarrui · 2022-03-22T12:32:45Z

A quick fix would be to check if the original file has execute permissions, set them to the copied file:

public static void copyFileToFolder(File source, File destFolder, boolean overwrite) throws Exception {
	Logger.info("Copying file [" + source + "] to folder [" + destFolder + "]");
	if (new File(destFolder, source.getName()).exists() && !overwrite) return;
	try {
		//copyFileToDirectory(source, destFolder);
		File destFile = new File(destFolder, source.getName());
		Files.copy(source.toPath(), destFile.toPath(), StandardCopyOption.COPY_ATTRIBUTES);
		if (source.canExecute()) {
			destFile.setExecutable(true, false); // set execution permissions for all (ugo+x)
		}
	} catch (IOException e) {
		throw new Exception(e.getMessage(), e);
	}
}

I've applied this change for JavaPackager 1.6.6.-SNAPSHOT and published in issue-155 branch. Please, could you test it and give some feedback?

Thanks!

maths22 · 2022-03-22T13:26:24Z

That didn't work for me, probably because that only checks the permissions of the parent file, and if the parent file is a directory it's not going to check and set permissions on the child files

fvarrui · 2022-03-22T20:09:01Z

Mmmmh ... yeah! you are right ... this change only works when an additionalResource is a file, but not when is a directory. I have to improve FileUtils.copyFolderToFolder(), used when additional resources are directories.

fvarrui · 2022-03-23T09:24:34Z

This is what JavaPackager does when copying additional resources:

	protected void copyAdditionalResources(List<File> resources, File destination) {

                [...]

		resources.stream().forEach(r -> {
			if (!r.exists()) {
				Logger.warn("Additional resource " + r + " doesn't exist");
				return;
			}
			try {
				if (r.isDirectory()) {
					FileUtils.copyFolderToFolder(r, destination);
				} else if (r.isFile()) {
					FileUtils.copyFileToFolder(r, destination);
				}
			} catch (Exception e) {
				Logger.error(e.getMessage(), e);
			}
		});

                [...]

	}

Whay if we use the copy native command on MacOS and Linux?

	public static void copyFileToFolder(File source, File destFolder, boolean overwrite) throws Exception {
		Logger.info("Copying file [" + source + "] to folder [" + destFolder + "]");
		if (new File(destFolder, source.getName()).exists() && !overwrite) return;
		try {
			File destFile = new File(destFolder, source.getName());
			if (Platform.windows.isCurrentPlatform())
				Files.copy(source.toPath(), destFile.toPath(), StandardCopyOption.COPY_ATTRIBUTES);
			else {
				CommandUtils.execute("cp", source, destFile);
			}			
		} catch (IOException e) {
			throw new Exception(e.getMessage(), e);
		}
	}
	
	public static void copyFolderToFolder(File from, File to) throws Exception {
		Logger.info("Copying folder [" + from + "] to folder [" + to + "]");
		if (!from.isDirectory()) throw new Exception("Source folder " + from + " is not a directory");
		try {
			if (Platform.windows.isCurrentPlatform())
				copyDirectoryToDirectory(from, to);
			else {
				CommandUtils.execute("cp", "-R", from, to);
			}
		} catch (IOException e) {
			throw new Exception(e.getMessage(), e);
		}
	}

so, it would keep the permissions of the original files

Ali-RS · 2022-03-23T10:29:50Z

Whay if we use the copy native command on MacOS and Linux?

Yes, sounds good to me.

fvarrui · 2022-03-23T22:36:03Z

Changes published in issue-155 branch.

fvarrui · 2022-04-05T12:01:00Z

JavaPackager v1.6.6 released to Maven Central

Ali-RS closed this as completed Feb 23, 2022

fvarrui reopened this Mar 22, 2022

fvarrui added bug Something isn't working feedback Waiting for feedback fixed Issue fixed and release pending labels Mar 25, 2022

fvarrui closed this as completed Apr 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Keep permisions of additionalResources on packaging [Linux] #155

Keep permisions of additionalResources on packaging [Linux] #155

Ali-RS commented Feb 23, 2022 •

edited

Loading

fvarrui commented Feb 23, 2022 •

edited

Loading

Ali-RS commented Feb 23, 2022 •

edited

Loading

fvarrui commented Feb 23, 2022

fvarrui commented Feb 23, 2022

Ali-RS commented Feb 23, 2022

Ali-RS commented Feb 23, 2022 •

edited

Loading

fvarrui commented Feb 23, 2022

fvarrui commented Mar 3, 2022

Ali-RS commented Mar 3, 2022

maths22 commented Mar 22, 2022

fvarrui commented Mar 22, 2022

Ali-RS commented Mar 22, 2022 •

edited

Loading

maths22 commented Mar 22, 2022

fvarrui commented Mar 22, 2022

maths22 commented Mar 22, 2022

fvarrui commented Mar 22, 2022

fvarrui commented Mar 23, 2022

Ali-RS commented Mar 23, 2022

fvarrui commented Mar 23, 2022

fvarrui commented Apr 5, 2022

Keep permisions of additionalResources on packaging [Linux] #155

Keep permisions of additionalResources on packaging [Linux] #155

Comments

Ali-RS commented Feb 23, 2022 • edited Loading

fvarrui commented Feb 23, 2022 • edited Loading

Ali-RS commented Feb 23, 2022 • edited Loading

fvarrui commented Feb 23, 2022

fvarrui commented Feb 23, 2022

Ali-RS commented Feb 23, 2022

Ali-RS commented Feb 23, 2022 • edited Loading

fvarrui commented Feb 23, 2022

fvarrui commented Mar 3, 2022

Ali-RS commented Mar 3, 2022

maths22 commented Mar 22, 2022

fvarrui commented Mar 22, 2022

Ali-RS commented Mar 22, 2022 • edited Loading

maths22 commented Mar 22, 2022

fvarrui commented Mar 22, 2022

maths22 commented Mar 22, 2022

fvarrui commented Mar 22, 2022

fvarrui commented Mar 23, 2022

Ali-RS commented Mar 23, 2022

fvarrui commented Mar 23, 2022

fvarrui commented Apr 5, 2022

Ali-RS commented Feb 23, 2022 •

edited

Loading

fvarrui commented Feb 23, 2022 •

edited

Loading

Ali-RS commented Feb 23, 2022 •

edited

Loading

Ali-RS commented Feb 23, 2022 •

edited

Loading

Ali-RS commented Mar 22, 2022 •

edited

Loading