diff mbox

[U-Boot,Reproducible-builds] Reproducible U-Boot build support, using SOURCE_DATE_EPOCH

Message ID 87fv1vgb32.fsf@aikidev.net
State Accepted
Delegated to: Tom Rini
Headers show

Commit Message

Vagrant Cascadian Sept. 30, 2015, 3:50 p.m. UTC 
On 2015-09-28, Paul Kocialkowski wrote:
> Le jeudi 24 septembre 2015 à 09:05 -0700, Vagrant Cascadian a écrit :
>> I think the use of "time = mktime(time_universal);" is where the problem
>> lies:
>
> […]
>
>> According to the mktime manpage:
>> 
>>        The  mktime()  function converts a broken-down time structure,
>>        expressed as local time, to calendar time representation.  
>> 
>> So my interpetation is that it's taking the UTC time and converts it
>> into local time using the configured timezone... not sure what would be
>> a viable alternative to mktime.
>
> That seems to make sense. Come to think of it, it probably was not
> necessary to call gmtime in the first place: if SOURCE_DATE_EPOCH is
> always in UTC, we should be able to stick that as-is in the time
> variable. At best, gmtime + mktime (assuming mktime working in UTC)
> would give us back the same timestamp.
>
> What do you think? Please let me know if I'm wrong.

This patch on top of 2015.10-rc4 seems to resolve the issue for me:



It still checks for the validity of SOURCE_DATE_EPOCH using gmtime, but
doesn't call mktime at all, just re-uses the value set from
SOURCE_DATE_EPOCH.


live well,
  vagrant

Comments

Paul Kocialkowski Oct. 2, 2015, 10:19 a.m. UTC | #1 
Le mercredi 30 septembre 2015 à 08:50 -0700, Vagrant Cascadian a écrit :
> On 2015-09-28, Paul Kocialkowski wrote:
> > What do you think? Please let me know if I'm wrong.
> 
> This patch on top of 2015.10-rc4 seems to resolve the issue for me:
> 
> Index: u-boot/tools/default_image.c
> ===================================================================
> --- u-boot.orig/tools/default_image.c
> +++ u-boot/tools/default_image.c
> @@ -108,8 +108,6 @@ static void image_set_header(void *ptr,
>  			fprintf(stderr, "%s: SOURCE_DATE_EPOCH is not valid\n",
>  				__func__);
>  			time = 0;
> -		} else {
> -			time = mktime(time_universal);
>  		}
>  	} else {
>  		time = sbuf->st_mtime;
> 
> 
> It still checks for the validity of SOURCE_DATE_EPOCH using gmtime, but
> doesn't call mktime at all, just re-uses the value set from
> SOURCE_DATE_EPOCH.

That's a good plan! I guess we should also fully get rid of the
time_universal variable and make the check inline:

if (gmtime(&time) == NULL)

and of course drop the else statement.

Would you like to craft that patch for upstream U-Boot?
If not, I'd be happy to do it.

Thanks for your work!
diff mbox

Patch

Index: u-boot/tools/default_image.c
===================================================================
--- u-boot.orig/tools/default_image.c
+++ u-boot/tools/default_image.c
@@ -108,8 +108,6 @@  static void image_set_header(void *ptr,
 			fprintf(stderr, "%s: SOURCE_DATE_EPOCH is not valid\n",
 				__func__);
 			time = 0;
-		} else {
-			time = mktime(time_universal);
 		}
 	} else {
 		time = sbuf->st_mtime;