On 16-05-24 12:06:48, James Clarke wrote: > Hi, > > On 24 May 2016, at 11:59, Dhole <dh...@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
signature.asc
Description: PGP signature
