Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix docker example and include pyproject.toml #1121

Merged
merged 7 commits into from
Apr 8, 2024

Conversation

tdejager
Copy link
Contributor

@tdejager tdejager commented Apr 5, 2024

Fixes the docker example as pointed out by @pavelzw, will be a bit better when the #1092 is merged.
Also removes pixi.toml in favor for a single pyproject.toml

@pavelzw
Copy link
Contributor

pavelzw commented Apr 5, 2024

We could probably already do something like this:

[tool.pixi.feature.prod.pypi-dependencies]
docker-project = { path = ".", editable = false }

And remove the pip host-dependencies and the postinstall-production and build-wheel part.
Although I would still like a cleaner solution that doesn't suffer of #1046 and maybe #1088

Comment on lines 54 to 55
default = ["test", "build", "dev"]
prod = ["prod"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both envs need to be in the same solve-group, otherwise this example is a bit pointless.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. This posed a problem, so it was good we tested this. Made an issue for it here: #1123

@@ -1,16 +1,44 @@
[build-system]
requires = ["hatchling"]
build-backend = "hatchling.build"
requires = ["flit_core>=3.2,<4"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reasoning behind switching to flit?
Isn't flit old-school nowadays?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, so I would rather use, hatch but currently because of PEP517 mismatch uv is currently not supporting the prepare_metadata_for_build_wheel. See: astral-sh/uv#2130. I'd rather not use setuptools cause it makes the .egg-info in the source and because of legacy reasons. Do you know of another good build backend?

@tdejager
Copy link
Contributor Author

tdejager commented Apr 5, 2024

As long as: #1123 is not resolved, I cannot add the dependencies to the same solve-group. Not sure if you feel like merging it before thats fixed @pavelzw

@pavelzw
Copy link
Contributor

pavelzw commented Apr 5, 2024

Or we go back the postinstall-production route and maybe wait until we find a better solution to the general problem as mentioned in #1088 (comment)
We might have stretched the possibilities of uv / [pypi-dependencies] too far and should maybe think of other less-hacky solutions to fix this?

@tdejager
Copy link
Contributor Author

tdejager commented Apr 8, 2024

Yeah, lets postpone that for now, I've reverted back to manual installation. Please check it out @pavelzw

Copy link
Contributor

@pavelzw pavelzw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@ruben-arts ruben-arts merged commit 4fecc12 into prefix-dev:main Apr 8, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants