Page MenuHomePhabricator

Update rsvg on the image scalers to 2.40.16 (to solve several SVG rendering issues)
Closed, ResolvedPublic

Description

Needs to be 2.40.4 or later, or include a backport of aa1f447e2, to fix T111815.

Needs to be 2.40.9 or later, or include a backport of 05480772, to fix T44090.

Needs to be 2.40.10 or later, or include a backport of 99805d95, to fix T97758.

Needs to be 2.40.10 or later, or include a backport of 9c718ff7, to fix T64987.

Needs to be 2.40.13 or later, or include a backport of cb7fd6b6 to (partially?) fix T65703.

Latest upstream version can be found here.

Related Objects

StatusAssignedTask
OpenNone
StalledNone
OpenNone
ResolvedMoritzMuehlenhoff
ResolvedMoritzMuehlenhoff
OpenNone
ResolvedMoritzMuehlenhoff
ResolvedJoe
ResolvedMoritzMuehlenhoff
ResolvedKrenair
Resolved AlexMonk-WMF
Resolvedfgiunchedi
Resolved AlexMonk-WMF
ResolvedKrenair
Resolvedfgiunchedi
ResolvedKrenair
DeclinedNone
Resolvedmobrovac
ResolvedKrinkle
ResolvedKartikMistry
ResolvedKartikMistry
Resolved bd808
InvalidNone
DeclinedNone
Resolveddduvall
Resolveddduvall

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Restricted Application added subscribers: Steinsplitter, Matanya, Aklapper. · View Herald TranscriptSep 13 2015, 12:52 AM

Well, if it's 2.40.10 it'll also fix T112421 and T97758

Well, if it's 2.40.10 it'll also fix T112421 and T97758

The first one is this one.

Tgr updated the task description. (Show Details)Sep 14 2015, 11:44 PM
Tgr updated the task description. (Show Details)
Restricted Application added a project: Commons. · View Herald TranscriptNov 17 2015, 5:52 AM

Who would be the person to poke to actually get it upgraded? I know @ArielGlenn used to handle stuff like this a long time ago, but I have no idea who's purview it is these days.

Andrew added a subscriber: Andrew.Nov 17 2015, 6:44 PM

It looks like Alexandros upgraded things in November. Here's what we currently serve up for Trusty:

root@carbon:~# reprepro list trusty-wikimedia | grep rsvg
trusty-wikimedia|main|amd64: gir1.2-rsvg-2.0 2.40.2-1+wm1
trusty-wikimedia|main|amd64: librsvg2-2 2.40.2-1+wm1
trusty-wikimedia|main|amd64: librsvg2-bin 2.40.2-1+wm1
trusty-wikimedia|main|amd64: librsvg2-common 2.40.2-1+wm1
trusty-wikimedia|main|amd64: librsvg2-dbg 2.40.2-1+wm1
trusty-wikimedia|main|amd64: librsvg2-dev 2.40.2-1+wm1
trusty-wikimedia|main|amd64: librsvg2-doc 2.40.2-1+wm1
trusty-wikimedia|main|i386: librsvg2-doc 2.40.2-1+wm1
trusty-wikimedia|main|source: librsvg 2.40.2-1+wm1

