mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From James Peach <jpe...@apache.org>
Subject Re: Review Request 60902: Moved the libnl3 configure checks into a macro.
Date Mon, 17 Jul 2017 04:30:35 GMT


> On July 17, 2017, 3:20 a.m., Qian Zhang wrote:
> > configure.ac
> > Line 350 (original)
> > <https://reviews.apache.org/r/60902/diff/1/?file=1777350#file1777350line350>
> >
> >     So `--with-nl` is removed, will it cause backward compatibility issue?

No, `--with-nl` is provided by the `MESOS_HAVE_LIBNL3` macro.


> On July 17, 2017, 3:20 a.m., Qian Zhang wrote:
> > m4/libnl3.m4
> > Lines 33-37 (patched)
> > <https://reviews.apache.org/r/60902/diff/1/?file=1777351#file1777351line33>
> >
> >     Mind to elaborate a bit about why introducing a new .m4 file? What about just
introducing a new configure option (like `--enable-libnl3`) and make `--enable_port_mapping_isolator`
depend on it? Just like the relationship between `--enable-ssl` and `--enable-libevent`: https://github.com/apache/mesos/blob/1.3.0/configure.ac#L1657:L1698
> >     
> >     And for `--enable-libnl3`, I think we should not do much checks, just some very
basic check should be OK, and on top of it, `--enable_port_mapping_isolator` can do some more
advanced/specific check on libnl3 for itself.

> Mind to elaborate a bit about why introducing a new .m4 file?

This is because a separate file makes it easier to maintain. We could inline the macro into
`configure.ac` but that's not typical autotool practice.

> What about just introducing a new configure option (like --enable-libnl3)

I don't see how `--enable-libnl3` makes anything simpler, unless you want to to use that option
to enable all the features that depend on `libnl3`. In that case, however, I'd just implement
that based on the existing `--with-nl`.

The semantics here are that we have `--enable-port-mapping-isolator`, `--enable-network-ports-isolator`
and `--with-nl` arguments. If either of the first 2 options is enabled, we will attempt to
use the `libnl3` from the location of the 3rd option.

> I think we should not do much checks

The checks we are applying are all ones that we already had. I didn't want to change the dependency
requirements here.


- James


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60902/#review180653
-----------------------------------------------------------


On July 16, 2017, 11:43 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60902/
> -----------------------------------------------------------
> 
> (Updated July 16, 2017, 11:43 p.m.)
> 
> 
> Review request for mesos and Qian Zhang.
> 
> 
> Bugs: MESOS-7675
>     https://issues.apache.org/jira/browse/MESOS-7675
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since the `network/ports` isolator will depend on libnl3, move those
> checks into a separate macro so that we can call it again when we
> add a configure option to enable it.
> 
> 
> Diffs
> -----
> 
>   configure.ac 4d7c4a4679e5c624ee750226d542e0d8c228507a 
>   m4/libnl3.m4 PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60902/diff/1/
> 
> 
> Testing
> -------
> 
> make check (Fedora 26), manual verification of configuration logs.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message