Re: [XEN PATCH 2/5] docs: set DATE to SOURCE_DATE_EPOCH if available

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 01/01/2025 7:03 pm, Maximilian Engelhardt wrote:
> On Montag, 30. Dezember 2024 23:28:42 CET Maximilian Engelhardt wrote:
>> On Montag, 30. Dezember 2024 22:38:24 CET Andrew Cooper wrote:
>>> On 30/12/2024 9:00 pm, Maximilian Engelhardt wrote:
>>>> Use the solution described in [1] to replace the call to the 'date'
>>>> command with a version that uses SOURCE_DATE_EPOCH if available. This
>>>> is needed for reproducible builds.
>>>>
>>>> The -d "@..." syntax was introduced in GNU date about 2005 (but only
>>>> added to the docuemntation in 2011), so I assume a version supporting
>>>> this syntax is available, if SOURCE_DATE_EPOCH is defined. If
>>>> SOURCE_DATE_EPOCH is not defined, nothing changes with respect to the
>>>> current behavior.
>>>>
>>>> [1] https://reproducible-builds.org/docs/source-date-epoch/
>>>>
>>>> Signed-off-by: Maximilian Engelhardt <maxi@xxxxxxxxxxxxx>
>>>> ---
>>>>
>>>>  docs/Makefile | 8 +++++++-
>>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/docs/Makefile b/docs/Makefile
>>>> index b30cc619f8..beba02a94f 100644
>>>> --- a/docs/Makefile
>>>> +++ b/docs/Makefile
>>>> @@ -3,7 +3,13 @@ include $(XEN_ROOT)/Config.mk
>>>>
>>>>  -include $(XEN_ROOT)/config/Docs.mk
>>>>  
>>>>  VERSION           := $(shell $(MAKE) -C $(XEN_ROOT)/xen --no-print-
> directory
>>>>  xenversion)>
>>>>
>>>> -DATE              := $(shell date +%Y-%m-%d)
>>>> +
>>>> +DATE_FMT  := +%Y-%m-%d
>>>> +ifdef SOURCE_DATE_EPOCH
>>>> +DATE              := $(shell date -u -d "@$(SOURCE_DATE_EPOCH)" 
>>>> "$(DATE_FMT)"
>>>> 2>/dev/null || date -u -r "$(SOURCE_DATE_EPOCH)" "$(DATE_FMT)"
>>>> 2>/dev/null || date -u "$(DATE_FMT)") +else
>>>> +DATE              := $(shell date "$(DATE_FMT)")
>>>> +endif
>>>>
>>>>  DOC_ARCHES      := arm x86_32 x86_64
>>>>  MAN_SECTIONS    := 1 5 7 8
>>> While this looks fine for docs, there's another (identical) use of date
>>> in tools/firmware/hvmloader/Makefile, as well as some differing uses to
>>> construct XEN_BUILD_{DATE,TIME}.  INSTALL talks about VGABIOS_REL_DATE
>>> too.
>>>
>>> Does something like this work for you?  It seems to DTRT for SMBIOS.  It
>>> needs adapting a bit more for vgabios and Xen, but I think having one
>>> common $(date) is going to be better than ad-hoc ones over the tree.
>>>
>>> ~Andrew
>> Hi Andrew,
>>
>> Thanks for your quick reply. Your patch looks fine to me. You can add my
>> Tested-by.
>>
>> We currently use "export XEN_BUILD_{DATE,TIME}=...", "export
>> SMBIOS_REL_DATE=..." and "export VGABIOS_REL_DATE=..." for building xen in
>> Debian, so we did not run into reproducibility problems with these. But
>> having them combined to all use SOURCE_DATE_EPOCH if available sounds like
>> a good idea and would also benefit other downstream users.
>>
>> Maxi
> Hi Andrew,
>
> I extended your patch to also cover the other uses of date. Please check if 
> this look reasonable as I'm not an expert in makefiles. It seems to DTRT in 
> the cases I tested.
>
> What I changed compared to your patch:
>
> * Add LC_ALL=C to all date commands. This was also missing in my original 
> patch, but I think it's a good thing to do and XEN_BUILD_{DATE,TIME} already 
> do it.
>
> * Change the quoting to allow calling the date command without any additional 
> (formatting) arguments.
>
> * Add an include of Config.mk to tools/firmware/vgabios/Makefile and moved 
> the 
> definition of XEN_BUILD_{DATE,TIME} further down in xen/Makefile to have the 
> newly defined date wrapper available.
>
> Does this look reasonable or are there parts that should be done differently?

That looks good to me.

Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>