chore: update includes with include-what-you-use

[e-b-olson]

Hacktoberfest 2023

This PR updates the include directives in the avogadrolibs submodule using the include-what-you-use tool.

Note: there were some build issues after running the tool; specifically with some missed includes of fstream and sstream, so these were manually added back in where needed.

The Process

The process was non-trivial and not particularly fun. I'm including an outline of the steps here for anyone in the future wishing to do the same.

1. Fork the main openchemistry project
2. Fork the specific submodule you will be working with (in this case avogadrolibs)
3. Clone the main openchemistry repo to your machine
4. Edit the submodule file (.gitmodules) to point the submodule in question (avogadrolibs) to your fork of the submodule
5. Clone the submodule repo

Once you have all the code on your machine. Time to set-up the toolchain.

1. You will need the correct version of CMake
2. You will need the latest version of include-what-you-use
3. You will need the correct version of Python
4. You will need the correct version of gcc and/or g++ (or whatever compiler you will use).

Now that you have your toolchain built, it's time to go through the process.

1. Create a build directory (openchemistry-build) next to the repo directory
2. Run CMake to configure the build $ cmake ../openchemistry
3. Run CMake to build the project cmake --build . --config Release
4. Once you have a working build, it's time to run with include-what-you-use
5. Edit the submodule's CMakeLists.txt file to include the following lines:

set(CMAKE_CXX_INCLUDE_WHAT_YOU_USE=include-what-you-use) set(CMAKE_EXPORT_COMPILE_COMMANDS ON CACHE INTERNAL "")

6. Run the iwyu_tool and redirect the output to a file $ iwyu_tool -p ./avogadrolibs/ >> fix_includes_input.txt
7. Now you can run the fix_include script, directing the output file as input to the script. This will perform the code updates. Note: this can take a while. Like a really long time. Like. REALLY long. And there's no progress indicator. It may seem like the process is frozen. Just wait. Go get a snack. Watch some tv. $ fix_include --nocomments -p ./avogadrolibs/ < fix_includes_input.txt
8. Once this is done, better check that it worked. Revert your changes to the CMakeLists.txt file. Then build the project with CMake again.
9. Fix any errors.
10. Repeat steps 4 through 8 until everything builds clean.
11. commit your code
12. push to your repo
13. realize you forgot to run this in your fork of the submodule. GOTO: 1

---

Developer Certificate of Origin Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors. 1 Letterman Drive Suite D4700 San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this license document, but changing it is not allowed.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I have the right to submit it under the open source license indicated in the file; or

(b) The contribution is based upon previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the same open source license (unless I am permitted to submit under a different license), as indicated in the file; or

(c) The contribution was provided directly to me by some other person who certified (a), (b) or (c) and I have not modified it.

(d) I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved.

[welcome[bot]]

Thanks for opening this pull request! Please check out our contributing guidelines and check for the automated tests.

[ghutchis]

I committed a merge so the runners can test. Not 100% sure on some of these, but seems like a win.

With the Qt headers, we've generally gone with <QtCore/QList> for example, but it seems like iwyu is sticking to things like <qlist.h> .. do you know if there's any difference?

[ghutchis]

But above all, thank you - I've been meaning to run iwyu for ages and it's definitely useful.

I might not merge this until after the upcoming 1.98 release just out of superstition though.

[ghutchis]

Looks like it's using ext/alloc_traits.h which is an internal header: https://github.com/include-what-you-use/include-what-you-use/issues/166

[github-actions[bot]]

Here are the build results Avogadro2.AppImage Artifacts will only be retained for 90 days.

[e-b-olson]

Oh. Wow. I intended this as just a draft PR (hadn't had time to finish the notes and do all the checks). Thanks for moving this along. I can take a look at the issues, but may need some help (I don't have a Windows machine, for example). Thanks!

[ghutchis]

I think you should be able to click on "Details" for any of the failing / skipped tests to see the results. Usually the Cmake builds are easier to understand than the Build Wheels (because the Python build actions hide the compile errors).

[ghutchis]

I'm also happy to take a look at what's going on, e.g., my comments above.

[e-b-olson]

Finally had some time to get back to this. Apologies for the delay. I've added some include-what-you-use pragmas to the code to prevent inclusion of <ext/alloc_traits.h>. Not particularly pretty or elegant, but it seems to work.

As for the Qt headers, I'm not sure why iwyu prefers <qtlist.h>. Wonder if it is related to the version of Qt is installed on my machine (5.15.3)?

Just double checking everything still builds now, and hope to have a PR up sometime in the next 24 hours.

[ghutchis]

Thanks. As I said, this has been hugely needed and something I put off myself.

It sounds like the Qt-preferred option is what we've used, e.g.

#include <QtCore/QList>

I can think of two strategies:

• add pragmas to avoid the #include <qtlist.h> style
• do some work to transform the iwyu form back into the preferred style, e.g. a set of sed rules

I'm not sure which is easier, although I'd probably go for option 2 myself because it's probably faster to grep the list of #include <q*.h> find the unique options and turn it into a list of transform rules.

[ghutchis]

Looks like avogadro/core/elements.cpp still has an <ext/alloc_traits.h> .. maybe that also goes on the list of "transform after iwyu"

[e-b-olson]

Looks like avogadro/core/elements.cpp still has an <ext/alloc_traits.h> .. maybe that also goes on the list of "transform after iwyu"

It does? Sorry, I'm not seeing that. I'm fairly new to requesting PRs from forked repos, so maybe I missed something in the process of updating this PR. Am I looking in the wrong place? I'm looking at the file here.

If you're using grep, it will report the string <ext/alloc_traits.h>, but that's just part of the pragma (it's not an actual include). Is that what you're seeing?

[ghutchis]

No, it looks like I saw old build results (i.e., the build error) because the runners didn't restart until just now. I think because you're a new contributor, I have to authorize every push to build.

I will probably hold off on this PR for a bit to make a 1.98.1 release first.

[github-actions[bot]]

Here are the build results Avogadro2.AppImage macOS.dmg Win64.exe Artifacts will only be retained for 90 days.