Move headers Part 1

[kodebach]

Note The release notes currently don't mention the moved/changed headers. I'll write a highlight section in one of the follow up PRs.

This PR starts implementing various decisions related to headers and repo structure. It is hard to define which decisions are implemented in this PR, because the changes will be split across multiple PRs for ease of reviewing.

The entire series of PRs, will implement:

• doc/decisions/4_decided/header_include.md
• doc/decisions/4_decided/header_file_structure.md
• doc/decisions/4_decided/library_directory_structure.md
• doc/decisions/4_decided/library_split.md

What happens in this PR?

This PR is very big so here is an overview what happens by commit (range):

1. 0db575f adds the script scripts/dev/check-includes.sh. This script checks the new rules for includes and is also executed in a new Github Actions workflow. Link
2. 496d1ad until d226f6b move the existing headers to their new locations. Commits fall into one of these groups: (1) moving/splitting headers (2) automatic refactoring (3) reformatting (4) manual fixes of things missed by automatic script Link
3. efa1537 until 0a65e50 fix various problems resulting from the previous commits. Note: fd1280a is a simple regex replace (using the regex from check-includes.sh), which adds the now required ./ to #include "..." lines. Link
4. 21b8c0d until 4bf8a75 are again like 2 Link
5. The remaining 8e124fd until fbeb633 are again mostly fixing problems from previous changes. Link The only notable exceptions are:
   • c257793 which removed the C89 cruft from types.h and thereby eliminated the need for configure_file. It also removes ELEKTRA_HAVE_KDB_LONG_DOUBLE and instead accepts a long double that is at least as big as double. Note: This part (long double) could be undone, if we don't want the change.
   • fbeb633 fixes a bug that comes from master, but seemingly wasn't detected in the CI
   • 2b0e948 temporarily disables external example tests. This will be undone in one of the follow-up PRs that finish the moving of headers.

I know the PR is still very big. However, splitting it up further would only create multiple PRs that touch about the same number of files as this PR.

Hints for reviewers

To reviewers I suggest using GitHub's "file viewed" checkbox feature. When you start the review go through all files quickly and check off all the files where just the #includes changed.

Okay... seems that doesn't quite work, at least for me GitHub lags a lot when I try to check off the files. Using github.dev works a bit better.

The remaining changes should be a lot more manageable to review. See #4920 (comment) and #4920 (comment) for a list of those files.

What's going to happen in follow-up PRs?

There will be 2-4 follow-up PRs depending on how easy it will be to find intermediary points where everything works.

1. The old_helper.h file will be split up and removed. That will allow us to re-enable the external example tests.
2. kdbprivate.h will be split up and removed.
3. Many CMake changes that will remove the need for include_directories all over the place.
4. (definitely separate) The headers from bindings also need to be moved. In particular the C++ binding still uses e.g. #include <key.hpp>.

Most likely, 1-2 will be one PR, 3 may be separate and 4 is definitely separate (since I haven't even started that yet). I'll keep 1 & 2 as a single PR, because these once again are things that touch large numbers of files.

Basics

• Short descriptions of your changes are in the release notes
  (added as entry in doc/news/_preparation_next_release.md which contains _(my name)_)
  Please always add them to the release notes.
• Details of what you changed are in commit messages
  (first line should have module: short statement syntax)
• References to issues, e.g. close #X, are in the commit messages.
• The buildservers are happy. If not, fix in this order:
  • add a line in doc/news/_preparation_next_release.md
  • reformat the code with scripts/dev/reformat-all
  • make all unit tests pass
  • fix all memleaks
  • fix the CI itself (or rebase if already fixed)
• The PR is rebased with current master.

Checklist

• I added unit tests for my code
• I fully described what my PR does in the documentation
  (not in the PR description)
• I fixed all affected documentation (see Documentation Guidelines)
• I fixed all affected decisions (see Decision Process)
• I added code comments, logging, and assertions as appropriate (see Coding Guidelines)
• I updated all meta data (e.g. README.md of plugins and METADATA.ini)
• I mentioned every code not directly written by me in reuse syntax

Review

• Documentation is introductory, concise, good to read and describes everything what the PR does
• Examples are well chosen and understandable
• Code is conforming to our Coding Guidelines
• APIs are conforming to our Design Guidelines
• Code is consistent to our Design Decisions

Labels

• Add the "work in progress" label if you do not want the PR to be reviewed yet.
• Add the "ready to merge" label if everything is done and no further pushes are planned by you.

[markus2330]

No other reviews done so far, two required. And it is quite clear why nobody reviewed yet, as it is very challenging. Please write in doc/news/_preparation_next_release.md what the goals of these changes were. What are the automatic changes? How were they done?

It would also be very helpful if you say what are the commit ranges where you manually changed something. Maybe you even need to create a separate PR for this (just for review). For me the GitHub UI was hanging several times, e.g., it was not possible to search for a file.

[kodebach]

No other reviews done so far, two require

Not really my fault... Can't really do more than request reviews from people. I already reduced the size of this PR, by keeping some changes from my fork for future PRs.

And it is quite clear why nobody reviewed yet, as it is very challenging. Please write in doc/news/_preparation_next_release.md what the goals of these changes were. What are the automatic changes? How were they done?

I did all of that already in the PR description and the first comment (linked from the description). IMO information like this has no place in the release notes. People using Elektra don't care how we made a commit. Yes, the why we did that should be in the release notes, but that's why I put a note at the top of the PR description (which nobody seems to read in this repo....)

[markus2330]

Unfortunately still no other reviews and no changes here. I described what would have improved the chance that I will take a look. There are plenty of things that can be done, we will talk about it in the next meeting.

[kodebach]

I described what would have improved the chance that I will take a look.

You wanted to know, what commits where automated, etc. I told you that this is already explained in the PR description. What else do you want from me?
If you really insist I can copy the text verbatim into doc/news/_preparation_next_release.md, but I really don't see the point since other than reviewers of this PR, nobody will ever care about which commits were automated...

EDIT: I have noticed the PR description doesn't actually mention the relevant decisions. I have added that now. I still haven't update the release notes, because none of the decisions are fully implemented. I may move the decision files to "partially implemented", if it starts to look like this PR will get merged.

I described what would have improved the chance that I will take a look.

Yeah, two other reviews, but I can't do anything about that...

At this point I've got to ask: Do you even want this PR? It really seems like you don't care at all, since you're seemingly not even willing to read the PR description.

There are plenty of things that can be done, we will talk about it in the next meeting.

The next meeting is too late for me. The PR is now open for almost 15 days (and the code is unchanged for the last 14 of those), with not a single useful comment about the content. I simply cannot afford to waste even more time, waiting for any meaningful response from you.

If this PR is not merged (or at least close to it) at the time of the next meeting, I will have to go back to developing my own fork for my thesis.

[atmaxinger]

Is the ./ really necessary?

[kodebach]

See doc/decisions/4_decided/header_include.md. We decided to enforce this syntax, to make sure #include "..." is only used for local files.

[atmaxinger]

LGTM, I think these changes make sense overall, and thanks to the split-up of the header files we're also getting a bit more guidance on where to put new function declarations.

How much of this can be automated? Could you apply your tool also to the open PRs?

[kodebach]

How much of this can be automated? Could you apply your tool also to the open PRs?

There are two changes in here that affect other PRs:

First, the new rules for includes enforced by check-includes.sh: This one is easy to fix. Almost all includes can be fixed with regex-replace using the regexes in check-includes.sh as a start.

Second, the moved headers. That's not as easy. The bigger PRs #4892 and #4922 are definitely candidates for automated updates with tree-surgeon. But for anything with less about 10-20 affected files (*), manually updating the includes is probably easier. My script currently doesn't account for transitive includes, so you sometimes need to add new headers even after the automatic update.

(*) Depending on the PR, not all changed files will be affected. Newly added files of course need to be changed, but modified files might be fixed with a merge/rebase after this PR.

However, if this PR is merged, I will definitely help with updating all the other PRs.

[kodebach]

Update for #4920 (comment): I have now moved some decisions to "partially implemented". For fixing the conflicts and (possibly) updating release notes, I'll wait until the release is done, so there won't be additional conflicts.

[kodebach]

I have created #4940 and #4941 to show what the next steps after this PR will be. IMO it could be done as one PR, but I split the final (mostly CMake-related) changes into #4941 to simplify the reviews. One or both of these PRs will contain the proper release notes for the header & library changes.

[kodebach]

@markus2330 @atmaxinger This PR is now up-to-date with master and should be mergeable again. To my absolute surprise the merge worked without any conflicts. I only had to update a few go-elektra files to the new includes.

In addition to the files listed in #4920 (comment) the following files now also contain noteworthy changes:

src/bindings/go-elektra/kdb/kdb.go
src/bindings/go-elektra/kdb/key.go
src/bindings/go-elektra/kdb/keyset.go
src/bindings/go-elektra/temp-elektra.pc.in

Technically the go-elektra/kdb file are also just #include changes, but I listed them, since they are not normal C/C++.

[kodebach]

@hannes99 Please review. To make things easier, read the PR description and the comments linked from there first.

[kodebach]

@markus2330 Any chance that we can merge this in the next few days (until end of May)?

#4464 was merged to day, so now I'll have to update this PR again. I'd really prefer, if we could merge this PR ASAP and I'll help update all the other open PRs. We a big PR like this one, every time master changes, it's big gamble whether there'll be complicated conflicts to solve... Much easier to update the smaller PRs one-by-one...

[hannes99]

LGTM, and seems to match what was decided in the decision.

[kodebach]

@markus2330 This PR now has two approving reviews. I have also updated it from master again (now includes xfconf binding). All new files have only updated #includes compared master (*). I hope you can at least take a quick look in the next days, so we can finally merge this.

(*) except for src/bindings/xfconf/elektra-xfconf-channel.h, where I removed an unnecessary ELEKTRA_UNUSED instead of adding the header for the attribute.

[kodebach]

For anyone interested: The tree-surgeon tool I wrote is now publicly available at https://github.com/kodebach/tree-surgeon. There are very few tests and zero documentation, it is now public.

[kodebach]

As it seems this PR is not getting merged into master immanently, I created new-repo-layout and changed the base branch of this PR and merged the PR into this new branch. This way I can continue with the rest of my work and all of it can get merged into master together at some later point.