[PATCH cygport] Add more checks of SOURCE_DATE_EPOCH

[PATCH cygport] Add more checks of SOURCE_DATE_EPOCH

[Christian Franke]

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: 0001-Add-more-checks-of-SOURCE_DATE_EPOCH.patch --]
[-- Type: text/plain, Size: 1601 bytes --]

From b04c8f5e9becd6e91095e2add551f72870c9e869 Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Fri, 16 Feb 2024 13:16:06 +0100
Subject: [PATCH] Add more checks of SOURCE_DATE_EPOCH

Fail if it is out of range.  Warn if it lies in the future.
Inform whether it is set or set but not exported.
---
 bin/cygport.in | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/bin/cygport.in b/bin/cygport.in
index 5fc89eaf..3fe8a52e 100755
--- a/bin/cygport.in
+++ b/bin/cygport.in
@@ -493,14 +493,32 @@ unset restrict
 
 if [ "${SOURCE_DATE_EPOCH+y}" = "y" ]
 then
-	if [ -n "$(echo "${SOURCE_DATE_EPOCH}" | sed -e 's/^$/X/' -e 's/[0-9]//g')" ]
+	if ! [[ "${SOURCE_DATE_EPOCH}" =~ ^[0-9]+$ ]]
 	then
 		error "SOURCE_DATE_EPOCH must be an integer number (seconds since the unix epoch)"
 	fi
+	if ! _d=$(date --iso-8601=seconds -d "@${SOURCE_DATE_EPOCH}" 2>/dev/null)
+	then
+		error "SOURCE_DATE_EPOCH is out of range"
+	fi
+	if [ "${SOURCE_DATE_EPOCH}" -le "$(printf '%(%s)T')" ]
+	then
+		inform "Reproducible build: SOURCE_DATE_EPOCH='${SOURCE_DATE_EPOCH}' [$_d]"
+	else
+		warning "SOURCE_DATE_EPOCH='${SOURCE_DATE_EPOCH}' [$_d] lies in the future"
+	fi
+	unset _d
+	if [ -z "$(printenv SOURCE_DATE_EPOCH 2>/dev/null)" ]
+	then
+		inform "SOURCE_DATE_EPOCH is not exported to the environment"
+	fi
+
 	case $(peflags --version 2>/dev/null | sed -n '1s/^.* //p') in
 		4.6.[6-9]|4.[7-9]*|[5-9]*) ;;
 		*) error "SOURCE_DATE_EPOCH requires peflags 4.6.6 or later"
 	esac
+else
+	inform "SOURCE_DATE_EPOCH is not set"
 fi
 
 
-- 
2.43.0

Re: [PATCH cygport] Add more checks of SOURCE_DATE_EPOCH

[Jon Turney]

On 16/02/2024 12:29, Christian Franke via Cygwin-apps wrote:
> Fail if it is out of range. Warn if it lies in the future. Inform 
> whether it is set or set but not exported.

What is the valid range here?

Would it not make more sense to just re-export it if set? (so that 
commands like "SOURCE_DATE_EPOCH=something cygport foo" work as expected?)

Re: [PATCH cygport] Add more checks of SOURCE_DATE_EPOCH

[Christian Franke]

Jon Turney wrote:
> On 16/02/2024 12:29, Christian Franke via Cygwin-apps wrote:
>> Fail if it is out of range. Warn if it lies in the future. Inform 
>> whether it is set or set but not exported.
>
> What is the valid range here?

The range accepted by 'date -d @EPOCH ...', later used to adjust the 
patch timestamps.

The test could also be removed as date interestingly accepts a (too) 
wide range of values :-)

$ date -d @$((1<<55))
Sun Jun 13 08:26:08 CEST 1141709097

$ date -d @$((1<<56))
date: time ‘72057594037927936’ is out of range

$ date -d @$((-(1<<55)))
Sun Jul 20 18:27:20 LMT -1141705158

$ date -d @$((-(1<<56)))
date: time ‘-72057594037927936’ is out of range


> Would it not make more sense to just re-export it if set?

If the cygport file decides to set but not export it, there is possibly 
no need to do it. An example is smartmontools.cygport which passes the 
unexported variable as a parameter to configure.


> (so that commands like "SOURCE_DATE_EPOCH=something cygport foo" work 
> as expected?)
>

Would make no difference as the 'VAR=val CMD...' syntax already exports 
the variable to the CMD:

$ unset FOO; FOO=bar sh -c 'sh -c "sh -c printenv\ FOO"'
bar

Re: [PATCH cygport] Add more checks of SOURCE_DATE_EPOCH

[Jon Turney]

On 26/02/2024 19:53, Christian Franke via Cygwin-apps wrote:
> 
>> Would it not make more sense to just re-export it if set?
> 
> If the cygport file decides to set but not export it, there is possibly 
> no need to do it. An example is smartmontools.cygport which passes the 
> unexported variable as a parameter to configure.

Ok, but exporting it is harmless there, right?

>> (so that commands like "SOURCE_DATE_EPOCH=something cygport foo" work 
>> as expected?)
>>
> 
> Would make no difference as the 'VAR=val CMD...' syntax already exports 
> the variable to the CMD:
> 
> $ unset FOO; FOO=bar sh -c 'sh -c "sh -c printenv\ FOO"'
> bar
> 
Ah, right.

So you seem to be saying that the only situation where it's set but not 
exported is where it's set in the cygport.

So we're just making people (need to remember to) explicitly write 
"export SOURCE_DATE_EPOCH" in their cygport where needed?

Re: [PATCH cygport] Add more checks of SOURCE_DATE_EPOCH

[Christian Franke]

Jon Turney wrote:
> On 26/02/2024 19:53, Christian Franke via Cygwin-apps wrote:
>>
>>> Would it not make more sense to just re-export it if set?
>>
>> If the cygport file decides to set but not export it, there is 
>> possibly no need to do it. An example is smartmontools.cygport which 
>> passes the unexported variable as a parameter to configure.
>
> Ok, but exporting it is harmless there, right?

I'm not aware of any corner cases where exporting would break something. 
But leaving the decision to the user would allow to handle such cases. 
It would also allow to check whether it makes a difference and if yes, 
which files are affected.


>
>
>>> (so that commands like "SOURCE_DATE_EPOCH=something cygport foo" 
>>> work as expected?)
>>>
>>
>> Would make no difference as the 'VAR=val CMD...' syntax already 
>> exports the variable to the CMD:
>>
>> $ unset FOO; FOO=bar sh -c 'sh -c "sh -c printenv\ FOO"'
>> bar
>>
> Ah, right.
>
> So you seem to be saying that the only situation where it's set but 
> not exported is where it's set in the cygport.
>
> So we're just making people (need to remember to) explicitly write 
> "export SOURCE_DATE_EPOCH" in their cygport where needed?

Exactly.