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

Resolving "cyclic" dependency with ruamel.yaml.clib #119

Open
mbargull opened this issue Nov 4, 2019 · 2 comments
Open

Resolving "cyclic" dependency with ruamel.yaml.clib #119

mbargull opened this issue Nov 4, 2019 · 2 comments

Comments

@mbargull
Copy link
Member

mbargull commented Nov 4, 2019

I gave the awkward "circular dependency" mentioned in conda-forge/ruamel.yaml.clib-feedstock#3 (comment) a look this morning.

Here is what we've got:

  • The Python package ruamel.yaml itself has an optional dependency on ruamel.yaml.clib:
    Guarded by :
    • try: ... from _ruamel_yaml import ... except: in ruamel.yaml.main
    • try: ... from .cyaml import ... except: in ruamel.yaml.__init__.py (ruamel.yaml.cyaml has the unconditional from _ruamel_yaml import ...)
  • The distribution ruamel.yaml has a hard* dependency on ruamel.yaml.clib:
    It is in extras_require but with an empty "extra" plus platform specifier which makes it a platform-dependent hard dependency, AFAIK.
    (* except for python >=3.8 for which ruamel.yaml.clib is not built for PyPI)
  • The Python module (or rather C/Cython extension ) _ruamel_yaml has a hard dependency on ruamel.yaml:
    Has a bunch of from ruamel.yaml... imports.
  • The distribution ruamel.yaml.clib has no dependencies.
    Which means its actual content, i.e., _ruamel_yaml, is simply broken if one pip/conda installs ruamel.yaml.clib (:tada: ... /s).

So, to cleanly resolve this messy situation (and assuming upstream doesn't change things in the foreseeable future), we can create yet another ruamel.yaml.* package that hosts only the Python code for ruamel.yaml (since the Python package has only an optional dependency on the C extension, as noted above).
My proposal for the ruamel packages is thus:

  • ruamel:
    • no changes (i.e., empty ruamel/__init__.py and nothing else).
  • ruamel.yaml:
    • convert to noarch: generic package which is itself empty and only has
      requirements:
        run:
          - ruamel.yaml.pylib {{ version }}
          - ruamel.yaml.clib >=0.1.2
  • ruamel.yaml.clib:
  • ruamel.yaml.pylib:

Now, a slight issue/decision remains:
We could make the current recipe have multiple outputs and create the proposed ruamel.yaml and ruamel.yaml.pylib packages from this repo. However, this would introduce a dependency cycle not in the actual package builds but between ruamel.yaml-feedstock and ruamel.yaml.clib-feedstock:

  • ruamel.yaml.clib-feedstock would depend on ruamel.yaml-feedstock due to ruamel.yaml.clib needing ruamel.yaml.pylib
  • ruamel.yaml-feedstock would depend on ruamel.yaml.clib-feedstock due to ruamel.yaml needing ruamel.yaml.clib

So, to avoid this "conda(-forge)-build-system-only" dependency, we'd need to not use outputs here, but

  1. Submit a new package ruamel.yaml.pylib to staged-recipes to create ruamel.yaml.pylib-feedstock.
  2. Change this repo's recipe to be noarch: generic and depend on ruamel.yaml.pylib and ruamel.yaml.clib
    (Even though this package will be "empty", we should retain the source: to continue getting updates via the bot and stay in-sync with the new ruamel.yaml.pylib.)
  3. Fix/update ruamel.yaml.clib's recipe to depend on ruamel.yaml.pylib.

Thoughts, @conda-forge/ruamel, @conda-forge/ruamel-yaml, @conda-forge/ruamel-yaml-clib, @jjhelmus, @mingwandroid, @msarahan?

@mbargull
Copy link
Member Author

mbargull commented Nov 4, 2019

If we update/split the packages as outlined above, one case we have to take into account is this:

  • ruamel.yaml=NEW: new build that depends on ruamel.yaml.pylib=NEW
  • ruamel.yaml=OLD: old build whose payload is identical to ruamel.yaml.pylib=NEW
# install ruamel.yaml=NEW and ruamel.yaml.pylib=NEW
conda install ruamel.yaml=NEW

# remove ruamel.yaml=NEW, leave ruamel.yaml.pylib=NEW in place, install ruamel.yaml=OLD
conda install ruamel.yaml=OLD

# if conda only warns in case of clobbering, ruamel.yaml.pylib=NEW is now broken

# remove ruamel.yaml.pylib=NEW, leave ruamel.yaml=OLD in place
conda remove ruamel.yaml.pylib
 
# now ruamel.yaml=OLD is broken, too, i.e., `import ruamel.yaml` fails

Solutions:

  1. Add a constrains requirements to ruamel.yaml.pylib for ruamel.yaml>=NEW:
    • This would introduce a dependency cycle in the constrains, which is not optimal.
      I know conda had issues with this at some point; not sure what the current state of this is.
    • >=NEW hits a limitation of conda if it does not indicate a version change; meaning:
      If we do not increase the version but only the build number, we are not able to express "either a new version or newer builds of the current version" with conda's current capabilities.
  • Add a constrains requirement on old ruamel.yaml builds:
    • metadata patching
    • can just do constrain: ["ruamel.yaml.pylib<0a0" ] to prevent ruamel.yaml=OLD and ruame.yaml.pylib from being simultaneously installed.

I'm generally no big fan of patching metadata. However, in this case it is the less troublesome solution and only affects constrains dependencies and "packages coming from the future" (from the PoV of ruamel.yaml=OLD) and thus should not change the consistency of current installations.

@mbargull
Copy link
Member Author

mbargull commented Nov 4, 2019

If we do not increase the version but only the build number

We could lump the build into a "custom" conda-forge-specific version number, of course, meaning: have contrains: [ "ruamel.yaml>=0.16.5.0.post0" ] or something. This still leaves the "cycle in constrains dependencies" issue, though..

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

1 participant