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

Respect SOURCE_DATE_EPOCH spec #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

onlyjob
Copy link

@onlyjob onlyjob commented Apr 4, 2016

ckbuilder puts a buildtime timestamp both in comments and in other part of
generated js. This patch make it observe SOURCE_DATE_EPOCH, so to allow for
reproducible builds
See https://reproducible-builds.org/specs/source-date-epoch/

Bug-Debian: https://bugs.debian.org/819726

@fredck
Copy link

fredck commented Apr 4, 2016

One of the purposes of the timestamp is helping on cache invalidation. In other words, browsers will usually take new sources form the server for every new build. This is intentional.

But using the proposed strategy, wouldn't it mean that the timestamp will always be the same, when building from the same environment? For example, two builds with totally different configurations will have the same timestamp if built on the same machine at the same time. Am I right?

If that is right, one of the possibilities we have is providing a command line parameter on the builder so one could pass the desired timestamp. The other option, much more complex so probably unfeasible, would be dropping the timestamp thing altogether and instead creating a hash based on the sources included in the build.

@onlyjob
Copy link
Author

onlyjob commented Apr 4, 2016

The idea it to be able to produce binary identical builds by setting SOURCE_DATE_EPOCH.
You can see that default behaviour is not changed unless SOURCE_DATE_EPOCH is set.

When we re-build the same version of package we want to produce identical binaries. New revision of package is built using new(er) time stamp so cache will be properly invalidated on package update. This change helps to produce identical builds when necessary.
I hope it make sense.

@fredck
Copy link

fredck commented Apr 4, 2016

Ok... I've totally misunderstood the spec... it makes total sense. Thanks for the clarification.

@onlyjob
Copy link
Author

onlyjob commented Apr 4, 2016

No worries. :)

@onlyjob
Copy link
Author

onlyjob commented May 11, 2016

I'm slightly disappointed that this improvement is still not merged (so it could not be incorporated into latest release 2.3.1). Is there any doubts or concerns regarding this change?

@mlewand
Copy link
Contributor

mlewand commented Jun 3, 2016

@onlyjob Sorry for a long wait, we were quite busy and not able to pay enough attention here. The idea looks fine, I'll just ask you to:

  • provide some tests for added functionality,
  • adjust code style (parenthesis spacing, consistent usage of a single quote)

Once that's done we should be fine with accepting your PR,
cheers!

@onlyjob
Copy link
Author

onlyjob commented Jun 3, 2016

Unfortunately I have no time or skills to implement tests. Please feel free to take over this pull request and craft it to your preferences. Also it is worth noticing that I am merely forwarding patch that was originally contributed to Debian...

@mlewand
Copy link
Contributor

mlewand commented Jun 6, 2016

Hi, I'm sorry, since you're not the author of the code we can't accept it from you in this case. Could you reach the original author of the contribution? Or do you know what license is used for code shared in a referenced place?

@boyska
Copy link

boyska commented Jun 6, 2016

I am the author of the code! I am quite flexible about the licensing, so just tell me what you prefer, or whatever I can do to solve the licensing issue.

@onlyjob
Copy link
Author

onlyjob commented Jun 6, 2016

@mlewand, FYI Debian project typically uses the same license as upstream for most patches (unless otherwise is explicitly stated). This contribution is already license-clean.

@mlewand
Copy link
Contributor

mlewand commented Jun 7, 2016

I am the author of the code! I am quite flexible about the licensing, so just tell me what you prefer, or whatever I can do to solve the licensing issue.

Since we have original author here, I'd simply ask you @boyska to create a new PR with your code and this will make the situation clear for us. Could you deliver it? :)

@onlyjob
Copy link
Author

onlyjob commented Jun 7, 2016

So much dance around trivial pull request... :( What's wrong with you people?

@mlewand
Copy link
Contributor

mlewand commented Jun 7, 2016

@onlyjob we just want to have it nice and clean, there's not much of effort to create a pull request.

 ckbuilder puts a buildtime timestamp both in comments and in other part of
 generated js. This patch make it observe SOURCE_DATE_EPOCH, so to allow for
 reproducible builds
 See https://reproducible-builds.org/specs/source-date-epoch/

Bug-Debian: https://bugs.debian.org/819726
Author: boyska <[email protected]>
Signed-off-by: Dmitry Smirnov <[email protected]>
@onlyjob onlyjob force-pushed the reproducible-build branch from 9b82746 to 1622a4f Compare August 25, 2016 05:45
@onlyjob
Copy link
Author

onlyjob commented Aug 25, 2016

I've rebased my commit. As far as I'm concerned it is ready to be merged...

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.

4 participants