Project

General

Profile

Bug #12566

Feature #5630: Reproducible builds

ikiwiki image size specification makes the ISO build unreproducible

Added by intrigeri 4 months ago. Updated 3 months ago.

Status:
Resolved
Priority:
Normal
Assignee:
-
Category:
-
Target version:
Start date:
05/19/2017
Due date:
% Done:

100%

QA Check:
Pass
Feature Branch:
bugfix/12566-dont-resize-image
Type of work:
Code
Blueprint:
Easy:
Affected tool:

Description

In [[!img introduction_to_gnome_and_the_tails_desktop/keyboard.png size="267x" link="no" alt="Menu in the top-right corner of the desktop to switch between different keyboard layouts"]], size="267x" makes ikiwiki generate files called 267x-keyboard.png in an unreproducible manner.

Is this 267x thing correct, or a typo for 267px?


Related issues

Related to Tails - Feature #12625: Make Ikiwiki resize images deterministically In Progress 05/31/2017

Associated revisions

Revision 14a77e5a (diff)
Added by anonym 4 months ago

Don't resize image.

When Ikiwiki resizes it a timestamp is added which breaks
reproducibility.

Refs: #12625
Will-fix: #12566

Revision 49dd3889
Added by intrigeri 4 months ago

Merge remote-tracking branch 'origin/bugfix/12566-dont-resize-image' into testing (Fix-committed: #12566).

History

#1 Updated by intrigeri 4 months ago

  • Assignee changed from sajolida to anonym
  • Type of work changed from End-user documentation to Code

Actually that's valid ikiwiki syntax (https://ikiwiki.info/ikiwiki/directive/img/) so it's not sajolida's problem. Reassigning to the person committed to do the bulk of the work on the reproducible builds deliverable.

The generated PNG has an embedded timestamp.

#2 Updated by anonym 4 months ago

Ikiwiki does the resizeing in IkiWiki/Plugin/img.pm, using an interface to ImageMagick. I think Ikiwiki just have to tell PerlMagick to do the equivalent of convert +set date:create +set date:modify -define png:exclude-chunk=time to fix this.

Any way, it's so easy for us to fix this by simply replacing the current image with version rescaled to 267x252, so I cannot really justify looking deeper into this (if I was more fluent in Perl, so this wouldn't take a lot of time, I'd consider doing it, though). Bonus: we won't get an identical (modulo the time stamp) resized PNG generated for each locale we translate for.

#3 Updated by anonym 4 months ago

anonym wrote:

I think Ikiwiki just have to tell PerlMagick to do the equivalent of convert +set date:create +set date:modify -define png:exclude-chunk=time to fix this.

FWIW, from my super quick look it seems the two +set ... things can be accomplished with:

--- a/IkiWiki/Plugin/img.pm
+++ b/IkiWiki/Plugin/img.pm
@@ -174,6 +174,9 @@ sub preprocess (@) {
                my $r = $im->Read("$format:$srcfile\[$pagenumber]");
                error sprintf(gettext("failed to read %s: %s"), $file, $r) if $r;

+                $im->Set('date:create' => NULL);
+                $im->Set('date:modify' => NULL);
+
                if (! defined $im->Get("width") || ! defined $im->Get("height")) {
                        error sprintf(gettext("failed to get dimensions of %s"), $file);
                }

but perhaps we only want to do this if SOURCE_DATE_EPOCH is set (and then set it to that value instead of NULL?). And I couldn't find how to do -define png:exclude-chunk=time.

#4 Updated by intrigeri 4 months ago

Ikiwiki does the resizeing in IkiWiki/Plugin/img.pm, using an interface to ImageMagick. I think Ikiwiki just have to tell PerlMagick to do the equivalent of convert +set date:create +set date:modify -define png:exclude-chunk=time to fix this.

Thanks for diving into the (Perl) code!

Any way, it's so easy for us to fix this by simply replacing the current image with version rescaled to 267x252, so I cannot really justify looking deeper into this (if I was more fluent in Perl, so this wouldn't take a lot of time, I'd consider doing it, though).

Regarding the short-term: I agree in principle that this would be a perfectly acceptable short-term fix. But this image is used in two different places on the testing branch: in one case it's downscaled, in the other case it's displayed full size (just reading the source, I didn't check the built result). So "simply replacing the current image with version rescaled to 267x252" may not be OK. As far as 3.0 is concerned, we might have to duplicate this image (ugly, sad, but well, if we have a better long-term plan it's fine with me).

Regarding the long-term: in practice this means something like "when a tech writer introduces such a reproducibility issue, Foundation Team or RM will notice it and resize images". I expect it'll happen very rarely and won't be a big burden in general. But it might also happen at the worst possible time, e.g. when merging release notes with new screenshots into the stable branch, at the last minute before building the final ISO… and then it can be stressful for the RM to spend time on this and make the critical path to the release longer. I'd rather avoid adding such stress++ possibilities to the release process. I am fluent in Perl, I know the ikiwiki code base, we have time left on this project, and we promised to fix as much stuff as we could upstream: so I could attempt a proper fix, perhaps with a time limit set in advance, e.g. no more than 8 hours of work. What do you think?

Bonus: we won't get an identical (modulo the time stamp) resized PNG generated for each locale we translate for.

I'm not sure I get this point. In theory, translators should add a localized picture every time the screenshot is different in the language they're translating to, so ideally we want a different picture per language.

#5 Updated by anonym 4 months ago

  • Assignee changed from anonym to intrigeri
  • QA Check set to Info Needed

intrigeri wrote:

Any way, it's so easy for us to fix this by simply replacing the current image with version rescaled to 267x252, so I cannot really justify looking deeper into this (if I was more fluent in Perl, so this wouldn't take a lot of time, I'd consider doing it, though).

Regarding the short-term: I agree in principle that this would be a perfectly acceptable short-term fix. But this image is used in two different places on the testing branch: in one case it's downscaled, in the other case it's displayed full size (just reading the source, I didn't check the built result).

Indeed.

So "simply replacing the current image with version rescaled to 267x252" may not be OK. As far as 3.0 is concerned, we might have to duplicate this image (ugly, sad, but well, if we have a better long-term plan it's fine with me).

I propose this short term fix: let's drop the resizing and show the image in full size in both instances. I had a look, and showing the image in full size on the startup options page doesn't look weird to me at least.

Agreed?

Regarding the long-term: in practice this means something like "when a tech writer introduces such a reproducibility issue, Foundation Team or RM will notice it and resize images". I expect it'll happen very rarely and won't be a big burden in general. But it might also happen at the worst possible time, e.g. when merging release notes with new screenshots into the stable branch, at the last minute before building the final ISO… and then it can be stressful for the RM to spend time on this and make the critical path to the release longer. I'd rather avoid adding such stress++ possibilities to the release process.

Very good point.

I am fluent in Perl, I know the ikiwiki code base, we have time left on this project, and we promised to fix as much stuff as we could upstream: so I could attempt a proper fix, perhaps with a time limit set in advance, e.g. no more than 8 hours of work. What do you think?

Sounds good to me! Let me know if you want me to create this ticket for you!

Bonus: we won't get an identical (modulo the time stamp) resized PNG generated for each locale we translate for.

I'm not sure I get this point. In theory, translators should add a localized picture every time the screenshot is different in the language they're translating to, so ideally we want a different picture per language.

Got it, I wasn't aware of this.

#6 Updated by intrigeri 4 months ago

but perhaps we only want to do this if SOURCE_DATE_EPOCH is set (and then set it to that value instead of NULL?). And I couldn't find how to do -define png:exclude-chunk=time.

ikiwiki doesn't do SOURCE_DATE_EPOCH at all (yet), but:

  • look for if ($config{deterministic})
  • perhaps using Unix epoch is good enough
  • in some areas (when it's not particularly expensive) the output was made deterministic by default

#7 Updated by intrigeri 4 months ago

  • Assignee changed from intrigeri to anonym
  • QA Check changed from Info Needed to Dev Needed

I propose this short term fix: let's drop the resizing and show the image in full size in both instances.

ACK

I am fluent in Perl, I know the ikiwiki code base, we have time left on this project, and we promised to fix as much stuff as we could upstream: so I could attempt a proper fix, perhaps with a time limit set in advance, e.g. no more than 8 hours of work. What do you think?

Sounds good to me! Let me know if you want me to create this ticket for you!

Yes, please.

#8 Updated by intrigeri 4 months ago

  • Related to Feature #12625: Make Ikiwiki resize images deterministically added

#9 Updated by intrigeri 4 months ago

  • Subject changed from Weird ikiwiki image size specification makes the ISO build unreproducible to ikiwiki image size specification makes the ISO build unreproducible

#10 Updated by anonym 4 months ago

  • Subject changed from ikiwiki image size specification makes the ISO build unreproducible to Weird ikiwiki image size specification makes the ISO build unreproducible
  • Status changed from Confirmed to In Progress
  • % Done changed from 0 to 30
  • QA Check changed from Dev Needed to Ready for QA
  • Feature Branch set to bugfix/12566-dont-resize-image

intrigeri wrote:

I propose this short term fix: let's drop the resizing and show the image in full size in both instances.

ACK

Implemented in the feature branch. I'll just make sure this actually works before asking for it to be merged.

I am fluent in Perl, I know the ikiwiki code base, we have time left on this project, and we promised to fix as much stuff as we could upstream: so I could attempt a proper fix, perhaps with a time limit set in advance, e.g. no more than 8 hours of work. What do you think?

Sounds good to me! Let me know if you want me to create this ticket for you!

Yes, please.

#12625

#11 Updated by anonym 4 months ago

  • Subject changed from Weird ikiwiki image size specification makes the ISO build unreproducible to ikiwiki image size specification makes the ISO build unreproducible

#12 Updated by anonym 4 months ago

  • Assignee changed from anonym to intrigeri
  • % Done changed from 30 to 50

anonym wrote:

intrigeri wrote:

I propose this short term fix: let's drop the resizing and show the image in full size in both instances.

ACK

Implemented in the feature branch. I'll just make sure this actually works before asking for it to be merged.

Works for me locally! Please review'n'merge into testing!

#13 Updated by intrigeri 4 months ago

  • % Done changed from 50 to 60

Code review passes, built the website locally and indeed it looks OK without resizing. I've set up https://jenkins.tails.boum.org/job/reproducibly_build_Tails_ISO_bugfix-12566-dont-resize-image/ to check reproducibility.

#14 Updated by intrigeri 4 months ago

  • Status changed from In Progress to Fix committed
  • % Done changed from 60 to 100

#15 Updated by intrigeri 4 months ago

  • Assignee deleted (intrigeri)
  • QA Check changed from Ready for QA to Pass

#16 Updated by intrigeri 3 months ago

  • Status changed from Fix committed to Resolved

Also available in: Atom PDF