build: add minimal python config to publish project on pypi#8818
build: add minimal python config to publish project on pypi#8818lypwig wants to merge 1 commit intobiomejs:mainfrom
Conversation
|
WalkthroughAdds build and packaging infrastructure for a Python–Rust hybrid package. Introduces a GitHub Actions workflow ( 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@pyproject.toml`:
- Line 10: Update the Python minimum version in pyproject.toml by changing the
requires-python metadata value from ">=3.8" to ">=3.11" and ensure CI/test
matrices (e.g., any GitHub Actions job names or matrix entries referencing
Python versions) are updated to include 3.11 and 3.12 and remove or
de-prioritize older EOL versions; look for the requires-python field and any CI
matrix entries or tox/pytest configuration referencing Python versions to keep
them consistent with the new baseline.
🧹 Nitpick comments (3)
python/biome_js/__main__.py (1)
5-8: Placeholder binary discovery may not work in all scenarios.Returning just
"biome"relies on the binary being onPATH. When installed via pip, maturin places the binary in Python'sbin/Scriptsdirectory, so this should work. However, the docstring says "Return the biome binary path" when it actually returns just the name.Given this is WIP, this is fine for now, but you may want to enhance this later to locate the binary relative to the package installation (e.g., using
importlib.resourcesorsysconfig.get_path('scripts')).pyproject.toml (1)
35-36: Empty exclude array is unnecessary.You can remove the empty
exclude = []if you have no exclusions—it's just noise..github/workflows/publish-pypi.yml (1)
6-6: Consider a more descriptive workflow name.The workflow is named "CI" but it's specifically for publishing to PyPI. A name like "Publish PyPI" or "Build and Publish Python Wheels" would be clearer in the Actions UI, especially if you have other CI workflows.
siketyan
left a comment
siketyan
left a comment
There was a problem hiding this comment.
Biome can be distributed in many ways, but I think the binary should be always same. Can we reuse the binaries built on our existing CI to make packages for pypi?
lypwig
force-pushed
the
main
branch
2 times, most recently
from
f3a3a97 to
011ea38
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@python/biome_js/__main__.py`:
- Line 20: The code checks sys._framework directly which can raise
AttributeError on some builds; update the guard in __main__.py to use
getattr(sys, "_framework", None) (or similar) instead of direct attribute access
so the branch becomes: elif sys.platform == "darwin" and getattr(sys,
"_framework", None): ensuring safe access to sys._framework.
- Line 10: The current concatenation in biome_exe = "biome" +
sysconfig.get_config_var("EXE") can raise a TypeError if
sysconfig.get_config_var("EXE") returns None; update the code around the
biome_exe assignment to fetch the EXE value into a variable (call it exe or
similar), guard against None by defaulting to an empty string (or another
appropriate suffix), and then build biome_exe using that safe value; refer to
sysconfig.get_config_var("EXE") and the biome_exe variable to locate where to
apply this fix.
🧹 Nitpick comments (2)
pyproject.toml (1)
14-29: Consider adding Python version classifiers.The classifiers are missing
Programming Language :: Python :: 3and version-specific entries (e.g.,Programming Language :: Python :: 3.11). These help with discoverability on PyPI but aren't strictly required.📦 Suggested addition
classifiers = [ "Environment :: Console", "Intended Audience :: Developers", "Operating System :: OS Independent", "Programming Language :: JavaScript", + "Programming Language :: Python :: 3", + "Programming Language :: Python :: 3.11", + "Programming Language :: Python :: 3.12", + "Programming Language :: Python :: 3.13", "Programming Language :: Rust", "Topic :: Software Development :: Quality Assurance", ]python/biome_js/__main__.py (1)
37-37: Improve error message to show all searched paths.The
FileNotFoundErroronly mentionsscripts_path, but three locations are checked. Listing all searched paths would help users debug installation issues.💡 Suggested improvement
- raise FileNotFoundError(scripts_path) + raise FileNotFoundError( + f"Could not find biome executable. Searched: {scripts_path}, {user_path}, {target_path}" + )
python/biome_js/__main__.py
Outdated
| def find_biome_bin() -> str: | ||
| """Return the Biome binary path.""" | ||
|
|
||
| biome_exe = "biome" + sysconfig.get_config_var("EXE") |
There was a problem hiding this comment.
Handle potential None from get_config_var("EXE").
sysconfig.get_config_var("EXE") can return None on some platforms, which would cause a TypeError when concatenating with the string "biome".
🔧 Suggested fix
- biome_exe = "biome" + sysconfig.get_config_var("EXE")
+ biome_exe = "biome" + (sysconfig.get_config_var("EXE") or "")🤖 Prompt for AI Agents
In `@python/biome_js/__main__.py` at line 10, The current concatenation in
biome_exe = "biome" + sysconfig.get_config_var("EXE") can raise a TypeError if
sysconfig.get_config_var("EXE") returns None; update the code around the
biome_exe assignment to fetch the EXE value into a variable (call it exe or
similar), guard against None by defaulting to an empty string (or another
appropriate suffix), and then build biome_exe using that safe value; refer to
sysconfig.get_config_var("EXE") and the biome_exe variable to locate where to
apply this fix.
python/biome_js/__main__.py
Outdated
| user_scheme = sysconfig.get_preferred_scheme("user") | ||
| elif os.name == "nt": | ||
| user_scheme = "nt_user" | ||
| elif sys.platform == "darwin" and sys._framework: |
There was a problem hiding this comment.
Guard against missing sys._framework attribute.
sys._framework is a private attribute that may not exist on all Python builds. Using getattr with a default avoids a potential AttributeError.
🔧 Suggested fix
- elif sys.platform == "darwin" and sys._framework:
+ elif sys.platform == "darwin" and getattr(sys, "_framework", None):🤖 Prompt for AI Agents
In `@python/biome_js/__main__.py` at line 20, The code checks sys._framework
directly which can raise AttributeError on some builds; update the guard in
__main__.py to use getattr(sys, "_framework", None) (or similar) instead of
direct attribute access so the branch becomes: elif sys.platform == "darwin" and
getattr(sys, "_framework", None): ensuring safe access to sys._framework.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@python/biome_js/__main__.py`:
- Around line 25-29: The code assumes sysconfig.get_path("scripts",
scheme=user_scheme) always returns a string when constructing user_path; add a
None guard: first retrieve the scripts path into a variable (e.g., scripts_path
= sysconfig.get_path("scripts", scheme=user_scheme)), check scripts_path is not
None before calling os.path.join to form user_path, and only then call
os.path.isfile(user_path) and return it if present; update the logic around
user_path, sysconfig.get_path, user_scheme, and biome_exe accordingly.
- Around line 12-14: Guard against sysconfig.get_path("scripts") returning None
before calling os.path.join: assign the result to a local variable (e.g.,
scripts_dir = sysconfig.get_path("scripts")), check that scripts_dir is truthy
(not None/empty) before using os.path.join(scripts_dir, biome_exe), and only
then call os.path.isfile on the constructed scripts_path; if scripts_dir is None
or empty, skip the join and handle accordingly (e.g., return None or continue
the fallback logic) so that scripts_path, sysconfig.get_path, biome_exe,
os.path.join, and os.path.isfile are used safely.
♻️ Duplicate comments (2)
python/biome_js/__main__.py (2)
10-10: Handle potentialNonefromget_config_var("EXE").As previously noted,
sysconfig.get_config_var("EXE")can returnNoneon some platforms.
20-20: Guard against missingsys._frameworkattribute.As previously noted,
sys._frameworkis a private attribute that may not exist on all Python builds.
🧹 Nitpick comments (1)
python/biome_js/__main__.py (1)
37-37: Improve the error message to list all checked locations.Currently only
scripts_pathis mentioned, but three locations are checked. A more helpful message would aid debugging, especially if the guards forNoneare applied (asscripts_pathmay not be defined).💡 Suggested improvement
- raise FileNotFoundError(scripts_path) + raise FileNotFoundError( + f"Could not find the biome executable '{biome_exe}'. " + "Searched scripts directory, user scripts directory, and package bin directory." + )
| scripts_path = os.path.join(sysconfig.get_path("scripts"), biome_exe) | ||
| if os.path.isfile(scripts_path): | ||
| return scripts_path |
There was a problem hiding this comment.
Guard against None from sysconfig.get_path("scripts").
sysconfig.get_path("scripts") can return None in certain configurations, which would cause a TypeError in os.path.join.
🔧 Suggested fix
- scripts_path = os.path.join(sysconfig.get_path("scripts"), biome_exe)
- if os.path.isfile(scripts_path):
- return scripts_path
+ scripts_dir = sysconfig.get_path("scripts")
+ if scripts_dir:
+ scripts_path = os.path.join(scripts_dir, biome_exe)
+ if os.path.isfile(scripts_path):
+ return scripts_path📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| scripts_path = os.path.join(sysconfig.get_path("scripts"), biome_exe) | |
| if os.path.isfile(scripts_path): | |
| return scripts_path | |
| scripts_dir = sysconfig.get_path("scripts") | |
| if scripts_dir: | |
| scripts_path = os.path.join(scripts_dir, biome_exe) | |
| if os.path.isfile(scripts_path): | |
| return scripts_path |
🤖 Prompt for AI Agents
In `@python/biome_js/__main__.py` around lines 12 - 14, Guard against
sysconfig.get_path("scripts") returning None before calling os.path.join: assign
the result to a local variable (e.g., scripts_dir =
sysconfig.get_path("scripts")), check that scripts_dir is truthy (not
None/empty) before using os.path.join(scripts_dir, biome_exe), and only then
call os.path.isfile on the constructed scripts_path; if scripts_dir is None or
empty, skip the join and handle accordingly (e.g., return None or continue the
fallback logic) so that scripts_path, sysconfig.get_path, biome_exe,
os.path.join, and os.path.isfile are used safely.
| user_path = os.path.join( | ||
| sysconfig.get_path("scripts", scheme=user_scheme), biome_exe | ||
| ) | ||
| if os.path.isfile(user_path): | ||
| return user_path |
There was a problem hiding this comment.
Same None guard needed for the user scheme path.
sysconfig.get_path("scripts", scheme=user_scheme) can also return None.
🔧 Suggested fix
- user_path = os.path.join(
- sysconfig.get_path("scripts", scheme=user_scheme), biome_exe
- )
- if os.path.isfile(user_path):
- return user_path
+ user_scripts_dir = sysconfig.get_path("scripts", scheme=user_scheme)
+ if user_scripts_dir:
+ user_path = os.path.join(user_scripts_dir, biome_exe)
+ if os.path.isfile(user_path):
+ return user_path🤖 Prompt for AI Agents
In `@python/biome_js/__main__.py` around lines 25 - 29, The code assumes
sysconfig.get_path("scripts", scheme=user_scheme) always returns a string when
constructing user_path; add a None guard: first retrieve the scripts path into a
variable (e.g., scripts_path = sysconfig.get_path("scripts",
scheme=user_scheme)), check scripts_path is not None before calling os.path.join
to form user_path, and only then call os.path.isfile(user_path) and return it if
present; update the logic around user_path, sysconfig.get_path, user_scheme, and
biome_exe accordingly.
I don't know why but i can't directly answer this comment. Yes I think it's possible, I will try something. |
lypwig
changed the title
| [tool.maturin] | ||
| bindings = "bin" | ||
| manifest-path = "crates/biome_cli/Cargo.toml" | ||
| module-name = "biome_js" |
There was a problem hiding this comment.
I don't know how the Python world works, but can we ask the owner of biome to transfer it to us? I can reach out to them
3 participants
Summary
This PR follows #8808 and is used to improve Biome integration with Python project.
For now, it only consists in publishing Biome to Pypi on https://pypi.org/project/biome-js/.
The build is done via Maturin (which also generated the CI file).
The pyproject content is based on Biome's package.json. I also took inspiration ruff pyproject.
This is a work in progress: I prefer to publish early in order to get feedback before to continue.
Some other things to do:
Any other suggestion is welcome.
Test Plan
uvx --from biome-js biome --help)sudo docker run --rm -v $(pwd):/io ghcr.io/pyo3/maturin build --release --manylinux 2_28 --target-dir distDocs
I will update docs when this PR will be in a more stable state.