Honestly, we could consider removing these include .m4 files given how short these PKG_CONFIG replacements are. Do you think that's worth while @hmc to have these in the parent configure.ac instead?

Besides the above comment, this changes looks fantastic.

@firstyear, I think that’s a good idea, given how short those files are now. Some of the unconverted files may be longer, depending on the code complexity, but we can judge them on a case-by-case basis.

I’ll update the PR to reflect this discussion.

Great! Thank you, I agree with your case-by-case assessment idea.

@firstyear, I've reworked the patch series per our earlier discussion. I've also converted a few more modules to use pkg-config.

openldap, ldapsdk (Mozilla) and db all use pkg-config, but Debian doesn't ship pkg-config files in the respective development packages, so I can't test these out.

netsnmp doesn't ship pkg-config files, but they will be included in the next upstream release.

Let me know if you need anything changed.

By the way, ldapsdk (Mozilla) is no longer in development, so pkg-config won't be an option in the future. Will support for this library be removed from 389-ds-base?

Yes, ldapsdk should be removed. I think there are options in the .C code that need removing, so let's open a ticket to say we should do this.

For now I see no issue in removing the ldapsdk option from the configure process however. @mreynolds ?

@firstyear, thanks for clarifying about ldapsdk's status in 389-ds-base. If support is going to be removed, then I won't need to modify src/svrcore/m4/*.m4 to use PKG_CHECK_MODULES.

I'll open a ticket about dropping ldapsdk support. [EDIT: See bug #50302]

Do you have any comments about this PR?

It looks good to me, I would like @mreynolds to check as well to be 100% sure, but I see nothing obvious blocking it from merging.

Thanks, @firstyear. Hopefully @mreynolds will get time to review this PR soon.

Works for me! ACK

Works for me! ACK

Once you rebase the PR I (or @firstyear) will merge it.

Through the magic of pagure, we can rebase for @hmc :) so I'll merge this now!

@hmc Thanks again for this contribution, I think this is a great cleanup for the project. We really appreciate it.

Through the magic of pagure, we can rebase for @hmc :) so I'll merge this now!

@firstyear Really? I was told there is no automatic rebase option in pagure. Am I missing something?

When merge is yellow, select it, and an extra option is there for rebase (to master). You select that first, then it'll refresh, merge goes green. Then you do the merge. It will also warn on rebase conflict and will "reset" so that nothing is lost.

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3354

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.