Project

General

Profile

Bug #6907

ikiwiki po plugin does not play well with inline directives

Added by BitingBird almost 5 years ago. Updated 8 days ago.

Status:
In Progress
Priority:
Normal
Assignee:
Category:
-
Target version:
Start date:
12/03/2018
Due date:
% Done:

100%

Estimated time:
4.00 h
QA Check:
Feature Branch:
Type of work:
Code
Blueprint:
Starter:
No
Affected tool:

Description

See screenshot in attachment.

  • Other symptoms: #9279, #10124, #9671. More such tickets are reported regularly.

broken-inline.png View (140 KB) sajolida, 04/11/2018 01:31 PM

ikiwiki-inline.zip (10.7 KB) sajolida, 04/20/2018 09:22 AM


Subtasks

Bug #16185: Please deploy ikiwiki po plugin fix from ticket #6907Resolved


Related issues

Related to Tails - Bug #9671: rss and atom feed of l10n news show po file source Resolved 07/03/2015
Related to Tails - Bug #9279: ISO file size unit is not translatable on the download page Resolved 04/26/2015
Related to Tails - Bug #10124: po_translatable_pages is unstable on boum.org Rejected 08/31/2015
Related to Tails - Feature #14588: Self-host our website Resolved 10/03/2018
Duplicated by Tails - Bug #7113: News titles are truncated on https://tails.boum.org/index.fr.html Duplicate 04/19/2014
Duplicated by Tails - Bug #9564: translated file doesn't get parsed in ikiwiki Duplicate 06/12/2015
Duplicated by Tails - Bug #12212: French version of https://tails.boum.org/news/ is broken Duplicate 02/03/2017
Duplicated by Tails - Bug #15032: Buggy “News” entries for non-English languages Duplicate 12/10/2017
Duplicated by Tails - Bug #12246: Untranslatable parts of documentation due to inline errors Duplicate 02/17/2017
Blocks Tails - Feature #15506: Core work 2018Q4: Foundations Team Confirmed 04/08/2018

History

#1 Updated by BitingBird over 4 years ago

  • Status changed from New to Confirmed

#2 Updated by intrigeri over 4 years ago

  • Assignee set to intrigeri
  • Type of work changed from Website to Code

That's a bug in the ikiwiki PO plugin. I'll take care of it, but will treat it as low-priority personally. Don't hesitate being faster than me :)

#3 Updated by intrigeri over 4 years ago

  • Subject changed from Problem with the "news" titles on Frensh version, and partly in :de and :pt to Recurring problems with the "news" titles on non-English homepage

#4 Updated by intrigeri over 4 years ago

  • Duplicated by Bug #7113: News titles are truncated on https://tails.boum.org/index.fr.html added

#5 Updated by sajolida about 4 years ago

  • Subject changed from Recurring problems with the "news" titles on non-English homepage to Truncated news titles on non-English homepage

#6 Updated by intrigeri about 3 years ago

  • Related to Bug #9671: rss and atom feed of l10n news show po file source added

#7 Updated by intrigeri about 3 years ago

  • Duplicated by Bug #9564: translated file doesn't get parsed in ikiwiki added

#8 Updated by intrigeri about 3 years ago

  • Subject changed from Truncated news titles on non-English homepage to ikiwiki po plugin does not play well with inline directives
  • Description updated (diff)
  • Assignee deleted (intrigeri)

Making this ticket (which is the oldest one we have about that ikiwiki bug) track the root cause of this problem. We have other tickets that track what looks like various symptoms of that bug (not sure what to do with them but people won't stop opening new ones anyway).

This bug is an old one, we've been seeing it from time to sime since years. And since years, I've tried to find a reliable reproducers for this bug a few times, but never managed to find one. One should instrument ikiwiki to debug it properly.

I doubt I'll have time to get to it any time soon. I think this ticket should have higher priority, though.

#9 Updated by intrigeri about 3 years ago

  • Description updated (diff)

#10 Updated by sajolida about 3 years ago

  • Related to Bug #9279: ISO file size unit is not translatable on the download page added

#11 Updated by sajolida about 3 years ago

  • Related to Bug #10124: po_translatable_pages is unstable on boum.org added

#12 Updated by sajolida about 3 years ago

Maybe Hole in the Roof?

#13 Updated by sajolida about 3 years ago

See also the files linked from #10309#note-2 to understand better how we're going to rely on inlines to factorize instructions in the installation assistant.

#14 Updated by sajolida about 3 years ago

While trying to reproduce #10124 I found out that cloning our repo and building the website was reproducing #9671 straight away (let's say using ikiwiki-cgi.setup on tag 1.5). Maybe it would be worth solving this one first as it's related and easy to reproduce?

#15 Updated by intrigeri about 3 years ago

  • Target version set to Hole in the Roof

sajolida wrote:

Maybe Hole in the Roof?

Totally qualifies, good catch.

#16 Updated by intrigeri about 3 years ago

sajolida wrote:

While trying to reproduce #10124 I found out that cloning our repo and building the website was reproducing #9671 straight away (let's say using ikiwiki-cgi.setup on tag 1.5).

Yeepee, a reproducer! Thanks a lot. Confirmed on current Debian unstable (3.20150614). Note to anyone wanting to work on this (or to report it upstream): one may need to modify the destdir in ikiwiki-cgi.setup after cloning, and before running ikiwiki. So "we" can now try and find someone with Perl skills (ikiwiki hacking experience would save them startup time but is not absolutely required) to work on it.

Maybe it would be worth solving this one first as it's related and easy to reproduce?

Yes (and then I'm curious to see if #9671 is just a symptom of #6907, or if one can fix it independently; everyone got what's my guess on it by now ;)

#17 Updated by sajolida about 3 years ago

  • Assignee set to sajolida

This is not assigned to anybody, so I'll take it and report it upstream with instructions to reproduce it. But my Perl skills will end up here :)

#18 Updated by sajolida over 2 years ago

Assignee:

Now I can't reproduce this anymore :(

I tried with both ikiwiki 3.20160121 and 3.20141016.2:

  1. git reset --hard 1.5
  2. Change destdir in ikiwiki-cgi.set to
    config/chroot_local-includes/usr/share/doc/tails/website.
  3. rm -r ./wiki/src/.ikiwiki
    ./config/chroot_local-includes/usr/shar/doc/tails/website/*
  4. ikiwiki -setup ikiwiki-cgi.setup -rebuild

And each time the resulting
config/chroot_local-includes/usr/share/doc/tails/website/security.de.rss
looked fine.

#19 Updated by intrigeri over 2 years ago

Now I can't reproduce this anymore :(

The content of the ikiwiki state directory is probably as important as the Git state and ikiwiki version, to reproduce it.

#20 Updated by sajolida over 2 years ago

  • Assignee deleted (sajolida)

I'm aware of this and that's why I'm removing wiki/src/.ikiwiki each time (for me that's the "ikiwiki state directory") and I really think I did that last time as well... I'll stay on the lookout for more occurrence of this but with no promise.

#21 Updated by anonym almost 2 years ago

  • Duplicated by Bug #12212: French version of https://tails.boum.org/news/ is broken added

#22 Updated by xin almost 2 years ago

  • Related to Bug #12246: Untranslatable parts of documentation due to inline errors added

#23 Updated by intrigeri 12 months ago

  • Duplicated by Bug #15032: Buggy “News” entries for non-English languages added

#24 Updated by sajolida 8 months ago

#25 Updated by sajolida 8 months ago

It seems to me that this is happening especially when we have two levels of inline:

  • /news
    • /news/version_3.6.1 inlined from /news
      • /news/version_3.5/manual_upgrade.inline inlined from /news/version_3.6.1

But I couldn't reproduce this with a prototype ikiwiki either :(

Did we ever see this happening with only one level of inline?

#26 Updated by sajolida 8 months ago

  • Related to deleted (Bug #12246: Untranslatable parts of documentation due to inline errors)

#27 Updated by sajolida 8 months ago

  • Duplicated by Bug #12246: Untranslatable parts of documentation due to inline errors added

#28 Updated by intrigeri 8 months ago

Did we ever see this happening with only one level of inline?

Yes, we see it often after a new blog post is published (even when we don't use inline in there).

#29 Updated by emmapeel 8 months ago

sajolida wrote:

But I couldn't reproduce this with a prototype ikiwiki either :(

I can reproduce this consistently on the page https://tails.boum.org/install/mac/dvd/index.es.html
some lines after the title: "Reiniciar en la memoria USB de Tails"

It is fixed if the inline: install/inc/steps/mac_startup_disks.inline is not translated to Spanish in wiki/src/install/inc/steps/restart_first_time.inline.es.po (or wiki/src/install/inc/steps/restart_second_time.inline.es.po)

#30 Updated by sajolida 8 months ago

  • Blocks Feature #15411: Core work 2018Q2 → 2018Q3: Technical writing added

#31 Updated by sajolida 8 months ago

Thanks a lot xin and Emma, I think this is a major finding!!!

https://tails.boum.org/install/mac/usb/index.es.html and https://tails.boum.org/install/mac/usb/index.fr.html are always broken on step 6/7. I tried rebuilding the website completetly and the problem occurs reliably (rm wiki/src/.ikiwiki and rm -r my destination folder).

The structure of the inlines on this page is as follow:

  • /install/mac/dvd
    • /install/inc/steps/restart_first_time.inline
      • /install/inc/steps/mac_startup_disks.inline
      • /install/inc/steps/not_at_all.inline
    • /install/inc/steps/install_final.inline
    • /install/inc/steps/restart_second_time.inline
      • /install/inc/steps/mac_startup_disks.inline
      • /install/inc/steps/not_at_all.inline
    • /install/inc/steps/create_persistence.inline

The inline that is broken on step 6/7 is mac_startup_disks.inline which is otherwise inlined correctly in step 4/7 (in English "Immediately press-and-hold", in Spanish "Inmediatamente aprieta-y-mantiene").

To go further I wrote a minimal ikiwiki repo that reproduces the problem every time. I'm putting the archive in attachment.

intrigeri: Can you confirm that we have a reproducer now and tell us what would be the next steps?

It might not be the same bug as always (as sometimes rebuilding the wiki fixes the issue) but at least now we have something clear to work on.

#32 Updated by sajolida 8 months ago

  • Assignee set to intrigeri
  • QA Check set to Info Needed

#33 Updated by intrigeri 8 months ago

https://tails.boum.org/install/mac/usb/index.es.html and https://tails.boum.org/install/mac/usb/index.fr.html are always broken on step 6/7.
I tried rebuilding the website completetly and the problem occurs reliably (rm wiki/src/.ikiwiki and rm -r my destination folder).

FTR the latter is currently not broken on our website but the former is (same on my local build) so it looks like the problem happens less reliably than you're suggesting.

To go further I wrote a minimal ikiwiki repo that reproduces the problem every time. I'm putting the archive in attachment.

intrigeri: Can you confirm that we have a reproducer now

Yes, I can reproduce the bug with this archive.

and tell us what would be the next steps?

Allocate some dev time (or find a volunteer) to find out why broken-imbricated-inlines/level-one/level-two.fr.po is included as-is (and not converted to Markdown) in broken-imbricated-inlines/level-one.fr.html. Given this currently breaks the installation assistant in at least one language, and there's now a chance we could fix this bug, I could work on it as part of my FT work. I'll see in a month when I can work on it, depending on how we're doing on our budget and team energy.

#34 Updated by intrigeri 8 months ago

  • QA Check changed from Info Needed to Dev Needed

#35 Updated by intrigeri 8 months ago

#36 Updated by sajolida 8 months ago

FTR the latter is currently not broken on our website but the former is (same on my local build) so it looks like the problem happens less reliably than you're suggesting.

Did you test translating install/inc/steps/mac_startup_disks.inline on
line 86 of install/inc/steps/restart_second_time.inline.fr.po?

#37 Updated by intrigeri 8 months ago

Did you test translating install/inc/steps/mac_startup_disks.inline on line 86 of install/inc/steps/restart_second_time.inline.fr.po?

Indeed, I did not. Doing so breaks the page.

#38 Updated by lamby 7 months ago

  • Assignee changed from intrigeri to lamby
  • Estimated time set to 4.00 h

Self-assigning ticket during meeting on tails-meeting with nod from intrigeri. Adding 4 hours to estimate/max-time as discussed.

#39 Updated by intrigeri 7 months ago

  • Target version changed from Hole in the Roof to Tails_3.8

#40 Updated by intrigeri 6 months ago

  • Target version changed from Tails_3.8 to Tails_3.9

#41 Updated by lamby 6 months ago

Just a heads-up that I:

  • Have been working on this issue
  • Can reproduce locally
  • Am creating a minimal testcase (otherwise my REPL for this is ~10 minutes!)

#42 Updated by intrigeri 6 months ago

#43 Updated by intrigeri 6 months ago

#44 Updated by lamby 6 months ago

  • Assignee changed from lamby to intrigeri
  • % Done changed from 0 to 100
  • QA Check changed from Dev Needed to Ready for QA

#45 Updated by intrigeri 5 months ago

  • Status changed from Confirmed to In Progress
  • % Done changed from 100 to 80
  • QA Check deleted (Ready for QA)
  • Type of work changed from Code to Communicate

lamby wrote:

Fixed, patch sent upstream: https://ikiwiki.info/todo/Re-use_translated_content_instead_of_skipping_if_previously_translated/

Excellent! Reviewed and approved there (I created the bug in the first place, mind you :)

I'll handle the next steps i.e. ping upstream if needed and ensure this gets deployed on our production website (possibly without waiting for the upstream release and package update, no idea which version boum.org runs in production).

#46 Updated by intrigeri 4 months ago

  • Target version changed from Tails_3.9 to Tails_3.10.1

I'll ping upstream in ~1 month, early in the 3.10 cycle.

#48 Updated by intrigeri 2 months ago

#49 Updated by intrigeri 2 months ago

#50 Updated by intrigeri 2 months ago

  • Assignee changed from intrigeri to lamby
  • Target version changed from Tails_3.10.1 to Tails_3.11

Actually, if you don't mind, I'd rather see you track this and keep pinging upstream: you're doing it just as well, if not better, than I would :) Feel free to batch this with similar tasks that are now mostly blocked on pinging upstream, this will make it slightly easier to clock non-ridiculous amounts of time.

Last ping was on Aug 24 so postponing to 3.11.

#51 Updated by intrigeri 2 months ago

#52 Updated by intrigeri 2 months ago

Also, by mid-November #14588 should be done, so if this is not merged upstream by then, I think we should run a custom ikiwiki with this patch applied => once we're there, don't hesitate filing a ticket for sysadmins about it.

#53 Updated by lamby about 2 months ago

I've:

  • Refreshed the patch against the latest version in Debian (as well as added a missing function prototype)
  • Also asked the Debian maintainer(s) to poke upstream on this and whether it could be
    applied in Debian in the meantime.

#54 Updated by lamby about 2 months ago

  • Assignee changed from lamby to intrigeri

In https://bugs.debian.org/911356#17:

If you can put together a regression test for this bug that renders a
translated page and inspects the HTML output (t/img.t, t/meta.t, or a
combination of t/po.t and t/render.t might be a good basis) that would
make me a lot more confident about accepting patches. At the moment I
don't think we have any "full stack" test coverage that actually
renders HTML from a translated page.

I believe would need a few hours assigned to run with this all the way through so assigning over to you for approval.

#55 Updated by lamby about 1 month ago

Gentle ping on this intri.

#56 Updated by intrigeri about 1 month ago

I believe would need a few hours assigned to run with this all the way through so assigning over to you for approval.

I have good news! hefee's feature/15355-po-plugin-disable-languages branch (https://salsa.debian.org/hefee/ikiwiki.git) adds infrastructure for the "full stack" test smcv requires. That branch is not ready to be submitted upstream yet for reasons that are unrelated to the test suite. But I think it will save you quite some time :) I think this should fit into 2h but as usual feel free to request more if/when you notice it's not going to be sufficient.

So I suggest you:

  1. import the relevant bits of hefee's t/po-integration.t into your branch (I say forget the Git history, just copy'n'paste)
  2. integrate into this new integration test the test case you came up with earlier on this ticket
  3. submit upstream
  4. tell hefee and Ulrike about it so that 1. they don't duplicate your additional work; 2. they're not confused next time they merge upstream master

#57 Updated by intrigeri about 1 month ago

  • Assignee changed from intrigeri to lamby
  • Estimated time changed from 4.00 h to 6.00 h

#58 Updated by intrigeri about 1 month ago

  • Type of work changed from Communicate to Code

#59 Updated by lamby about 1 month ago

Today I spent some time merging the integration suite into my local branch and trying to get a testcase for the double-inline issue running. I'm currently not managing to get it to trigger, alas, so will step away for another day.

#60 Updated by sajolida 24 days ago

  • Blocks deleted (Feature #15411: Core work 2018Q2 → 2018Q3: Technical writing)

#61 Updated by lamby 23 days ago

lamby wrote:

Today I spent some time merging the integration suite into my local branch and trying to get a testcase for the double-inline issue running. I'm currently not managing to get it to trigger, alas, so will step away for another day.

I had another run at this today but with no luck. :( :( There is something fundamentally at issue with the way we are doing the test though in that it one cannot translate properly when we call via $perl$ with a getcwd for some reason. It is basically generating "real" ikiwiki pages for the PO files instead of translating. If I use the system-installed ikiwiki it works just fine, even with the same ikiwiki.setup.

Any ideas?

#62 Updated by lamby 20 days ago

  • Assignee changed from lamby to intrigeri

I just had another run at this but, alas, I did not manage to get a test harness working as mentioned above. I fear I have reached my "approved" hours here, so I this is a soft-request for additional time to dedicate to this. I wonder if we should follow the testing methodology in po.t instead instead calling perl directly, but I am unsure. Anyway, you thoughts and opiniion are always welcome of course.

#63 Updated by intrigeri 12 days ago

  • Assignee changed from intrigeri to lamby
  • Estimated time changed from 6.00 h to 8.00 h

Sorry for the delay!

lamby wrote:

There is something fundamentally at issue with the way we are doing the test though in that it one cannot translate properly when we call via $perl$ with a getcwd for some reason. It is basically generating "real" ikiwiki pages for the PO files instead of translating. If I use the system-installed ikiwiki it works just fine, even with the same ikiwiki.setup.

Any ideas?

I've tried hefee's test with git clean -fdx && perl Makefile.PL && prove t/po-integration.t and it worked just fine. Then I've passed CLEANUP => 0 to tempdir and confirmed that the generated PO (in the in directory) and HTML (in the out directory) have the correct content. So it seems that this can possibly work. To understand better what's going on on your system I would suggest two things:

  • Try the test procedure I've used from hefee's branch. If you still see weird behaviour, unset PERL_MM_OPT (in the past I've had issues with it when doing this sort of ikiwiki work) and retry. If that still fails, retry in a clean chroot or VM with a clean environment, to eliminate any possibility that your local system is tainted in a way that breaks this.
  • Share the current state of your work so I can try to reproduce the issue.

Adding 2 hours to the budget for this debugging and whatever next steps we'll come up with after that.

#64 Updated by intrigeri 12 days ago

  • Estimated time changed from 8.00 h to 4.00 h

(Removing what's already been accounted for in Q2.)

#65 Updated by intrigeri 9 days ago

  • Target version changed from Tails_3.11 to Tails_3.12

#66 Updated by lamby 9 days ago

Whilst we don't have an upstream-blessed or upstream-merged patch (yet), do we patch this for our main site yet? If not, I think we should as I might prioritise other Tails tasks and it would be silly to not have this fixed and humans seeing it.

How would we do this? I'm assuming the website build builds from our packages. Also, please do let me know if we should start a child ticket for this.

#67 Updated by intrigeri 9 days ago

That part of the discussion was moved to the new dedicated subtask :)

#68 Updated by intrigeri 8 days ago

The upstream bug was moved to https://ikiwiki.info/bugs/po:_second_or_subsequent_inline_of_translated_page_inlines_.po_file__44___not_translated_content/ and smcv is asking input to the author of the PO plugin, that is: yours truly. I'm not sure which one of us two will be more efficient at answering Simon: on the one hand I did wrote that code, OTOH that was many years ago and you're the one who worked on it more recently. Can you please take a look at Simon's questions and reassign to me if you feel I specifically am needed? TIA!

Also available in: Atom PDF