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

Jump to bottom

include <elektra/kdb.h> #817

Open

markus2330 opened this issue Jun 30, 2016 · 10 comments

Labels

Projects

Milestone

Contributor

markus2330 commented Jun 30, 2016 •

edited

We could install all header files also to /usr/include/elektra, which allows us to include <elektra/kdb.h> without any include path.

The text was updated successfully, but these errors were encountered:

markus2330 added the proposal label

Contributor Author

markus2330 commented Feb 8, 2017

@domhof Is this a 1.0 issue?

Member

sanssecours commented Feb 8, 2017

If the alternative “Macro magic to support both inclusion options“ means that we do not need to install each header file twice, then I am for that option.

How about creating a link /usr/include/elektra/legacy that points to /usr/include/elektra. Would that also work?

Contributor Author

markus2330 commented Feb 9, 2017 •

edited

means that we do not need to install each header file twice, then I am for that option.

What is so bad about installing the files twice? They are not really large... (388K)

How about creating a link /usr/include/elektra/legacy that points to /usr/include/elektra. Would that also work?

You are right, actually we do not need to install them twice. (even the symlink does not seem to be necessary.) We simply internally always use <elektra/file.h> and if the user sets the include path to /usr/include/elektra the internal elektra/kdb.h should still work?

Within the source repo, however, we would need to create a folder called elektra, so the include files would be in `src/include/elektra.

markus2330 mentioned this issue

Prefix Key and KeySet with KDB #1331

Closed

Member

sanssecours commented Feb 9, 2017

What is so bad about installing the files twice? They are not really large... (388K)

It’s not that bad, but I am usually a fan of keeping things small and tidy.

Within the source repo, however, we would need to create a folder called elektra, so the include files would be in src/include/elektra.

Provided that this solution works, creating an additional subfolder does not sound that bad. I certainly could live with it.

markus2330 added this to to discuss in lcdproc

markus2330 moved this from to discuss to to implement in lcdproc

Contributor Author

markus2330 commented Mar 16, 2017

lets do it

markus2330 self-assigned this

markus2330 mentioned this issue

First Version of YAML CPP Plugin & Minor Enhancements #1613

Merged

5 tasks

markus2330 added this to the 0.8.22 milestone

markus2330 modified the milestones: 0.8.22, 0.8.23

markus2330 modified the milestones: 0.8.23, 0.8.24

markus2330 removed their assignment

markus2330 modified the milestones: 0.8.24, 0.8.25, 0.9.0

markus2330 mentioned this issue

highlevel: header file names #2344

Closed

bauhaus93 added a commit to bauhaus93/libelektra that referenced this issue


          src,tests,examples,benchmarks: Replaced kdb.h includes

68b86bb

Includes for <kdb.h> and "kdb.h" were replaced by <elektra/kdb.h> and "elektra/kdb.h".
Targeted files: *.h/*.hpp/*.c/*.cpp/*.chs/*.i (including matching files with suffix .in) in folders src, tests, examples, benchmarks.
Resolving ElektraInitiative#817

Script used for replacement:

replace()
{
	    if [[ $(sed -i "s/$2/$3/gw /dev/stdout" $1) ]]
		then
		echo "$1"
	    fi
}

for file in $(find libelektra/src libelektra/tests libelektra/examples libelektra/benchmarks -regex ".*\.\(h\|\(hpp\)\|c\|\(cpp\)\|\(chs\)\|i\)\(\.in\)?"); do
	replace "$file" '#include <kdb\.h>' '#include <elektra\/kdb\.h>'
	replace "$file" '#include "kdb\.h"' '#include "elektra\/kdb\.h"'
	replace "$file" '%include "kdb\.h"' '%include "elektra\/kdb\.h"'
done

bauhaus93 added a commit to bauhaus93/libelektra that referenced this issue


          include: Moved kdb.h.in, updated CMakeLists

30c916a

kdb.h.in moved into subfolder elektra, so that kdb.h can be included by <elektra/kdb.h> and "elektra/kdb.h".
The configure_file and install commands in the CMakeLists.txt were moved/updated.
For custom target elektra_config_headers the path of kdb.h was updated.
Resolving ElektraInitiative#817

bauhaus93 mentioned this issue

Move kdb header #3098

Closed

16 tasks

markus2330 assigned bauhaus93

bauhaus93 added a commit to bauhaus93/libelektra that referenced this issue


          src,tests,examples,benchmarks: Replaced kdb.h includes

a7e546d

Includes for <kdb.h> and "kdb.h" were replaced by <elektra/kdb.h> and "elektra/kdb.h".
Targeted files: *.h/*.hpp/*.c/*.cpp/*.chs/*.i (including matching files with suffix .in) in folders src, tests, examples, benchmarks.
Resolving ElektraInitiative#817

Script used for replacement:

replace()
{
	    if [[ $(sed -i "s/$2/$3/gw /dev/stdout" $1) ]]
		then
		echo "$1"
	    fi
}

for file in $(find libelektra/src libelektra/tests libelektra/examples libelektra/benchmarks -regex ".*\.\(h\|\(hpp\)\|c\|\(cpp\)\|\(chs\)\|i\)\(\.in\)?"); do
	replace "$file" '#include <kdb\.h>' '#include <elektra\/kdb\.h>'
	replace "$file" '#include "kdb\.h"' '#include "elektra\/kdb\.h"'
	replace "$file" '%include "kdb\.h"' '%include "elektra\/kdb\.h"'
done

bauhaus93 added a commit to bauhaus93/libelektra that referenced this issue


          include: Moved kdb.h.in, updated CMakeLists

663a0d5

kdb.h.in moved into subfolder elektra, so that kdb.h can be included by <elektra/kdb.h> and "elektra/kdb.h".
The configure_file and install commands in the CMakeLists.txt were moved/updated.
For custom target elektra_config_headers the path of kdb.h was updated.
Resolving ElektraInitiative#817

bauhaus93 added a commit to bauhaus93/libelektra that referenced this issue


          src,tests,examples,benchmarks: Replaced kdb.h includes

515c829

Includes for <kdb.h> and "kdb.h" were replaced by <elektra/kdb.h> and "elektra/kdb.h".
Targeted files: *.h/*.hpp/*.c/*.cpp/*.chs/*.i (including matching files with suffix .in) in folders src, tests, examples, benchmarks.
Resolving ElektraInitiative#817

Script used for replacement:

replace()
{
	    if [[ $(sed -i "s/$2/$3/gw /dev/stdout" $1) ]]
		then
		echo "$1"
	    fi
}

for file in $(find libelektra/src libelektra/tests libelektra/examples libelektra/benchmarks -regex ".*\.\(h\|\(hpp\)\|c\|\(cpp\)\|\(chs\)\|i\)\(\.in\)?"); do
	replace "$file" '#include <kdb\.h>' '#include <elektra\/kdb\.h>'
	replace "$file" '#include "kdb\.h"' '#include "elektra\/kdb\.h"'
	replace "$file" '%include "kdb\.h"' '%include "elektra\/kdb\.h"'
done

bauhaus93 added a commit to bauhaus93/libelektra that referenced this issue


          include: Moved kdb.h.in, updated CMakeLists

72a2e63

kdb.h.in moved into subfolder elektra, so that kdb.h can be included by <elektra/kdb.h> and "elektra/kdb.h".
The configure_file and install commands in the CMakeLists.txt were moved/updated.
For custom target elektra_config_headers the path of kdb.h was updated.
Resolving ElektraInitiative#817

bauhaus93 added a commit to bauhaus93/libelektra that referenced this issue


          src,tests,examples,benchmarks: Replaced kdb.h includes

3365fa9

Includes for <kdb.h> and "kdb.h" were replaced by <elektra/kdb.h> and "elektra/kdb.h".
Targeted files: *.h/*.hpp/*.c/*.cpp/*.chs/*.i (including matching files with suffix .in) in folders src, tests, examples, benchmarks.
Resolving ElektraInitiative#817

Script used for replacement:

replace()
{
	    if [[ $(sed -i "s/$2/$3/gw /dev/stdout" $1) ]]
		then
		echo "$1"
	    fi
}

for file in $(find libelektra/src libelektra/tests libelektra/examples libelektra/benchmarks -regex ".*\.\(h\|\(hpp\)\|c\|\(cpp\)\|\(chs\)\|i\)\(\.in\)?"); do
	replace "$file" '#include <kdb\.h>' '#include <elektra\/kdb\.h>'
	replace "$file" '#include "kdb\.h"' '#include "elektra\/kdb\.h"'
	replace "$file" '%include "kdb\.h"' '%include "elektra\/kdb\.h"'
done

bauhaus93 added a commit to bauhaus93/libelektra that referenced this issue


          include: Moved kdb.h.in, updated CMakeLists

94de704

kdb.h.in moved into subfolder elektra, so that kdb.h can be included by <elektra/kdb.h> and "elektra/kdb.h".
The configure_file and install commands in the CMakeLists.txt were moved/updated.
For custom target elektra_config_headers the path of kdb.h was updated.
Resolving ElektraInitiative#817

markus2330 added enhancement and removed proposal labels

markus2330 changed the title ~~proposal: include <elektra/kdb.h>~~ include <elektra/kdb.h>

kodebach mentioned this issue

Integrate kdbEnsure into kdbOpen #3651

Merged

20 tasks

Member

kodebach commented Mar 3, 2021

I think a big issue here is also that we often use e.g. #include <kdbmacros.h> with <>. AFAIK the C standard doesn't have an opinion on this, but most compilers will start only look in include paths with <>. But with #include "kdbmacros.h" they first look in the directory the current file is in, so that way it should work no matter where we put our headers, as long as the have the same relative layout as in the source tree.

kodebach added the cm2022s label

markus2330 removed the cm2022s label

mpranj pushed a commit to mpranj/libelektra that referenced this issue


          src,tests,examples,benchmarks: Replaced kdb.h includes

02de7e3

Includes for <kdb.h> and "kdb.h" were replaced by <elektra/kdb.h> and "elektra/kdb.h".
Targeted files: *.h/*.hpp/*.c/*.cpp/*.chs/*.i (including matching files with suffix .in) in folders src, tests, examples, benchmarks.
Resolving ElektraInitiative#817

Script used for replacement:

replace()
{
	    if [[ $(sed -i "s/$2/$3/gw /dev/stdout" $1) ]]
		then
		echo "$1"
	    fi
}

for file in $(find libelektra/src libelektra/tests libelektra/examples libelektra/benchmarks -regex ".*\.\(h\|\(hpp\)\|c\|\(cpp\)\|\(chs\)\|i\)\(\.in\)?"); do
	replace "$file" '#include <kdb\.h>' '#include <elektra\/kdb\.h>'
	replace "$file" '#include "kdb\.h"' '#include "elektra\/kdb\.h"'
	replace "$file" '%include "kdb\.h"' '%include "elektra\/kdb\.h"'
done

mpranj pushed a commit to mpranj/libelektra that referenced this issue


          include: Moved kdb.h.in, updated CMakeLists

kdb.h.in moved into subfolder elektra, so that kdb.h can be included by <elektra/kdb.h> and "elektra/kdb.h".
The configure_file and install commands in the CMakeLists.txt were moved/updated.
For custom target elektra_config_headers the path of kdb.h was updated.
Resolving ElektraInitiative#817

mpranj mentioned this issue

Move kdb header #3813

Closed

20 tasks

Contributor Author

markus2330 commented May 1, 2021

As discussed in #3813 we do not need any legacy header files. The only supported way to include files should be <elektra/NAME.h>, where NAME usually does not start with "kdb", so e.g. it would be <elektra/macros.h>.

markus2330 unassigned bauhaus93

markus2330 mentioned this issue

[decisions] headers #4246

Merged

markus2330 added floss2022w and removed floss2022w labels

Janldeboer self-assigned this

Contributor

Janldeboer commented Dec 1, 2022

To proceed with this, we have to move all the header files into the src/include/elektra folder, is that correct?

Member

kodebach commented Dec 1, 2022

This issue is a bit outdated. We now have a few decisions that describe what the everything should look like:

doc/decisions/3_decided/header_file_structure.md: Describes where the various header files should be located in the repo.
doc/decisions/3_decided/header_include.md: Describes how the headers should be #included into other files.

There are also some decisions that are tangentially related:

doc/decisions/3_decided/private_api.md: Requires kdbprivate.h to be split up.
doc/decisions/3_decided/library_split.md: Describes how the API should be defined into libraries, which has an impact on the headers.

For this issue I'd say the main part is header_file_structure.md. Fully implementing that, also requires updating includes and CMake config. So doing everything at once might a bit much. Maybe start with moving things to src/include/elektra and src/include/internal and updating the CMake config.

Contributor

Janldeboer commented Dec 1, 2022

Thank you for the feedback, I will continue with that!

kodebach mentioned this issue

[FLOSS H3] Change includes of header files #4747

Closed

23 tasks

markus2330 unassigned Janldeboer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment