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

Migrate to PEP517-compliant pyproject.toml build system #622

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

baldurmen
Copy link
Contributor

This PR migrates the build system from the old setup.py to the new PEP517-compliant pyproject.toml-based one. I stuck with setuptools, since this is what we were already using.

Note that sadly, to my knowledge, it is not currently possible to install scripts like it was before, which means that the accuraterip-checksum script has to be installed manually.

The setup.py file isn't removed either, as it is still needed to build the C extension.

Happy to change things if necessary, but AFAIK this works fine, as I'm building the Debian package with it.

Cheers,

Signed-off-by: Louis-Philippe Véronneau <pollo@debian.org>
…ache__ artifacts

Signed-off-by: Louis-Philippe Véronneau <pollo@debian.org>
@MerlijnWajer
Copy link
Collaborator

Thanks for the pull request, on first look, things look fine. Could you elaborate a little more on having to manually install the accuraterip-checksum program? Does that mean that we currently don't install this any more in say the debian package, or did you work around this in the debian/rules file?

@baldurmen
Copy link
Contributor Author

In the old setup.py script, this bit used to install the scripts/accuraterip-checksum file in /usr/bin/accuraterip-checksum/ when running python3 setup.py install:

    scripts=[
        'scripts/accuraterip-checksum',
    ],

As I said, as far as I understand, this is not possible anymore. This means running pip install against the proposed pyproject.toml file will not install the scripts/accuraterip-checksum file in /usr/bin/accuraterip-checksum/.

For the Debian package, it is not an issue, as we're now manually installing the script. I was mostly concerned for use cases where people install whipper manually on their machines.

@MerlijnWajer
Copy link
Collaborator

Thanks for the explanation, I would feel better if we can find a way to install the script any way, we do need it for whipper to function properly, don't we? Or did we turn this into a Python extension as well?

@baldurmen
Copy link
Contributor Author

You probably know the codebase better than I, but I do not believe the accuraterip-checksum script is needed for whipper to work properly. AFAIU, whipper itself loads the C extension via import accuraterip.

accuraterip-checksum mainly seems to be a "nice to have" cli tool that can give you the accuraterip checksum of individual files without having to run whipper.

Signed-off-by: Louis-Philippe Véronneau <pollo@debian.org>
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.

None yet

2 participants