-
Notifications
You must be signed in to change notification settings - Fork 349
sof-docs: build out of source directory #9878
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
Conversation
scripts/gen-doc.sh
Outdated
| # this needs to run from the Zephyr python virtual environment | ||
| # so that the python packages are available (and more up to date than | ||
| # the system packages). | ||
| source ~/zephyrproject/.venv/bin/activate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Firstly I don't think a requirement of zephyrproject being called that way and being in $HOME should be imposed. This isn't just for docker images or DUTs, this is for developers, using these manually on their development machines, right? Secondly, I also use venv, I'm not against it in general, but has it become an absolute requirement yet? Thirdly, the venv directory doesn't necessarily have to be called .venv, I've seen multiple examples just using venv and that's also what I use FWIW. Wouldn't it be better, if we do want to enforce the use of venv, to just check that it's enabled and error out otherwise? In general, I'd leave it to the user to activate and deactivate the venv.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This really needs to be simple and align to the getting started documentation for both SOF and Zephyr. Going off track here just add complexity.
Firstly I don't think a requirement of
zephyrprojectbeing called that way and being in$HOMEshould be imposed.
Zephyr is locked in as a hard requirement now and the path aligns with the instructions that most developers will use.
This isn't just for docker images or DUTs, this is for developers, using these manually on their development machines, right?
This is for developers, github runners and docker. Its really simple now to get the right ingredients to build docs.
Secondly, I also use venv, I'm not against it in general, but has it become an absolute requirement yet? Thirdly, the venv directory doesn't necessarily have to be called
.venv, I've seen multiple examples just usingvenvand that's also what I use FWIW. Wouldn't it be better, if we do want to enforce the use of venv, to just check that it's enabled and error out otherwise? In general, I'd leave it to the user to activate and deactivate the venv.
Again this is following the default instructions. Yes a Python virtual environment is needed as pip is better managed for python support than apt-get based python packages. Leaving it to the user is just an extra step and then the system python packages could be used and fail (and hence developers may then waste time/effort debugging). This scripts are for convenience, developers can still manually invoke a venv and manually run make if they so desire.
kv2019i
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will surprise (e.g. all my working trees on all my machines), but I can live with making the default setup easier. We do have a "choose-your-workspace" approach in our own SOF docs https://thesofproject.github.io/latest/getting_started/build-guide/build-with-zephyr.html
System tools for sphinx etc tend to be behind the versions available via pip. Prefer the pip version and automatically enable the Python virtual environment when starting the script. Signed-off-by: Liam Girdwood <liam.r.girdwood@intel.com>
Move the intermediate doxygen build output out of the source directory. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
841e74f to
3e3ce61
Compare
jsarha
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to build the sof-docs with this PR, but I am not sure if Zephyr's python virtual environment is better for the job than any other virtual environment. AFAIK sof-docs build does not need west or any other Zephyr dependencies, but it needs several other packages that are not in the Zephyr's python virtual environment.
Then again it I guess it does not matter which virtual environment is used, but it would be nice if the script would install the dependecies, and one would not need to do so one by one after hitting each of them. So could there be flag or test to check if running pip install for sphinx, sphinx_rtd_theme, breathe, sphinxcontrib.plantuml, and sphinxcontrib.blockdiag is needed?
In any case this gives a hint that a python virtual environment helps in building the docs, so this is an improvement.
Build the sof docs outside of the source directory and within a Python virtual environment.