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.

Sign up for GitHub

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

Jump to bottom

Sphinx: Unable to disable making man pages #2155

Closed
psampathkumar opened this issue Jan 18, 2024 · 3 comments · Fixed by #2161
Closed

Sphinx: Unable to disable making man pages #2155

psampathkumar opened this issue Jan 18, 2024 · 3 comments · Fixed by #2161
Labels
bug Something isn't working tools Issues related to mp, ruledit, etc along with CI and build tools.

Comments

@psampathkumar
Copy link
Contributor

Describe the bug
When people compile the code for themselves in master, they should be able to disable making the RTD man pages, or ignore it when there is a error.

To Reproduce
Steps to reproduce the behavior:

  1. Compile without sphinx_rtd_theme in system.
  2. See Error
  3. Set -DFREECIV_ENABLE_MANPAGES flag to OFF in CMake
  4. Still See error

Expected behavior
Man pages are skipped with the flag.

Screenshots
image

Platform and version (please complete the following information):

  • OS: Arch Linux
  • Freeciv21 version: [e.g. 3.1-alpha.4]

Additional context
It works in stable without this error, with the same PKGBUILD on arch.
@psampathkumar psampathkumar added bug Something isn't working Untriaged This issue or PR needs triaging labels Jan 18, 2024
@blabber
Copy link
Collaborator

blabber commented Jan 19, 2024

Of course, FREECIV_ENABLE_MANPAGES should be taken into account during the build.
But this issue raises another question: Why is sphinx_rtd_theme even needed to render a manpage?

@jwrober
Copy link
Collaborator

jwrober commented Jan 19, 2024

Is part of our requirements file - https://github.com/longturn/freeciv21/blob/master/docs/requirements.txt

blabber added a commit to blabber/freeciv21 that referenced this issue Jan 19, 2024
@blabber
cmake: Fix the option to disable man pages
8125947 
This commit fixes the handling of the `FREECIV_ENABLE_MANPAGES` option,
leveraging the fact, that an `option` call is a no-op if the value is
already set.

While here, allow users on non-Unix systems to build manpages.

Closes longturn#2155
@blabber blabber mentioned this issue Jan 19, 2024
Merged
@psampathkumar
Copy link
Contributor Author

psampathkumar commented Jan 20, 2024 •
edited
Loading

Why is sphinx_rtd_theme even needed to render a manpage?

It is for the readthedocs website (https://longturn.readthedocs.io/en/latest/index.html). The end user mostly doesnt need that build unless for some reason they want to have a local copy of that website. They'll get all the information from text files without the manpage compile..

@lmoureaux lmoureaux added tools Issues related to mp, ruledit, etc along with CI and build tools. and removed Untriaged This issue or PR needs triaging labels Jan 20, 2024
lmoureaux added a commit to lmoureaux/freeciv21 that referenced this issue Jan 20, 2024
@lmoureaux
Docs: make external packages optional
e9decb2 
It was noted in longturn#2155 that when packaging Freeciv21, sphinx_rtd_theme and
sphinx_last_updated_by_git were needed at build time to create the man pages.
The rtd theme is obviously only needed for HTML output and last_updated_by_git
doesn't seem to be used with the man pages. Make them both optional by checking
if they can be imported when evaluating the config.

This will allow package maintainers to ship man pages even when if of the two
packages are missing from their distribution.
@lmoureaux lmoureaux mentioned this issue Jan 20, 2024
Merged
lmoureaux added a commit to lmoureaux/freeciv21 that referenced this issue Jan 20, 2024
@lmoureaux
Docs: make external packages optional
f53d414 
It was noted in longturn#2155 that when packaging Freeciv21, sphinx_rtd_theme and
sphinx_last_updated_by_git were needed at build time to create the man pages.
The rtd theme is obviously only needed for HTML output and last_updated_by_git
doesn't seem to be used with the man pages. Make them both optional by checking
if they can be imported when evaluating the config.

This will allow package maintainers to ship man pages even when one of the two
packages are missing from their distribution.
lmoureaux added a commit that referenced this issue Jan 20, 2024
@lmoureaux
Docs: make external packages optional
eb5119a 
It was noted in #2155 that when packaging Freeciv21, sphinx_rtd_theme and
sphinx_last_updated_by_git were needed at build time to create the man pages.
The rtd theme is obviously only needed for HTML output and last_updated_by_git
doesn't seem to be used with the man pages. Make them both optional by checking
if they can be imported when evaluating the config.

This will allow package maintainers to ship man pages even when one of the two
packages are missing from their distribution.
lmoureaux pushed a commit that referenced this issue Jan 21, 2024
@blabber @lmoureaux
cmake: Fix the option to disable man pages
c9d544d 
This commit fixes the handling of the `FREECIV_ENABLE_MANPAGES` option,
leveraging the fact, that an `option` call is a no-op if the value is
already set.

While here, allow users on non-Unix systems to build manpages.

Closes #2155
lmoureaux added a commit to lmoureaux/freeciv21 that referenced this issue Mar 17, 2024
@lmoureaux
Docs: make external packages optional
0ec0ead 
It was noted in longturn#2155 that when packaging Freeciv21, sphinx_rtd_theme and
sphinx_last_updated_by_git were needed at build time to create the man pages.
The rtd theme is obviously only needed for HTML output and last_updated_by_git
doesn't seem to be used with the man pages. Make them both optional by checking
if they can be imported when evaluating the config.

This will allow package maintainers to ship man pages even when one of the two
packages are missing from their distribution.
lmoureaux pushed a commit to lmoureaux/freeciv21 that referenced this issue Mar 17, 2024
@blabber @lmoureaux
cmake: Fix the option to disable man pages
56987a6 
This commit fixes the handling of the `FREECIV_ENABLE_MANPAGES` option,
leveraging the fact, that an `option` call is a no-op if the value is
already set.

While here, allow users on non-Unix systems to build manpages.

Closes longturn#2155
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working tools Issues related to mp, ruledit, etc along with CI and build tools.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

cmake: Fix the option to disable man pages blabber/freeciv21
4 participants
@blabber @lmoureaux @jwrober @psampathkumar