* [PATCH] Ensure source_date_epoch is always initialised
@ 2016-05-19 0:02 James Clarke
2016-05-19 13:07 ` [PATCH v2] " James Clarke
0 siblings, 1 reply; 5+ messages in thread
From: James Clarke @ 2016-05-19 0:02 UTC (permalink / raw)
To: gcc-patches
gcc/c-family
PR preprocessor/71183
* c-common.c (get_source_date_epoch): Move to libcpp/init.c.
* c-common.h (get_source_date_epoch): Remove definition, as it
is now internal to libcpp/init.c.
* c-lex.c (c_lex_with_flags): Remove source_date_epoch
initialization, as this is now done by libcpp.
gcc/testsuite/
PR preprocessor/71183
* gcc.dg/cpp/special/date-time.c: New testcase.
* gcc.dg/cpp/special/date-time.exp: New file. Sets the
SOURCE_DATE_EPOCH environment variable for date-time.c.
libcpp/
PR preprocessor/71183
* include/cpplib.h (cpp_init_source_date_epoch): Remove
definition, as it is now internal to init.c.
* init.c (cpp_create_reader): Initialize source_date_epoch.
(get_source_date_epoch): Moved from gcc/c-family/c-common.c, and
uses cpp_error instead of fatal_error.
(cpp_init_source_date_epoch): Drop source_date_epoch argument
and call get_source_date_epoch to get the value.
---
gcc/c-family/c-common.c | 33 --------------------
gcc/c-family/c-common.h | 5 ---
gcc/c-family/c-lex.c | 3 --
gcc/testsuite/gcc.dg/cpp/special/date-time.c | 5 +++
gcc/testsuite/gcc.dg/cpp/special/date-time.exp | 35 +++++++++++++++++++++
libcpp/include/cpplib.h | 3 --
libcpp/init.c | 43 ++++++++++++++++++++++++--
7 files changed, 80 insertions(+), 47 deletions(-)
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 146e805..83f38dd 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -12791,39 +12791,6 @@ valid_array_size_p (location_t loc, tree type, tree name)
return true;
}
-/* Read SOURCE_DATE_EPOCH from environment to have a deterministic
- timestamp to replace embedded current dates to get reproducible
- results. Returns -1 if SOURCE_DATE_EPOCH is not defined. */
-time_t
-get_source_date_epoch ()
-{
- char *source_date_epoch;
- long long epoch;
- char *endptr;
-
- source_date_epoch = getenv ("SOURCE_DATE_EPOCH");
- if (!source_date_epoch)
- return (time_t) -1;
-
- errno = 0;
- epoch = strtoll (source_date_epoch, &endptr, 10);
- if ((errno == ERANGE && (epoch == LLONG_MAX || epoch == LLONG_MIN))
- || (errno != 0 && epoch == 0))
- fatal_error (UNKNOWN_LOCATION, "environment variable $SOURCE_DATE_EPOCH: "
- "strtoll: %s\n", xstrerror(errno));
- if (endptr == source_date_epoch)
- fatal_error (UNKNOWN_LOCATION, "environment variable $SOURCE_DATE_EPOCH: "
- "no digits were found: %s\n", endptr);
- if (*endptr != '\0')
- fatal_error (UNKNOWN_LOCATION, "environment variable $SOURCE_DATE_EPOCH: "
- "trailing garbage: %s\n", endptr);
- if (epoch < 0)
- fatal_error (UNKNOWN_LOCATION, "environment variable $SOURCE_DATE_EPOCH: "
- "value must be nonnegative: %lld \n", epoch);
-
- return (time_t) epoch;
-}
-
/* Check and possibly warn if two declarations have contradictory
attributes, such as always_inline vs. noinline. */
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 0ee9f56..63fd2b9 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1482,9 +1482,4 @@ extern bool valid_array_size_p (location_t, tree, tree);
extern bool cilk_ignorable_spawn_rhs_op (tree);
extern bool cilk_recognize_spawn (tree, tree *);
-/* Read SOURCE_DATE_EPOCH from environment to have a deterministic
- timestamp to replace embedded current dates to get reproducible
- results. Returns -1 if SOURCE_DATE_EPOCH is not defined. */
-extern time_t get_source_date_epoch (void);
-
#endif /* ! GCC_C_COMMON_H */
diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c
index 38a428d..5bab8d1 100644
--- a/gcc/c-family/c-lex.c
+++ b/gcc/c-family/c-lex.c
@@ -389,9 +389,6 @@ c_lex_with_flags (tree *value, location_t *loc, unsigned char *cpp_flags,
enum cpp_ttype type;
unsigned char add_flags = 0;
enum overflow_type overflow = OT_NONE;
- time_t source_date_epoch = get_source_date_epoch ();
-
- cpp_init_source_date_epoch (parse_in, source_date_epoch);
timevar_push (TV_CPP);
retry:
diff --git a/gcc/testsuite/gcc.dg/cpp/special/date-time.c b/gcc/testsuite/gcc.dg/cpp/special/date-time.c
new file mode 100644
index 0000000..3304b75
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/cpp/special/date-time.c
@@ -0,0 +1,5 @@
+/* { dg-do preprocess } */
+__DATE__
+__TIME__
+/* { dg-final { scan-file date-time.i "\"Jul 4 1978\"" } } */
+/* { dg-final { scan-file date-time.i "\"21:24:16\"" } } */
diff --git a/gcc/testsuite/gcc.dg/cpp/special/date-time.exp b/gcc/testsuite/gcc.dg/cpp/special/date-time.exp
new file mode 100644
index 0000000..3c43143
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/cpp/special/date-time.exp
@@ -0,0 +1,35 @@
+# Copyright (C) 1997-2016 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with GCC; see the file COPYING3. If not see
+# <http://www.gnu.org/licenses/>.
+
+# GCC testsuite that uses the `dg.exp' driver.
+
+load_lib gcc-dg.exp
+
+# If a testcase doesn't have special options, use these.
+global DEFAULT_CFLAGS
+if ![info exists DEFAULT_CFLAGS] then {
+ set DEFAULT_CFLAGS " -ansi -pedantic-errors"
+}
+
+setenv SOURCE_DATE_EPOCH 268435456
+
+# Initialize `dg'.
+dg-init
+
+dg-runtest $srcdir/$subdir/date-time.c "" $DEFAULT_CFLAGS
+
+# All done.
+dg-finish
diff --git a/libcpp/include/cpplib.h b/libcpp/include/cpplib.h
index 4998b3a..35b0375 100644
--- a/libcpp/include/cpplib.h
+++ b/libcpp/include/cpplib.h
@@ -784,9 +784,6 @@ extern void cpp_init_special_builtins (cpp_reader *);
/* Set up built-ins like __FILE__. */
extern void cpp_init_builtins (cpp_reader *, int);
-/* Initialize the source_date_epoch value. */
-extern void cpp_init_source_date_epoch (cpp_reader *, time_t);
-
/* This is called after options have been parsed, and partially
processed. */
extern void cpp_post_options (cpp_reader *);
diff --git a/libcpp/init.c b/libcpp/init.c
index f5ff85b..9594ae6 100644
--- a/libcpp/init.c
+++ b/libcpp/init.c
@@ -36,6 +36,7 @@ along with this program; see the file COPYING3. If not see
static void init_library (void);
static void mark_named_operators (cpp_reader *, int);
+static void cpp_init_source_date_epoch (cpp_reader *);
static void read_original_filename (cpp_reader *);
static void read_original_directory (cpp_reader *);
static void post_options (cpp_reader *);
@@ -267,6 +268,9 @@ cpp_create_reader (enum c_lang lang, cpp_hash_table *table,
_cpp_init_hashtable (pfile, table);
+ /* Initialize the source_date_epoch value. */
+ cpp_init_source_date_epoch (pfile);
+
return pfile;
}
@@ -533,11 +537,44 @@ cpp_init_builtins (cpp_reader *pfile, int hosted)
_cpp_define_builtin (pfile, "__OBJC__ 1");
}
+/* Read SOURCE_DATE_EPOCH from environment to have a deterministic
+ timestamp to replace embedded current dates to get reproducible
+ results. Returns -1 if SOURCE_DATE_EPOCH is not defined. */
+static time_t
+get_source_date_epoch (cpp_reader *pfile)
+{
+ char *source_date_epoch;
+ long long epoch;
+ char *endptr;
+
+ source_date_epoch = getenv ("SOURCE_DATE_EPOCH");
+ if (!source_date_epoch)
+ return (time_t) -1;
+
+ errno = 0;
+ epoch = strtoll (source_date_epoch, &endptr, 10);
+ if ((errno == ERANGE && (epoch == LLONG_MAX || epoch == LLONG_MIN))
+ || (errno != 0 && epoch == 0))
+ cpp_error (pfile, CPP_DL_FATAL, "environment variable $SOURCE_DATE_EPOCH: "
+ "strtoll: %s\n", xstrerror(errno));
+ if (endptr == source_date_epoch)
+ cpp_error (pfile, CPP_DL_FATAL, "environment variable $SOURCE_DATE_EPOCH: "
+ "no digits were found: %s\n", endptr);
+ if (*endptr != '\0')
+ cpp_error (pfile, CPP_DL_FATAL, "environment variable $SOURCE_DATE_EPOCH: "
+ "trailing garbage: %s\n", endptr);
+ if (epoch < 0)
+ cpp_error (pfile, CPP_DL_FATAL, "environment variable $SOURCE_DATE_EPOCH: "
+ "value must be nonnegative: %lld \n", epoch);
+
+ return (time_t) epoch;
+}
+
/* Initialize the source_date_epoch value. */
-void
-cpp_init_source_date_epoch (cpp_reader *pfile, time_t source_date_epoch)
+static void
+cpp_init_source_date_epoch (cpp_reader *pfile)
{
- pfile->source_date_epoch = source_date_epoch;
+ pfile->source_date_epoch = get_source_date_epoch (pfile);
}
/* Sanity-checks are dependent on command-line options, so it is
--
2.8.1.369.geae769a
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] Ensure source_date_epoch is always initialised
2016-05-19 0:02 [PATCH] Ensure source_date_epoch is always initialised James Clarke
@ 2016-05-19 13:07 ` James Clarke
2016-05-24 11:07 ` Dhole
0 siblings, 1 reply; 5+ messages in thread
From: James Clarke @ 2016-05-19 13:07 UTC (permalink / raw)
To: gcc-patches
gcc/c-family
PR preprocessor/71183
* c-common.c (get_source_date_epoch): Move to libcpp/init.c.
* c-common.h (get_source_date_epoch): Remove definition, as it
is now internal to libcpp/init.c.
* c-lex.c (c_lex_with_flags): Remove source_date_epoch
initialization, as this is now done by libcpp.
gcc/testsuite/
PR preprocessor/71183
* gcc.dg/cpp/special/date-time.c: New testcase.
* gcc.dg/cpp/special/date-time.exp: New file. Sets the
SOURCE_DATE_EPOCH environment variable for date-time.c.
libcpp/
PR preprocessor/71183
* include/cpplib.h (cpp_init_source_date_epoch): Remove
definition, as it is now internal to init.c.
* init.c (get_source_date_epoch): Moved from
gcc/c-family/c-common.c, and uses cpp_error instead of
fatal_error.
(cpp_init_source_date_epoch): Drop source_date_epoch argument
and call get_source_date_epoch to get the value.
(cpp_post_options): Call cpp_init_source_date_epoch.
---
Changes from v1:
* Moved cpp_init_source_date_epoch call from
cpp_init_source_date_epoch to cpp_post_options, as the error
callback is otherwise not set and would lead to an ICE if
cpp_error was called.
* Dropped forward declaration of cpp_init_source_date_epoch, as
the call to it now comes after its definition.
gcc/c-family/c-common.c | 33 --------------------
gcc/c-family/c-common.h | 5 ---
gcc/c-family/c-lex.c | 3 --
gcc/testsuite/gcc.dg/cpp/special/date-time.c | 5 +++
gcc/testsuite/gcc.dg/cpp/special/date-time.exp | 35 +++++++++++++++++++++
libcpp/include/cpplib.h | 3 --
libcpp/init.c | 42 ++++++++++++++++++++++++--
7 files changed, 79 insertions(+), 47 deletions(-)
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 146e805..83f38dd 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -12791,39 +12791,6 @@ valid_array_size_p (location_t loc, tree type, tree name)
return true;
}
-/* Read SOURCE_DATE_EPOCH from environment to have a deterministic
- timestamp to replace embedded current dates to get reproducible
- results. Returns -1 if SOURCE_DATE_EPOCH is not defined. */
-time_t
-get_source_date_epoch ()
-{
- char *source_date_epoch;
- long long epoch;
- char *endptr;
-
- source_date_epoch = getenv ("SOURCE_DATE_EPOCH");
- if (!source_date_epoch)
- return (time_t) -1;
-
- errno = 0;
- epoch = strtoll (source_date_epoch, &endptr, 10);
- if ((errno == ERANGE && (epoch == LLONG_MAX || epoch == LLONG_MIN))
- || (errno != 0 && epoch == 0))
- fatal_error (UNKNOWN_LOCATION, "environment variable $SOURCE_DATE_EPOCH: "
- "strtoll: %s\n", xstrerror(errno));
- if (endptr == source_date_epoch)
- fatal_error (UNKNOWN_LOCATION, "environment variable $SOURCE_DATE_EPOCH: "
- "no digits were found: %s\n", endptr);
- if (*endptr != '\0')
- fatal_error (UNKNOWN_LOCATION, "environment variable $SOURCE_DATE_EPOCH: "
- "trailing garbage: %s\n", endptr);
- if (epoch < 0)
- fatal_error (UNKNOWN_LOCATION, "environment variable $SOURCE_DATE_EPOCH: "
- "value must be nonnegative: %lld \n", epoch);
-
- return (time_t) epoch;
-}
-
/* Check and possibly warn if two declarations have contradictory
attributes, such as always_inline vs. noinline. */
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 0ee9f56..63fd2b9 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1482,9 +1482,4 @@ extern bool valid_array_size_p (location_t, tree, tree);
extern bool cilk_ignorable_spawn_rhs_op (tree);
extern bool cilk_recognize_spawn (tree, tree *);
-/* Read SOURCE_DATE_EPOCH from environment to have a deterministic
- timestamp to replace embedded current dates to get reproducible
- results. Returns -1 if SOURCE_DATE_EPOCH is not defined. */
-extern time_t get_source_date_epoch (void);
-
#endif /* ! GCC_C_COMMON_H */
diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c
index 38a428d..5bab8d1 100644
--- a/gcc/c-family/c-lex.c
+++ b/gcc/c-family/c-lex.c
@@ -389,9 +389,6 @@ c_lex_with_flags (tree *value, location_t *loc, unsigned char *cpp_flags,
enum cpp_ttype type;
unsigned char add_flags = 0;
enum overflow_type overflow = OT_NONE;
- time_t source_date_epoch = get_source_date_epoch ();
-
- cpp_init_source_date_epoch (parse_in, source_date_epoch);
timevar_push (TV_CPP);
retry:
diff --git a/gcc/testsuite/gcc.dg/cpp/special/date-time.c b/gcc/testsuite/gcc.dg/cpp/special/date-time.c
new file mode 100644
index 0000000..3304b75
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/cpp/special/date-time.c
@@ -0,0 +1,5 @@
+/* { dg-do preprocess } */
+__DATE__
+__TIME__
+/* { dg-final { scan-file date-time.i "\"Jul 4 1978\"" } } */
+/* { dg-final { scan-file date-time.i "\"21:24:16\"" } } */
diff --git a/gcc/testsuite/gcc.dg/cpp/special/date-time.exp b/gcc/testsuite/gcc.dg/cpp/special/date-time.exp
new file mode 100644
index 0000000..3c43143
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/cpp/special/date-time.exp
@@ -0,0 +1,35 @@
+# Copyright (C) 1997-2016 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with GCC; see the file COPYING3. If not see
+# <http://www.gnu.org/licenses/>.
+
+# GCC testsuite that uses the `dg.exp' driver.
+
+load_lib gcc-dg.exp
+
+# If a testcase doesn't have special options, use these.
+global DEFAULT_CFLAGS
+if ![info exists DEFAULT_CFLAGS] then {
+ set DEFAULT_CFLAGS " -ansi -pedantic-errors"
+}
+
+setenv SOURCE_DATE_EPOCH 268435456
+
+# Initialize `dg'.
+dg-init
+
+dg-runtest $srcdir/$subdir/date-time.c "" $DEFAULT_CFLAGS
+
+# All done.
+dg-finish
diff --git a/libcpp/include/cpplib.h b/libcpp/include/cpplib.h
index 4998b3a..35b0375 100644
--- a/libcpp/include/cpplib.h
+++ b/libcpp/include/cpplib.h
@@ -784,9 +784,6 @@ extern void cpp_init_special_builtins (cpp_reader *);
/* Set up built-ins like __FILE__. */
extern void cpp_init_builtins (cpp_reader *, int);
-/* Initialize the source_date_epoch value. */
-extern void cpp_init_source_date_epoch (cpp_reader *, time_t);
-
/* This is called after options have been parsed, and partially
processed. */
extern void cpp_post_options (cpp_reader *);
diff --git a/libcpp/init.c b/libcpp/init.c
index f5ff85b..8f21d3d 100644
--- a/libcpp/init.c
+++ b/libcpp/init.c
@@ -533,11 +533,44 @@ cpp_init_builtins (cpp_reader *pfile, int hosted)
_cpp_define_builtin (pfile, "__OBJC__ 1");
}
+/* Read SOURCE_DATE_EPOCH from environment to have a deterministic
+ timestamp to replace embedded current dates to get reproducible
+ results. Returns -1 if SOURCE_DATE_EPOCH is not defined. */
+static time_t
+get_source_date_epoch (cpp_reader *pfile)
+{
+ char *source_date_epoch;
+ long long epoch;
+ char *endptr;
+
+ source_date_epoch = getenv ("SOURCE_DATE_EPOCH");
+ if (!source_date_epoch)
+ return (time_t) -1;
+
+ errno = 0;
+ epoch = strtoll (source_date_epoch, &endptr, 10);
+ if ((errno == ERANGE && (epoch == LLONG_MAX || epoch == LLONG_MIN))
+ || (errno != 0 && epoch == 0))
+ cpp_error (pfile, CPP_DL_FATAL, "environment variable $SOURCE_DATE_EPOCH: "
+ "strtoll: %s\n", xstrerror(errno));
+ if (endptr == source_date_epoch)
+ cpp_error (pfile, CPP_DL_FATAL, "environment variable $SOURCE_DATE_EPOCH: "
+ "no digits were found: %s\n", endptr);
+ if (*endptr != '\0')
+ cpp_error (pfile, CPP_DL_FATAL, "environment variable $SOURCE_DATE_EPOCH: "
+ "trailing garbage: %s\n", endptr);
+ if (epoch < 0)
+ cpp_error (pfile, CPP_DL_FATAL, "environment variable $SOURCE_DATE_EPOCH: "
+ "value must be nonnegative: %lld \n", epoch);
+
+ return (time_t) epoch;
+}
+
/* Initialize the source_date_epoch value. */
-void
-cpp_init_source_date_epoch (cpp_reader *pfile, time_t source_date_epoch)
+static void
+cpp_init_source_date_epoch (cpp_reader *pfile)
{
- pfile->source_date_epoch = source_date_epoch;
+ pfile->source_date_epoch = get_source_date_epoch (pfile);
}
/* Sanity-checks are dependent on command-line options, so it is
@@ -600,6 +633,9 @@ cpp_post_options (cpp_reader *pfile)
{
int flags;
+ /* Initialize the source_date_epoch value. */
+ cpp_init_source_date_epoch (pfile);
+
sanity_checks (pfile);
post_options (pfile);
--
2.8.1.369.geae769a
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] Ensure source_date_epoch is always initialised
2016-05-19 13:07 ` [PATCH v2] " James Clarke
@ 2016-05-24 11:07 ` Dhole
2016-05-24 11:07 ` James Clarke
0 siblings, 1 reply; 5+ messages in thread
From: Dhole @ 2016-05-24 11:07 UTC (permalink / raw)
To: James Clarke; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1053 bytes --]
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.
In my latest patch it's:
-2: no yet set
-1: disabled
non-negative: use use this value SOURCE_DATE_EPOCH
[1] https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01026.html
Cheers,
--
Dhole
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] Ensure source_date_epoch is always initialised
2016-05-24 11:07 ` Dhole
@ 2016-05-24 11:07 ` James Clarke
2016-05-24 12:32 ` Dhole
0 siblings, 1 reply; 5+ messages in thread
From: James Clarke @ 2016-05-24 11:07 UTC (permalink / raw)
To: Dhole; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1373 bytes --]
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.
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.
Regards,
James
[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] Ensure source_date_epoch is always initialised
2016-05-24 11:07 ` James Clarke
@ 2016-05-24 12:32 ` Dhole
0 siblings, 0 replies; 5+ messages in thread
From: Dhole @ 2016-05-24 12:32 UTC (permalink / raw)
To: James Clarke; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2924 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-05-24 12:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-19 0:02 [PATCH] Ensure source_date_epoch is always initialised James Clarke
2016-05-19 13:07 ` [PATCH v2] " James Clarke
2016-05-24 11:07 ` Dhole
2016-05-24 11:07 ` James Clarke
2016-05-24 12:32 ` Dhole
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).