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

Reducing the number of dependencies #601

Closed
njh opened this issue Jun 20, 2024 · 9 comments
Closed

Reducing the number of dependencies #601

njh opened this issue Jun 20, 2024 · 9 comments

Comments

@njh
Copy link

njh commented Jun 20, 2024

I have been looking at packaging the meshtastic CLI, so that it can easily be installed using brew install meshtastic on a Mac.
There are a lot of dependencies listed in requirements.txt, although many of them are just needed for testing.
In #587, @FedericoCeratto has been looking at reducing the number of dependencies by removing timeago, which seems like a good idea to me.

pexpect is listed in both setup.py and requirements.txt and but doesn't seem to be being used anywhere? Maybe I am missing something? Can it be removed?

There two are listed in requirements.txt but don't seem to be being used, however I can see why they might be useful:

  • webencodings
  • pyparsing

And maybe they are an optional extra for one of the other dependencies?

@geeksville
Copy link
Member

I have a WIP branch that changes over to Poetry which addresses much of this. I'll try to send in a PR soon.

@ianmcorvidae
Copy link
Contributor

poetry is definitely the way forward; I think that a more condensed list of dependencies is in setup.py (which the poetry migration will replace), if it's helpful to get started on that now for the purposes of packaging. I also just pushed a commit removing pexpect from setup.py (and one unused import of dotmap), which can go out next release.

I'll try to take a look at 587 again soon. I'm not sure Federico is has looked at it in a bit, so I'll maybe try to write some tests for it/try it out a bit so it can get in and remove that timeago dependency.

for now that does at least cut the install requirements down to: pyserial, protobuf, requests, pypubsub, dotmap, pyqrcode, tabulate, timeago, pyyaml, bleak, and packaging (plus, for tunnel, but that's linux-only, pytap2). If you're packaging the CLI in brew, you might also include wcwidth for #598

@njh
Copy link
Author

njh commented Jun 21, 2024

That all sounds great, thanks Ian! 👍

I have a working Homebrew Formula, which passes brew audit --strict --online meshtastic:
https://gist.github.com/njh/d4c8a61bdf7b3df7ee6cd0ac4e8432f2

All of the dependencies have a Source Distribution (.tar.gz) on PyPi apart from pypubsub, which I have asked for here:
schollii/pypubsub#48

An optimisation would be to not install mesh-tunnel CLI if pytap2 isn't available (or not on Linux).

Oh and it would be good to clarify the license (#582), which I currently have listed as Apache-2.0.

@geeksville geeksville assigned geeksville and unassigned geeksville Jun 21, 2024
@geeksville
Copy link
Member

added an issue for the poetry stuff #604.

@ianmcorvidae
Copy link
Contributor

I've merged the removal of timeago and the change to using poetry, both of which will hopefully help with this! I might see if I can sometime soon restructure the one spot that's currently using dotmap to get rid of that one too.

As far as licensing, I don't think Apache2 is an inaccurate license to use since we haven't really done a proper formal relicensing, but GPL3 is probably the more future-proof choice. The idea, as I was told, is that this is largely the only Meshtastic thing that's not already GPL3, so the intent has been to move it over to that, which should be a compatible relicensing as far as I can tell (IANAL, of course, and if anyone knows a reason it's not, please do let us know).

I'd love to set it up so mesh-tunnel and pytap2 aren't needed or installed except on Linux -- perhaps poetry has a good way of doing this? I'll need to dig around a bit to get familiar with poetry to be sure, though. I think beyond that we're down to stuff that at least the CLI is definitely using. I would like, potentially, to have a more limited version for library-only users, who probably don't need pyqrcode, tabulate, and possibly requests & packaging (which are used for checking if there's a new version of the library/CLI), but that's probably a separate thing.

@garthvh
Copy link
Member

garthvh commented Jun 26, 2024

Apache > GPL3 is fine, the other way is potentially an issue. https://www.apache.org/licenses/GPL-compatibility.html#:~:text=Apache%202%20software%20can%20therefore,be%20included%20in%20Apache%20projects.

@alexmyczko
Copy link

just did it for debian, but also need it with brew on macOS: https://ftp-master.debian.org/new/meshtastic_2.3.12-1.html

@ianmcorvidae
Copy link
Contributor

#711 reduced the required dependencies for this even more, and 1a5ca78 and 89b41c1 removed a couple which I'm not sure why they were still present. I'm going to close this for now, but am happy to entertain any potential reductions in dependencies, especially for using this code as a library.

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

No branches or pull requests

5 participants