-
Notifications
You must be signed in to change notification settings - Fork 25
Make upip installable #56
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
Make upip installable #56
Conversation
setup.py (setuptools) uses the sdist_upip module (included in this commit) to generate packages in the correct format for use with upip. To generate a distribution package, make sure the VERSION is set as desired in setup.py and then run `python3 setup.py sdist`. The result will be a tar.gz file in the dist/ subdirectory.
gzip_4k(outbuf, self.archive_files[0]) | ||
|
||
return r | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code quality here could be better.
unclear: is this the usual code used for this purpose / is there anything better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also: do we need to bundle this or can we just pip install something to get this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used the file as-is. Other than the comments at the topic referring to "pycopy-lib" the file is essentially the same as the file which was part of "micropython-lib". I would keep the file "as-is" (including to allow comparing it to the source in the future, if the source might have changed/improved).
About bundling: I searched hard to find an installable version of this. I didnt manage. I did find a number of micropython modules (e.g. micropython-umqtt.simple2) that do have a setup.py, which import sdist_upip, but that don't ship along that file. So there could be a way yet to install that file in a way I couldn't figure out. I did find one module that shipped the file along.
Interestingly micropython-umqtt.simple2 imports sdist_upip from the parent directory ..
(see here), which doesn't feel very "installed". Instead it feels like the author cloned the "pycopy-lib" repo, which has sdist_upip in its root, and then created a subdirectory for his module (or copying the existing umqtt.simple dir), and following the same pattern as other modules in the "pycopy-lib" repo. All modules in that repo import sdist_upip from ..
which is the root of that repo.
So to summarise: I could not find a place to pip install it from and others seem to also rely on the file from either micropython-lib or pycopy-lib, so I chose the bundling option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. I agree to use 3rd party code "as is" to simplify updates (if any).
If there is no alternative, this is as good as it gets.
HERE = Path(__file__).parent | ||
README = (HERE / 'README.rst').read_text() | ||
|
||
VERSION = "1.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
over time, we'll need to improve how this is handled.
either by using a tool that updates versions everywhere (here, code, docs, ...) or by deriving version from git (for other "normal python" projects i use setuptools_scm).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you saying, it's ok to have it hard coded for now, and only improve later? (It might be good to not take on too much more before getting to v1.0.0)
But I like the approach of deriving the version from git. Especially when putting the build/publish into a Github Action, it would allow understanding where each build artifact actually came from. Also, given that this setup.py is run with python3 (the dependencies alone make it not run on MicroPython), I would expect setuptools_scm to work just fine (i.e. that there's nothing MicroPython specific here).
Let me know if you feel this should be addressed before launch of v1.0.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok for now.
not sure if setuptools_scm can be used, it is made for normal python code in a git repo.
we have half of that (== git) on the dev machine, but none of that on the target device.
setup.py
Outdated
'License :: OSI Approved :: MIT License', | ||
'Programming Language :: Python :: Implementation :: MicroPython', | ||
], | ||
platforms=["esp32"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, guess we also have linux/unix here. just leave it away? not sure what the valid values here are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added "linux" and "darwin". (pushed as extra commit on top)
It appears this field is not used by any tools, so we could also omit it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as long as it is valid and not rejected by pypi, we can keep it.
It appears the "platforms" fields it not actually used by any tools but can be used to communicate to users what platforms the package is meant to work on. (https://stackoverflow.com/a/34994245) Including linux and darwin here, because the code runs well on the unix port of MicroPython, which itself runs well on Linux and macOS (darwin). In fact most of the testing had been done using the unix port.
I added a setup.py to build a upip distribution package of py-esp32-ulp. I called the package "micropython-py-esp32-ulp" trying to follow the convention that micropython packages in pypi start with the "micropython-" prefix.
Setup.py uses the "sdist_upip" module (included in the commit) to create a package in the correct format for upip. The main "correct" part is that upip requires a smaller (non-default) gzip dictionary size, so that the archives can actually be uncompressed on memory-constrained devices.
Two questions/concerns:
sdist_upip.py
originally comes from themicropython-lib
project. They removed this recently without any explanation, but it lives on as part of the pycopy project inpycopy-lib
. I have taken a copy there and included this in this project. It's MIT licensed and begins with a long attribution header, so it should be ok, but I am not entirely sure about including files from other projects. The alternative could be a build/make script that downloads that file when needed, but it seems a lot more complicated. So, for now I chose the path of including the file (as I have seen some other micropython modules have also done). Is this ok?Who should publish this to pypi? I have created an account on test.pypi.org to test how this all works, but perhaps it's better that you @ThomasWaldmann actually publish it, as owner of the project and github repo we reference? Perhaps later we can automate the publishing with Github Actions, as PyPi appears to allow the creation of API tokens for this purpose, but even here it might be better if you own this?