| Message ID | 20161212145230.11412-7-mike.holmes@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 12 December 2016 at 15:52, Mike Holmes <mike.holmes@linaro.org> wrote: > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > --- > configure.ac | 4 ++-- > platform/linux-generic/m4/configure.m4 | 2 ++ > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/configure.ac b/configure.ac > index f17bcae..eb5436c 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -158,9 +158,9 @@ ODP_CFLAGS="$ODP_CFLAGS -DIMPLEMENTATION_NAME=$ > IMPLEMENTATION_NAME" > ############################################################ > ############## > AC_ARG_WITH([os], > [AS_HELP_STRING([--with-os=os], > - [select platform to be used, default linux])], > + [select platform to be used, default ${default_helper_os}])], > [], > - [with_os=linux > + [with_os=${default_helper_os} > ]) > > AC_SUBST([with_os]) > diff --git a/platform/linux-generic/m4/configure.m4 > b/platform/linux-generic/m4/configure.m4 > index d3e5528..bf6bbe2 100644 > --- a/platform/linux-generic/m4/configure.m4 > +++ b/platform/linux-generic/m4/configure.m4 > @@ -38,3 +38,5 @@ m4_include([platform/linux-generic/m4/odp_schedule.m4]) > > AC_CONFIG_FILES([platform/linux-generic/Makefile > platform/linux-generic/include/odp/api/plat/static_ > inline.h]) > +#define the odp helper implimentation to be built > implementation > +default_helper_os=linux > -- > 2.9.3 > > Christophe.
Le 12/12/2016 à 03:52 PM, Mike Holmes a écrit : > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > --- > configure.ac | 4 ++-- > platform/linux-generic/m4/configure.m4 | 2 ++ > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/configure.ac b/configure.ac > index f17bcae..eb5436c 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -158,9 +158,9 @@ ODP_CFLAGS="$ODP_CFLAGS -DIMPLEMENTATION_NAME=$IMPLEMENTATION_NAME" > ########################################################################## > AC_ARG_WITH([os], > [AS_HELP_STRING([--with-os=os], > - [select platform to be used, default linux])], > + [select platform to be used, default ${default_helper_os}])], > [], > - [with_os=linux > + [with_os=${default_helper_os} > ]) > > AC_SUBST([with_os]) > diff --git a/platform/linux-generic/m4/configure.m4 b/platform/linux-generic/m4/configure.m4 > index d3e5528..bf6bbe2 100644 > --- a/platform/linux-generic/m4/configure.m4 > +++ b/platform/linux-generic/m4/configure.m4 > @@ -38,3 +38,5 @@ m4_include([platform/linux-generic/m4/odp_schedule.m4]) > > AC_CONFIG_FILES([platform/linux-generic/Makefile > platform/linux-generic/include/odp/api/plat/static_inline.h]) > +#define the odp helper implimentation to be built > +default_helper_os=linux I don't think this would work well with multiple platforms. Either all the platforms m4 are included and they would just override this value for the others. Either only one is included, depending on the --with-platform flag, and I'm not sure how this would work. Until platform is selected, the default_helper_os will not be set. I think the AC_ARG_WITH --with-os should have an empty default value. Platform configure.m4 should only set the value if it is empty. Nicolas
On 13 December 2016 at 03:05, Nicolas Morey-Chaisemartin <nmorey@kalray.eu> wrote: > > > Le 12/12/2016 à 03:52 PM, Mike Holmes a écrit : > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > > --- > > configure.ac | 4 ++-- > > platform/linux-generic/m4/configure.m4 | 2 ++ > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/configure.ac b/configure.ac > > index f17bcae..eb5436c 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -158,9 +158,9 @@ ODP_CFLAGS="$ODP_CFLAGS -DIMPLEMENTATION_NAME=$ > IMPLEMENTATION_NAME" > > ############################################################ > ############## > > AC_ARG_WITH([os], > > [AS_HELP_STRING([--with-os=os], > > - [select platform to be used, default linux])], > > + [select platform to be used, default ${default_helper_os}])], > > [], > > - [with_os=linux > > + [with_os=${default_helper_os} > > ]) > > > > AC_SUBST([with_os]) > > diff --git a/platform/linux-generic/m4/configure.m4 > b/platform/linux-generic/m4/configure.m4 > > index d3e5528..bf6bbe2 100644 > > --- a/platform/linux-generic/m4/configure.m4 > > +++ b/platform/linux-generic/m4/configure.m4 > > @@ -38,3 +38,5 @@ m4_include([platform/linux- > generic/m4/odp_schedule.m4]) > > > > AC_CONFIG_FILES([platform/linux-generic/Makefile > > platform/linux-generic/include/odp/api/plat/static_ > inline.h]) > > +#define the odp helper implimentation to be built > > +default_helper_os=linux > > I don't think this would work well with multiple platforms. > Either all the platforms m4 are included and they would just override this > value for the others. > Either only one is included, depending on the --with-platform flag, and > I'm not sure how this would work. Until platform is selected, the > default_helper_os will not be set. > > I think the AC_ARG_WITH --with-os should have an empty default value. > Platform configure.m4 should only set the value if it is empty. > I was following the idea that with the platform set this appears it would auto set a default helper OS for that platfrom, and would I assume allow a different platform to select a different default helper os. Thus it could make it simpler in majority use cases - but I was hoping for your feedback, I would say that alternatively the default is Linux as that will be most common and thus a sensible default. > > Nicolas > > -- Mike Holmes Program Manager - Linaro Networking Group Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs "Work should be fun and collaborative, the rest follows"
Le 12/13/2016 à 02:25 PM, Mike Holmes a écrit : > > > On 13 December 2016 at 03:05, Nicolas Morey-Chaisemartin <nmorey@kalray.eu <mailto:nmorey@kalray.eu>> wrote: > > > > Le 12/12/2016 à 03:52 PM, Mike Holmes a écrit : > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org <mailto:mike.holmes@linaro.org>> > > --- > > configure.ac <http://configure.ac> | 4 ++-- > > platform/linux-generic/m4/configure.m4 | 2 ++ > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/configure.ac <http://configure.ac> b/configure.ac <http://configure.ac> > > index f17bcae..eb5436c 100644 > > --- a/configure.ac <http://configure.ac> > > +++ b/configure.ac <http://configure.ac> > > @@ -158,9 +158,9 @@ ODP_CFLAGS="$ODP_CFLAGS -DIMPLEMENTATION_NAME=$IMPLEMENTATION_NAME" > > ########################################################################## > > AC_ARG_WITH([os], > > [AS_HELP_STRING([--with-os=os], > > - [select platform to be used, default linux])], > > + [select platform to be used, default ${default_helper_os}])], > > [], > > - [with_os=linux > > + [with_os=${default_helper_os} > > ]) > > > > AC_SUBST([with_os]) > > diff --git a/platform/linux-generic/m4/configure.m4 b/platform/linux-generic/m4/configure.m4 > > index d3e5528..bf6bbe2 100644 > > --- a/platform/linux-generic/m4/configure.m4 > > +++ b/platform/linux-generic/m4/configure.m4 > > @@ -38,3 +38,5 @@ m4_include([platform/linux-generic/m4/odp_schedule.m4]) > > > > AC_CONFIG_FILES([platform/linux-generic/Makefile > > platform/linux-generic/include/odp/api/plat/static_inline.h]) > > +#define the odp helper implimentation to be built > > +default_helper_os=linux > > I don't think this would work well with multiple platforms. > Either all the platforms m4 are included and they would just override this value for the others. > Either only one is included, depending on the --with-platform flag, and I'm not sure how this would work. Until platform is selected, the default_helper_os will not be set. > > I think the AC_ARG_WITH --with-os should have an empty default value. Platform configure.m4 should only set the value if it is empty. > > > I was following the idea that with the platform set this appears it would auto set a default helper OS for that platfrom, and would I assume allow a different platform to select a different default helper os. > Thus it could make it simpler in majority use cases - but I was hoping for your feedback, I would say that alternatively the default is Linux as that will be most common and thus a sensible default. I guess it work if you select a platform. If you don't (./configure --help), it'll probably just show up an empty string if the default platform is not set. And it's not really standard or user friendly to have a default option changing its value when changing another option. (platform = linux => os = linux, platform = mppa => os = mOS) Maybe a simple check after including the configure.m4 files to make sure the value is not empty is needed.
On 13 December 2016 at 08:35, Nicolas Morey-Chaisemartin <nmorey@kalray.eu> wrote: > > > Le 12/13/2016 à 02:25 PM, Mike Holmes a écrit : > > > > On 13 December 2016 at 03:05, Nicolas Morey-Chaisemartin <nmorey@kalray.eu > > wrote: > >> >> >> Le 12/12/2016 à 03:52 PM, Mike Holmes a écrit : >> > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >> > --- >> > configure.ac | 4 ++-- >> > platform/linux-generic/m4/configure.m4 | 2 ++ >> > 2 files changed, 4 insertions(+), 2 deletions(-) >> > >> > diff --git a/configure.ac b/configure.ac >> > index f17bcae..eb5436c 100644 >> > --- a/configure.ac >> > +++ b/configure.ac >> > @@ -158,9 +158,9 @@ ODP_CFLAGS="$ODP_CFLAGS >> -DIMPLEMENTATION_NAME=$IMPLEMENTATION_NAME" >> > ############################################################ >> ############## >> > AC_ARG_WITH([os], >> > [AS_HELP_STRING([--with-os=os], >> > - [select platform to be used, default linux])], >> > + [select platform to be used, default ${default_helper_os}])], >> > [], >> > - [with_os=linux >> > + [with_os=${default_helper_os} >> > ]) >> > >> > AC_SUBST([with_os]) >> > diff --git a/platform/linux-generic/m4/configure.m4 >> b/platform/linux-generic/m4/configure.m4 >> > index d3e5528..bf6bbe2 100644 >> > --- a/platform/linux-generic/m4/configure.m4 >> > +++ b/platform/linux-generic/m4/configure.m4 >> > @@ -38,3 +38,5 @@ m4_include([platform/linux-gen >> eric/m4/odp_schedule.m4]) >> > >> > AC_CONFIG_FILES([platform/linux-generic/Makefile >> > platform/linux-generic/includ >> e/odp/api/plat/static_inline.h]) >> > +#define the odp helper implimentation to be built >> > +default_helper_os=linux >> >> I don't think this would work well with multiple platforms. >> Either all the platforms m4 are included and they would just override >> this value for the others. >> Either only one is included, depending on the --with-platform flag, and >> I'm not sure how this would work. Until platform is selected, the >> default_helper_os will not be set. >> >> I think the AC_ARG_WITH --with-os should have an empty default value. >> Platform configure.m4 should only set the value if it is empty. >> > > I was following the idea that with the platform set this appears it would > auto set a default helper OS for that platfrom, and would I assume allow a > different platform to select a different default helper os. > Thus it could make it simpler in majority use cases - but I was hoping > for your feedback, I would say that alternatively the default is Linux as > that will be most common and thus a sensible default. > > > I guess it work if you select a platform. If you don't (./configure > --help), it'll probably just show up an empty string if the default > platform is not set. > And it's not really standard or user friendly to have a default option > changing its value when changing another option. (platform = linux => os = > linux, platform = mppa => os = mOS) > > Maybe a simple check after including the configure.m4 files to make sure > the value is not empty is needed. > Ok will fix in v2 -- Mike Holmes Program Manager - Linaro Networking Group Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs "Work should be fun and collaborative, the rest follows"
diff --git a/configure.ac b/configure.ac index f17bcae..eb5436c 100644 --- a/configure.ac +++ b/configure.ac @@ -158,9 +158,9 @@ ODP_CFLAGS="$ODP_CFLAGS -DIMPLEMENTATION_NAME=$IMPLEMENTATION_NAME" ########################################################################## AC_ARG_WITH([os], [AS_HELP_STRING([--with-os=os], - [select platform to be used, default linux])], + [select platform to be used, default ${default_helper_os}])], [], - [with_os=linux + [with_os=${default_helper_os} ]) AC_SUBST([with_os]) diff --git a/platform/linux-generic/m4/configure.m4 b/platform/linux-generic/m4/configure.m4 index d3e5528..bf6bbe2 100644 --- a/platform/linux-generic/m4/configure.m4 +++ b/platform/linux-generic/m4/configure.m4 @@ -38,3 +38,5 @@ m4_include([platform/linux-generic/m4/odp_schedule.m4]) AC_CONFIG_FILES([platform/linux-generic/Makefile platform/linux-generic/include/odp/api/plat/static_inline.h]) +#define the odp helper implimentation to be built +default_helper_os=linux
Signed-off-by: Mike Holmes <mike.holmes@linaro.org> --- configure.ac | 4 ++-- platform/linux-generic/m4/configure.m4 | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) -- 2.9.3