event/external: misc configury fixes

[ggouaillardet]

• Always use the external component when configure'd with --with-libevent=external
• Fix the external libevent library version detection
  by testing _EVENT_NUMERIC_VERSION and EVENT__NUMERIC_VERSION macros
• Use the event2/event.h header (event.h is deprecated since libevent 2.0

Signed-off-by: Gilles Gouaillardet gilles@rist.or.jp

[ggouaillardet]

@jsquyres could you please review this ?

@hppritcha that could be a blocker for v4.0.0. If the external libevent is requested with --with-libevent=external, the internal libevent might be used anyway. The expected behavior is the external libevent should be used or configure should abort, but we should never fallback to the internal event

[jsquyres]

The code itself looks fine; thanks for the fixes.

Question, though: this PR is really 2 things:

1. Bug fixes
2. Upgrade to require libevent >= 2.0.22 (we previously allowed >= 2.0.21).

Is there a reason for the upgrade? Do we know if this will affect our packagers?

[jsquyres]

@ggouaillardet Do you know if this affects 3.1.x / 3.0.x / 2.1.x?

[rhc54]

There is no reason to favor libevent 2.0.22 over 2.0.21 - the changes were irrelevant to us, so FAICT.

[ggouaillardet]

The bug in only in master and v4.0.x and was introduced when we decide to prefer external library over the internal one.

My understanding is we prefer the external library if its version is greater or equal than the internal one, so I thought 2.0.21 was a typo or something we forgot to update.
I will replace the 2.0.22 changes with a comment that explains the rationale.

[hppritcha]

@gpaulsen might want to discuss on Monday

[rhc54]

@ggouaillardet @jsquyres He makes a correct point - we have libevent 2.0.22 in the OMPI distribution. Our policy is indeed to prefer external only if it is more current than the internal. Thus, there is no point to check for 2.0.21 as it will never be selected.

[ggouaillardet]

I split the PR into two commits (bug fix + libevent version)

If here is a consensus we should make an exception to the policy and allow 2.0.21 to be a default external component, I'll be happy to replace the second commit with a comment in opal/mca/event/external/configure.m4

[ggouaillardet]

@jsquyres as far as I am concerned, packagers should always configure --with-libevent=external instead of assuming their libevent will be used by default.

[rhc54]

Looks correct to me

[jsquyres]

Per #5031, I thought we agreed to default to external if the external version >= the internal version (not >)...? I.e., really bias towards external -- even if the external version is the same as the internal version.

Specifically, I think that condition was created for our downstream packagers who will package exactly the version of libevent that we want/ask for (which, by default, they may just use the version that we already have embedded).

I don't think I care too strongly about this, but that what I remember us deciding.

[rhc54]

I'm afraid I don't understand your concern - this PR does exactly what you just described, doesn't it?

if external libevent version is 2.0.22 or greater

[jsquyres]

Hah -- silly me not noticing that the internal libevent is currently 2.0.22 (I just noticed the change in this PR to 2.0.22).

Good to go.