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

Add python-constraint to avoid conda-forge dependencies. #61

Closed
wants to merge 1 commit into from
Closed

Add python-constraint to avoid conda-forge dependencies. #61

wants to merge 1 commit into from

Conversation

HackerFoo
Copy link
Contributor

@HackerFoo HackerFoo commented Dec 16, 2020

Any packages with a condarc specifying outside channels will require those channels to be available to install them.

Depending in conda-forge is undesirable because it can conflict with many of the default packages. pip cannot be used directly in a conda recipe, so PyPI packages must be converted to Conda packages.

This has caused problems with vtr-optimized (PR), which this PR aims to resolve, but there are other packages using condarc as well.

@PiotrZierhoffer
Copy link
Contributor

Ha, we just discussed adding this, like minutes ago.

LGTM for me, but I'll ask @ajelinski to take a look as well

.travis.yml Show resolved Hide resolved
Copy link
Contributor

@litghost litghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

os: linux
dist: xenial
env:
- PACKAGE=misc/python-constraint
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should, probably, go to litex-conda-misc, but since it's a direct dependency of vtr-optimized, I don't think it's a big problem to have it here

@@ -0,0 +1,51 @@
{% set name = "python-constraint" %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this file autogenerated with something like conda skeleton pypi {packagename}? I actually never used it before, but if this is the result, then it's pretty good

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but I had to edit a bit, and change from .gz to .bz2.

@acomodi
Copy link
Contributor

acomodi commented Dec 16, 2020

There should be another way to fix the python-constraint problem. Basically the condarc file in vtr-optimized should include also the defaults package.

All the dependencies for vtr-optimized will be taken from defaults apart from python-constraint which will be taken from conda-forge. Given that python-constraint does not have any other dependency itself, it should not generate any conflicts.

So basically the condarc file would look like the following:

channels:
  - defaults
  - conda-forge

in the current implementation of the vtr-optimized package, all the various dependencies are taken from conda-forge, hence the conflicts

@HackerFoo
Copy link
Contributor Author

There should be another way to fix the python-constraint problem. Basically the condarc file in vtr-optimized should include also the defaults package.

All the dependencies for vtr-optimized will be taken from defaults apart from python-constraint which will be taken from conda-forge. Given that python-constraint does not have any other dependency itself, it should not generate any conflicts.

So basically the condarc file would look like the following:

channels:
  - defaults
  - conda-forge

in the current implementation of the vtr-optimized package, all the various dependencies are taken from conda-forge, hence the conflicts

@acomodi I tried this, based on this tip, except reversed to avoid conda-forge, but unfortunately, it didn't work. There were still dependencies from conda-forge in info/about.json.

@HackerFoo
Copy link
Contributor Author

@ajelinski Any comments?

@ajelinski
Copy link
Contributor

Are you sure you tried with channels in this exact order? The order matters and in the link provided conda-forge is the most important channel.

I've tested how the recipe is rendered after adding the defaults channel on top in condarc like @acomodi mentioned. It definitely solves the problem which is caused by requiring libraries that are not available on the defaults (ie. main) channel:

  run:
  - libstdcxx-ng >=9.3.0
  - libgcc-ng >=9.3.0
  - tbb >=2020.2

This is because conda-build gets all packages it needs from conda-forge and there is gcc/gxx 9.3.0. After making defaults the most important channel (the first one in condarc) it becomes:

  run:
  - libstdcxx-ng >=7.3.0
  - tbb >=2020.3
  - libgcc-ng >=7.3.0

because conda-build will use all packages it can from the defaults channel. Only those not available will be taken from the conda-forge channel.

Those requirements can be satisfied without conda-forge so it won't be needed to install vtr-optimized after such change.

@ajelinski
Copy link
Contributor

Btw. info/about.json doesn't matter during dependency resolving. The depends key from info/index.json inside the package is what matters the most and there you can see this exact problem with requiring libgcc/libstdcxx >=9.3.0.

@ajelinski
Copy link
Contributor

I have added a branch with the aforementioned condarc change and all run requirements of the built package can be satisfied without conda-forge: https://travis-ci.com/github/litex-hub/litex-conda-eda/jobs/461371641#L2925

The package built in that CI can be installed with:

conda install -c litex-hub/label/travis-fix-vtr-optimized-209189450 vtr-optimized

@HackerFoo
Copy link
Contributor Author

HackerFoo commented Dec 16, 2020

@ajelinski Thanks. I'll try your package and close this if it works, unless there is still value in it.

@HackerFoo
Copy link
Contributor Author

@ajelinski It seems to work, so made a PR with your branch: #62

@HackerFoo
Copy link
Contributor Author

Superseded by #62

@HackerFoo HackerFoo closed this Dec 17, 2020
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

Successfully merging this pull request may close these issues.

5 participants