[PATCH v2] Ensure source_date_epoch is always initialised
Dhole
dhole@openmailbox.org
Tue May 24 12:32:00 GMT 2016
On 16-05-24 12:06:48, James Clarke wrote:
> Hi,
> > On 24 May 2016, at 11:59, Dhole <dhole@openmailbox.org> wrote:
> >
> > Hey!
> >
> > I'm the original author of the SOURCE_DATE_EPOCH patch.
> >
> > I've just seen this. I believe that this bug was fixed in the the
> > rework of the patch I sent some days ago [1], although the latest
> > version of that patch has not been reviewed yet. In [1] the
> > initialization of source_date_epoch is done at init.c
> > (cpp_create_reader), so now it should be initialized properly even when
> > just calling the preprocessor. I tested your example and it gives the
> > expected output.
> >
> > Although thinking further, maybe it would be more wise to use "0" as a
> > default value, to mean "not yet set", so that errors like this are
> > avoided. So source_date_epoch could be:
> > 0: not yet set
> > negative: disabled
> > positive: use this value as SOURCE_DATE_EPOCH
> >
> > In such case, SOURCE_DATE_EPOCH would need to be a positive number
> > instead of a non-negative number.
>
> 0 *is* a valid SOURCE_DATE_EPOCH, ie Jan 1 1970 00:00:00, and should
> definitely be allowed.
You're right in the sense that 0 is a valid unix epoch. In my
suggestion I was considering that SOURCE_DATE_EPOCH is used to set the
date the source code was last modified, and I guess no build process
nowadays has code that was last modified in 1970. But it may be easier
to understand if 0 is left as a valid value.
> I see your patch continues to put some of the code inside c-family? Is
> there a reason for doing that instead of keeping it all inside libcpp
> like mine, given it’s inherently preprocessor-only? You’ve also merged
> all the error paths into one message which is not as helpful.
I merged the error paths into one as suggested in [1]. I'm not that
knowledgable of GCC to give a call on this, so I just followed the
suggestion from Martin. But it could be reverted if needed.
Regarding having the code inside c-family, I'm following the suggestion
from Joseph [2]:
Joseph Myers wrote:
> Since cpplib is a library and doesn't have any existing getenv calls, I
> wonder if it would be better for the cpplib client (i.e. something in the
> gcc/ directory) to be what calls getenv and then informs cpplib of the
> timestamp it should treat as being the time of compilation.
Jakub also found it reasonable [3]:
Jakub Jelinek wrote:
> Doing this on the gcc/ side is of course reasonable, but can be done through
> callbacks, libcpp already has lots of other callbacks into the gcc/ code,
> look for e.g. cpp_get_callbacks in gcc/c-family/* and in libcpp/ for
> corresponding code.
[1] https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01889.html
[2] https://gcc.gnu.org/ml/gcc-patches/2015-06/msg02270.html
[3] https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01930.html
Cheers,
--
Dhole
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20160524/8520c7b2/attachment.sig>
More information about the Gcc-patches
mailing list