You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ant.apache.org by bu...@apache.org on 2018/08/10 18:34:20 UTC

[Bug 62617] New: Honor SOURCE_DATE_EPOCH to allow reproducible builds

https://bz.apache.org/bugzilla/show_bug.cgi?id=62617

            Bug ID: 62617
           Summary: Honor SOURCE_DATE_EPOCH to allow reproducible builds
           Product: Ant
           Version: unspecified
          Hardware: PC
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: Core tasks
          Assignee: notifications@ant.apache.org
          Reporter: apache-bugzilla@lepiller.eu
  Target Milestone: ---

Created attachment 36086
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=36086&action=edit
patch for tstamp

Hi,

the <tstamp> task allows programmers to refer to the current time in the build
process.

I would like to suggest honoring the SOURCE_DATE_EPOCH environment variable.
This variable is set by distributions during build and refer to a fixed
timestamp that can be used reproducibly. More information on
https://reproducible-builds.org/specs/source-date-epoch/.

This bug report is related to #61269.

Attached is a patch for this issue. By default, the old behavior is preserved,
unless SOURCE_DATE_EPOCH is defined in the environment, in which case it is
used as the current date.

Thank you :)

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62617] Honor SOURCE_DATE_EPOCH to allow reproducible builds

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62617

Jaikiran Pai <ja...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 OS|                            |All

--- Comment #1 from Jaikiran Pai <ja...@apache.org> ---
Hi Julien,

>> By default, the old behavior is preserved, unless SOURCE_DATE_EPOCH is defined in the environment, in which case it is used as the current date.

I think, the proposed patch might not preserve the current behaviour. Since,
right now, even if the SOURCE_DATE_EPOCH is set in the system where the build
is run, the tstamp task will not use it and instead will use the current time.
With this patch, the behaviour will change in such build files. Maybe it's a
better idea to have an explicitly set attribute (a new attribute) on the task
which will start honouring this environment variable? That way the default
semantics of the attribute can continue to ignore this environment variable
unless explicitly set to honour it. Some attribute like "useSourceDateEpoch".


As for the patch:

-            Date d = new Date();
+            Date d;
+            String epoch = System.getenv("SOURCE_DATE_EPOCH");
+            if(epoch.compareTo("") == 0)
+                d = new Date();

I believe you will need a epoch != null check here. In its current form, this
will lead to a NPE on systems where the environment variable isn't set.
Furthermore, I don't think the compareTo call is required. The "spec" that you
pointed to, in the linked content, explicitly states that the value of such an
environment variable MUST be an integer, so I think you can just try to use it
as an Integer value. If you still want to check if the environment variable
value is empty, then I think epoch.isEmpty() is a better API to use.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62617] Honor SOURCE_DATE_EPOCH to allow reproducible builds

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62617

--- Comment #6 from Jaikiran Pai <ja...@apache.org> ---
https://gitbox.apache.org/repos/asf?p=ant.git;a=commitdiff;h=1bfa7880afc7cbbc1e3d73be5cfa77dc2fd216d2

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62617] Honor SOURCE_DATE_EPOCH to allow reproducible builds

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62617

Emmanuel Bourg <eb...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |61269


Referenced Bugs:

https://bz.apache.org/bugzilla/show_bug.cgi?id=61269
[Bug 61269] Ant should support reproducible builds
-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62617] Honor SOURCE_DATE_EPOCH to allow reproducible builds

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62617

--- Comment #10 from Emmanuel Bourg <eb...@apache.org> ---
The <touch> task is also sensitive to the current time, SOURCE_DATE_EPOCH
should override the calls to System.currentTimeMillis().

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62617] Honor SOURCE_DATE_EPOCH to allow reproducible builds

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62617

Emmanuel Bourg <eb...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|FIXED                       |---
             Status|RESOLVED                    |REOPENED

--- Comment #8 from Emmanuel Bourg <eb...@apache.org> ---
Reopening because DateUtils.getDateForHeader() also needs to support
SOURCE_DATE_EPOCH.

Suggested fix:
https://salsa.debian.org/java-team/ant/blob/debian/1.10.6-1/debian/patches/0011-reproducible-propertyfile-task.patch

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62617] Honor SOURCE_DATE_EPOCH to allow reproducible builds

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62617

--- Comment #7 from Emmanuel Bourg <eb...@apache.org> ---
Thank you for adding SOURCE_DATE_EPOCH support to Ant. Note that the timezone
should also be clamped to a fixed value, for example UTC, otherwise the
formatted dates will depend on the user timezone and the result won't be
reproducible.

Here is for example the patch I've implemented in Debian to get reproducible
timestamps:

https://salsa.debian.org/java-team/ant/blob/debian/1.10.6-1/debian/patches/0009-reproducible-timestamp-task.patch

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62617] Honor SOURCE_DATE_EPOCH to allow reproducible builds

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62617

Emmanuel Bourg <eb...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebourg@apache.org

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62617] Honor SOURCE_DATE_EPOCH to allow reproducible builds

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62617

Jonas Witschel <di...@gmx.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |diabonas@gmx.de

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62617] Honor SOURCE_DATE_EPOCH to allow reproducible builds

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62617

Jaikiran Pai <ja...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |1.10.8
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #5 from Jaikiran Pai <ja...@apache.org> ---
Thank you very much for this patch. I've done minor modifications to it and
committed it upstream. This change will be available in our 1.10.8 release.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62617] Honor SOURCE_DATE_EPOCH to allow reproducible builds

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62617

--- Comment #3 from Stefan Bodewig <bo...@apache.org> ---
Actually I don't think it is very likely that SOURCE_DATE_EPOCH has been set by
accident when a user tuns the build, so I would probably live with the
backwards incompatibility. Point it out in WHATSNEW and maybe log a prominent
message in Tstamp when it is used.

As for the patch itself, I'd probably drop the check for empty string but catch
a NumberFormatException that parseInt may throw. If the env varibale holds
anything that connot be parsed log a warning and continue with a new Date()
instance.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62617] Honor SOURCE_DATE_EPOCH to allow reproducible builds

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62617

--- Comment #9 from Jaikiran Pai <ja...@apache.org> ---
Hello Emmanuel, would you be interested in contributing those patches that you
have included in Debian, to upstream Ant project itself? If so, can you open
individual pull requests for them, so that they can be reviewed?

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62617] Honor SOURCE_DATE_EPOCH to allow reproducible builds

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62617

Julien Lepiller <ap...@lepiller.eu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #36086|0                           |1
        is obsolete|                            |

--- Comment #4 from Julien Lepiller <ap...@lepiller.eu> ---
Created attachment 36845
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=36845&action=edit
new patch

Here is a new patch (although untested) rebased on ant master. Please let me
know what you think of it. Thank you for your interest in this issue :)

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62617] Honor SOURCE_DATE_EPOCH to allow reproducible builds

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62617

--- Comment #2 from Jonas Witschel <di...@gmx.de> ---
Hi Jaikiran,

I would be very interested in reproducible build support in Apache Ant as well.

>(In reply to Jaikiran Pai from comment #1)
> I think, the proposed patch might not preserve the current behaviour. Since,
> right now, even if the SOURCE_DATE_EPOCH is set in the system where the
> build is run, the tstamp task will not use it and instead will use the
> current time. With this patch, the behaviour will change in such build
> files.
I don't see the point in preserving backwards compatibility in this case: if
SOURCE_DATE_EPOCH is set, the user explicitly expects time stamps to be set to
this fixed date instead of the current time as per the specification Julien
linked above. Using the current time instead is undesirable behaviour if the
environment variable is set, so I would consider the current behaviour a bug
that requires backwards-incompatible changes in order to be fixed.

Maybe it's a better idea to have an explicitly set attribute (a new
> attribute) on the task which will start honouring this environment variable?
> That way the default semantics of the attribute can continue to ignore this
> environment variable unless explicitly set to honour it. Some attribute like
> "useSourceDateEpoch".
That would defeat the goal of having reproducible support by default: every
projects needs to be aware of this attribute and explicitly set it, which will
not happen e.g. for legacy projects. All artifacts created by Apache Ant should
be reproducible out of the box if SOURCE_DATE_EPOCH is set.

> I believe you will need a epoch != null check here. In its current form,
> this will lead to a NPE on systems where the environment variable isn't set.
> Furthermore, I don't think the compareTo call is required. The "spec" that
> you pointed to, in the linked content, explicitly states that the value of
> such an environment variable MUST be an integer, so I think you can just try
> to use it as an Integer value. If you still want to check if the environment
> variable value is empty, then I think epoch.isEmpty() is a better API to use.
ACK on the review of the patch.

-- 
You are receiving this mail because:
You are the assignee for the bug.