[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

This revision was automatically updated to reflect the committed changes. Closed by commit rG2c090162746a: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135045/new/

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

ychen accepted this revision as: ychen. ychen added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135045/new/ https://reviews.llvm.org/D135045

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay updated this revision to Diff 466891. MaskRay marked an inline comment as done. MaskRay added a comment. Don't test SOURCE_DATE_EPOCH=253402300799 on Windows (which only supports upto 32536850399) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay marked 2 inline comments as done. MaskRay added inline comments. Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1612 + time_t TT = *getPreprocessorOpts().SourceDateEpoch; + std::tm *TM = std::gmtime(); Result = asctime(TM); ychen wrote:

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

ychen added inline comments. Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1612 + time_t TT = *getPreprocessorOpts().SourceDateEpoch; + std::tm *TM = std::gmtime(); Result = asctime(TM); MaskRay wrote: > ychen wrote: > > To be consistent with

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay marked an inline comment as done. MaskRay added inline comments. Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1612 + time_t TT = *getPreprocessorOpts().SourceDateEpoch; + std::tm *TM = std::gmtime(); Result = asctime(TM); ychen wrote:

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

ychen added a comment. Looks good to me except for one nit. And the test still fails on Windows for some reason. Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1612 + time_t TT = *getPreprocessorOpts().SourceDateEpoch; + std::tm *TM = std::gmtime(); Result =

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

emaste added a comment. > The time_t value may be negative True, but we do not need to allow a negative `SOURCE_DATE_EPOCH` though. The spec implies that it must be nonnegative, although doesn't say so explicitly. https://reproducible-builds.org/specs/source-date-epoch/ Repository: rG LLVM

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay marked an inline comment as done. MaskRay added inline comments. Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1092 +TT = *PP.getPreprocessorOpts().SourceDateEpoch; +TM = std::gmtime(); + } else { ychen wrote: > MaskRay wrote: > > ychen wrote:

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay updated this revision to Diff 465649. MaskRay marked an inline comment as done. MaskRay edited the summary of this revision. MaskRay added a comment. Use `??? ?? ` or `??:??:??` is gmtime/localtime returns null Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

ychen added inline comments. Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1092 +TT = *PP.getPreprocessorOpts().SourceDateEpoch; +TM = std::gmtime(); + } else { MaskRay wrote: > ychen wrote: > > MaskRay wrote: > > > ychen wrote: > > > > Why not use

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay marked 2 inline comments as done. MaskRay added inline comments. Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1092 +TT = *PP.getPreprocessorOpts().SourceDateEpoch; +TM = std::gmtime(); + } else { ychen wrote: > MaskRay wrote: > > ychen wrote:

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

ychen added inline comments. Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1092 +TT = *PP.getPreprocessorOpts().SourceDateEpoch; +TM = std::gmtime(); + } else { MaskRay wrote: > ychen wrote: > > Why not use localtime as the else branch? > > > > As

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay marked an inline comment as done. MaskRay added inline comments. Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1092 +TT = *PP.getPreprocessorOpts().SourceDateEpoch; +TM = std::gmtime(); + } else { ychen wrote: > Why not use localtime as the

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay updated this revision to Diff 465501. MaskRay marked an inline comment as done. MaskRay added a comment. Make error message precise Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135045/new/ https://reviews.llvm.org/D135045 Files:

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

ychen added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4313 +// 253402300799 is the UNIX timestamp of -12-31T23:59:59Z. +if (StringRef(Epoch).getAsInteger(10, V) || V > 253402300799) +

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay updated this revision to Diff 464815. MaskRay added a comment. Use `%if clang-target-64-bits` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135045/new/ https://reviews.llvm.org/D135045 Files: clang/docs/ReleaseNotes.rst

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

ychen added a comment. The Windows premerge failure looks relevant. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135045/new/ https://reviews.llvm.org/D135045 ___ cfe-commits mailing list

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay updated this revision to Diff 464581. MaskRay edited the summary of this revision. MaskRay added a comment. Herald added a reviewer: ctetreau. https://wiki.debian.org/ReproducibleBuilds/TimestampsFromCPPMacros says > We missed out __TIMESTAMP__ by mistake, but we will probably send in

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

MaskRay created this revision. MaskRay added reviewers: clang-language-wg, aaron.ballman, appcs, efriedma, emaste. Herald added a subscriber: StephenFan. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.