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

Ability to pin Sphinx and friends in a conda environment #3829

Closed
Tracked by #51
jakirkham opened this issue Mar 21, 2018 · 37 comments
Closed
Tracked by #51

Ability to pin Sphinx and friends in a conda environment #3829

jakirkham opened this issue Mar 21, 2018 · 37 comments
Labels
Accepted Accepted issue on our roadmap Bug A bug Status: blocked Issue is blocked on another issue

Comments

@jakirkham
Copy link
Contributor

Details

Expected Result

We are currently pinning Sphinx with the expectation that RTD honors this pinning.

Actual Result

However it appears that RTD overrides our Sphinx pinning later, which is causing a different problem ( #3769 ).

cc @humitos

@humitos
Copy link
Member

humitos commented Mar 21, 2018

In my comment #3769 (comment) I mentioned that the problem is the order of the commands executed.

RTD first:

  1. create the env using the yaml provided by the user
  2. install the needed packages to build the documentation (mock pillow sphinx sphinx_rtd_theme)

The last step that installs these packages will UPGRADE them if they are already installed in the conda env.

I've been checking the option --no-update-dependencies (from conda install) but it seems that doesn't work as I want. So, at the moment I don't know how to do something like "ensure installed but not upgrade it".

/tmp ⌚ 12:06:29
$ conda install --no-update-dependencies sphinx          

The following NEW packages will be INSTALLED:

    packaging:                17.1-py27_0                     
    pyparsing:                2.2.0-py27hf1513f8_1            

[...]

The following packages will be UPDATED:

    sphinx:                   1.5.1-py27_0         conda-forge --> 1.7.1-py27_0    

The following packages will be SUPERSEDED by a higher-priority channel:

    asn1crypto:               0.24.0-py27_0        conda-forge --> 0.24.0-py27_0   
[...]

Proceed ([y]/n)? n

Exiting

(output cropped)

I don't have too much knowledge about conda, but it would be great if someone who knows it more help us here saying if there is an option that solves this.

Otherwise, we will need to think in another solution. For example, "remove that command" from the flow, which involves force the user to always add those default packages (sphinx, mock, pillow, rtd_sphinx_theme) to their YAML file, which I think it's not a good "solution".

cc @juanlu001, @willingc do you know if there is an option in conda install to ensure a package is installed (or install it of not) but do not upgrade it if it's already installed? Thanks

@jakirkham
Copy link
Contributor Author

cc @kalefranz

@kalefranz
Copy link

"ensure installed but not upgrade it"

Until conda/conda#6845 was filed, I hadn't recognized that we didn't cover that use case. Right now, unfortunately, there's no good way :(

@stsewd
Copy link
Member

stsewd commented Mar 21, 2018

This is the relevant part of the code

https://github.com/rtfd/readthedocs.org/blob/673bfc7938d8d50d9bda39a62aed08ff41d481ed/readthedocs/doc_builder/python_environments.py#L333-L357

Where conda is installed first and then rtd installs its pip requirements

@humitos isn't an option pin the sphinx version like is done with the python environment?

https://github.com/rtfd/readthedocs.org/blob/673bfc7938d8d50d9bda39a62aed08ff41d481ed/readthedocs/doc_builder/python_environments.py#L219-L227

@stsewd
Copy link
Member

stsewd commented Mar 21, 2018

Kind of related to #1364

@humitos
Copy link
Member

humitos commented Mar 21, 2018

@stsewd we decided to unpin them some time ago because we had incompatibility problems with conda. Check this PR #2876

I think what we are doing now is correct, but we need conda to behaves like pip in that way of "ensure installed without upgrade"

I'm marking this as blocked on conda/conda#6845 for now (when that feature gets implemented I think this will be fixed automatically in RTD without code changes)

If there are someone with a better solution for the general problem that works in these different scenarios, we should probably implement it --but for now, we don't have a better solution :(

@wkerzendorf
Copy link

@stsewd does this mean you can't pin sphinx at the moment (sorry am not quite understanding it)

@stsewd
Copy link
Member

stsewd commented May 17, 2018

@wkerzendorf no, you can't. If you pin sphinx it will be updated by rtd later, this issue depends on conda/conda#6845

@wkerzendorf
Copy link

That’s unfortunate- any idea for some work around. Could I revert to pip or does that also not work. Could RTD just change the order of when which conda is called

@stsewd
Copy link
Member

stsewd commented May 17, 2018

@wkerzendorf you can do it with pip, this is only a conda problem.

@kalefranz
Copy link

conda/conda#6845 should be resolved in conda 4.6 with conda/conda#7291

@wkerzendorf
Copy link

Any idea when this is coming out?

@kalefranz
Copy link

kalefranz commented May 17, 2018 via email

@willingc
Copy link
Contributor

@kalefranz I wonder what happens if you have Sphinx install in the pip section of the environment.yml. I'm on limited bandwidth right now or I would try it now.

@kalefranz
Copy link

I don't think we add any type of --upgrade flag when we shell out to pip for the environment.yml stuff. So my hunch is that it works as you want.

@humitos
Copy link
Member

humitos commented May 22, 2018

I'm marking this as blocked on conda/conda#6845 for now (when that feature gets implemented I think this will be fixed automatically in RTD without code changes)

This issue was closed in conda (implemented by conda/conda#7291)

I suppose that we will need to wait for the next mini conda release that contains this feature and then:

  • update our docker image with that conda version
  • update our build servers with that docker image
  • update our python code to call the conda command with the -S, --satisfied-skip-solve argument

@humitos
Copy link
Member

humitos commented Jul 10, 2018

Although the conda issue is already solved in code, it will be available in 4.6.0 which is unreleased at this time. I'd like to work on the PR to add/fix this once it's released.

humitos added a commit that referenced this issue Jul 18, 2019
We run 3 steps when a project depends on conda.

1. create the whole environment based on user's YAML file
2. run `conda install` with our own dependencies
3. run `pip install` with some of our dependencies that are not
published on conda repositories.

This commit changes this to make it in just one step (at environment
creation). To do this, it appends our own requirements to the
`environment.yml` file under `dependencies` and `dependencies.pip`
config (see https://docs.conda.io/projects/conda/en/latest/user-guide/tasks/manage-environments.html#create-env-file-manually)

It also shows the resulting `environment.yml` in the build output.

This behavior is added behind a feature flag so we can test it first.

This allow users to be able to pin their dependencies as they want
without us upgrading them in the second step. Also, this should
improve the build time, since dependencies resolution is done just
once.

Related to

* #3829
* #5631
@humitos
Copy link
Member

humitos commented Jul 28, 2019

Hi everybody!

This issue has been opened for a long time and we tried different ways to achieve it. Unfortunately, most of them failed and we found that conda does not have the same behavior that pip has regarding packages that are already installed in the environment.

Now, we deployed a new Feature Flag (CONDA_APPEND_CORE_REQUIREMENTS) to try to attack this problem and finally be able to pin "Read the Docs common dependencies" (sphinx, recommonmark, sphinx-rtd-theme, mock, pillow, etc). This work is done at #5956 and #5986

This feature is under a Feature Flag because it's not broadly tested yet and may need some extra work once we have more data on projects using it and having common issues. I'd be happy to enable this flag to any project that has been involved in this issue and get some feedback from you.

As a side note, I also added another Feature Flag (UPDATE_CONDA_STARTUP) to update the version of conda to the latest release at the beginning of the process, which it may be good to enable as well.

Please, let me know what are the projects you would like to enable these flags, and I will do it next week probably.

Thanks you all for the support on this!

@adam-urbanczyk
Copy link

I believe CQ is also suffering from this issue: https://readthedocs.org/projects/cadquery/builds/10978472/
Could you enable this CONDA_APPEND_CORE_REQUIREMENTS flag for CQ @humitos ?

CC: @jmwright

@humitos
Copy link
Member

humitos commented May 21, 2020

@adam-urbanczyk Done! Let me know if there is any problem (better in a new issue or support email, so we don't notify all the people involved in this issue). Thanks!

@stsewd
Copy link
Member

stsewd commented Feb 2, 2021

Just a note here, looks like the default conda channel has really old versions of sphinx and the rtd-theme. If you are going to pin your deps to an updated version, you should include the conda-forge channel (when doing using the CONDA_APPEND_CORE_REQUIREMENTS feature flag).

Screenshot from 2021-02-02 15-13-57

We could also try to use that channel explicitly to install our deps, but I'm not sure if that could break something. But I think we should just always use the CONDA_APPEND_CORE_REQUIREMENTS flag and recommend users to include the conda-forge channel to get their latest versions.

@astrojuanlu
Copy link
Contributor

I just skimmed this thread to understand the current status of the issue, but still need a bit more time to read. For now, let me leave a quick answer to this one concern:

We could also try to use that channel explicitly to install our deps, but I'm not sure if that could break something.

I think it's worth trying. Today I got confirmation on Twitter from @isuruf that the conda-forge channel is independent from defaults on Linux (IIUC, this means that it's "self-sufficient", but this might be slightly imprecise).

Just for future reference, the new (April 2020) Anaconda Inc. Terms of Service impose several restrictions on free of charge use of the defaults channel (also confirmed by the CEO on Twitter and clarified in the Anaconda Inc. blog). This is pushing more users to conda-forge, which is being more battle-tested than ever (it has been quite reliable over the past years anyway).

joaander added a commit to glotzerlab/hoomd-blue that referenced this issue Jan 12, 2023
Also switch to the = syntax to pull the latest bugfix release 3.11.*.
Problems with the bulleted lists should have been fixed: readthedocs/readthedocs.org#8252
But conda environments don't work well when the users specifies sphinx and sphinx_rtd_theme: readthedocs/readthedocs.org#3829
joaander added a commit to glotzerlab/hoomd-blue that referenced this issue Jan 12, 2023
Bulleted lists were not rendering properly. Apparently, this is
an old bug (readthedocs/sphinx_rtd_theme#1115)
that has recently resurfaced. The solution is to pin newer versions
of sphinx and sphinx_rtd_theme (https://stackoverflow.com/questions/67542699/readthedocs-sphinx-not-rendering-bullet-list-from-rst-file/71069918#71069918).
However, readthedocs conda build environment does not allow pinning:
readthedocs/readthedocs.org#3829

Thus, the only solution is to convert to a requirements.txt build
on readthedocs. While configuring this build, I also updated to the
latest system image, python version, and to a non-deprecated
name for the readthedocs configuration file
(https://docs.readthedocs.io/en/stable/config-file/v2.html).
@humitos
Copy link
Member

humitos commented Feb 21, 2024

Is this issue still valid? I think we have removed CONDA_APPEND_CORE_REQUIREMENTS feature flag and enabled it for all the projects by default.

@willingc
Copy link
Contributor

I think that this probably can be closed. @jakirkham?

@astrojuanlu
Copy link
Contributor

Also these days there’s https://docs.readthedocs.io/en/stable/config-file/v2.html#build-tools-python to choose between conda and mamba

@humitos
Copy link
Member

humitos commented Feb 22, 2024

OK, thanks for the feedback here. I think we can close this issue then, but feel free to re-open or ping me here if you consider it shouldn't be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Bug A bug Status: blocked Issue is blocked on another issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.