• This repository requires the use of Signed-off-by in commits. (Technically, we cannot add this header for you.)
• Please use the appropriate headers for the issue you are fixing in keeping with other commits in this repository.
• Please rewrite your commit message to explain the effect of the change, rather than the change itself. This should ideally produce something shorter (i.e., no body paragraph) and more useful to see in a log.
• I still do not agree with this behavior; you will need to eventually get a review/merge from Simo.
• Please rebase onto master while you are here.

• Fixed signed off
• Headers fixed
• Commit message replaced
• ack
• Rebased

I really did mean Resolves: #194. See git log.

Per discussion: the issue says we need to yell about having to do this. I think the flag is INI_MS_DETECT or so.

(blocked on https://pagure.io/SSSD/ding-libs/issue/3167)

Now that ding-libs has merged the upstream PR, I've updated and tested the functionality. This includes autoconf magic to check for the relevant patch. Thoughts on how to move the c blob out of configure.ac and into its own file?

Build failed

Btw you can move big checks into an m4 file, see the files in the build root and in external/ for examples.
external/ contains m4 scripts that come from external sources so it should not be put there.
We can crate a new m4/ folder like in most projects and stick a file in there. Perhaps also move some of the .m4 file in the root there too now that the pool is growing.
But that would be another commit/PR.

@simo: Build failed was desired behavior:

configure: error: "ini_config library must support extended INI_MS_DETECT. See: https://pagure.io/SSSD/ding-libs/pull-request/3172"

Without extended INI_MS_DETECT (which IIRC @rharwood will backport), we'd have to provide an extensive fallback that isn't written; thus, build failure. But yes, I'll refactor m4 code.

Uhmmm, can't we maintain current behavior if MS_DETECT is not available ?

@simo -- We could probably put setting merge_flags in an #ifdef but I'm not sure what that accomplishes.

Since we're intending to backport ding-libs + this PR together, I'd not know why we'd build gssproxy without the ding-libs patch but with this patch (if it maintains current behavior). Its a reasonable requirement, IMO, to have a matching version of ding-libs as no existing functionality is changed with the patch, and only new features are added.

That said, just tell me to do so and it'll be done. :)

Because gss-proxy is compiled on other distro's as well, and this would be a breaking change for a minor behavior change that can be optional.
So yeah, I very much would like to see an #ifedf based approach.

Btw, why do you need such a huge C file to detect if the INI_MS_DETECT flag is available ?

Email replies on pagure seem to be broken, so sorry i this shows up twice.

Without extended INI_MS_DETECT (which IIRC @rharwood will backport), we'd have to provide an extensive fallback that isn't written; thus, build failure. But yes, I'll refactor m4 code.

Correct, it's already in ding-libs-0.6.0-33.fc27

Because gss-proxy is compiled on other distro's as well, and this would be a breaking change for a minor behavior change that can be optional.

It's only a breaking change if they decide to backport the feature. A normal update of ding-libs and a normal update of gssproxy will produce something fine. And the only distros that have even asked for this change in gssproxy are Fedora/CentOS family.

@cipherboy, did you check this on rawhide with the appropriate ding-libs? I can't get it to work there.

So I just grabbed the rawhide iso from here, with hash ed0e3c8341b08567a4c85b1ff6193e3950536dc9631f9679352c0b69bc169036. I then performed a dnf builddep gssproxy, which resulted in installing libini_config-devel-1.3.0-30.fc26.x86_64.rpm, which does not match the libini_config-devel-1.3.0-33.fc27.x86_64.rpm from your build. I'm guessing r30 doesn't have the patch that you added to r33.

(this reduces the amount of autoconf tooling by only checking the return status of ini_config_augment(...) instead of also validating the contents of the resulting config struct)

(this fixes Robbie's issue -- in particular, #include "config.h" isn't appropriate at ./configure time).

Yup, testing good for me now, thanks.

Okay, maybe instead, return 0 if ret is 0, and 1 otherwise. Then remove the majority of the ret = 1 manipulation above.

Please group like types here.

This isn't something that will ever be run outside the build tools, so we don't have to actually clean up after ourselves properly.

This variable seems unused.

(some comments inline)

@simo The alternative to this large check is to gate on a newer version of libini_config - the drawback to that approach is that it hasn't been released yet, and we don't know when it will be released.

I believe this to fix the comments inline.

/lgtm codewise

Commit c0d85387 fixes this pull-request

Commit c0d85387 fixes this pull-request

Commit abce3165 fixes this pull-request