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.

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

Extend documentation with CMake options #1812

Closed
ntamas opened this issue Sep 14, 2021 · 11 comments
Closed

Extend documentation with CMake options #1812

ntamas opened this issue Sep 14, 2021 · 11 comments
Labels
documentation Issues that are related to the documentation of igraph; good candidates for first-time contributors good first issue Candidates for first-time contributors who are already familiar with C

Comments

@ntamas
Copy link
Member

ntamas commented Sep 14, 2021

What is the feature or improvement you would like to see?

The installation part in the documentation should be extended with a listing of the most common CMake options.

Use cases for the feature

E.g., turning off IGRAPH_WARNINGS_AS_ERRORS is particularly useful for downstream projects that compile igraph as part of their toolchain if the toolchain itself throws warnings that we did not anticipate.

References

N/A

@ntamas ntamas added documentation Issues that are related to the documentation of igraph; good candidates for first-time contributors good first issue Candidates for first-time contributors who are already familiar with C PR welcome labels Sep 14, 2021
@szhorvat
Copy link
Member

I was thinking about fixing this yesterday, and one thought I had was to not put it directly into the installation instructions. Instead, create a Troubleshooting (or Possible Issues) section, and mention it there, along with other potential problems.

We don't want most people (e.g. distro maintainers) to set this to OFF by default. Ideally, if a warning is triggered, people would come and report it to us. The risk of documenting it along with more common options is that people will just set it to OFF from the get-go.

Thoughts?

@vtraag
Copy link
Member

vtraag commented Sep 14, 2021

I agree that separating it from the main instructions is a good idea. We could also rubric it under something like "Advanced Configuration" or "Advanced Settings" or the like? A "Troubleshooting" or "Possible Issues" section also sounds good to me, but it might be nice to simply have a list of possible configuration options, instead of necessarily looking at the troubleshooting section to look for configuration options.

@ntamas
Copy link
Member Author

ntamas commented Sep 14, 2021

I'd probably go for an "Advanced Configuration" settings chapter, with a big disclaimer at the beginning ("Abandon all expectations, ye who enter here"). Not all CMake options are related to troubleshooting; e.g., the possibility to turn off GraphML support to spare a dependency on libxml2 is not "troubleshooting", and neither is the option to configure igraph as 32-bit on a 64-bit platform.

@iosonofabio
Copy link
Member

iosonofabio commented Sep 14, 2021 via email

@ntamas
Copy link
Member Author

ntamas commented Sep 14, 2021

I know but I felt that it wouldn't be appropriate here after all :)

@szhorvat
Copy link
Member

I won't fix this for 0.9.5. Almost all options are already documented:

https://igraph.org/c/doc/igraph-Installation.html#igraph-Installation-notable-configuration-options

A GUI (either ccmake or cmake-gui) will display relevant options, including IGRAPH_WARNINGS_AS_ERRORS, with a short description. Thus, the option is discoverable.

We can restructure the docs with an Advanced Configuration section for 0.10. Right now this would be the only "advanced" option.

ntamas added a commit that referenced this issue Oct 18, 2021
@ntamas
Copy link
Member Author

ntamas commented Oct 18, 2021

Added IGRAPH_WARNINGS_AND_ERRORS and BUILD_TESTING; I think that the rest are really niche and the CMake GUI help strings are enough.

@ntamas ntamas closed this as completed Oct 18, 2021
@szhorvat
Copy link
Member

@ntamas BUILD_TESTING: The tests are not built unless one is explicitly asking for it (building the build_tests target, or something that depends on it such as test / check). Why then do we document it?

I think we should remove it as the current text is confusing. It suggests that the tests are built by default, which is not true.

@szhorvat
Copy link
Member

@ntamas IGRAPH_WARNINGS_AND_ERRORS: Should we not ask people to report it to us if they encounter warnings that prevent the build? Warnings are only treated as errors with the main compilers that we use, such as clang / GCC. It won't happen with other, exotic compilers such as PGI (I don't recall Intel). So people should really not be using this setting, except as a last resort.

@ntamas
Copy link
Member Author

ntamas commented Oct 18, 2021

Re BUILD_TESTING: you are right. The name of the option is misleading (it misled me at least), but note that the option is not created by us; it is created by CTest automatically and we are supposed to handle it in CMakeLists.txt such that if the user turns this option OFF then we don't even create the test targets. I'll try to rephrase it in the docs.

Re IGRAPH_WARNINGS_AND_ERRORS: actually, we handle it for GCC, Clang and Intel compilers, and sometimes it happens that a particular version of igraph was created with an older GCC/Clang version in mind and some new version introduces warnings that the old version of igraph did not account for. In this case I'd rather not receive reports as I don't want to update older, non-maintained versions just to keep the build clean for newer compilers.

@ntamas
Copy link
Member Author

ntamas commented Oct 18, 2021

Okay, I'll just remove BUILD_TESTING; it's too complicated to explain and it really does not have much use, considering that we have a separate target for testing and tests are not built by default.

ntamas added a commit that referenced this issue Oct 18, 2021
ntamas added a commit that referenced this issue Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issues that are related to the documentation of igraph; good candidates for first-time contributors good first issue Candidates for first-time contributors who are already familiar with C
Projects
None yet
Development

No branches or pull requests

4 participants