| Message ID | 20200506005528.2897-1-sonnysasaka@chromium.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Marcel Holtmann |
| Headers | show |
| Series | build: Add an option to explicitly disable installing hid2hci | expand |
Hi Sonny, > --- > configure.ac | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/configure.ac b/configure.ac > index 1433ace4a..ba9937a16 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -218,8 +218,11 @@ if (test "${enable_udev}" != "no" && test -z "${path_udevdir}"); then > fi > AC_SUBST(UDEV_DIR, [${path_udevdir}]) > > +AC_ARG_ENABLE(hid2hci, AC_HELP_STRING([--disable-hid2hci], > + [disable hid2hci tool]), [enable_hid2hci=${enableval}]) > AM_CONDITIONAL(HID2HCI, test "${enable_tools}" != "no" && > - test "${enable_udev}" != "no") > + test "${enable_udev}" != "no" && > + test "${enable_hid2hci}" != "no") can you give me a bit of background why you need that. We did have that and I removed it in favor of putting everything behind udev. Mainly since we really don’t need most of these things anymore. Can’t you just disable udev support and get the same result? Regards Marcel
Hi Marcel, Chrome OS doesn't support HID-HCI switchable controllers so we would like to remove this from being installed to reduce maintenance burden. Disabling udev unfortunately also uninstalls sixaxis plugin, which we still need. Do you have a suggestion how we can achieve this? On Wed, May 6, 2020 at 4:06 AM Marcel Holtmann <marcel@holtmann.org> wrote: > > Hi Sonny, > > > --- > > configure.ac | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/configure.ac b/configure.ac > > index 1433ace4a..ba9937a16 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -218,8 +218,11 @@ if (test "${enable_udev}" != "no" && test -z "${path_udevdir}"); then > > fi > > AC_SUBST(UDEV_DIR, [${path_udevdir}]) > > > > +AC_ARG_ENABLE(hid2hci, AC_HELP_STRING([--disable-hid2hci], > > + [disable hid2hci tool]), [enable_hid2hci=${enableval}]) > > AM_CONDITIONAL(HID2HCI, test "${enable_tools}" != "no" && > > - test "${enable_udev}" != "no") > > + test "${enable_udev}" != "no" && > > + test "${enable_hid2hci}" != "no") > > can you give me a bit of background why you need that. We did have that and I removed it in favor of putting everything behind udev. Mainly since we really don’t need most of these things anymore. Can’t you just disable udev support and get the same result? > > Regards > > Marcel >
Hi Marcel, Could you please take another look at this patch? Thanks! On Wed, May 6, 2020 at 10:05 AM Sonny Sasaka <sonnysasaka@chromium.org> wrote: > > Hi Marcel, > > Chrome OS doesn't support HID-HCI switchable controllers so we would > like to remove this from being installed to reduce maintenance burden. > Disabling udev unfortunately also uninstalls sixaxis plugin, which we > still need. Do you have a suggestion how we can achieve this? > > On Wed, May 6, 2020 at 4:06 AM Marcel Holtmann <marcel@holtmann.org> wrote: > > > > Hi Sonny, > > > > > --- > > > configure.ac | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/configure.ac b/configure.ac > > > index 1433ace4a..ba9937a16 100644 > > > --- a/configure.ac > > > +++ b/configure.ac > > > @@ -218,8 +218,11 @@ if (test "${enable_udev}" != "no" && test -z "${path_udevdir}"); then > > > fi > > > AC_SUBST(UDEV_DIR, [${path_udevdir}]) > > > > > > +AC_ARG_ENABLE(hid2hci, AC_HELP_STRING([--disable-hid2hci], > > > + [disable hid2hci tool]), [enable_hid2hci=${enableval}]) > > > AM_CONDITIONAL(HID2HCI, test "${enable_tools}" != "no" && > > > - test "${enable_udev}" != "no") > > > + test "${enable_udev}" != "no" && > > > + test "${enable_hid2hci}" != "no") > > > > can you give me a bit of background why you need that. We did have that and I removed it in favor of putting everything behind udev. Mainly since we really don’t need most of these things anymore. Can’t you just disable udev support and get the same result? > > > > Regards > > > > Marcel > >
Hi Sonny, > --- > configure.ac | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/configure.ac b/configure.ac > index 1433ace4a..ba9937a16 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -218,8 +218,11 @@ if (test "${enable_udev}" != "no" && test -z "${path_udevdir}"); then > fi > AC_SUBST(UDEV_DIR, [${path_udevdir}]) > > +AC_ARG_ENABLE(hid2hci, AC_HELP_STRING([--disable-hid2hci], > + [disable hid2hci tool]), [enable_hid2hci=${enableval}]) > AM_CONDITIONAL(HID2HCI, test "${enable_tools}" != "no" && > - test "${enable_udev}" != "no") > + test "${enable_udev}" != "no" && > + test "${enable_hid2hci}" != "no") I looked at this again. Lets turn this around and make this an --enable-hid2hci similar to --enable-sixaxis. And you need to change the ${enable_tools} into ${enable_hid2hci} actually and not just add another dependency. Everything else seems to be already in the Makefile.tools for this. Regards Marcel
Thanks for the suggestion, Marcel. I have sent the v2 patch, please take another look. Thanks. On Wed, May 13, 2020 at 1:02 AM Marcel Holtmann <marcel@holtmann.org> wrote: > > Hi Sonny, > > > --- > > configure.ac | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/configure.ac b/configure.ac > > index 1433ace4a..ba9937a16 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -218,8 +218,11 @@ if (test "${enable_udev}" != "no" && test -z "${path_udevdir}"); then > > fi > > AC_SUBST(UDEV_DIR, [${path_udevdir}]) > > > > +AC_ARG_ENABLE(hid2hci, AC_HELP_STRING([--disable-hid2hci], > > + [disable hid2hci tool]), [enable_hid2hci=${enableval}]) > > AM_CONDITIONAL(HID2HCI, test "${enable_tools}" != "no" && > > - test "${enable_udev}" != "no") > > + test "${enable_udev}" != "no" && > > + test "${enable_hid2hci}" != "no") > > I looked at this again. Lets turn this around and make this an --enable-hid2hci similar to --enable-sixaxis. > > And you need to change the ${enable_tools} into ${enable_hid2hci} actually and not just add another dependency. Everything else seems to be already in the Makefile.tools for this. > > Regards > > Marcel >
diff --git a/configure.ac b/configure.ac index 1433ace4a..ba9937a16 100644 --- a/configure.ac +++ b/configure.ac @@ -218,8 +218,11 @@ if (test "${enable_udev}" != "no" && test -z "${path_udevdir}"); then fi AC_SUBST(UDEV_DIR, [${path_udevdir}]) +AC_ARG_ENABLE(hid2hci, AC_HELP_STRING([--disable-hid2hci], + [disable hid2hci tool]), [enable_hid2hci=${enableval}]) AM_CONDITIONAL(HID2HCI, test "${enable_tools}" != "no" && - test "${enable_udev}" != "no") + test "${enable_udev}" != "no" && + test "${enable_hid2hci}" != "no") AC_ARG_ENABLE(cups, AC_HELP_STRING([--disable-cups], [disable CUPS printer support]), [enable_cups=${enableval}])