[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