Synapse doesn't detect URL preview dependencies if built with poetry-core 1.3.0, raising runtime errors

[DMRobertson]

This is causing CI failures:

Synapse 1.69.0rc1 does not provide the feature 'url_preview'

even though the requisite dependencies are installed. The problem is that python-poetry/poetry-core#476 now enforces https://peps.python.org/pep-0685/#specification. Wheels built by poetry-core include metadata describing an extra url-preview with a - but NOT url_preview with an _. This means the dependency-checking code in Synapse here

synapse/synapse/config/repository.py

Lines 206 to 208 in 7b67e93

	self.url_preview_enabled = config.get("url_preview_enabled", False)
	if self.url_preview_enabled:
	check_requirements("url_preview")

will fail.

People who've also installed from source after the release of poetry-core 1.3.0 are going to be seeing this.

Assuming we want to be able to use future poetry-core releases, we'll need to update the dependency-checking code to account for this.

[DMRobertson]

#14080 should avoid the problem by pinning the build-system requirements. Leaving this issue open though in case we do want to use newer versions of poetry-core in the future.

[gdt]

This just turns a build failure because of one problem into a build failure because the poetry version is wrong. It doesn't really fix the problem.

[gdt]

Is 1.2.1 safe? I don't see a 1.3.0 release on poetry github, so I'm a little confused. I don't understand requiring 1.2.0 exactly if there is 1.2.1 which should be a micro with bugfixes. And a change like new rules in a micro doesn't make sense either.

[DMRobertson]

I don't see a 1.3.0 release on poetry github

I'm talking about poetry-core here (latest: 1.3.1), not poetry (latest: 1.2.1).

[gdt]

Sorry, missed that. poetry-core in pkgsrc is today at 1.0.8. Same comment about should probably be updated, but hasn't, and that if you need to avoid 1.3, no need to also require >= 1.2.0 because of that.

[DMRobertson]

Are you certain that pkgsrc's infrastructure will use its version poetry-core (1.0.8) to build Synapse? Whenever I have build synapse locally with poetry build, the output suggests that both tools fetch the latest version of poetry-core matching the requirements specified in the build system.

$ poetry build
Preparing build environment with build-system requirements poetry-core>=1.0.0, setuptools_rust>=1.3
Building matrix-synapse (1.69.0rc1)

[...]

^C

$ strace poetry build 2>&1 | grep -E 'poetry[-_]core'
openat(AT_FDCWD, "/home/dmr/.local/pipx/venvs/poetry/lib64/python3.10/site-packages/poetry_core-1.2.0.dist-info/entry_points.txt", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/home/dmr/.local/pipx/venvs/poetry/lib64/python3.10/site-packages/poetry_core-1.2.0.dist-info/entry_points.txt", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/home/dmr/.local/pipx/venvs/poetry/lib64/python3.10/site-packages/poetry_core-1.2.0.dist-info/entry_points.txt", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
write(1, "Preparing build environment with"..., 99Preparing build environment with build-system requirements poetry-core>=1.0.0, setuptools_rust>=1.3) = 99
read(3, "Collecting poetry-core>=1.0.0\n", 8192) = 30
read(3, "  Using cached poetry_core-1.3.1"..., 8162) = 59

[...]

We see problems related to this when installing older versions of poetry itself in #13849.

[gdt]

I am not certain, but if so tools fetching things over the net at runtime is a bug that needs fixing, from the packaging viewpoint.
Do you know if this is packaged in guix? They are very hard-core on reproducible builds, and I admire them for that.

We also have a fake homedir and warn if anything is left in that. work/.home is empty after the build.

Re #13849 My quick reaction is that poetry is too unstable and complicated to depend on, but I realize all build tools have issues and it's not an easy decision....

[DMRobertson]

Cross-posting from private communication:

I had assumed that the done thing was to fetch the latest version of the build-time dependencies, because that's what python -m build and poetry build seem to do, and I wasn't aware of any poetry/python/pep517-specific machinery that packagers were using to prevent this.

I don't like any of that fetching at build time either---hence why I wanted a specific pin. I can see that an upper bound also achieves that without screwing others over though; it just didn't occur to me.

I'll make sure we have something more flexible in place for the 1.69 release proper---whether that's a range like poetry-core>=1.0,<1.3 or application fixes to work with poetry-core 1.3.x.

[ahesford]

Isn't the fundamental problem here the whole notion of dependecy checking used by this project?

For example, https://github.com/matrix-org/synapse/blob/develop/pyproject.toml#L177-L185 causes hard failures when setuptools_rust is not available at runtime even though it only exists as a hack around build-system problems.

[DMRobertson]

Isn't the fundamental problem here the whole notion of dependecy checking used by this project?

The checking is long-standing (circa 2015). My understanding is that it was originally intended to protect people who pip installed once, then git pulled a new version with stricter version requirements but didn't pip install again to process them.

Distributors who are specifically managing Synapse's dependencies are welcome to patch out the this mechanism.

For example, develop/pyproject.toml#L177-L185 causes hard failures when setuptools_rust is not available at runtime even though it only exists as a hack around build-system problems.

See #13926 and #13952.

[ahesford]

I definitely share your distaste for Python packaging noted in #13926. The ever-changing official view on how to build and distribute Python software, ill-formed build ecosystem and tendency to laugh off distribution package problems with a "just use pip" response have been increasingly large obstacles to sensible handling of Python in Void Linux.

We've already patched out the setuptools-rust dependency but generally try to avoid carrying downstream patches unless they fix problems uniquely related to our build infrastructure.

[ShadowJonathan]

This caused a startup error for me after building 1.67 AND 1.68 from source.

[darix]

Any plans to long term support newer poetry-core versions?

[DMRobertson]

Any plans to long term support newer poetry-core versions?

There are some thoughts on this in #14085 (comment).

[bnavigator]

This should be fixed in poetry-core, have you tried 1.3.2? There is already a change regarding the normaloized directory name: python-poetry/poetry-core#495

Edit: I was wrong. The core metadata specification mandates a hyphen.

[DMRobertson]

Closing as #14085 has been merged into develop via the 1.69 release.

[voegelas]

Why is poetry-core pinned to >=1.0.0,<=1.3.1 in pyproject.toml? Version 1.3.2 was released almost two weeks ago.

And what about "cache_memory"? I don't use that feature, but there's a check for "cache_memory" in synapse/config/cache.py.

[DMRobertson]

Why is poetry-core pinned to >=1.0.0,<=1.3.1 in pyproject.toml? Version 1.3.2 was released almost two weeks ago.

This is a defensive choice---we've been bitten twice now by problems introduced by unbounded poetry-core requirements: here, and indirectly in #13849. Quoting #14085 (comment):

Discussed among the team. We'll constrain to <= 1.3.1 and will raise that upper bound to 1.3.2/1.4.0/another version if a) someone wants to use that version, and b) we can check that it's safe.

This happened in #14217.

[DMRobertson]

And what about "cache_memory"? I don't use that feature, but there's a check for "cache_memory" in synapse/config/cache.py.

Thanks, I missed this and will fixup. Edit: #14221.