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

Better error message when using build.tools.python without build.os #8912

Closed
adamjstewart opened this issue Feb 12, 2022 · 10 comments
Closed
Labels
Accepted Accepted issue on our roadmap Improvement Minor improvement to code

Comments

@adamjstewart
Copy link

Details

Expected Result

I would expect the following simple .readthedocs.yaml file to work:

# Read the Docs configuration file
# See https://docs.readthedocs.io/en/stable/config-file/v2.html for details

# Required
version: 2

# Set the version of Python
build:
  tools:
    python: "3.10"

# Configuration for Sphinx documentation
sphinx:
   configuration: docs/conf.py
   fail_on_warning: true

Actual Result

I see the following build error:

Problem in your project's configuration. Invalid "build.tools.python": .readthedocs.yaml: Invalid configuration option: build.tools.python. Make sure the key name is correct.

For now I'll just remove this option.

@choi-jiwoo
Copy link

Had same issue when building documentation of my project, and found out that I had to configure build.os before build.tools. It is stated as a Required field in their documentation.

See the documentation about build.os here.

# Read the Docs configuration file
# See https://docs.readthedocs.io/en/stable/config-file/v2.html for details

# Required
version: 2

# Set the version of Python
build:
  os: ubuntu-20.04  # <- add this line
  tools:
    python: "3.10"

# Configuration for Sphinx documentation
sphinx:
   configuration: docs/conf.py
   fail_on_warning: true

@adamjstewart
Copy link
Author

Hmm, is there any reason that build.os needs to be required to use build.tools? There's only one option at the moment and I'm not sure if most users will care what version of Linux they build on.

@humitos
Copy link
Member

humitos commented Feb 14, 2022

Hi! Yeah, build.os is required as you already figured it out. However, the error message should be clearer and communicate this in a better way. @stsewd is there an easy way to add this check to run before the build.tools.python and update the error message?

Note that I created a branch in test-builds to test this. See the build output at https://readthedocs.org/projects/test-builds/builds/16080135/

is there any reason that build.os needs to be required to use build.tools?

In simple words, having a default here has avoided us to move forward in the past. We experienced similar issues with build.image that defaulted to latest and because of that we weren't able to touch latest image anymore because it always breaks someone's else project.

@stsewd
Copy link
Member

stsewd commented Feb 14, 2022

Yes, it should be possible to provide a better message

@adamjstewart
Copy link
Author

In simple words, having a default here has avoided us to move forward in the past. We experienced similar issues with build.image that defaulted to latest and because of that we weren't able to touch latest image anymore because it always breaks someone's else project.

But presumably if I don't specify builds.os or build.tools I'm still getting some default image, right? Why should the requirements be any different if I also want to specify build.tools? Other CI platforms like GitHub Actions allow me to select ubuntu-latest so that I can test my package against newer OSes.

@humitos
Copy link
Member

humitos commented Feb 14, 2022

Other CI platforms like GitHub Actions allow me to select ubuntu-latest so that I can test my package against newer OSes.

We have an open issue for this at #8861 where you can read and comment about the discussion.

@humitos humitos added Accepted Accepted issue on our roadmap Improvement Minor improvement to code labels Feb 14, 2022
@stsewd stsewd changed the title Invalid configuration option: build.tools.python Better error message when using build.tools.python without build.os Oct 3, 2022
bendnorman added a commit to catalyst-cooperative/pudl-catalog that referenced this issue Feb 7, 2023
Need to specify the os to specify python version
readthedocs/readthedocs.org#8912
circumspect added a commit to circumspect/White-Rabbit that referenced this issue Mar 30, 2023
@cjw296
Copy link

cjw296 commented May 15, 2023

Why is build.os required at all? I write libraries that are OS agnostic, so why can't a "default" OS be in place? (likely "latest ubuntu").

@humitos
Copy link
Member

humitos commented May 22, 2023

@cjw296 the process to build the documentation itself has to run on a OS. We used the approach of having a "default OS" in the past and it's more problematic than useful to the large scale of users we have. We think that explicitly defining the dependencies is good practice and helps people to create reproducible builds over time.

stephenfin added a commit to getpatchwork/git-pw that referenced this issue Jun 27, 2023
This is required. The error message is misleading [1].

[1] readthedocs/readthedocs.org#8912

Signed-off-by: Stephen Finucane <stephen@that.guru>
stephenfin added a commit to getpatchwork/patchwork that referenced this issue Jun 27, 2023
This is required. The error message is misleading [1].

[1] readthedocs/readthedocs.org#8912

Signed-off-by: Stephen Finucane <stephen@that.guru>
stephenfin added a commit to getpatchwork/pwclient that referenced this issue Jun 27, 2023
This is required. The error message is misleading [1].

[1] readthedocs/readthedocs.org#8912

Signed-off-by: Stephen Finucane <stephen@that.guru>
@humitos humitos added this to the YAML File Completion milestone Jul 4, 2023
JonathonReinhart added a commit to JonathonReinhart/scuba that referenced this issue Dec 11, 2023
JonathonReinhart added a commit to JonathonReinhart/scuba that referenced this issue Dec 18, 2023
mristin added a commit to Parquery/icontract that referenced this issue Jan 7, 2024
The ``build.os`` needs to be set in the readthedocs configuration file,
see readthedocs/readthedocs.org#8912.
mristin added a commit to Parquery/icontract that referenced this issue Jan 7, 2024
The ``build.os`` needs to be set in the readthedocs configuration file,
see readthedocs/readthedocs.org#8912.
mristin added a commit to Parquery/icontract that referenced this issue Jan 7, 2024
The ``build.os`` needs to be set in the readthedocs configuration file,
see readthedocs/readthedocs.org#8912.
JonathonReinhart added a commit to JonathonReinhart/staticx that referenced this issue Jan 8, 2024
mberr added a commit to pykeen/pykeen that referenced this issue Feb 19, 2024
@humitos
Copy link
Member

humitos commented Feb 21, 2024

build.tools.python is no longer required. You can only one of the other tools now. See an example only using build.tools.nodejs at https://beta.readthedocs.org/projects/test-builds/builds/23326369/

Also, we have improved this message as well:

Screenshot_2024-02-21_20-32-20

I think this issue is already solved. Feel free to re-open if you consider.

@humitos humitos closed this as completed Feb 21, 2024
@adamjstewart
Copy link
Author

Looks like there is an ubuntu-lts-latest option now for people who always want to run with the latest ubuntu version 😄

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 Improvement Minor improvement to code
Projects
None yet
Development

No branches or pull requests

5 participants