| Message ID | 20200305193427.17434-1-dev@kresin.me |
|---|---|
| State | Accepted |
| Delegated to: | Mathias Kresin |
| Headers | show |
| Series | [OpenWrt-Devel] iproute2: revert add libcap support, enabled in ip-full | expand |
On Thu, Mar 5, 2020 at 8:34 PM Mathias Kresin <dev@kresin.me> wrote: > > This reverts commit a6da3f9ef746101b84a6f530f5a40de28341b69a. > > The libcap isn't as optional as the commit messages suggests. A hard > dependency to the libcap package is added, which is only available in > the external packages feed. Therefore it is impossible to package > ip-full without having the external packages feed up and running, which > is a regression to the former behaviour. > > Signed-off-by: Mathias Kresin <dev@kresin.me> Acked-by: Hans Dedecker <dedeckeh@gmail.com> > --- > package/network/utils/iproute2/Makefile | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/package/network/utils/iproute2/Makefile b/package/network/utils/iproute2/Makefile > index 34b768a906..cff582283c 100644 > --- a/package/network/utils/iproute2/Makefile > +++ b/package/network/utils/iproute2/Makefile > @@ -47,7 +47,7 @@ $(call Package/iproute2/Default) > VARIANT:=full > PROVIDES:=ip > ALTERNATIVES:=300:/sbin/ip:/usr/libexec/ip-full > - DEPENDS:=+libnl-tiny +libelf +(PACKAGE_devlink||PACKAGE_rdma):libmnl +libcap > + DEPENDS:=+libnl-tiny +libelf +(PACKAGE_devlink||PACKAGE_rdma):libmnl > endef > > define Package/tc > @@ -55,43 +55,43 @@ $(call Package/iproute2/Default) > TITLE:=Traffic control utility > VARIANT:=tc > PROVIDES:=tc > - DEPENDS:=+kmod-sched-core +libxtables +libelf +(PACKAGE_devlink||PACKAGE_rdma):libmnl +PACKAGE_ip-full:libcap > + DEPENDS:=+kmod-sched-core +libxtables +libelf +(PACKAGE_devlink||PACKAGE_rdma):libmnl > endef > > define Package/genl > $(call Package/iproute2/Default) > TITLE:=General netlink utility frontend > - DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap > + DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf > endef > > define Package/ip-bridge > $(call Package/iproute2/Default) > TITLE:=Bridge configuration utility from iproute2 > - DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap > + DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf > endef > > define Package/ss > $(call Package/iproute2/Default) > TITLE:=Socket statistics utility > - DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap > + DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf > endef > > define Package/nstat > $(call Package/iproute2/Default) > TITLE:=Network statistics utility > - DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap > + DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf > endef > > define Package/devlink > $(call Package/iproute2/Default) > TITLE:=Network devlink utility > - DEPENDS:=+libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap > + DEPENDS:=+libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf > endef > > define Package/rdma > $(call Package/iproute2/Default) > TITLE:=Network rdma utility > - DEPENDS:=+libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap > + DEPENDS:=+libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf > endef > > ifeq ($(BUILD_VARIANT),tiny) > @@ -100,7 +100,7 @@ endif > > ifeq ($(BUILD_VARIANT),full) > HAVE_ELF:=y > - HAVE_CAP:=y > + HAVE_CAP:=n > endif > > ifeq ($(BUILD_VARIANT),tc) > -- > 2.17.1 > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
On Thu, Mar 5, 2020 at 8:34 PM Mathias Kresin <dev@kresin.me> wrote: > > This reverts commit a6da3f9ef746101b84a6f530f5a40de28341b69a. Not exactly a revert, since it keeps HAVE_CAP logic. > The libcap isn't as optional as the commit messages suggests. A hard > dependency to the libcap package is added, which is only available in > the external packages feed. Therefore it is impossible to package > ip-full without having the external packages feed up and running, which > is a regression to the former behaviour. The libcap support is by all means optional, otherwise iproute2 build will fail when its missing. Besides,your commit actually removes this dependency. If this is not a living proof of the facultative nature of this dependency, I don't know what else is. One would argue that ip-full should correspond to the full fledged version of the packet. If the dependency of an external package is the issue, how about making a different variant with HAVE_CAP support? It could be called ip-really-full (or ip-fullest). Anyway, I don't really need libcap support in my iproute2. As long as iproute2 continues to build successfully in the presence of libcap, it would be fine for me. > Signed-off-by: Mathias Kresin <dev@kresin.me> > --- > package/network/utils/iproute2/Makefile | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/package/network/utils/iproute2/Makefile b/package/network/utils/iproute2/Makefile > index 34b768a906..cff582283c 100644 > --- a/package/network/utils/iproute2/Makefile > +++ b/package/network/utils/iproute2/Makefile > @@ -47,7 +47,7 @@ $(call Package/iproute2/Default) > VARIANT:=full > PROVIDES:=ip > ALTERNATIVES:=300:/sbin/ip:/usr/libexec/ip-full > - DEPENDS:=+libnl-tiny +libelf +(PACKAGE_devlink||PACKAGE_rdma):libmnl +libcap > + DEPENDS:=+libnl-tiny +libelf +(PACKAGE_devlink||PACKAGE_rdma):libmnl > endef > > define Package/tc > @@ -55,43 +55,43 @@ $(call Package/iproute2/Default) > TITLE:=Traffic control utility > VARIANT:=tc > PROVIDES:=tc > - DEPENDS:=+kmod-sched-core +libxtables +libelf +(PACKAGE_devlink||PACKAGE_rdma):libmnl +PACKAGE_ip-full:libcap > + DEPENDS:=+kmod-sched-core +libxtables +libelf +(PACKAGE_devlink||PACKAGE_rdma):libmnl > endef > > define Package/genl > $(call Package/iproute2/Default) > TITLE:=General netlink utility frontend > - DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap > + DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf > endef > > define Package/ip-bridge > $(call Package/iproute2/Default) > TITLE:=Bridge configuration utility from iproute2 > - DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap > + DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf > endef > > define Package/ss > $(call Package/iproute2/Default) > TITLE:=Socket statistics utility > - DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap > + DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf > endef > > define Package/nstat > $(call Package/iproute2/Default) > TITLE:=Network statistics utility > - DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap > + DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf > endef > > define Package/devlink > $(call Package/iproute2/Default) > TITLE:=Network devlink utility > - DEPENDS:=+libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap > + DEPENDS:=+libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf > endef > > define Package/rdma > $(call Package/iproute2/Default) > TITLE:=Network rdma utility > - DEPENDS:=+libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap > + DEPENDS:=+libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf > endef > > ifeq ($(BUILD_VARIANT),tiny) > @@ -100,7 +100,7 @@ endif > > ifeq ($(BUILD_VARIANT),full) > HAVE_ELF:=y > - HAVE_CAP:=y > + HAVE_CAP:=n > endif > > ifeq ($(BUILD_VARIANT),tc) > -- > 2.17.1 >
05/03/2020 23:29, Alin Năstac: > On Thu, Mar 5, 2020 at 8:34 PM Mathias Kresin <dev@kresin.me> wrote: >> >> This reverts commit a6da3f9ef746101b84a6f530f5a40de28341b69a. > > Not exactly a revert, since it keeps HAVE_CAP logic. Sure, I want to make sure that HAVE_CAP is really disabled. >> The libcap isn't as optional as the commit messages suggests. A hard >> dependency to the libcap package is added, which is only available in >> the external packages feed. Therefore it is impossible to package >> ip-full without having the external packages feed up and running, which >> is a regression to the former behaviour. > > The libcap support is by all means optional, otherwise iproute2 build > will fail when its missing. You describe exactly the issue I hit while building ip-full. During development I don't have any external/third-party feeds installed. And the OpenWrt base system shouldn't depend on external/third-party feeds. These feeds are an add on and by no means a requirement. > Besides, your commit actually removes this > dependency. If this is not a living proof of the facultative nature of > this dependency, I don't know what else is. Sorry, I don't get what you're trying to say. > One would argue that ip-full should correspond to the full fledged > version of the packet. If the dependency of an external package is the > issue, how about making a different variant with HAVE_CAP support? It > could be called ip-really-full (or ip-fullest). Try to get libcap into the OpenWrt base system and enable HAVE_CAP afterwards? Mathias
Hi Mathias, On Sun, Mar 15, 2020 at 11:40 PM Mathias Kresin <dev@kresin.me> wrote: > > 05/03/2020 23:29, Alin Năstac: > > On Thu, Mar 5, 2020 at 8:34 PM Mathias Kresin <dev@kresin.me> wrote: > >> > >> This reverts commit a6da3f9ef746101b84a6f530f5a40de28341b69a. > > > > Not exactly a revert, since it keeps HAVE_CAP logic. > > Sure, I want to make sure that HAVE_CAP is really disabled. > > >> The libcap isn't as optional as the commit messages suggests. A hard > >> dependency to the libcap package is added, which is only available in > >> the external packages feed. Therefore it is impossible to package > >> ip-full without having the external packages feed up and running, which > >> is a regression to the former behaviour. > > > > The libcap support is by all means optional, otherwise iproute2 build > > will fail when its missing. > > You describe exactly the issue I hit while building ip-full. During > development I don't have any external/third-party feeds installed. And > the OpenWrt base system shouldn't depend on external/third-party feeds. > These feeds are an add on and by no means a requirement. > > > Besides, your commit actually removes this > > dependency. If this is not a living proof of the facultative nature of > > this dependency, I don't know what else is. > > Sorry, I don't get what you're trying to say. > > > One would argue that ip-full should correspond to the full fledged > > version of the packet. If the dependency of an external package is the > > issue, how about making a different variant with HAVE_CAP support? It > > could be called ip-really-full (or ip-fullest). > > Try to get libcap into the OpenWrt base system and enable HAVE_CAP > afterwards? I'm in favor of this approach as currently a part of the ip-full functionality is broken and I see ip-full for now and in the future as a core package Hans > > Mathias > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
On Sun, Mar 15, 2020 at 11:40 PM Mathias Kresin <dev@kresin.me> wrote: > > 05/03/2020 23:29, Alin Năstac: > > On Thu, Mar 5, 2020 at 8:34 PM Mathias Kresin <dev@kresin.me> wrote: > >> > >> This reverts commit a6da3f9ef746101b84a6f530f5a40de28341b69a. > > > > Not exactly a revert, since it keeps HAVE_CAP logic. > > Sure, I want to make sure that HAVE_CAP is really disabled. > > >> The libcap isn't as optional as the commit messages suggests. A hard > >> dependency to the libcap package is added, which is only available in > >> the external packages feed. Therefore it is impossible to package > >> ip-full without having the external packages feed up and running, which > >> is a regression to the former behaviour. > > > > The libcap support is by all means optional, otherwise iproute2 build > > will fail when its missing. > > You describe exactly the issue I hit while building ip-full. During > development I don't have any external/third-party feeds installed. And > the OpenWrt base system shouldn't depend on external/third-party feeds. > These feeds are an add on and by no means a requirement. > > > Besides, your commit actually removes this > > dependency. If this is not a living proof of the facultative nature of > > this dependency, I don't know what else is. > > Sorry, I don't get what you're trying to say. I'm trying to say that libcap dependency is optional from iproute2 tarball pov. I guess you were trying to say that openwrt +libcap dependency was not optional, but I don't really understand why you think it isn't. ip package has 2 variants (tiny and full) and I only enabled it on ip-full variant.Since the variant selection is at user's discretion, the ip openwrt package dependency of libcap *is* - by all means of the word - optional. > > One would argue that ip-full should correspond to the full fledged > > version of the packet. If the dependency of an external package is the > > issue, how about making a different variant with HAVE_CAP support? It > > could be called ip-really-full (or ip-fullest). > > Try to get libcap into the OpenWrt base system and enable HAVE_CAP > afterwards? As I said in the previous message, my use case doesn't require libcap functionality, so I have no problem with disabling HAVE_CAP in the full variant, but from the semantic pov, full should really mean "all functionality enabled". Maybe it would be wise to create a third variant called standard that would be equivalent with the full variant minus HAVE_CAP.
On 16/03/2020 09:37, Hans Dedecker wrote: >>> One would argue that ip-full should correspond to the full fledged >>> version of the packet. If the dependency of an external package is the >>> issue, how about making a different variant with HAVE_CAP support? It >>> could be called ip-really-full (or ip-fullest). Yet another ip package seems messy to me. >> Try to get libcap into the OpenWrt base system and enable HAVE_CAP >> afterwards? > I'm in favor of this approach as currently a part of the ip-full > functionality is broken and I see ip-full for now and in the future as > a core package > +1 for moving libcap to core. Other packages could benefit from it too (lldpd comes to mind). Stijn
diff --git a/package/network/utils/iproute2/Makefile b/package/network/utils/iproute2/Makefile index 34b768a906..cff582283c 100644 --- a/package/network/utils/iproute2/Makefile +++ b/package/network/utils/iproute2/Makefile @@ -47,7 +47,7 @@ $(call Package/iproute2/Default) VARIANT:=full PROVIDES:=ip ALTERNATIVES:=300:/sbin/ip:/usr/libexec/ip-full - DEPENDS:=+libnl-tiny +libelf +(PACKAGE_devlink||PACKAGE_rdma):libmnl +libcap + DEPENDS:=+libnl-tiny +libelf +(PACKAGE_devlink||PACKAGE_rdma):libmnl endef define Package/tc @@ -55,43 +55,43 @@ $(call Package/iproute2/Default) TITLE:=Traffic control utility VARIANT:=tc PROVIDES:=tc - DEPENDS:=+kmod-sched-core +libxtables +libelf +(PACKAGE_devlink||PACKAGE_rdma):libmnl +PACKAGE_ip-full:libcap + DEPENDS:=+kmod-sched-core +libxtables +libelf +(PACKAGE_devlink||PACKAGE_rdma):libmnl endef define Package/genl $(call Package/iproute2/Default) TITLE:=General netlink utility frontend - DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap + DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf endef define Package/ip-bridge $(call Package/iproute2/Default) TITLE:=Bridge configuration utility from iproute2 - DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap + DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf endef define Package/ss $(call Package/iproute2/Default) TITLE:=Socket statistics utility - DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap + DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf endef define Package/nstat $(call Package/iproute2/Default) TITLE:=Network statistics utility - DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap + DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf endef define Package/devlink $(call Package/iproute2/Default) TITLE:=Network devlink utility - DEPENDS:=+libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap + DEPENDS:=+libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf endef define Package/rdma $(call Package/iproute2/Default) TITLE:=Network rdma utility - DEPENDS:=+libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap + DEPENDS:=+libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf endef ifeq ($(BUILD_VARIANT),tiny) @@ -100,7 +100,7 @@ endif ifeq ($(BUILD_VARIANT),full) HAVE_ELF:=y - HAVE_CAP:=y + HAVE_CAP:=n endif ifeq ($(BUILD_VARIANT),tc)
This reverts commit a6da3f9ef746101b84a6f530f5a40de28341b69a. The libcap isn't as optional as the commit messages suggests. A hard dependency to the libcap package is added, which is only available in the external packages feed. Therefore it is impossible to package ip-full without having the external packages feed up and running, which is a regression to the former behaviour. Signed-off-by: Mathias Kresin <dev@kresin.me> --- package/network/utils/iproute2/Makefile | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)