gee-community / qgis-earthengine-plugin

Integrates Google Earth Engine and QGIS using Python API
http://qgis-ee-plugin.appspot.com
MIT License
437 stars 113 forks source link

External libraries refactoring #144

Open XavierCLL opened 3 months ago

XavierCLL commented 3 months ago

With this, we are going to reduce the complexity of generating and packaging the external libraries (and package size). We need to check the new extlibs through two tests ( @gena and @SiggyF ? )

Test Windows Linux Mac
GEE Authentication :white_check_mark: Xavier
tester 2
:white_check_mark: Xavier
tester 2
:white_check_mark: SiggyF
tester 2
GEE Example :white_check_mark: Xavier
tester 2
:white_check_mark: Xavier
tester 2
:white_check_mark: SiggyF
tester 2

For testing, use the Artifact at https://github.com/gee-community/qgis-earthengine-plugin/actions/runs/8509882708 (the package is created by Ubuntu OS but should work for any OS)

SiggyF commented 1 month ago

I think the pycrypto library was the only native (c-based) dependency (which also required manual compilation). If that's no longer needed, we can see the distribution as an "all" / "any" / "python only" distribution.

I left a few minor comments:

You could also consider keeping it as is. Once there will be a system specific dependency added back, we need to revert back to this approach.

I tested it, and it works on my m1 mac.

XavierCLL commented 1 month ago

The file is now double zipped with linux in the name: name: ee_plugin-${{ matrix.os }}.zip

That is a Github action issue/limitation

XavierCLL commented 1 month ago

I think the pycrypto library was the only native (c-based) dependency (which also required manual compilation). If that's no longer needed, we can see the distribution as an "all" / "any" / "python only" distribution.

I left a few minor comments:

* crawling and removing files feels a bit uncomfortable.

* the earthengine version is now pinned, I added a comment to revert that.

* It's named linux, while it's now all/any.  The package is now (probably was already) double zipped.

You could also consider keeping it as is. Once there will be a system specific dependency added back, we need to revert back to this approach.

I tested it, and it works on my m1 mac.

Hi @SiggyF thanks so much for your review!

As soon as I realized that EE API no longer needed pycrypto, I tested and created this PR to have an independent OS for the external libs for the plugin. And, of course, if in the future the EE-API includes a library, not OS-independent, we can roll back to the old code. The good thing to have this, simplified and not OS-depend: is easy to maintain and release it, a lot of size reduction for the final plugin package, fewer GitHub actions.

Regards! Xavier