conda / menuinst

Cross platform menu item installation
https://conda.github.io/menuinst/
BSD 3-Clause "New" or "Revised" License
36 stars 42 forks source link

Use `apipkg` to provide compatibility forwarders with v1 #151

Closed jaimergp closed 1 year ago

jaimergp commented 1 year ago

Description

Old menuinst v1 import paths are still in use in conda.core.initialize. To avoid having to issue repodata patches for a lot of conda versions we are providing this little forwarding logic using apipkg (only on Windows).

I don't know how the team feels about adding one more dependency to the conda tree. If that's a problem we could vendor the apipkg project.

Checklist - did you ...

jaimergp commented 1 year ago

Ok this should be ready to go @conda/constructor. I have two main questions:

Thanks to the forwarders, menuinst v2 is backwards compatible with all the public calls I could find on Github (mostly conda.core.initialize).

jezdez commented 1 year ago

I don't know how the team feels about adding one more dependency to the conda tree. If that's a problem we could vendor the apipkg project.

I know that apipkg has been around for a LONG time, so it's probably pretty well tested and used, but I also remember that Holger suggested to always vendor it as it's relatively small.

jaimergp commented 1 year ago

I also remember that Holger suggested to always vendor it as it's relatively small.

The README suggests is a single module, but all I see is a package with several (small) modules. Am I missing something? 🤔

jaimergp commented 1 year ago

Ah, that's v3. Maybe I try v2, which is indeed single-module.

jezdez commented 1 year ago

https://github.com/pytest-dev/apipkg/commit/2acee5096f9843fda2ae608169cf5950a32f4f65 seems to have changed that, I didn't know that before

jezdez commented 1 year ago

@jaimergp Hmm, seeing that the v3 also added Python 3.11 support (or at least testing), I wonder if dependening might actually be better :-/ It exists both on defaults and conda-forge: https://anaconda.org/anaconda/apipkg https://anaconda.org/conda-forge/apipkg but the versions don't match.

jaimergp commented 1 year ago

defaults is reaaally behind with 1.5.

We can vendor the whole v3 package too. Even if they are a few files. :shrug:

jaimergp commented 1 year ago

WDYT about this way @jezdez?

jezdez commented 1 year ago

@jaimergp that's alright, I think it might be a little easier to maintain when using pip's vendoring lib, but that can be done separately

jezdez commented 1 year ago

(need to review the API here in depth, next week though)

jaimergp commented 1 year ago

Solving the conflicts dismissed the review, sorry @jezdez. Can you reapprove when you have a chance? Thanks 🙏