Skip to content

Update to modern packaging #141

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Apr 16, 2025
Merged

Update to modern packaging #141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d7fa7a7
Update to modern packaging
tamird Feb 11, 2025
98317f8
Add dependabot config
tamird Feb 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .github/dependabot.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Please see the documentation for all configuration options:
# https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file

version: 2
updates:
- package-ecosystem: "github-actions"
directory: "/"
schedule:
interval: "monthly"
- package-ecosystem: "pip"
directory: "/"
schedule:
interval: "monthly"
groups:
pip-packages:
patterns:
- "*"
19 changes: 11 additions & 8 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,24 @@ jobs:
name: Test on python ${{ matrix.python-version }} and ${{ matrix.os }}
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
python-version: ["3.8", "3.9", "3.10"]
Copy link

Choose a reason for hiding this comment

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

I guess this could also get an update? https://devguide.python.org/versions/ -> 3.8 is EOL, 3.11-3.13 are missing

Copy link
Owner

Choose a reason for hiding this comment

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

If 3.8 is EOL, I think we can probably drop support, and it'd be nice to start running tests on some more recent python releases, though perhaps that should be a separate PR.

os: [ubuntu-latest, windows-latest, macOS-latest]

steps:
- uses: actions/checkout@v4
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v5

- uses: astral-sh/setup-uv@v5
with:
python-version: ${{ matrix.python-version }}
- name: Install tox
run: |
python -m pip install --upgrade pip
pip install tox tox-gh-actions
- name: Test with tox
run: tox
# TODO(https://github.com/astral-sh/setup-uv/issues/226): Remove this.
prune-cache: ${{ matrix.os != 'windows-latest' }}

- run: uv sync --all-extras --dev

- run: uv pip install tox tox-gh-actions

- run: uv run tox
env:
PLATFORM: ${{ matrix.os }}
1 change: 0 additions & 1 deletion MANIFEST.in
View file
Open in desktop

This file was deleted.

6 changes: 3 additions & 3 deletions docs/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@ BUILDDIR = _build

# Put it first so that "make" without argument is like "make help".
help:
@$(SPHINXBUILD) -M help "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O)
uv run @$(SPHINXBUILD) -M help "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O)

.PHONY: help man Makefile

# Copy generated manfile into project root for distribution.
man: Makefile
@$(SPHINXBUILD) -M man "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O)
uv run @$(SPHINXBUILD) -M man "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O)
cp "$(BUILDDIR)/man/git-revise.1" "$(SOURCEDIR)/.."

# Catch-all target: route all unknown targets to Sphinx using the new
# "make mode" option. $(O) is meant as a shortcut for $(SPHINXOPTS).
%: Makefile
@$(SPHINXBUILD) -M $@ "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O)
uv run @$(SPHINXBUILD) -M $@ "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O)
4 changes: 3 additions & 1 deletion docs/conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import os
import sys

from sphinx.application import Sphinx

sys.path.insert(0, os.path.abspath(".."))

import gitrevise
Expand Down Expand Up @@ -122,5 +124,5 @@
# -- Extension configuration -------------------------------------------------


def setup(app):
def setup(app: Sphinx) -> None:
app.add_object_type("gitconfig", "gitconfig", objname="git config value")
24 changes: 12 additions & 12 deletions docs/contributing.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ for testing, and :command:`black` for code formatting.

.. code-block:: shell
$ tox # All python versions
$ tox -e py38 # Python 3.8
$ tox -e py39 # Python 3.9
$ tox -e py310 # Python 3.10
$ uv run tox # All python versions
$ uv run tox -e py38 # Python 3.8
$ uv run tox -e py39 # Python 3.9
$ uv run tox -e py310 # Python 3.10
$ tox -e mypy # Mypy Typechecking
$ tox -e lint # Linting
$ tox -e format # Check Formatting
$ uv run tox -e mypy # Mypy Typechecking
$ uv run tox -e lint # Linting
$ uv run tox -e format # Check Formatting
Code Formatting
---------------
Expand All @@ -26,8 +26,8 @@ This project uses ``isort`` and ``black`` for code formatting.

.. code-block:: shell
$ isort . # sort imports
$ black . # format all python code
$ uv run isort . # sort imports
$ uv run ruff format # format all python code
Copy link
Owner

Choose a reason for hiding this comment

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

I take it here ruff is a new replacement for black? Is the formatting the same as the existing code?

Copy link

Choose a reason for hiding this comment

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

Yes, ruff format is mostly the same apart form some obscure edge cases. Thy actually have some testing against black formatting.

You can probably also remove isort, that's also taken care of by check --fix-only --select I --fixable I ("I" are the isort rules). If you have any other linters (flake8, pylint), you can probably also replace them with ruff check --select 'ALL'...

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah, It does appear that this patch adds a call to ruff check, though it does not remove the pylint check (which appears to still be called). Is ruff check preferable to pylint nowadays?

https://github.com/mystor/git-revise/pull/141/files#diff-ef2cef9f88b4fe09ca3082140e67f5ad34fb65fb6e228f119d3812261ae51449R31-L35

