initc3 / HoneyBadgerMPC

Robust MPC-based confidentiality layer for blockchains
GNU General Public License v3.0
131 stars 64 forks source link

Fix sphinx-build error to generate docs #378

Closed sbellem closed 4 years ago

sbellem commented 4 years ago

The error:

root@15a832adeb00:/usr/src/HoneyBadgerMPC# make -C docs html
usage: sphinx-build [-h] [-d] [-f CONFIG_FILE_PATH]          
sphinx-build: error: unrecognized arguments: -M html . _build

This perhaps is caused by a combination of using autodoc to generate documentation from the docstrings and not having an if __name__ == "__main__" clause in a python module that could be run as a script. So the problem seems to be with the following code:

https://github.com/initc3/HoneyBadgerMPC/blob/6aec65f9a1e430316077bc98d548bda42571b4b3/honeybadgermpc/__init__.py#L16-L18

Now adding the if clause as in:

if __name__ == "__main__":

    # Skip loading the config for tests since the would have different values for sys.argv.
    if "pytest" not in sys.modules:
        HbmpcConfig.load_config()

works as far as building the docs but it creates problems for actually running the code as in the case of tutorial 2 for instance, because the config does not get loaded as it is expected to be:

$ scripts/launch-tmuxlocal.sh apps/tutorial/hbmpc-tutorial-2.py conf/mpc/local
WARNING: the $CONFIG_PATH environment variable wasn't set. Please run this file with `scripts/launch-tmuxlocal.sh apps/tutorial/hbmpc-tutorial-2.py conf/mpc/local`

So it seems that to properly solve this issue the logic that handles the loading of the config may need to be reviewed (#382). The main observation one may make is that the "loading config" code that reads command line arguments does not need to be invoked by default in the project root __init__.py. Doing so is obviously convenient, so the proposed solution should aim at preserving the convenience of the current approach meanwhile resolving the problem it causes for generating documentation.

sbellem commented 4 years ago

This issue seems to have been encountered by @Drake-Eidukas as he points out in https://github.com/initc3/HoneyBadgerMPC/blob/0914c0269a5d3636b01f40557e8ea432f721bd04/.travis.yml#L48-L49

The comment seems to be now misplaced and should be for the Sphinx Tests job https://github.com/initc3/HoneyBadgerMPC/blob/0914c0269a5d3636b01f40557e8ea432f721bd04/.travis.yml#L40

So it looks like the documentation is actually not being built on Travis CI, and the job should actually error out.