Add new option to allow override pdflatex binary

[skirpichev]

Any pdflatex-compatible binary could be used instead. For example, xelatex (as requested in rtfd/readthedocs.org#1556).

This is a very primitive version of that can be used to solve mentioned issue. Some other variants are mentioned in the issue thread.

[skirpichev]

@agjohnson, any feedback?

[asr]

I'm interested in the fix of #1556. Any feedback on this PR?

[skirpichev]

Ok, I did requested change.

This should make sense otherwise, with a companion patch to rtfd/readthedocs.org to use this setting.

Maybe it's better to use latex_engine option from conf.py? In this case - probably no changes should be made here, but all in rtfd/readthedocs.org repo.

[skirpichev]

Test failure seems to be unrelated - master branch is broken.

[humitos]

@skirpichev I'm not the one taking the decisions but I would suggest to add documentation (maybe in spec.rst), and also changing the name of the pdflatex option to something more descriptive like pdf_bin or pdf_binary. Although, I think it shouldn't be a simple string but a multiple choice value because if it will be executed as is, RTD would be executing arbitrary commands.

On the other hand, this PR will need another one into the readthedocs.org code to finally execute this command, right?

[skirpichev]

but I would suggest to add documentation

Sure. But I don't know where it should be. Most yml settings are documented now in readthedocs.org main repo.

something more descriptive like pdf_bin or pdf_binary

Maybe. But your suggestions looks even more cryptic.

I think it shouldn't be a simple string but a multiple choice value because if it will be executed as is, RTD would be executing arbitrary commands.

Or just add more validation for this option, e.g. test if this value resolves to some binary in $PATH. But I think, multivalued option is fine. Thank you.

On the other hand, this PR will need another one into the readthedocs.org code to finally execute this command, right?

Sure. I'm waiting here, because otherwise PR against readthedocs.org doesn't make sense.

Here is another way to solve issue. So, it's possible without changes in readthedocs-build, in principle.

[agjohnson]

So, this seems a little more complicated than just swapping out the binary used. I agree with @humitos that the option shouldn't be just a string, and that it should probably be moved to a different namespace in the space.

Bear with us, this is going to be a spec related decision.

I believe we'll be adding a number of options to configure sphinx eventually, so perhaps putting it under the sphinx namespace makes more sense. Here are some options:

sphinx:
  latex:
    binary: (?:xelatex|latex)
sphinx:
  latex_binary: (?:xelatex|latex)

I think i lean towards option 1, perhaps there are others? This would take adding parsing of the sphinx and sphinx['latex'] dictionaries, and then finally the binary parameter.

[skirpichev]

I believe we'll be adding a number of options to configure sphinx eventually

@agjohnson, could you provide an example? Most sphinx stuff - configurable via conf.py, I think.

[humitos]

I think i lean towards option 1, perhaps there are others? This would take adding parsing of the sphinx and sphinx['latex'] dictionaries, and then finally the binary parameter.

I also prefer the option 1 since it prepares RTD for the future in case we want to teak a little bit more sphinx or even the latex builder.

@agjohnson, could you provide an example? Most sphinx stuff - configurable via conf.py, I think.

I think the idea of the .readthedocs.yml is to finally get rid of the conf.py or at least most of it. Not sure anyway.

[skirpichev]

I think the idea of the .readthedocs.yml is to finally get rid of the conf.py or at least most of it.

@humitos, I hope, it's not...

[skirpichev]

@agjohnson, thank you for review and sorry for your time. I'll close this pr per https://github.com/rtfd/readthedocs.org/pull/2362