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

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

Merged
merged 3 commits into from
May 26, 2016
Merged

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

merged 3 commits into from
May 26, 2016

Conversation

ChristianSchulte
Copy link
Contributor

No description provided.

…ting an

archive using an encoding different from UTF-8 instead of only when a name is
not encodeable. Additionally support that extra field when unarchiving.

This closes #39
@ChristianSchulte ChristianSchulte merged commit a2cb2e1 into codehaus-plexus:master May 26, 2016
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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants