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

mkbuildinf.pl: improve reproducible builds #4639

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
6 participants
@jurobystricky

jurobystricky commented Oct 31, 2017

If the environment contains SOURCE_DATE_EPOCH, generate output such as:

#define DATE "built on: reproducible build, date unspecified"

instead of the usual, such as:

#define DATE "built on: Tue Oct 24 17:20:35 2017"

The message is consistent with other messages if SOURCE_DATE_EPOCH
is defined.

Signed-off-by: Juro Bystricky juro.bystricky@intel.com

Checklist
  • documentation is added or updated
  • tests are added or updated
mkbuildinf.pl: improve reproducible builds
If the environment contains SOURCE_DATE_EPOCH, generate output such as:

 #define DATE "built on: reproducible build, date unspecified"

instead of the usual, such as:

 #define DATE "built on: Tue Oct 24 17:20:35 2017"

The message is consistent with other messages if SOURCE_DATE_EPOCH
is defined.

Signed-off-by: Juro Bystricky <juro.bystricky@intel.com>
@richsalz

This comment has been minimized.

Show comment
Hide comment
@richsalz

richsalz Oct 31, 2017

Contributor

There's already an ifdef, OPENSSL_USE_BUILD_DATE, that is used in crypto/cversion.c, that does the same thing.

Thanks for the idea, tho.

Contributor

richsalz commented Oct 31, 2017

There's already an ifdef, OPENSSL_USE_BUILD_DATE, that is used in crypto/cversion.c, that does the same thing.

Thanks for the idea, tho.

@richsalz richsalz closed this Oct 31, 2017

@jurobystricky

This comment has been minimized.

Show comment
Hide comment
@jurobystricky

jurobystricky Oct 31, 2017

Yes, but quite the same. This script 'mkbuildinf.pl" generates the file crypto/buildinf.h. The date there ends up breaking reproducible build for openssl-dbg packages (or any package containing this file)

jurobystricky commented Oct 31, 2017

Yes, but quite the same. This script 'mkbuildinf.pl" generates the file crypto/buildinf.h. The date there ends up breaking reproducible build for openssl-dbg packages (or any package containing this file)

@jurobystricky

This comment has been minimized.

Show comment
Hide comment
@jurobystricky

jurobystricky Oct 31, 2017

sorry not quite the same

jurobystricky commented Oct 31, 2017

sorry not quite the same

@richsalz

This comment has been minimized.

Show comment
Hide comment
@richsalz

richsalz Oct 31, 2017

Contributor

Then it would be good if buildinf.h used the same ifdef style as crypto/cversion.c

Contributor

richsalz commented Oct 31, 2017

Then it would be good if buildinf.h used the same ifdef style as crypto/cversion.c

@richsalz

This comment has been minimized.

Show comment
Hide comment
@richsalz

richsalz Oct 31, 2017

Contributor

But is this really needed? Reproducible builds tend to compare output, not sources, no?

If this is needed, you'll have to sign our CLA and get Intel to put you on their list. Or put the text "CLA: trivial" in your commit text (not the title).

Contributor

richsalz commented Oct 31, 2017

But is this really needed? Reproducible builds tend to compare output, not sources, no?

If this is needed, you'll have to sign our CLA and get Intel to put you on their list. Or put the text "CLA: trivial" in your commit text (not the title).

@jurobystricky

This comment has been minimized.

Show comment
Hide comment
@jurobystricky

jurobystricky Oct 31, 2017

open-embedded splits openssl into several packages, some of them are affected by the generated buildinf.h file,
(I believe those are openssl-dbg and openssl-staticdev packages, when I build them on different days, they differ. )
I will rework the patch along the lines you suggested.
Thanks

jurobystricky commented Oct 31, 2017

open-embedded splits openssl into several packages, some of them are affected by the generated buildinf.h file,
(I believe those are openssl-dbg and openssl-staticdev packages, when I build them on different days, they differ. )
I will rework the patch along the lines you suggested.
Thanks

@kaduk

This comment has been minimized.

Show comment
Hide comment
@kaduk

kaduk Nov 1, 2017

Contributor

@richsalz SOURCE_DATE_EPOCH is the de facto standard variable used by reproducible builds all over the place. Need we really be a special snowflake in this regard?

Contributor

kaduk commented Nov 1, 2017

@richsalz SOURCE_DATE_EPOCH is the de facto standard variable used by reproducible builds all over the place. Need we really be a special snowflake in this regard?

@richsalz

This comment has been minimized.

Show comment
Hide comment
@richsalz

richsalz Nov 1, 2017

Contributor

I did not know that. It would be good to have this PR change openssl to use that.

My other question still stands: is it necessary that buildinf.h NEVER CHANGE if the variable is set?

Contributor

richsalz commented Nov 1, 2017

I did not know that. It would be good to have this PR change openssl to use that.

My other question still stands: is it necessary that buildinf.h NEVER CHANGE if the variable is set?

@kaduk

This comment has been minimized.

Show comment
Hide comment
@kaduk

kaduk Nov 1, 2017

Contributor

The requirement is actually fairly weak: we need to be reproducible only when invoked with the same compiler and configuration flags on the same platform and the same source tree (i.e., git commit), on different hosts and at different times. https://wiki.debian.org/ReproducibleBuilds/Howto does not even require that we are reproducible when built at a different path in the filesystem, though I don't know how universal that relaxation is.

Contributor

kaduk commented Nov 1, 2017

The requirement is actually fairly weak: we need to be reproducible only when invoked with the same compiler and configuration flags on the same platform and the same source tree (i.e., git commit), on different hosts and at different times. https://wiki.debian.org/ReproducibleBuilds/Howto does not even require that we are reproducible when built at a different path in the filesystem, though I don't know how universal that relaxation is.

@kaduk

This comment has been minimized.

Show comment
Hide comment
@kaduk

kaduk Nov 1, 2017

Contributor

[deleted]

Contributor

kaduk commented Nov 1, 2017

[deleted]

@levitte

This comment has been minimized.

Show comment
Hide comment
@levitte

levitte Nov 1, 2017

Member

Doesn't this mean we can remove the ifdef soup around the date in cversion.c too?

Member

levitte commented Nov 1, 2017

Doesn't this mean we can remove the ifdef soup around the date in cversion.c too?

@richsalz

This comment has been minimized.

Show comment
Hide comment
@richsalz

richsalz Nov 1, 2017

Contributor

See #4644

Contributor

richsalz commented Nov 1, 2017

See #4644

@lamby

This comment has been minimized.

Show comment
Hide comment
@lamby

lamby Nov 2, 2017

(FYI there's no real reason to replace it with a string describing that it is reprocubible, ie:)

use POSIX qw(strftime);
my $date = strftime("%Y-%m-%d", gmtime($ENV{SOURCE_DATE_EPOCH} || time));

:)

lamby commented Nov 2, 2017

(FYI there's no real reason to replace it with a string describing that it is reprocubible, ie:)

use POSIX qw(strftime);
my $date = strftime("%Y-%m-%d", gmtime($ENV{SOURCE_DATE_EPOCH} || time));

:)

@richsalz

This comment has been minimized.

Show comment
Hide comment
@richsalz

richsalz Nov 2, 2017

Contributor

That's nice, but I think the time is useful :)

Contributor

richsalz commented Nov 2, 2017

That's nice, but I think the time is useful :)

@lamby

This comment has been minimized.

Show comment
Hide comment
@lamby

lamby Nov 2, 2017

That's nice, but I think the time is useful :)

Oh sure, adjust the format to suit. I was only demonstrating that we don't need to replace the date with a string such as "reproducible build".

lamby commented Nov 2, 2017

That's nice, but I think the time is useful :)

Oh sure, adjust the format to suit. I was only demonstrating that we don't need to replace the date with a string such as "reproducible build".

@richsalz

This comment has been minimized.

Show comment
Hide comment
@richsalz

richsalz Nov 2, 2017

Contributor

Ah, I see. I'm clearly a noob on this stuff, so I'll just be the typing-monkey and do whatever @kaduk @levitte @kroeckx tell me to type.
:)

Contributor

richsalz commented Nov 2, 2017

Ah, I see. I'm clearly a noob on this stuff, so I'll just be the typing-monkey and do whatever @kaduk @levitte @kroeckx tell me to type.
:)

@richsalz

This comment has been minimized.

Show comment
Hide comment
@richsalz

richsalz Nov 27, 2017

Contributor

Fix merged to master, thanks!

Contributor

richsalz commented Nov 27, 2017

Fix merged to master, thanks!

@richsalz richsalz closed this Nov 27, 2017

@lamby

This comment has been minimized.

Show comment
Hide comment
@lamby

lamby Nov 27, 2017

Thanks all :)

lamby commented Nov 27, 2017

Thanks all :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment