Skip to content

Support the Info-ZIP Unicode Path Extra Field. #41

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
May 26, 2016
Merged

Support the Info-ZIP Unicode Path Extra Field. #41

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a080e0a
Updated to always add the Info-ZIP Unicode Path Extra Field when crea…
ChristianSchulte May 13, 2016
00e12b8
Updated to correct test cases failing on windows.
ChristianSchulte May 16, 2016
2ecffa3
Updated to upgrade 'plexus-utils' to latest patch release.
ChristianSchulte May 16, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-utils</artifactId>
<version>3.0.23</version>
<version>3.0.24</version>
</dependency>
<dependency>
<groupId>org.codehaus.plexus</groupId>
Expand Down
38 changes: 23 additions & 15 deletions src/main/java/org/codehaus/plexus/archiver/jar/JdkManifestFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* limitations under the License.
*/

import java.io.InputStream;
import java.io.IOException;
import java.util.Map;
import java.util.Properties;
import java.util.jar.Attributes;
Expand All @@ -33,25 +33,33 @@ class JdkManifestFactory
public static java.util.jar.Manifest getDefaultManifest()
throws ArchiverException
{
final java.util.jar.Manifest defaultManifest = new java.util.jar.Manifest();
defaultManifest.getMainAttributes().putValue( "Manifest-Version", "1.0" );
try
{
final java.util.jar.Manifest defaultManifest = new java.util.jar.Manifest();
defaultManifest.getMainAttributes().putValue( "Manifest-Version", "1.0" );

String createdBy = "Plexus Archiver";
String createdBy = "Plexus Archiver";

InputStream inputStream = JdkManifestFactory.class.getResourceAsStream( "/META-INF/"
+ "maven/org.codehaus.plexus/plexus-archiver/pom.properties" );
Properties properties = PropertyUtils.loadProperties( inputStream );
if ( properties != null )
{
String plexusArchiverVersion = properties.getProperty( "version" );
if ( plexusArchiverVersion != null )
final Properties properties = PropertyUtils.loadProperties( JdkManifestFactory.class.getResourceAsStream(
"/META-INF/maven/org.codehaus.plexus/plexus-archiver/pom.properties" ) );

if ( properties != null )
{
createdBy += " " + plexusArchiverVersion;
String plexusArchiverVersion = properties.getProperty( "version" );
if ( plexusArchiverVersion != null )
{
createdBy += " " + plexusArchiverVersion;
}
}
}
defaultManifest.getMainAttributes().putValue( "Created-By", createdBy );

return defaultManifest;
defaultManifest.getMainAttributes().putValue( "Created-By", createdBy );

return defaultManifest;
}
catch ( final IOException e )
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What purpose serves this try catch block? No one throws an IOException. Not the PropertyUtils.

Copy link
Member

@michael-o michael-o May 26, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I consider the swallow here was intentional. The local was actually borrowed from other spots where the version is omitted of the pom.properties could not be read. This needs a rewrite now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am 05/27/16 um 00:38 schrieb Michael Osipov:

In
src/main/java/org/codehaus/plexus/archiver/jar/JdkManifestFactory.java
#41 (comment):

  •    return defaultManifest;

  •        defaultManifest.getMainAttributes().putValue( "Created-By", createdBy );

  •        return defaultManifest;

  •    }

  •    catch ( final IOException e )

  •    {

What purpose serves this try catch block? No one throws an
|IOException|. Not the |PropertyUtils|.

PropertyUtils started throwing the exception with the latest patch
release of 'plexus-utils'.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am 05/27/16 um 00:57 schrieb Michael Osipov:

In
src/main/java/org/codehaus/plexus/archiver/jar/JdkManifestFactory.java
#41 (comment):

  •    return defaultManifest;

  •        defaultManifest.getMainAttributes().putValue( "Created-By", createdBy );

  •        return defaultManifest;

  •    }

  •    catch ( final IOException e )

  •    {

I consider the swallow here
codehaus-plexus/plexus-utils@ba1c194
was intentional. The local was actually borrowed from other spots where
the version is omitted of the |pom.properties| could not be read.

Just restored that behaviour. I even considered throwing an
AssertionError if the properties cannot be read. Omitting the version
shouldn't be an issue, of course.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am 2016-05-27 um 02:51 schrieb Christian Schulte:

  •    return defaultManifest;

  •        defaultManifest.getMainAttributes().putValue( "Created-By", createdBy );

  •        return defaultManifest;

  •    }

  •    catch ( final IOException e )

  •    {

Am 05/27/16 um 00:57 schrieb Michael Osipov:

In
src/main/java/org/codehaus/plexus/archiver/jar/JdkManifestFactory.java
#41 (comment):

  •    return defaultManifest;

  •        defaultManifest.getMainAttributes().putValue( "Created-By", createdBy );

  •        return defaultManifest;

  •    }

  •    catch ( final IOException e )

  •    {

I consider the swallow here
codehaus-plexus/plexus-utils@ba1c194
was intentional. The local was actually borrowed from other spots where
the version is omitted of the |pom.properties| could not be read.

Just restored that behaviour. I even considered throwing an
AssertionError if the properties cannot be read. Omitting the version

I am not convinced that AssertionError is the right thing because
Errors are reverved for the VM. Please search for pom.properties in
maven-core (DefaultRepositorySystemSessionFactory and
DefaultRuntimeInformation). You will see the basic ida it was taken from.

throw new ArchiverException( "Failure reading default manifest.", e );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is obviously a wrong message now because no default manifest is read here.

}
}

public static void merge( java.util.jar.Manifest target, java.util.jar.Manifest other, boolean overwriteMain )
Expand Down
35 changes: 21 additions & 14 deletions src/main/java/org/codehaus/plexus/archiver/jar/Manifest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -735,25 +735,32 @@ public Iterator<String> iterator()
public static Manifest getDefaultManifest()
throws ArchiverException
{
final Manifest defaultManifest = new Manifest();
defaultManifest.getMainAttributes().putValue( "Manifest-Version", "1.0" );
try
{
final Manifest defaultManifest = new Manifest();
defaultManifest.getMainAttributes().putValue( "Manifest-Version", "1.0" );

String createdBy = "Plexus Archiver";
String createdBy = "Plexus Archiver";

InputStream inputStream = Manifest.class.getResourceAsStream( "/META-INF/"
+ "maven/org.codehaus.plexus/plexus-archiver/pom.properties" );
Properties properties = PropertyUtils.loadProperties( inputStream );
if ( properties != null )
{
String plexusArchiverVersion = properties.getProperty( "version" );
if ( plexusArchiverVersion != null )
final Properties properties = PropertyUtils.loadProperties( Manifest.class.getResourceAsStream(
"/META-INF/maven/org.codehaus.plexus/plexus-archiver/pom.properties" ) );

if ( properties != null )
{
createdBy += " " + plexusArchiverVersion;
String plexusArchiverVersion = properties.getProperty( "version" );
if ( plexusArchiverVersion != null )
{
createdBy += " " + plexusArchiverVersion;
}
}
}
defaultManifest.getMainAttributes().putValue( "Created-By", createdBy );
defaultManifest.getMainAttributes().putValue( "Created-By", createdBy );

return defaultManifest;
return defaultManifest;
}
catch ( final IOException e )
{
throw new ArchiverException( "Failure reading default manifest.", e );
}
}

/**
Expand Down