Copy link

Choose a reason for hiding this comment

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

It depends: ruff check (replacement for a lot of flake8 thingies and parts of pylint; an isort) is different from ruff format (replacement for black), so IMO the patch is right to not remove pylint (or at least not remove it without a replacement)

Re comparison with pylint: I would guess it implements about 2/3 of pylints rules (source). Mostly everything which needs context which is outside of the current file is not implmented: ruff is strictly one file only. They have some chatter that multiple files linting is on the radar, but thats seems to be still a bit off as well: "red not", which also will be a mypy replacement, if I understand that right).

My personal take: ruff format + ruff check --select ALL + mypy catches about everything what I care to catch. We have implemented that setup at work (with mypy in struct modusl and a handful of explicitly ignored ruff rules), where it runs against sonarqube and mostly leaves nothing for sonarqube to find. And it's so damn fast that running linter and formatter is fun again :-)

Building Documentation
----------------------
Expand All @@ -44,6 +44,6 @@ Publishing

.. code-block:: shell
$ python3 setup.py sdist bdist_wheel
$ twine check dist/*
$ twine upload dist/*
$ uv build
$ uv run twine check dist/*
$ uv run twine upload dist/*
Copy link

Choose a reason for hiding this comment

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

uv also has some upload capabilities: uv publish... https://docs.astral.sh/uv/guides/package/#publishing-your-package

Copy link
Owner

Choose a reason for hiding this comment

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

That's good to know, might be worth switching to that, but I'd want to look into it more.

14 changes: 8 additions & 6 deletions gitrevise/odb.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
if TYPE_CHECKING:
from subprocess import _FILE

from typing_extensions import Self
Copy link
Owner

Choose a reason for hiding this comment

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

I don't love needing to pull in a dependency for Self when we can do the existing TypeVar stuff and it looks like it should work OK? What does this do differently?

Copy link

Choose a reason for hiding this comment

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

Depends how recent you wanna be: it's included in python proper as of py3.11: https://peps.python.org/pep-0673/

from typing import Self

Copy link
Owner

Choose a reason for hiding this comment

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

Oh that's nice.

Looks like I last increased the minimum python version in 2022 from 3.6 to 3.8 (4a7e852). I imagine we can get away with bumping the minimum version again so long as the major LTS distros (debian stable / ubuntu) are shipping that version of Python or newer.

Copy link

@jankatins jankatins Apr 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

debian stable (bookworm) is 3.11
Ubuntu 22.04 is on 3.10 and 24.04 on 3.12

Not sure how dependent you are on system python for installing a newer version of any python package though: if you can install a newer python package you probably also can install (and probably already are installing) a newer python :-)

With uv, you simply do a uv tool install git-revise@latest you probably anyway get a py 3.13^W3.12 installed:

[16:34:18] λ  uv tool install git-revise@latest
Resolved 1 package in 282ms
Audited 1 package in 0.00ms
Installed 1 executable: git-revise

[23:04:45] λ  which git-revise
~/.local/bin/git-revise

[23:05:32] λ  /home/jankatins/.local/share/uv/tools/git-revise/bin/python --version
Python 3.12.8

(That 3.12 is downloaded and installed by uv -> In this case it was already downloaded and extracted/hardlinked)

Copy link
Owner

Choose a reason for hiding this comment

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

Cool. I'll admit I'm out-of-the-loop on these modern python packaging changes, and so I worry about things like "can someone on the Ubuntu LTS do pip3 install --user git-revise and have it work", but I suppose that's less of a thing folks are doing nowadays?

I think I'd definitely be OK with bumping the python version to 3.10, less certain about 3.11 given 22.04 is still on 3.10, but perhaps that is also OK.



class MissingObject(Exception):
"""Exception raised when a commit cannot be found in the ODB"""
Expand All @@ -57,7 +59,7 @@ class Oid(bytes):
def __new__(cls, b: bytes) -> Oid:
if len(b) != 20:
raise ValueError("Expected 160-bit SHA1 hash")
return super().__new__(cls, b) # type: ignore
return super().__new__(cls, b)

@classmethod
def fromhex(cls, instr: str) -> Oid:
Expand Down Expand Up @@ -165,8 +167,8 @@ class Repository:
"""path to GnuPG binary"""

_objects: Dict[int, Dict[Oid, GitObj]]
_catfile: Popen
_tempdir: Optional[TemporaryDirectory]
_catfile: Popen[bytes]
_tempdir: Optional[TemporaryDirectory[str]]

__slots__ = [
"workdir",
Expand Down Expand Up @@ -480,7 +482,7 @@ class GitObj:

__slots__ = ("repo", "body", "oid", "persisted")

def __new__(cls: Type[GitObjT], repo: Repository, body: bytes) -> GitObjT:
def __new__(cls, repo: Repository, body: bytes) -> "Self":
oid = Oid.for_object(cls._git_type(), body)
cache = repo._objects[oid[0]] # pylint: disable=protected-access
if oid in cache:
Expand All @@ -495,7 +497,7 @@ def __new__(cls: Type[GitObjT], repo: Repository, body: bytes) -> GitObjT:
self.persisted = False
cache[oid] = self
self._parse_body() # pylint: disable=protected-access
return cast(GitObjT, self)
return self

@classmethod
def _git_type(cls) -> str:
Expand Down Expand Up @@ -834,7 +836,7 @@ def git(
trim_newline: bool = True,
) -> bytes:
"""Invoke git with the given index as active"""
env = dict(**env) if env is not None else dict(**os.environ)
env = {**env} if env is not None else {**os.environ}
env["GIT_INDEX_FILE"] = str(self.index_file)
return self.repo.git(
*cmd,
Expand Down
2 changes: 1 addition & 1 deletion gitrevise/tui.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ def noninteractive(

# Update the commit message on the target commit if requested.
if args.message:
message = b"\n".join(l.encode("utf-8") + b"\n" for l in args.message)
message = b"\n".join(line.encode("utf-8") + b"\n" for line in args.message)
current = current.update(message=message)

# Prompt the user to edit the commit message if requested.
Expand Down
33 changes: 0 additions & 33 deletions pylintrc
View file
Open in desktop

This file was deleted.

92 changes: 92 additions & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
[build-system]
requires = ["hatchling"]
build-backend = "hatchling.build"

Comment on lines +1 to +4
Copy link
Owner

Choose a reason for hiding this comment

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

What is hatchling, and how is it connected to uv? On first look it appears it's another different build system?

Copy link

@jankatins jankatins Apr 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

uv as of today has no officially released build backend: poetry has poetry-core, hatch has hatchling, pip has .. build? There is also a setuptools one. So essentially you have to pick one and some more.

uv us currently developing one, but that looks still a bit rough. astral-sh/uv#3957 Might already be enough though...

Copy link
Contributor

Choose a reason for hiding this comment

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

Arch Linux packager here...

I fully support the switch to a proper PEP 517 build, and the ruff and uv tooling is almost universally recognized as better these days than black and pip and such. I don't have strong feelings about hatch/hatchling over poetry, flit, setuptools, or others, so that's fine I suppose.

On the other hand just a warning that the uv build backend is not only rough but controversial. I would avoid it for the foreseeable future as it introduces lots of binary requirements on the host environment when pure Python would do fine. It's very heavy weight for an application like this.

Copy link
Owner

Choose a reason for hiding this comment

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

That works for me, happy to use whatever works best for folks building the actual packages :-)

[project]
name = "git-revise"
dynamic = ["version"]
requires-python = ">=3.8"
authors = [{ name = "Nika Layzell", email = "[email protected]" }]
description = "Efficiently update, split, and rearrange git commits"
readme = "README.md"
license = { file = "LICENSE" }
keywords = ["git", "revise", "rebase", "amend", "fixup"]
classifiers = [
"Development Status :: 4 - Beta",
"Intended Audience :: Developers",
"Environment :: Console",
"Topic :: Software Development :: Version Control",
"Topic :: Software Development :: Version Control :: Git",
"License :: OSI Approved :: MIT License",
"Programming Language :: Python :: 3",
"Programming Language :: Python :: 3.8",
"Programming Language :: Python :: 3.9",
"Programming Language :: Python :: 3.10",
]

[project.scripts]
git-revise = "gitrevise.tui:main"

[dependency-groups]
dev = [
"isort~=5.13.2",
"mypy~=1.14.1",
"pylint~=3.2.7",
"pytest-xdist~=3.6.1",
"pytest~=8.3.4",
"ruff~=0.9.6",
"sphinx~=7.1.2",
"tox-uv~=1.13.1",
"tox~=4.24.1",
"twine~=6.1.0",
"typing-extensions~=4.12.2",
]

[project.urls]
Homepage = "https://github.com/mystor/git-revise/"
Issues = "https://github.com/mystor/git-revise/issues/"
Repository = "https://github.com/mystor/git-revise/"
Documentation = "https://git-revise.readthedocs.io/en/latest/"

[tool.hatch.version]
path = "gitrevise/__init__.py"

[tool.hatch.build.targets.wheel]
packages = ["/gitrevise"]

[tool.hatch.build.targets.wheel.shared-data]
"git-revise.1" = "share/man/man1/git-revise.1"

[tool.hatch.build.targets.sdist]
include = ["/gitrevise"]

[tool.pylint.messages_control]
disable = [
"missing-docstring",
"too-few-public-methods",
"too-many-arguments",
"too-many-branches",
"too-many-instance-attributes",
"too-many-return-statements",
"cyclic-import",
"fixme",

# Currently broken analyses which are also handled (better) by mypy
"class-variable-slots-conflict",
"no-member",
]

good-names = [
# "Exception as e" is perfectly fine.
"e",
# "with open(…) as f" is idiomatic.
"f",
# Other contextually-unambiguous names.
"fn",
"repo",
"ed",
]

# TODO(https://github.com/astral-sh/ruff/issues/14813): Remove this once ruff properly respects
# requires-python.
[tool.ruff]
41 changes: 0 additions & 41 deletions setup.py
View file
Open in desktop

This file was deleted.

Loading
Loading