Missing dependency to lxml for URL previewing

[babolivier]

URL previewing in Synapse requires the lxml library (python[2]-lxml) to work (source: tried to run Synapse without the lxml package installed and with URL previewing enabled and it crashed), but this optional dependency is missing from synapse/python_dependencies.py.

Should be an easy one to fix but I'm unsure about the lowest working version of lxml for this feature.

[gpierron]

Got the same problem here. (debian stable package from repository (0.22.1-1)

[ara4n]

we deliberately didn't include lxml in the deps for synapse as it's a beast and only needed for optional url previews. so the readme asks users to install manually when needed. this was @erikjohnston's idea; i forget why we don't put it in the optional deps (other than possibly optional deps get installed anyway?)

[kyrias]

I mean, if it's not listed as a conditional dependency for URL previewing, that makes that section of the dependencies script entirely useless and misleading, so removing it entirely and listing it in e.g. the README or related document would be much better.

[turt2live]

fwiw, synapse does complain about the missing dependency in a sensible manner.

[richvdh]

it is listed in the readme. I think this is working as intended.

[kyrias]

I'd say it still feels very inconsistent and misleading to list some dependencies of URL previewing under CONDITIONAL_REQUIREMENTS but not all of them.

[richvdh]

oh, yes it is. sorry, I hadn't realised that we have a separate section of CONDITIONAL_REQUIREMENTS for preview_url. Could somebody just make a PR for this?

[erikjohnston]

CONDITIONAL_REQUIREMENTS is a bit weird in that I think that all the packages listed will be installed when using e.g. pip, but on the other hand won't stop synapse from starting. If we added lxml then pip would try and install it and likely fail due to lack of libxml2-dev library, which wouldn't be ideal.

Whether we necessarily want to install CONDITIONAL_REQUIREMENTS by default with pip is a possible question.

[richvdh]

@erikjohnston: so why do we bother listing netaddr there?

[hawkowl]

Fixed in #4298 .

[rubo77]

what is the workaround? I tried

apt install python3-lxml 

but no luck

[rubo77]

Workaround:

pip install lxml