-
Notifications
You must be signed in to change notification settings - Fork 71
[MASSEMBLY-791] overrideUmask option to ensure permissions in packaged archive match … #148
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
40777 dir-a/ | ||
100666 dir-a/a1.txt | ||
100666 dir-a/a2.txt | ||
40755 dir-b/ | ||
100644 dir-b/b.txt | ||
40777 dir-c/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<!-- | ||
Licensed to the Apache Software Foundation (ASF) under one | ||
or more contributor license agreements. See the NOTICE file | ||
distributed with this work for additional information | ||
regarding copyright ownership. The ASF licenses this file | ||
to you under the Apache License, Version 2.0 (the | ||
"License"); you may not use this file except in compliance | ||
with the License. You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, | ||
software distributed under the License is distributed on an | ||
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
KIND, either express or implied. See the License for the | ||
specific language governing permissions and limitations | ||
under the License. | ||
--> | ||
<project xmlns="http://maven.apache.org/POM/4.0.0" | ||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
<modelVersion>4.0.0</modelVersion> | ||
<parent> | ||
<groupId>org.apache.maven.plugin.assembly.test</groupId> | ||
<artifactId>it-project-parent</artifactId> | ||
<version>1</version> | ||
</parent> | ||
|
||
<groupId>org.apache.maven.its</groupId> | ||
<artifactId>massembly-791</artifactId> | ||
<packaging>pom</packaging> | ||
<version>1.0</version> | ||
|
||
<properties> | ||
<project.build.outputTimestamp>2023-06-07T10:18:31Z</project.build.outputTimestamp> | ||
</properties> | ||
|
||
<build> | ||
<plugins> | ||
<plugin> | ||
<groupId>org.apache.maven.plugins</groupId> | ||
<artifactId>maven-assembly-plugin</artifactId> | ||
<executions> | ||
<execution> | ||
<id>make-assembly</id> | ||
<phase>package</phase> | ||
<goals> | ||
<goal>single</goal> | ||
</goals> | ||
<configuration> | ||
<!-- Follow permissions defined in assembly descriptor --> | ||
<umask>0</umask> | ||
<descriptors> | ||
<descriptor>src/assembly/src.xml</descriptor> | ||
</descriptors> | ||
</configuration> | ||
</execution> | ||
</executions> | ||
</plugin> | ||
</plugins> | ||
</build> | ||
|
||
</project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
<?xml version='1.0'?> | ||
<!-- | ||
Licensed to the Apache Software Foundation (ASF) under one | ||
or more contributor license agreements. See the NOTICE file | ||
distributed with this work for additional information | ||
regarding copyright ownership. The ASF licenses this file | ||
to you under the Apache License, Version 2.0 (the | ||
"License"); you may not use this file except in compliance | ||
with the License. You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, | ||
software distributed under the License is distributed on an | ||
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
KIND, either express or implied. See the License for the | ||
specific language governing permissions and limitations | ||
under the License. | ||
--> | ||
<assembly xmlns="http://maven.apache.org/plugins/maven-assembly-plugin/assembly/1.1.0" | ||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
xsi:schemaLocation="http://maven.apache.org/plugins/maven-assembly-plugin/assembly/1.1.0 http://maven.apache.org/xsd/assembly-1.1.0.xsd"> | ||
<id>src</id> | ||
<formats> | ||
<format>tar</format> | ||
</formats> | ||
<includeBaseDirectory>false</includeBaseDirectory> | ||
<fileSets> | ||
<fileSet> | ||
<outputDirectory/> | ||
<directory>src/main/resources</directory> | ||
<includes> | ||
<include>dir-a/</include> | ||
</includes> | ||
<fileMode>0666</fileMode> | ||
<directoryMode>0777</directoryMode> | ||
</fileSet> | ||
<fileSet> | ||
<outputDirectory/> | ||
<directory>src/main/resources</directory> | ||
<includes> | ||
<include>dir-b/</include> | ||
</includes> | ||
<fileMode>0644</fileMode> | ||
<directoryMode>0755</directoryMode> | ||
</fileSet> | ||
<fileSet> | ||
<outputDirectory/> | ||
<directory>src/main/resources</directory> | ||
<includes> | ||
<include>dir-c</include> | ||
</includes> | ||
<directoryMode>0777</directoryMode> | ||
</fileSet> | ||
</fileSets> | ||
</assembly> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
# Licensed to the Apache Software Foundation (ASF) under one | ||
# or more contributor license agreements. See the NOTICE file | ||
# distributed with this work for additional information | ||
# regarding copyright ownership. The ASF licenses this file | ||
# to you under the Apache License, Version 2.0 (the | ||
# "License"); you may not use this file except in compliance | ||
# with the License. You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, | ||
# software distributed under the License is distributed on an | ||
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
# KIND, either express or implied. See the License for the | ||
# specific language governing permissions and limitations | ||
# under the License. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
# Licensed to the Apache Software Foundation (ASF) under one | ||
# or more contributor license agreements. See the NOTICE file | ||
# distributed with this work for additional information | ||
# regarding copyright ownership. The ASF licenses this file | ||
# to you under the Apache License, Version 2.0 (the | ||
# "License"); you may not use this file except in compliance | ||
# with the License. You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, | ||
# software distributed under the License is distributed on an | ||
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
# KIND, either express or implied. See the License for the | ||
# specific language governing permissions and limitations | ||
# under the License. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
# Licensed to the Apache Software Foundation (ASF) under one | ||
# or more contributor license agreements. See the NOTICE file | ||
# distributed with this work for additional information | ||
# regarding copyright ownership. The ASF licenses this file | ||
# to you under the Apache License, Version 2.0 (the | ||
# "License"); you may not use this file except in compliance | ||
# with the License. You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, | ||
# software distributed under the License is distributed on an | ||
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
# KIND, either express or implied. See the License for the | ||
# specific language governing permissions and limitations | ||
# under the License. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
# Workaround to keep empty directory under source control |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one | ||
* or more contributor license agreements. See the NOTICE file | ||
* distributed with this work for additional information | ||
* regarding copyright ownership. The ASF licenses this file | ||
* to you under the Apache License, Version 2.0 (the | ||
* "License"); you may not use this file except in compliance | ||
* with the License. You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
import org.apache.commons.compress.archivers.tar.TarArchiveEntry | ||
import org.apache.commons.compress.archivers.tar.TarFile | ||
|
||
List<String> expectedEntries = new File( basedir, "expected-entries.txt" ).readLines() | ||
|
||
File assembly = new File( new File( basedir, "target" ), "massembly-791-1.0-src.tar" ) | ||
|
||
assert assembly.exists() | ||
|
||
List<String> effectiveEntries = new ArrayList<>() | ||
try ( TarFile tarFile = new TarFile( assembly ) ) { | ||
for ( TarArchiveEntry entry : tarFile.getEntries() ) | ||
{ | ||
effectiveEntries.add( String.format( "%o %s", entry.getMode(), entry.getName() ) ) | ||
} | ||
} | ||
|
||
assert expectedEntries == effectiveEntries |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -223,4 +223,12 @@ public interface AssemblerConfigurationSource { | |
* @return Override group name. | ||
*/ | ||
String getOverrideGroupName(); | ||
|
||
/** | ||
* @return mask which is applied to permissions of files/directories before they are put into the assembly. | ||
* If {@code null} then the mask is not explicitly configured and remains implementation-specific. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it really implementation specific? Not just that there simply isn't a mask? How many implementations are there? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it is. It is implementation-specific, because it depends on Plexus Archiver internals. Refer to that comment for details, please. |
||
*/ | ||
default Integer getUmask() { | ||
return null; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current implementation of Plexus Archiver (used by Maven Assembly Plugin) -
org.codehaus.plexus.archiver.AbstractArchiver
class - applies its default umask (which is022
) only whenconfigureReproducibleBuild
method is called. Current version of Maven Assembly Plugin invokes that method only whenproject.build.outputTimestamp
maven property is specified. It means that0
as value ofoverrideUmask
configuration option changes behavior of Maven Assembly Plugin only whenproject.build.outputTimestamp
maven property is specified. It's sad (complicated for the users of Maven Assembly Plugin) behavior, but it's an issue of Plexus Archiver, which should not apply its default umask to file/directory permissions for the case when both conditions are met:fileMode
/directoryMode
are defined forfileSet
(as far as I understand, bothfileMode
anddirectoryMode
options offileSet
have default values -0644
and0755
respectively - so it means that this condition is always true).tar
,zip
andjar
) supports configuration of file/directory permissions independent of OS.This is the reason
overrideUmask
configuration option of Maven Assembly Plugin is introduced:0
asoverrideUmask
configuration option to explicitly override umask which is sometimes (see above) implicitly used by Plexus Archiver.dir
- supports setting same permissions as defined in assembly descriptor or source file/directory has in an OS independent way).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm losing the plot here, but it sounds like maybe this is something that should be fixed in plexus-archvier instead? If a fix there could avoid adding extra complexity and API here that would be preferable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overrideUmask
in Maven Assembly Plugin, because:overrideUmask
configuration option in Maven Assembly Plugin for flexibility, i.e. it's not just quick & dirty workaround.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional public API is too heavyweight a solution for a bug that can be fixed at the source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maven users use Assembly plugin and not Plexus Archiver, so for the users of Maven it is a bug of Assembly plugin introduced in 3.6.0 version (by migration to the new version of Plexus Archiver). We can provide a quick workaround. Changing Plexus Archiver is too complex. It can require Plexus Archiver API change and change of Assembly plugin. Unfortunately, I won’t be able to help with this kind of changes (I am even not the author of this pull request).
Assembly plugin already has
override*
configuration options, which are close tooverrideUmask
configuration option (which are the same sort workaround). IMHO, it’s a trade-off which Maven maintainers decide if they agree or not. I tried my best to provide all details for making informed decision :)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not agree with new public API as a quick workaround. If the bug was introduced by a new version of plexus archiver, then revert to the old version as a workaround until plexus archiver is fixed.
New public API needs to be justified on its own merits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure "override" means the same thing in both cases. If I'm following this, in the other cases the override replaces the existing GIDs, UIDs, etc. set on the files being added to the assembly. In this case it's not doing that. Rather it's changing the behavior of the plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maven Assembly Plugin 3.6.0 is not compatible with older version of Plexus Archiver (that was the first thing I tried to workaround this bug), so there is no way to "revert to old version" of Plexus Archiver when using Maven Assembly Plugin 3.6.0. Instead my team has to stay on 3.5.0 version of Maven Assembly Plugin (which cases some "false" warnings when used with Maven 3.9.x).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant the following Assembly plugin configuration options:
overrideUid
overrideUserName
overrideGid
overrideGroupName
are similar to this pull request, because instead of setting these values in Assembly plugin configuration, they had to be a part of respective sections of assembly descriptor - just like
fileMode
anddirectoryMode
are part offileSet
section of assembly descriptor. Unfortunately, that change was more complex and required changes in API of Plexus Archiver (which I was working on). It seems that maintainers of Assembly plugin decided to chose simpler (but less generic / agile) path, which is the same as this pull request suggests.