Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

setuptools_rust should not be a runtime dep #13926

Closed
darix opened this issue Sep 28, 2022 · 7 comments · Fixed by #13952
Closed

setuptools_rust should not be a runtime dep #13926

darix opened this issue Sep 28, 2022 · 7 comments · Fixed by #13952
Assignees
Labels
A-Packaging Our Debian packages, docker images; or issues relevant to downstream packagers O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. X-Regression Something broke which worked on a previous release X-Release-Blocker Must be resolved before making a release

Comments

@darix
Copy link

darix commented Sep 28, 2022

Description

This is a build time dep and not a runtime dep. At startup synapse should not check for this anymore.

Steps to reproduce

try to start synapse without setuptools_rust installed.

Homeserver

another home server

Synapse Version

1.68.0

Installation Method

Other (please mention below)

Platform

openSUSE Tumbleweed rpm

Relevant log output

Sep 28 03:06:30 tengu matrix-synapse[4251]: ERROR:root:Synapse 1.68.0 needs setuptools_rust, but it is not installed
Sep 28 03:06:30 tengu matrix-synapse[4251]: Missing Requirements: "setuptools_rust"
Sep 28 03:06:30 tengu matrix-synapse[4251]: To install run:
Sep 28 03:06:30 tengu matrix-synapse[4251]:     pip install --upgrade --force "setuptools_rust"

Anything else that would be useful to know?

No response

@DMRobertson
Copy link
Contributor

synapse/pyproject.toml

Lines 177 to 185 in 1254eb2

# This is for building the rust components during "poetry install", which
# currently ignores the `build-system.requires` directive (c.f.
# https://github.com/python-poetry/poetry/issues/6154). Both `pip install` and
# `poetry build` do the right thing without this explicit dependency.
#
# This isn't really a dev-dependency, as `poetry install --no-dev` will fail,
# but the alternative is to add it to the main list of deps where it isn't
# needed.
setuptools_rust = ">=1.3"

Erm wat. Was this meant to be a dev dependency or not @erikjohnston?

@DMRobertson DMRobertson added A-Packaging Our Debian packages, docker images; or issues relevant to downstream packagers S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. O-Uncommon Most users are unlikely to come across this or unexpected workflow labels Sep 28, 2022
@erikjohnston
Copy link
Member

Argh, that comment is confused.

The problem is that we don't have the ability to set build dependencies, so we have to set it as a "runtime" dependency in pyproject.toml. We could add it as a "dev" dependency, but decided against that as poetry install --no-dev would then fail to build in source checkouts

We could special case it in Synapse so that it doesn't check that particular dependency.

@DMRobertson
Copy link
Contributor

The problem is that we don't have the ability to set build dependencies

Isn't that what this is trying to do? Or does poetry ignore these?

synapse/pyproject.toml

Lines 309 to 311 in 1254eb2

[build-system]
requires = ["poetry-core>=1.0.0", "setuptools_rust>=1.3"]
build-backend = "poetry.core.masonry.api"

We could special case it in Synapse so that it doesn't check that particular dependency.

I'd be fine with this... begrudingly.

@DMRobertson DMRobertson added the X-Regression Something broke which worked on a previous release label Sep 28, 2022
@erikjohnston
Copy link
Member

That's what the first part of the comment is talking about:

This is for building the rust components during "poetry install", which
currently ignores the build-system.requires directive (c.f.
python-poetry/poetry#6154). Both pip install and
poetry build do the right thing without this explicit dependency.

i.e. if the build requirements are ignored when doing poetry install

@DMRobertson
Copy link
Contributor

i.e. if the build requirements are ignored when doing poetry install

I would like to once again declare that I hate Python packaging.

@darix
Copy link
Author

darix commented Sep 28, 2022

could you advice which lines i should patch to workaround this issue until a fix is ready?

@DMRobertson
Copy link
Contributor

could you advice which lines i should patch to workaround this issue until a fix is ready?

It's difficult to say without knowing how the openSUSE Tumbleweed rpm is built. You could comment out this line for the time being.

raise DependencyException(deps_unfulfilled)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Packaging Our Debian packages, docker images; or issues relevant to downstream packagers O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. X-Regression Something broke which worked on a previous release X-Release-Blocker Must be resolved before making a release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants