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

Update build system to prepare for Flatpak release #254

Merged
merged 10 commits into from
Apr 13, 2024
Merged

Conversation

chase9
Copy link
Contributor

@chase9 chase9 commented Dec 18, 2023

The goal of this PR is to make adjustments needed for eventually building Yin-Yang into flatpak format.

Essentially, it makes some updates to how we build the application in order to modernize and prepare for building with Flatpak. The add-flatpak branch depends on this being merged first.

To-Do:

  • Update build system
  • Ensure dependencies are up to date
  • Update Documentation for new development workflow
  • Update CI

Add template to cover future use cases
Adding setup.py and pyproject.toml allows easier installation with native Python tooling
@chase9
Copy link
Contributor Author

chase9 commented Dec 18, 2023

I'm still working on Flatpak support but I realized that my beta branch has gotten a bit out of date, so this PR is to get us in sync.

Copy link
Collaborator

@l0drex l0drex left a comment

Choose a reason for hiding this comment

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

Looks good overall, except for a few tiny things

setup.py Outdated Show resolved Hide resolved
.vscode/c_cpp_properties.json Outdated Show resolved Hide resolved
@chase9
Copy link
Contributor Author

chase9 commented Jan 21, 2024

Sorry, this got lost in my inbox. I pushed the changes you requested!

@chase9 chase9 requested a review from l0drex January 21, 2024 21:47
setup.py Outdated
],
python_requires='>=3.10',
install_requires=[
'psutil==5.9.5',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you try to collect the dependencies in a single location? With these changes, we would have three different entries to change with an update:

  • requirements.txt
  • setup.py and
  • pyproject.toml.

It would be great if we could just rely on one location.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My understanding of these files is that requirements.txt is for development and setup.py/pyproject.toml is for packaging/distribution, so I think we need to have at least two files? It does appear that pyproject.toml can completely replace setup.py but it is still recommended that both pyproject.toml and setup.py stick around1, but I would appreciate feedback from someone who is more knowledgeable on python packaging then myself.

Footnotes

  1. https://packaging.python.org/en/latest/guides/modernize-setup-py-project/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, it appears that the modern recommendation is indeed to forego setup.py in favor of pyproject.toml, if scripting is not needed. I'll work on that now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My latest commit sets up the project to use poetry for building, which means we only need to have pyproject.toml. If you are on board with switching to poetry, I will need to also update the build documentation since the commands will change.

@chase9 chase9 force-pushed the beta branch 4 times, most recently from 42c07ab to ebde87c Compare April 7, 2024 23:50
Builds off of 088434b, since the newer calls are not compatible with the older calls.
This also adds pytest and flake8 to DEV deps since they are needed for CI.
@chase9 chase9 requested a review from l0drex April 8, 2024 00:43
@chase9 chase9 changed the title Merge chase9:beta into oskarsh:beta Update build system to prepare for Flatpak release Apr 8, 2024
@l0drex
Copy link
Collaborator

l0drex commented Apr 10, 2024

Ok, the only thing left now is to update the instructions in the readme to use poetry, right? And then the wiki once its merged.

@chase9
Copy link
Contributor Author

chase9 commented Apr 12, 2024

I've updated the install script and README.md with the new instructions. Tested on a live ISO for Ubuntu 22.04 and everything worked! I do think that it would be cool to eventually do away with the current build instructions and instead be able to have users install using PIP or Flatpak. What do you think about this?

@l0drex
Copy link
Collaborator

l0drex commented Apr 12, 2024

Yes, definitely. I'd love to even go flatpak-only at some point, just to make bug reports more easily reproducable.

@l0drex l0drex merged commit 0e0e5d8 into oskarsh:beta Apr 13, 2024
1 check passed
@chase9 chase9 deleted the beta branch April 13, 2024 17:32
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.

2 participants