After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 758148 - Please support timestamps from environment
Please support timestamps from environment
Status: RESOLVED FIXED
Product: libxslt
Classification: Platform
Component: general
unspecified
Other All
: Normal enhancement
: ---
Assigned To: Daniel Veillard
libxml QA maintainers
Depends on:
Blocks:
 
 
Reported: 2015-11-15 22:44 UTC by Eduard Sanou
Modified: 2016-05-18 06:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Replace date timestamp by SOURCE_DATE_EPOCH env var (1.42 KB, text/plain)
2015-11-15 22:44 UTC, Eduard Sanou
Details

Description Eduard Sanou 2015-11-15 22:44:23 UTC
Created attachment 315641 [details]
Replace date timestamp by SOURCE_DATE_EPOCH env var

While working on the “reproducible builds” effort [1], we have noticed
that libxslt embeds timestamps when generating documentation.

We have a proposal for using a deterministic timestamp [2] (that can be set from a changelog or version control commit timestamp when building the package in a distribution; we use the latest debian/changelog entry in Debian) which is contained in the environment variable SOURCE_DATE_EPOCH (that can be exported by the building framework).

The attached patch proposes a way to use this variable to get
reproducible timestamps when generating docs, if the variable has been
set (if not, it falls back to the old behaviour).


 [1]: https://wiki.debian.org/ReproducibleBuilds
 [2]: https://reproducible-builds.org/specs/source-date-epoch/
Comment 1 Mattia Rizzolo 2016-01-07 10:58:30 UTC
Hi Daniel!

did you find some time to look at this too? :)
Comment 2 Mattia Rizzolo 2016-05-16 10:55:32 UTC
Hey Daniel!

So, it seems you're working on finishing 1.1.29, and I wonder if you could include this (or at least comment on it!) :)
Comment 3 Daniel Veillard 2016-05-16 15:54:35 UTC
Okay one more thing to do before release time !

 thanks for the heads-up !

Daniel
Comment 4 Daniel Veillard 2016-05-17 03:22:50 UTC
So your patch:

   - leaks ret data on returning NULL
   - doesn't use gmtime_r when available as code 20 lines below in same routine 
     does
   - seems very fragile in case of user error setting up the variable
   - the dependency on erro being available need to be expressed

so a convenience for packager ends up as a runtime risk for users, bad tradeoff
TBH. I hope you didn't patch soo many packages with similar code.

I'm still supportive of the concept, so I rewrote your patch and commited
the modified version, this should be in 1.1.29:

https://git.gnome.org/browse/libxslt/commit/?id=e57df303eca25a2a3f9e0625c29f4b20177858cc

If you could double check it works for you,

  thanks !

Daniel
Comment 5 Eduard Sanou 2016-05-17 19:06:15 UTC
Hi Daniel,

First of all, thanks for committing the rewrite of the patch!

Your patch looks good, sorry for the mistakes I did.  We have a comment to make regarding the case where SOURCE_DATE_EPOCH contains an invalid value: According to the spec [1]:
"If the value is malformed, the build process SHOULD exit with a non-zero error code."

I'm aware that my patch didn't do this either (moreover, it had a memory leak).  By looking at libexslt/date.c, and not being familiar with the library code, I haven't seen a straightforward way to error at exsltDateCurrent so that the execution terminates.  Do you think this behavior could be implemented?  Maybe the fact that this is a library makes it harder.

[1] https://reproducible-builds.org/specs/source-date-epoch/
Comment 6 Eduard Sanou 2016-05-17 19:26:20 UTC
I forgot to mention, I think that in your patch, "override" is only declared when HAVE_ERRNO_H is defined, but then it's read unconditionally

if (override == 0) {
...

Maybe the line

int override = 0;

should be outside the

+#ifdef HAVE_ERRNO_H
+    char *source_date_epoch;
+    int override = 0;
+#endif /* HAVE_ERRNO_H */


Also, we [the reproducible builds folk] have discussed a bit the issue regarding malformed SOURCE_DATE_EPOCH values.  Our suggestion, considering that this is a library, would be to return NULL (freeing ret this time :P) and maybe set errno to -1, so that callers to the library function can check if the function succeeded or not.  What do you think about this?
Comment 7 Daniel Veillard 2016-05-18 06:47:07 UTC
   Hi Eduard,

w.r.t. exiting the XSLT process doesn't sound right or possible, it's a SHOULD
so well need to be able to live with the current behaviour. The library
can't exit, and there is no good way to process this up to the upper layers.
Returning NULL can't work, this is code executed from XSLT there is no such
thing as error handling there, there is no errno, there is a missing value on
XSLT stack of the XSLT interpretor. No way the process who started the XSLT
transform (by forking xsltproc or whatever) will be able to capture that.
You must return a value, or the user will just see an error down the line
that he will in no way associate with SOURCE_DATE_EPOCH, he will just think
there is a bug in the DocBook XSLT or something.
To get an idea, I suggest you run xsltproc -v on some of the docbook transform
used for docs, save those in a log and try to find something in there, no normal
packager will ever go debug this.

However, you are right with override, I'm fixing this ASAP, that's a serious
portability issue,

https://git.gnome.org/browse/libxslt/commit/?id=8f9303f3b2a73523767b158f9be8dbf2a89ba3a2

   thanks !

Daniel