Skip to content
Permalink
Browse files

MSHARED-837 add an API to configure outputTimestamp

  • Loading branch information
hboutemy committed Oct 5, 2019
1 parent 60ce414 commit 5f07f227aa89e0bb4163c125a46fbd4c78445301
@@ -93,7 +93,7 @@
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-archiver</artifactId>
<version>4.1.0</version>
<version>4.2.0-SNAPSHOT</version>
</dependency>
<!--
! plexus-archiver needs this, or else maven-artifact will
@@ -42,8 +42,12 @@
import javax.lang.model.SourceVersion;
import java.io.File;
import java.io.IOException;
import java.text.DateFormat;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
import java.util.List;
import java.util.Map;
import java.util.Properties;
@@ -758,4 +762,52 @@ public void setBuildJdkSpecDefaultEntry( boolean buildJdkSpecDefaultEntry )
this.buildJdkSpecDefaultEntry = buildJdkSpecDefaultEntry;
}

/**
* Parse output timestamp configured for Reproducible Builds' archive entries, formatted as ISO-8601
* <code>yyyy-MM-dd'T'HH:mm:ssX</code>.
*
* @param outputTimestamp the value of <code>${project.build.outputTimestamp}</code> (may be <code>null</code>)
* @return the parsed timestamp, may be <code>null</code> if <code>null</code> input or input contains only 1
* character
* @since 3.4.1
*/
public Date parseOutputTimestamp( String outputTimestamp )
{
if ( outputTimestamp == null || outputTimestamp.length() < 2 )
{
// no timestamp configured (1 character configuration is useful to override a full value during pom
// inheritance)
return null;
}

DateFormat df = new SimpleDateFormat( "yyyy-MM-dd'T'HH:mm:ssX" );

This comment has been minimized.

Copy link
@michael-o

michael-o Oct 6, 2019

Member

This is wrong. It has to be XXX. Otherwise it will parse hour offsets only.

This comment has been minimized.

Copy link
@hboutemy

hboutemy Oct 6, 2019

Author Member

don't hesitate to add more unit tests to show which additional ways of specifying a timestamp you want

This comment has been minimized.

Copy link
@michael-o

michael-o Oct 6, 2019

Member

This is not what I meant. The standard mandates that all components are either in basic or extended form. You are not allowed to mix it. X allows to mix a basic offset with an extended timestamp. Thus, making the input invalid according to the spec. We have to comply with the spec and not reinvent the wheel. It has to be yyyy-MM-dd'T'HH:mm:ssXXX.

This comment has been minimized.

Copy link
@michael-o

michael-o Oct 6, 2019

Member

If you want me to cite the standard, I can look it up tomorrow at work. I have access to all of them from the ISO.

This comment has been minimized.

Copy link
@hboutemy

hboutemy Oct 6, 2019

Author Member

I want you to provide unit tests to have me a chance to understand what you're talking about: you're using strong wording about something being wrong, and I'm exhausted by this attitude

This comment has been minimized.

Copy link
@michael-o

michael-o Oct 6, 2019

Member

I can't, because SDF is incorrectly implemented and you don't really understand the specific sematics I am trying to tell. I will modify with a subsequent commit and the FastDateParser then you will hopefully understand.

This comment has been minimized.

Copy link
@hboutemy

hboutemy Oct 6, 2019

Author Member

I confess I don't read ISO: I'm just a guy who uses Java JDK implementation, and my objective currently is not to review the JDK
I'm not convinced forcing every packaging plugin to require a new lib is really a good idea, just because for 20 years JDK implementation is not strictly ISO compliant

This comment has been minimized.

Copy link
@michael-o

michael-o Oct 6, 2019

Member

@hboutemy I have added now two branches with suffixes _SDF and _FDF. The tests now fail as expected.

try
{
return df.parse( outputTimestamp );
}
catch ( ParseException pe )
{
throw new IllegalArgumentException( "Invalid project.build.outputTimestamp value '" + outputTimestamp + "'",

This comment has been minimized.

Copy link
@michael-o

michael-o Oct 6, 2019

Member

Do you really what to tie this parsing logic to the explicit property name? It seems like to high coupling.

This comment has been minimized.

Copy link
@hboutemy

hboutemy Oct 6, 2019

Author Member

yes, I want: this is exactly why it is done in maven-archiver, and not plexus-archiver

pe );
}
}

/**
* Configure Reproducible Builds archive creation if a timestamp is provided.
*
* @param outputTimestamp the value of <code>${project.build.outputTimestamp}</code> (may be <code>null</code>)
* @return the parsed timestamp
* @since 3.4.1
* @see #parseOutputTimestamp
* @see Archiver#configureReproducible
*/
public Date configureReproducible( String outputTimestamp )
{
Date outputDate = parseOutputTimestamp( outputTimestamp );
if ( outputDate != null )
{
getArchiver().configureReproducible( outputDate );
}
return outputDate;
}
}
@@ -1503,4 +1503,22 @@ public Manifest getJarFileManifest( File jarFile )
}

}

@Test
public void testParseOutputTimestamp()
{
MavenArchiver archiver = new MavenArchiver();

assertNull( archiver.parseOutputTimestamp( null ) );
assertNull( archiver.parseOutputTimestamp( "" ) );
assertNull( archiver.parseOutputTimestamp( "." ) );
assertNull( archiver.parseOutputTimestamp( " " ) );
assertNull( archiver.parseOutputTimestamp( "_" ) );

assertEquals( 1570300662000L, archiver.parseOutputTimestamp( "2019-10-05T18:37:42Z" ).getTime() );
assertEquals( 1570300662000L, archiver.parseOutputTimestamp( "2019-10-05T20:37:42+02:00" ).getTime() );
assertEquals( 1570300662000L, archiver.parseOutputTimestamp( "2019-10-05T16:37:42-02:00" ).getTime() );
assertEquals( 1570300662000L, archiver.parseOutputTimestamp( "2019-10-05T20:37:42+0200" ).getTime() );

This comment has been minimized.

Copy link
@michael-o

michael-o Oct 6, 2019

Member

This one is invalid. You cannot have an extended date time with a basic timezone offset.

This comment has been minimized.

Copy link
@hboutemy

hboutemy Oct 6, 2019

Author Member

the unit test proves I can

This comment has been minimized.

Copy link
@michael-o

michael-o Oct 6, 2019

Member

Because your code is wrong.

This comment has been minimized.

Copy link
@hboutemy

hboutemy Oct 6, 2019

Author Member

it would be wrong if the unit test was failing: if it does not fail, it not wrong
or we need to define the semantics of "wrong" word

assertEquals( 1570300662000L, archiver.parseOutputTimestamp( "2019-10-05T16:37:42-0200" ).getTime() );

This comment has been minimized.

Copy link
@michael-o

michael-o Oct 6, 2019

Member

This one too.

This comment has been minimized.

Copy link
@hboutemy
}
}

1 comment on commit 5f07f22

@michael-o

This comment has been minimized.

Copy link
Member

michael-o commented on 5f07f22 Oct 6, 2019

This commit needs some changes...

Please sign in to comment.
You can’t perform that action at this time.