Move esptool from user-venv dependency to vendor it with Mu.

[carlosperate]

As discussed in:

• https://github.com/mu-editor/mu/issues/2120#issuecomment-1309468274

Only a single script was needed from the entire esptool package, and the only dependency needed for that script is pyserial, which is already included with Mu. So, vendoring esptool means we don't need to pip download, package, and install its other dependencies, which have caused trouble in the past (specially cryptography when it moved to be a rust package, as discussed in https://github.com/mu-editor/mu/issues/1952).

This changes the "category" of this dependency, from a wheel packaged with Mu to install in the user venv, to be included as a Mu app runtime dependency. As esptool is used in the Mu settings, is probably closer to a "Mu runtime" dependency, than a "user running their code in a venv" dependency.

Apart from that, esptool and some of its dependencies don't have wheels in PyPI. To be able to use the --platform flag with pip download it needs all dependencies to have wheels, so removing this from the user wheels helps with that as well.

[carlosperate]

I haven't tested this properly yet, but it is added here for visibility and review.

[lgtm-com[bot]]

This pull request introduces 10 alerts when merging b81b0f430ea9615e6ac0259d1fb51d98066715b6 into f000daebf367b6a01b44a0494dcbfe94ccb3bb56 - view on LGTM.com

new alerts:

• 5 for Wrong number of arguments in a class instantiation
• 2 for Modification of parameter with default
• 1 for Unnecessary pass
• 1 for Variable defined multiple times
• 1 for `__init__` method calls overridden method

[carlosperate]

Tested this in a couple of ESP8266 devices and works as expected.

[ntoll]

This explains how to constrain LGTM so it won't analyse our vendored code.

https://lgtm.com/help/lgtm/customizing-file-classification

[carlosperate]

Since Semmle was adquire by GitHub and LGTM will be shut down in December, we might as well move to the current way to run it:

• https://github.com/mu-editor/mu/pull/2347

And how to exclude files: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#specifying-directories-to-scan

I'll merge the PR here, confirm it shows the same errors, update the config to ignore the vendored file, and we can merge it all together to the main branch.

[lgtm-com[bot]]

This pull request introduces 10 alerts when merging b48e5b31b365d44fd373aa302e515da441eb7441 into f000daebf367b6a01b44a0494dcbfe94ccb3bb56 - view on LGTM.com

new alerts:

• 5 for Wrong number of arguments in a class instantiation
• 2 for Modification of parameter with default
• 1 for Unnecessary pass
• 1 for Variable defined multiple times
• 1 for `__init__` method calls overridden method

[lgtm-com[bot]]

This pull request introduces 10 alerts when merging 91e56d2abc1423149475ac45a9d9b0025f03da2e into f000daebf367b6a01b44a0494dcbfe94ccb3bb56 - view on LGTM.com

new alerts:

• 5 for Wrong number of arguments in a class instantiation
• 2 for Modification of parameter with default
• 1 for Unnecessary pass
• 1 for Variable defined multiple times
• 1 for `__init__` method calls overridden method

[carlosperate]

Ah, took me a while to figure it out, the CodeQL CI job doesn't fail or show the issues, instead they are annotations in the PR (you have to go to the files view to see them) and they can also be viewed in the "security" section of the repository: https://github.com/mu-editor/mu/security/code-scanning

By default it shows the alerts for the main branch (which can be changed, so we can see the alerts for this vendor-esptool branch), once this is merged it will show all the alerts by default in that page without selecting anything else.

I'll now exclude the esptool file form the scanning and once that is done we should see the number reduce from 48:

[lgtm-com[bot]]

This pull request introduces 10 alerts when merging 3b66514ced27ac47ac903da0d27dadaed3bd4cc6 into f000daebf367b6a01b44a0494dcbfe94ccb3bb56 - view on LGTM.com

new alerts:

• 5 for Wrong number of arguments in a class instantiation
• 2 for Modification of parameter with default
• 1 for Unnecessary pass
• 1 for Variable defined multiple times
• 1 for `__init__` method calls overridden method

[carlosperate]

Okay, it's now configured to ignore the esptool script and the warnings went down to 27:

I think this PR is ready to be merged 🎉

[lgtm-com[bot]]

This pull request introduces 10 alerts when merging 7d9cab5d241e87400ef71100c8811512dc17ee9c into f000daebf367b6a01b44a0494dcbfe94ccb3bb56 - view on LGTM.com

new alerts:

• 5 for Wrong number of arguments in a class instantiation
• 2 for Modification of parameter with default
• 1 for Unnecessary pass
• 1 for Variable defined multiple times
• 1 for `__init__` method calls overridden method

[carlosperate]

@ntoll As now we are using CodeQL directly from GitHub I guess we should deactivate LGTM, which I assume you'd have to do from their website?

[lgtm-com[bot]]

This pull request introduces 10 alerts when merging fd818b659931239a126183c15573e5bde4388ec1 into f000daebf367b6a01b44a0494dcbfe94ccb3bb56 - view on LGTM.com

new alerts:

• 5 for Wrong number of arguments in a class instantiation
• 2 for Modification of parameter with default
• 1 for Unnecessary pass
• 1 for Variable defined multiple times
• 1 for `__init__` method calls overridden method

[ntoll]

@carlosperate yeah... lemme try to remember how to do that... :rofl:

[ntoll]

OK... LGTM is now switched off.

[carlosperate]

Great, thanks! I'll merge this as done then 👍