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

Add support for specifying the build date #6788

Merged
merged 1 commit into from
Sep 24, 2018
Merged

Add support for specifying the build date #6788

merged 1 commit into from
Sep 24, 2018

Conversation

peterhoeg
Copy link
Contributor

Currently the compiler gets the compilation date embedded into it - instead allow a date to be specified (would typically be the release date) so that subsequent compilations would not be different.

Further to #3973

Do you still require specs for something this small?

Tangibly, this will be used by NixOS/nixpkgs#47166 instead of the ugly replacement I'm doing over there.

@bew
Copy link
Contributor

bew commented Sep 24, 2018

Why would you want to set a specific build date?

@peterhoeg
Copy link
Contributor Author

To have a more deterministic build. On NixOS we generally set the date in the build environment to the start of the unix epoch.

Copy link
Member

@bcardiff bcardiff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually prefer to have the date/build number as additional information of the exact tag/commit because for a given release you might have multiple builds/packaging.

But I am ok to add this if it helps for some platforms.

@jhass
Copy link
Member

jhass commented Sep 24, 2018

Should this be CRYSTAL_CONFIG_BUILD_DATE?

@peterhoeg
Copy link
Contributor Author

I have changed the variable name to @jhass's suggestion.

I noticed that CircleCI fails on Darwin with this change. Is that expected? Also, do you require a test?

Copy link
Member

@bcardiff bcardiff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peterhoeg thanks. There is no test required for this.
Darwin is failing for unrelated reasons (llvm7 support will fix that).

@bcardiff bcardiff merged commit 4abd8d7 into crystal-lang:master Sep 24, 2018
@bcardiff bcardiff added this to the 0.27.0 milestone Sep 24, 2018
@peterhoeg peterhoeg deleted the f/build_date branch September 24, 2018 15:22
@peterhoeg peterhoeg restored the f/build_date branch September 24, 2018 16:22
@peterhoeg peterhoeg deleted the f/build_date branch October 2, 2018 05:11
ezrast pushed a commit to ezrast/crystal that referenced this pull request Oct 2, 2018
@peterhoeg peterhoeg restored the f/build_date branch October 2, 2018 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants