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

distribute config.h #745

Closed
ihrwein opened this issue Oct 23, 2015 · 3 comments · Fixed by #782
Closed

distribute config.h #745

ihrwein opened this issue Oct 23, 2015 · 3 comments · Fixed by #782
Assignees

Comments

@ihrwein
Copy link
Contributor

ihrwein commented Oct 23, 2015

If somebody want to use syslog-ng's header files in a project then he/she needs syslog-ng's config.h file as well. It is produced during the build but make install doesn't install it.

The link [1] says that config.h needs to be distributed if it affects the interface. Probably it is included in the header files through syslog-ng.h.

I see the following solutions:

  1. install config.h, perhaps renaming it to syslog-ng-config.h
  2. include config.h only in C files, move elsewhere the dependent defines from syslog-ng.h (YYDEBUG and PATH_SYSLOGNG, they are used only from .c files), include config.h in .c files where it is needed

This "feature" is needed to support Rust modules without a locally built syslog-ng.

I think the second approach is better but maybe it's impossible.

@bazsi @lbudai I'd like to hear your opinions.

@bazsi
Copy link
Collaborator

bazsi commented Oct 24, 2015

The header files will probably also use the result of autoconf tests, e.g.
when the name of a header is system dependent. So option #1 is more
realistic.

However I do agree that option #2 would be better. I am just a little
afraid that at any time we could establish a new dependency that will
affect our interface. So even if its doable today, we might have to rework
this in the future.

With all this said, I would install config.h as syslog-ng-config.h much
smaller change, and is more flexible.

Bazsi
On Oct 23, 2015 20:52, "Tibor Benke" notifications@github.com wrote:

If somebody want to use syslog-ng's header files in a project then he/she
needs syslog-ng's config.h file as well. It is produced during the build
but make install doesn't install it.

The link [1] says that config.h needs to be distributed if it affects the
interface. Probably it is included in the header files through syslog-ng.h
.

I see the following solutions:

  1. install config.h, perhaps renaming it to syslog-ng-config.h
  2. include config.h only in C files, move elsewhere the dependent defines
    from syslog-ng.h (YYDEBUG and PATH_SYSLOGNG, they are used only from .c
    files), include config.h in .c files where it is needed

This "feature" is needed to support Rust modules without a locally built
syslog-ng.

I think the second approach is better but maybe it's impossible.

@bazsi https://github.com/bazsi @lbudai https://github.com/lbudai I'd
like to hear your opinions.


Reply to this email directly or view it on GitHub
#745.

@ihrwein ihrwein mentioned this issue Oct 24, 2015
4 tasks
@ihrwein
Copy link
Contributor Author

ihrwein commented Nov 2, 2015

Probably we'll need to prefix our defines to avoid name collisions. I found this macro for this job:
http://www.gnu.org/software/autoconf-archive/ax_prefix_config_h.html

So we'll have SYSLOG_NG_HAVE_JAVA instead of HAVE_JAVA. Is it ok for you?

@bazsi
Copy link
Collaborator

bazsi commented Nov 3, 2015

Yes, lets hope it doesn't affect too many files, if we did things right
this should not be huge.

If it is, that's a smell that should be fixed up in the future.
On Nov 2, 2015 11:50 AM, "Tibor Benke" notifications@github.com wrote:

Probably we'll need to prefix our defines to avoid name collisions. I
found this macro for this job:
http://www.gnu.org/software/autoconf-archive/ax_prefix_config_h.html

So we'll have SYSLOG_NG_HAVE_JAVA instead of HAVE_JAVA. Is it ok for you?


Reply to this email directly or view it on GitHub
#745 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants