Hi,

> +++ libisoburn-1.3.2/libisoburn/isoburn.c

Oh nostalgy. :))

-------------------------------------------------------------------------

General questions:

Is it really desirable/correct to override the timestamps of files when
they get put into the ISO image ? I.e. isn't reproducible build supposed to
be identical only if the input is identical ?

If desirable: What about other file attributes which might change without
file content change ?

The question arises whether we need a general overrider for file dates.
xorriso generic commands could be used to set attrbutes and times after
grafting the files into the upcoming ISO. debian-cd normally uses only
one generic xorriso command: "-as" with first parameter "mkisofs".
The parameter list of this command "-as" would end at the first "--"
parameter. Afterwards one could perform commands like
-alter_date_r , -chmod_r , -chown_r , -chgrp_r in order to force file
attributes.

Currently it seems to me that xorriso lacks only two features:

- User defined  target->now  in libisofs.
  (Its use seems to be overrideable by other libisofs features.
   I'm still verifying whether this is true and whether they are
   reachable via xorriso ...)

- User defined MBR "Disc signature" at bytes 440 to 443.

This does not mean that i would not see the convenience of a single
option which overrides all variable time stamps. But if we decide that
input file dates shall not be overridden, then we need a new plan anyway.


With which xorriso commands (or xorriso -as mkisofs options) did you test
whether the proposed changes suffice for reproducible ISOs ?
("A normal debian-cd run on arch XYZ" would be sufficient as answer.
 I just wonder whether there are more features like isohybrid pseudo-random
 which might not have been touched by your test run.)


Some detail thoughts on the patches:

-------------------------------------------------------------------------
> 0001-source_date_epoch.patch  (libisoburn)
-------------------------------------------------------------------------

> +    source_date_epoch = getenv("SOURCE_DATE_EPOCH");

This would be the first env variable to influence libisoburn behavior.
Very economic shortcut but probably inappropriate on the long run.
I will have to think about an API call and a xorriso command to control
this.


> +    epoch = strtoull(source_date_epoch, &endptr, 10); 

That would be the first usage of strtoull() in the libraries.
The man page looks like it is not always available. (More retro would
be to read the string as type double and then convert to some int type.)


> +time_t Xorriso__current_time()
> ...
> +    if ((errno == ERANGE && (epoch == ULLONG_MAX || epoch == 0))
> +            || (errno != 0 && epoch == 0))
> +        return now; 

Since the application wants a constant time, shouldn't we return some
constant default value in this case ? (Proposal for a value ? 1970 ?)


-------------------------------------------------------------------------
> 0002-set-default-timestamp.patch
-------------------------------------------------------------------------

Looks ok, except the pending decision about getenv().


-------------------------------------------------------------------------
> 0003-set-scdbackup_tag_time-from-source_date_epoch.patch
-------------------------------------------------------------------------

Ok.


-------------------------------------------------------------------------
> 0004-normalize-wday-yday.patch
-------------------------------------------------------------------------

Where did you get negative effects from not setting  erg->wday , erg->yday ?

The patch would look dangerous because of the volatility of the gmtime()
result but actually seems to be without lasting effect because erg is only
overwritten on the stack but not in the caller's variable space:

 int Decode_ecma119_format(struct tm *erg, char *text, int flag)
...
+ erg = gmtime(&normalized);

If the two members are really needed then i would not regenerate the whole
erg by gmtime() but only the two missing struct members:

    normalized_tm = gmtime(&normalized);
    erg->wday = normalized_tm->wday;
    erg->yday = normalized_tm->yday;

The Unix time functions are tricky and the purpose of the timestamp might
be the job of an id string when converted back to some time format.
So it seems best to keep all significant struct members as they are now.


-------------------------------------------------------------------------
> 0001-source_date_epoch.patch (for libisofs)
-------------------------------------------------------------------------

Like with 0001-source_date_epoch.patch(libisoburn) i am not happy
with the environment variable.

Maybe one should make the time producer function of libisofs public
in the API and use it from libisoburn too, instead of having a
separate Xorriso__current_time().


-------------------------------------------------------------------------
> 0002-set-target-now-from-source_date_epoch.patch
-------------------------------------------------------------------------

Ok.


-------------------------------------------------------------------------
> 0003-isohybrid-mbr.patch
-------------------------------------------------------------------------

This weakens the weak random number even in the case of non-constant time:

-    id = 0xffffffff & (tv.tv_sec ^ (tv.tv_usec * 2000));
+    id = 0xffffffff & iso_current_time();

So one would have to let iso_current_time() produce struct timeval rather
than time_t.

But i ponder whether employing a user defined time to make a random number
non-random isn't too much "hintenrum" (approach from behind), anyways.

-------------------------------------------------------------------------


Have a nice day :)

Thomas


_______________________________________________
Reproducible-builds mailing list
Reproducible-builds@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/reproducible-builds

Reply via email to