Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

do not embed version and timestamp information #90

Merged
merged 1 commit into from
Dec 16, 2019

Conversation

hannesm
Copy link
Contributor

@hannesm hannesm commented Dec 14, 2019

#54 introduced the timestamp date to honor the SOURCE_DATE_EPOCH. This is insufficient if the reproducibility goal is to reproduce under any LOCALE (since the output of date depends on the locale set in your environment).

From #54 Actually, I don't even remember why I chose to have the compilation date as part of the information contained in ocamlgraph. I could simply remove it., this is why I open this PR which removes the timestamp and version information from ocamlgraph.

If you think there's value in version or date informaion, I'm fine adjusting this PR to remove less. I (non-exhaustively) searched for users of Graph.Version. and couldn't find any. I usually install ocamlgraph via opam, and in this case the version information is carried by other means (META) already.

@hannesm
Copy link
Contributor Author

hannesm commented Dec 14, 2019

other possibilities than removing all of it:

  • remove date only (keep val version : string)
  • modify date to date -Ru (for locale- and timezone-independent output) -- though only -u is specified by posix, not sure how portable -R is (it works on FreeBSD -- Use RFC 2822 date and time output format)
  • set LANG/LC_ALL/ ... in Makefile to something default (C?) if SOURCE_DATE_EPOCH is set to avoid locale-dependent output

@backtracking
Copy link
Owner

I've asked colleagues that are heavy users of OCamlGraph about this. If they do not complain, I'll merge.

@backtracking
Copy link
Owner

They are not using Version. So I'll merge.

@backtracking backtracking merged commit e527c82 into backtracking:master Dec 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants