haikuports / haikuporter

The tool that builds HaikuPorts recipes.
MIT License
41 stars 44 forks source link

Create modern python package for HaikuPorter #259

Closed jurgenwigg closed 1 year ago

jurgenwigg commented 1 year ago

Main goal is to create proper python package which could be installed via pip install command. At this moment package is created in a such way that it is not possible to install package flawless on newer python versions.

Creating unit tests, doing cleanup and removing all python2.7 leftovers, enabling lint, types and other static code analysis will be done under separate PR.

jurgenwigg commented 1 year ago

@scottmc I had to rename file due to issue I had on my computer. MacOS is case insensitive and it caused that while cloning the repository I had a big problem. File names will be as in the original repository - haikuporter. But file itself will be .py file instead of HaikuPorter or I can create directory for the source code of haikuporter itself. Both options are good but it depends on our decision.

In terms of future changes, for Python PEP8 suggests pretty clearly how to style names for classes, functions, etc. Enabling checking formatting for the python code would require setting custom RC file for pylint or changing the names to comply with PEP8 standard. This is open point for future PR.

pulkomandy commented 1 year ago

with some reorganization of the sources it should be possible to keep the name as "haikuporter", lowercase, for both the file and the directory. The file is meant to be installed in some /bin directory while the directory is meant to be a python module.

I think there will be a lot of people running haikuporter directly from the git repository, but they can probably handle some reorganization.

I agree that renaming everything to pep8 is a good idea, but we can still keep the "haikuporter" name :)

jurgenwigg commented 1 year ago

We're all on the same page :) my idea was to preserve whole functionality as it was before but in modern python package with all advantages of newer version and with possibility to release it on pypi :)

jurgenwigg commented 1 year ago

After migration HaikuPorter seems to work as before.

Importing module in the python itself:

» python
Python 3.11.4 (main, Jun  7 2023, 12:45:49) [GCC 9.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import HaikuPorter
>>> import haikuporter
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ModuleNotFoundError: No module named 'haikuporter'
>>> import haiku_porter
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ModuleNotFoundError: No module named 'haiku_porter'
>>>

Executing haikuporter script from CLI:

» ./haikuporter mesa -j4
Error: Unable to find haikuports.conf in known search paths.
Error: See haikuports-sample.conf for more information

Migration to src based structure instead of flat one requires installation of HaikuPorter package with pip install . or pip install -e ..

waddlesplash commented 1 year ago

Haiku itself must be built on a case-sensitive system. You can make a case-sensitive disk in MacOS to check the repository out on and work on it that way.

jurgenwigg commented 1 year ago

The question is if HaikuPorter should be created in such way that it require case sensitive system? Proposed solution should work as previous package also in terms of case sensitive/insensitive. I don't see any obstacles in having haikuporter script 'as is' in root directory and HaikuPorter directory in src place.

jurgenwigg commented 1 year ago

Ok, I've decided that this PR should cover only updating existing file structure to be able to install and create proper modern python package. Everything should work like before the update. Everything related to updating documentation (use mkdocs or sphinx), CI and pre-commit hooks should be done under separate PRs.

waddlesplash commented 1 year ago

The question is if HaikuPorter should be created in such way that it require case sensitive system?

Using HaikuPorter for cross-compilation requires a Haiku build tree, which needs a case-sensitive filesystem. Using HaikuPorter on Haiku will always have a case-sensitive filesystem. So there's no reason to support case-insensitive ones.

pulkomandy commented 1 year ago

But it's just moving a few files to an src/ subdirectory, so there is not really any problem with doing it anyways. And being able to checkout the git repository on a case insensitive filesystem sounds useful for many reasons (archiving, for example).

waddlesplash commented 1 year ago

It's not just moving "a few files" but the entire sources, and furthermore due to the changes the renames aren't even tracked so we are losing all the history here.

I would prefer the haikuporter command/script be moved elsewhere (1 file) than all the others.

jurgenwigg commented 1 year ago

It's not just moving "a few files" but the entire sources, and furthermore due to the changes the renames aren't even tracked so we are losing all the history here.

I would prefer the haikuporter command/script be moved elsewhere (1 file) than all the others.

The easiest way is just to add .py extension to existing haikuporter script. This is the easiest way to keep original directory for source files (HaikuPorter) and keep script file for it in the root directory. If this proposition would be a good deal for now? :)

waddlesplash commented 1 year ago

Sounds fine.

jurgenwigg commented 1 year ago

Now everything should work correctly and package itself should allow to install it through pip install . or pip install -e . command.

Begasus commented 1 year ago

Did a few check builds with these changes (only renamed the symlink in ~/config/non-packaged/bin to haikuporter instead of haikuporter.py (didn't want to change all the other calls to it or restart Terminal to be able to use it). Things works fine for the buildprocess (not mingling with the internals). :)

jurgenwigg commented 1 year ago

@waddlesplash @scottmc please check if your comments and findings were solved and if this PR can be merged :)

Begasus commented 1 year ago

Thanks for working on this, first problem arizes when running the checks on haikuports, it can't find haikuporter (as it's renamed it's no wonder but that should be addressed). :)

Run haikuporter --config=haikuports.conf --licenses=haiku-licenses-master --repository-update --quiet
/home/runner/work/_temp/[7](https://github.com/haikuports/haikuports/actions/runs/5627420031/job/15249910170?pr=9063#step:5:8)7c4164b-1e77-44f2-[8](https://github.com/haikuports/haikuports/actions/runs/5627420031/job/15249910170?pr=9063#step:5:9)5ad-e23976c92744.sh: line 1: haikuporter: command not found
Error: Process completed with exit code 127.

My guess is that if haikuporter gets a new release buildmaster(s) will be affected also by this? (eg haikuporter vs haikuporter.py in the conf file)

EDIT: haikuports-sample.conf should also reflect the changes

waddlesplash commented 1 year ago

It may make sense to restore the old filename, then...

jurgenwigg commented 1 year ago

I'm back from the weekend - I'm checking what went wrong with changing the filename.

jurgenwigg commented 1 year ago

262 I've created PR for fixing this issue.