I'm sorry I'm a little late but I have a couple of minor comments
on the patch:
+ 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);
In most texts (e.g. the C and POSIX standards), the name of
an environment variable doesn't include the dollar sign. In
other diagnostic messages GCC doesn't print one. I suggest
to follow the established practice and remove the dollar sign
from this error message as well.
I would also suggest to issue a single generic error message
explaining what the valid value of the variable is instead of
trying to describe what's wrong with it, for example as follows
(note also the hyphen in "non-negative" which is the prevalent
style used by other GCC messages and GNU documentation).
"environment variable SOURCE_DATE_EPOCH must expand to a non-
negative integer less than or equal to %qlli", LLONG_MAX
One comment about the documentation:
> +The value of @env{SOURCE_DATE_EPOCH} must be a UNIX timestamp,
> +defined as the number of seconds (excluding leap seconds) since
> +01 Jan 1970 00:00:00 represented in ASCII, identical to the output of
> +@samp{@command{date +%s}}.
The +%s option to the date command is a non-standard extension
that's not universally available. To avoid confusing users on
systems that don't support it I would suggest to either avoid
mentioning or to clarify that it's a Linux command.
Martin
