Build: use environment variable $READTHEDOCS_OUTPUT to define output directory

[humitos]

READTHEDOCS_OUTPUT points to the path where the user's repository was cloned plus _readthedocs/ subdirectory. This way we can make the command nicer:

python -m sphinx -T -E -j auto -b singlehtml -d _build/doctrees -D language=en . $READTHEDOCS_OUTPUT/html

Also, we gives our users a clear contract to where to find those files, even if we change the path of the underlying variable in the future.

This PR is a continuation of #9888. I split it into a different PR because I'm having problems to make the Sphinx command resolving the variable into its real value. The output folders are being saved into $READTHEDOCS_OUTPUT being that string the name of the folder 🤦🏼 . We need to research/debug a little more how to make this work properly.

[ericholscher]

I played around with this a little bit, and it looks like everything is set properly on docker... just need to figure out how to make it expand it. 🙃

[humitos]

Meh, because this PR was based in another PR that now is "Squashed & Merged" we have a bunch of individual commits in this one. We should rebase it to make it consistent now 😞

[humitos]

I merge main into this branch because it was the easiest way to handle with merge conflicts. Then I rebased this branch to remove all the "duplicated commits" and only leave the ones done in this particular PR. I think everything is fine and cleaner now, I hope.

[humitos]

I found why the variable was not working properly and I fixed it in 300d303

We need to discuss how to do it correctly to avoid that hack that I put there temporary to show the concept.

[ericholscher]

I found why the variable was not working properly and I fixed it in 300d303

Heh -- that is definitely a hack! Thanks for figuring that out, I really couldn't understand what was happening, but that makes sense.

Not 100% sure the best option here. I actually do think there's some useful cases for using env vars in the build commands. I wish we had a comment explaining why we added the escaping. I'm guessing just to be extra careful when allowing users to start modifying build commands.

[humitos]

I think that "build.jobs" and "build.commands" go over a different workflow because they support environment variables.

I think we added this to protect ourselves, maybe? I don't remember. However, we can simply use "escape=False" argument for these 2 commands

[humitos]

I think we added this to protect ourselves, maybe? I don't remember. However, we can simply use "escape=False" argument for these 2 commands

Yeap, we are using escape=False for build.jobs and build.commands:

readthedocs.org/readthedocs/doc_builder/director.py

Lines 332 to 333 in 0c0d2c6

	for command in commands:
	environment.run(*command.split(), escape_command=False, cwd=cwd)

[humitos]

I found why we need the escape_command=True in some of our commands. It's because we are using things like pip install setuptools<58. That < breaks the command otherwise.

I noted that I want to avoid escaping another environment variable from ourselves (in #9971), so I may use this hack for now 😄

[humitos]

@ericholscher this is ready for another round of review 👍🏼

[ericholscher]

This feels really close. I mainly just have a question on the output dir variables.

[humitos]

@ericholscher please, take a final look and let me know if this is ready to merge and deploy tomorrow 👍🏼

[ericholscher]

🚢