However, the latest version in gerrit (https://gerrit.wikimedia.org/r/#/admin/projects/operations/debs/librsvg) looks to be 2.36. So there must be a competing build tree someplace. The gerrit tree looks to have been mostly maintained by Tim Starling.

ok, further digging suggests that Giuseppe built the latest package.

Andrew raised the priority of this task from Low to Normal.Nov 18 2015, 5:06 PM

Giuseppe says:

  • The current package was a clean backport with the security patch so he didn't check anything into gerrit.
  • He's pretty sure that that patch is in the new upstream packages.

So, I will check the source to make sure, and then (perhaps) we can start using upstream builds.

I have new backports ready for testing. Can I get a volunteer to install them on a beta-cluster box and verify that things work ok?

andrew@copper:~$ ls /var/cache/pbuilder/result/trusty-amd64/*rsvg*
/var/cache/pbuilder/result/trusty-amd64/gir1.2-rsvg-2.0_2.40.11-1_amd64.deb
/var/cache/pbuilder/result/trusty-amd64/librsvg2-2_2.40.11-1_amd64.deb
/var/cache/pbuilder/result/trusty-amd64/librsvg_2.40.11-1_amd64.changes
/var/cache/pbuilder/result/trusty-amd64/librsvg_2.40.11-1.debian.tar.gz
/var/cache/pbuilder/result/trusty-amd64/librsvg_2.40.11-1.debian.tar.xz
/var/cache/pbuilder/result/trusty-amd64/librsvg_2.40.11-1.dsc
/var/cache/pbuilder/result/trusty-amd64/librsvg_2.40.11.orig.tar.xz
/var/cache/pbuilder/result/trusty-amd64/librsvg2-bin_2.40.11-1_amd64.deb
/var/cache/pbuilder/result/trusty-amd64/librsvg2-common_2.40.11-1_amd64.deb
/var/cache/pbuilder/result/trusty-amd64/librsvg2-dbg_2.40.11-1_amd64.deb
/var/cache/pbuilder/result/trusty-amd64/librsvg2-dev_2.40.11-1_amd64.deb
/var/cache/pbuilder/result/trusty-amd64/librsvg2-doc_2.40.11-1_all.deb

Giuseppe says:

  • The current package was a clean backport with the security patch so he didn't check anything into gerrit.
  • He's pretty sure that that patch is in the new upstream packages.

I double-checked and that's the case:
2.40.2-1+wm1 introduced the patch debian/patches/30_fix_number_parsing.patch, which was based on upstream commit 5ba4343bccc7e1765f38f87490b3d6a3a500fde1 which was included in the 2.40.6 upstream release.

I have new backports ready for testing. Can I get a volunteer to install them on a beta-cluster box and verify that things work ok?

paging @greg @Tgr

Tgr added a comment.Nov 30 2015, 10:47 PM

Sorry for being unresponsive. How do I access these packages?

@Tgr I've copied them to /tmp/rsvg-T112421/ on deployment-bastion

Tgr added a comment.Dec 2 2015, 9:42 PM

Tested with beta Commons file uploads after updating librsvg2-2 / librsvg2-bin / librsvg2-common on deployment-mediawiki02 (per T84950 that's probably the right place).

Tried to do some regression tests as well, but

  • fonts are broken, as said above, probably not an rsvg issue
  • lang parameter handling is completely broken (clearly not a thumbnail rendering issue)
  • Bawolff recommended looking at the most linked files list: most files picked at random from there worked but the Commons logo is badly broken (in some sizes).
Tgr added a comment.Dec 2 2015, 9:46 PM

So,

  • the beta appserver does not have the right fonts installed. This is expected, beta cluster has no real image scalers.
  • beta cluster's thumb.php hack does not handle extra parameters
  • there is an error where sometimes the image is badly clipped (depending on dimensions) - not sure if that's an rsvg issue or a problem with thumb.php.

So I guess testing on beta is blocked by T84950. Maybe we could take a production imagescaler out of circulation and test there?

  • beta cluster's thumb.php hack does not handle extra parameters

I submitted https://gerrit.wikimedia.org/r/#/c/256589/ for this.

Tgr added a comment.Dec 4 2015, 12:29 AM

there is an error where sometimes the image is badly clipped (depending on dimensions)

Maybe librsvg bug #757484? It's weird that it only happens for certain thumbnail sizes though.

I've removed the block on operations since this seems blocked on regressions now

@fgiunchedi: Could you create a Phabricator ticket for that issue and add it as a blocker to this ticket? Are there any other known regressions?

Tgr added a comment.Mar 8 2016, 8:02 PM

This is really blocked on having a sane way to test thumbnailing. @fgiunchedi do you think we could use a production cluster scaler for that?

@kaldari @Tgr I think we might be close to unblocking thumbnail generation in beta since there's now a swift cluster there. I've outlined what I think the next steps are in https://phabricator.wikimedia.org/T64835#1947921 but I'm not going to have time at least until the end of the quarter. Is there anyone that could pick it up? I'm not opposed to using production imagescalers but I think fixing T84950: Thumbnail generation should happen via the same setup in the beta cluster and in production (tracking) will serve us better

Tgr added a comment.Mar 9 2016, 9:14 PM

As far as I can see Swift is not relevant here. Having it in beta will help reproducing/early catching of missing file errors and the like, but it does not affect issues with thumbnail rendering software. What would need improvements there is system packages (especially fonts) matching production, and having the same or similar varnish/apache setup instead of the custom nginx+php code that currently proxies thumbnail requests to the real appservers.

@Tgr, thanks that makes sense. I was looking at the blocker chain of imagescaling in beta (e.g. https://phabricator.wikimedia.org/T84950#1711999) and T64835 is at the bottom of that. Do you know of plans to work on T84950 ? Possibly also given T66214 it'd be definitely helpful to have it fixed

Tgr added a comment.Mar 10 2016, 10:36 AM

T84950 is a mixed bag of things, most not broken out into a separate task. The second and third bullet point of that list is probably what's relevant here. I would like to work on it eventually but I don't have anything more specific than that (and definitely not in this quarter).

Gilles added a subscriber: Gilles.Mar 10 2016, 3:17 PM
Joe added a subscriber: Joe.EditedApr 20 2016, 12:40 PM

FYI, we build 2.40.5 for jessie (see T132584), should we move to 2.40.11 there too?

Restricted Application added a subscriber: Poyekhali. · View Herald TranscriptApr 20 2016, 12:40 PM

Is there any progress on this? Or is it stuck on any subprocess/review?

@Dvorapa: See "Blocked By" above.

(For the records, upstream 2.40.11 had some regressions. Latest releases can be found here: http://ftp.gnome.org/pub/GNOME/sources/librsvg/2.40/ )

Dvorapa added a comment.EditedMay 21 2016, 5:31 PM

@Aklapper Could this be provisionally fixed in an old way (before the blocked by task will be resolved)? There are still too many SVG files all over Wikimedia projects, which are completely valid, but look broken because of bugs in rsvg, which were fixed a long time ago.

Aklapper updated the task description. (Show Details)May 22 2016, 4:31 PM
Aklapper updated the task description. (Show Details)
Aklapper updated the task description. (Show Details)

@Dvorapa: I do not know more than what's written in T84950, sorry. :(

Menner added a subscriber: Menner.Jun 18 2016, 8:38 AM

Regression from 2.40.11 has been fixed in recent 2.40.16. I've compiled it on Ubuntu 14.04 and I've seen no issues on a short test.

I've used http://ftp.gnome.org/pub/GNOME/sources/librsvg/2.40/ but there is a recent Ubuntu package too. You may use http://packages.ubuntu.com/yakkety/libs/librsvg2-common for Backport.

Tell me if I can assist. Looking forward for an update. It fixes numerous issues valuable to Wikipedia.

Tgr added a comment.Jun 19 2016, 10:10 PM

@Andrew / @fgiunchedi wanna try again with 2.40.16?

@Tgr We're in the process of moving the mw* systems to jessie, so we can take the migration to jessie as an opportunity to move to 2.40.16. I had already opened a task to investigate the segfaults of rsvg-convert last week (T137876) and starting with 2.40.16 seems preferable.

Recent Debain has librsvg 2.40.16 ready for backport too. https://packages.debian.org/source/stretch/librsvg

MoritzMuehlenhoff closed this task as Resolved.Jul 4 2016, 2:46 PM

Eight image scalers based on Debian jessie running a backport of 2.40.16 have been added and the older trusty-based image scalers have now been disabled, so I'm marking this task as resolved.

Aklapper renamed this task from Update rsvg on the image scalers to Update rsvg on the image scalers to 2.40.16 (to solve several SVG rendering issues).Jul 4 2016, 4:55 PM
Aklapper added a project: User-notice.

Thank you, @MoritzMuehlenhoff!

Menner added a comment.Jul 4 2016, 7:46 PM

&action=purge produces random results. Once with showing known bugs and the other time without. Try out on:
File:Rsvg_marker_element_bug.svg

Menner added a comment.Jul 6 2016, 5:08 PM

Ping @MoritzMuehlenhoff : Noticed my last comment?

@Menner: I'll have a look tomorrow.

Font anti-aliasing seems to be partially broken since the upgrade: T139543.