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

requirements.txt installed when not indicated #2812

Closed
jaraco opened this issue May 1, 2017 · 18 comments
Closed

requirements.txt installed when not indicated #2812

jaraco opened this issue May 1, 2017 · 18 comments
Labels
Needed: design decision A core team decision is required

Comments

@jaraco
Copy link
Contributor

jaraco commented May 1, 2017

Details

Expected Result

Since the config specifies nothing for requirements.txt, it should not attempt to install any requirements.

Actual Result

Even with requirements.txt set to nothing and with a .readthedocs.yml indicating nothing for requirements_file, the build attempts to install requirements and fails.

@jaraco
Copy link
Contributor Author

jaraco commented May 1, 2017

I tried setting requirements_file: null in the .readthedocs.yml, but that only caused another error.

@jaraco
Copy link
Contributor Author

jaraco commented May 1, 2017

Creating an empty requirements file and pointing to that seems to work around the issue.

@jaraco
Copy link
Contributor Author

jaraco commented Aug 27, 2017

Here's another, related failure: https://readthedocs.org/projects/pathpy/builds/5890289/

In that build, I'd once set the requirements file to docs/requirements.txt. I've since removed that setting. Also, there's no requirements set in the YAML. Yet still the builder attempts to install docs/requirements.txt and fails (because it's not there).

Correction - I thought I'd tried to clear that setting, but when I went back to the admin, I'd apparently not succeeded in clearing that setting. Ensuring that the setting was cleared fixed the issue and that build is working now.

@jaraco
Copy link
Contributor Author

jaraco commented Aug 27, 2017

I do suspect this originally-reported issue is still present, that there's apparently no way to ask RTD not to install the requirements.txt file if it exists except to specify an alternate file that's empty.

@humitos
Copy link
Member

humitos commented Dec 27, 2017

I'm not sure to fully understand your problem. I mean, even if your repository doesn't have a reqs.txt RTD will install some base dependencies to build the docs. There is no way to avoid that. Is that what you mean?

@humitos humitos added the Needed: more information A reply from issue author is required label Dec 27, 2017
@xflr6
Copy link

xflr6 commented Jan 12, 2018

I think I have a related issue: I tend to use requirements.txt for the development environment (including dependencies for testing, pinning versions, etc.). I don't want this file to be used in the RTD build.

Currently I need to have an empty docs/requirements.txt (configured to be used in the webinterface) to avoid the RTD build replicating my development environment with all its dependencies not needed at all to build the docs.

If I configure it like this it will still use the requirements.txt from the project root (by default):
capture
I think, it would be nice if there was a checkbox here (or a special name like __none__ if the former is too hard to add) and/or a .readthedocs.yml option to disable using the requirements.txt from the project root completely (I suspect using it by default has proven to be useful for most users in practise).

@humitos humitos added Needed: design decision A core team decision is required and removed Needed: more information A reply from issue author is required labels Jan 15, 2018
@humitos
Copy link
Member

humitos commented Jan 15, 2018

@xflr6 thanks for your report.

RTD automatically finds a requirements.txt (first under the docs_dir and then under the root). If it finds it, it installs the dependencies automatically (even if the option from the web interface is empty):

https://github.com/rtfd/readthedocs.org/blob/8c7123b4ba2b5401e3a351f124cf45b04975e503/readthedocs/doc_builder/python_environments.py#L249-L280

Reading the code, with an empty docs/requirements.txt should be enough. Anyway, it could be considered kind of a hack.

I think changing this default behaviour will break tons of projects, though.

@xflr6
Copy link

xflr6 commented Jan 15, 2018

Thanks: I agree. It is probaly enough to provide some form of opt-out (on the website and/or .readthedocs.yml) from this default.

@tkdchen
Copy link

tkdchen commented Mar 12, 2018

I have same problem. My case is

  • Requirement file is empty in Advanced Settings
  • I create .readthedocs.yml with this content and put it in the project root directory.
    python:
      version: 3
      extra_requirements:
        - docs
    
  • There is a requirements.txt in the project root directory as well.

Actual result is same as the original report. readthedocs finds the requirements.txt and starts to install packages listed within it.

With the support of .readthedocs.yml, my understand is that readthedocs should read user requirements file in this order

  1. requirements_file option in .readthedocs.yml if there is such a YAML file. If it is set, use that file. If it is set to None and extra_requirements is set, run pip command just like the document[1] mentions.
  2. requirements file in the docs_dir()
  3. requirements file in project root directory

I think the fix to the first step in above order must be much helpful for fixing the problem this issue reports. And that should not break anyone else who does not use .readthedocs.yml.

[1] https://docs.readthedocs.io/en/latest/yaml-config.html#python-extra-requirements

@tkdchen
Copy link

tkdchen commented Mar 12, 2018

After reading the code, I find I was wrong.

            self.python_env.setup_base()
            self.python_env.save_environment_json()
            self.python_env.install_core_requirements()
            self.python_env.install_user_requirements()
            self.python_env.install_package()

extra_requirements in .readthedocs.yml is handled inside install_package as the last step.

But, I really think that settings in .readthedocs.yml should override the install_user_requirements rather than as extras.

BTW, the current behavior of handling extra_requirements after all possible requirements files is not documented in https://docs.readthedocs.io/en/latest/yaml-config.html.

tkdchen added a commit to Nitrate/Nitrate that referenced this issue Mar 12, 2018
The empty docs/requirements.txt is a workaround to make readthedocs
not install anything from possible requirements files found from default
places automatically.

Refer to: readthedocs/readthedocs.org#2812

Signed-off-by: Chenxiong Qi <qcxhome@gmail.com>
@jaraco
Copy link
Contributor Author

jaraco commented Mar 12, 2018

I'm not sure to fully understand your problem. I mean, even if your repository doesn't have a reqs.txt RTD will install some base dependencies to build the docs. There is no way to avoid that. Is that what you mean?

I apologize for not seeing this message or responding sooner. The issue I have is the same as xflr6 described. We have a requirements.txt there for heroku to install the project, but those aren't the requirements needed to build the docs.

Reading the code, with an empty docs/requirements.txt should be enough.

That's a better workaround than the one I found (setting the requirements file to another, empty file).

I suggest .readthedocs.yml should allow a null value for the requirements.txt file and that should disable the search for requirements.txt. That change depends on #2813, but would satisfy our use-cases. It might also be nice if there were a UI-equivalent, some way in the UI to say "don't use requirements.txt files implicitly", though personally I'd be satisfied with a supported interface in .readthedocs.yml.

@jaraco
Copy link
Contributor Author

jaraco commented Mar 12, 2018

an empty docs/requirements.txt should be enough.

I find otherwise. In this commit, I removed the explicit pointer to docs/requirements.txt, but the RTD build failed when it tried to install reqs from requirements.txt.

So I'll be restoring that workaround.

@stsewd
Copy link
Member

stsewd commented Mar 12, 2018

I find otherwise. In this commit, I removed the explicit pointer to docs/requirements.txt, but the RTD build failed when it tried to install reqs from requirements.txt.

mmm, that's strange, I going to take a look at the code. Probably that is a bug.

@stsewd
Copy link
Member

stsewd commented Mar 12, 2018

I found it, it's a bug. The break here only breaks for the first inner loop, so it keeps searching for a requirements file on the root.

https://github.com/rtfd/readthedocs.org/blob/8c7123b4ba2b5401e3a351f124cf45b04975e503/readthedocs/doc_builder/python_environments.py#L255-L260

@xflr6
Copy link

xflr6 commented Mar 12, 2018

How about this?

for path, req_file in itertools.product([docs_dir, ''], ['pip_requirements.txt', 'requirements.txt']):
    test_path = os.path.join(self.checkout_path, path, req_file) 
    if os.path.exists(test_path):
        requirements_file_path = test_path
        break

@stsewd
Copy link
Member

stsewd commented Mar 13, 2018

@xflr6 I think that would work, will you like to make a PR? Also, some tests will be needed.

@xflr6
Copy link

xflr6 commented Mar 14, 2018

@stsewd I took a glimpse at the test setup: I don't think I'll find enough time to add that soon.

@stsewd
Copy link
Member

stsewd commented Mar 14, 2018

@xflr6 don't worry, I'll take care of submit the fix with the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needed: design decision A core team decision is required
Projects
None yet
Development

No branches or pull requests

5 participants