Skip to content

Commit

Permalink
firewall/firewall4: provide uci-firewall
Browse files Browse the repository at this point in the history
Provide uci-firewall via PROVIDES in both firewall and firewall4. This
will allow us to change the dependency of luci-app-firewall to
uci-firewall, making it possible to use it with either implementation.

Move CONFLICTS from firewall4 to firewall, to solve this recursive
dependency problem:

tmp/.config-package.in:307:error: recursive dependency detected!
tmp/.config-package.in:307:     symbol PACKAGE_firewall is selected by PACKAGE_firewall4
tmp/.config-package.in:328:     symbol PACKAGE_firewall4 depends on PACKAGE_firewall

Signed-off-by: Stijn Tintel <stijn@linux-ipv6.be>
Reviewed-by: Jo-Philipp Wich <jo@mein.io>
  • Loading branch information
stintel committed Jan 6, 2022
1 parent 3ec25a6 commit 53b87a7
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 2 deletions.
4 changes: 3 additions & 1 deletion package/network/config/firewall/Makefile
Expand Up @@ -9,7 +9,7 @@
include $(TOPDIR)/rules.mk

PKG_NAME:=firewall
PKG_RELEASE:=1
PKG_RELEASE:=2

PKG_SOURCE_PROTO:=git
PKG_SOURCE_URL=$(PROJECT_GIT)/project/firewall3.git
Expand All @@ -29,6 +29,8 @@ define Package/firewall
CATEGORY:=Base system
TITLE:=OpenWrt C Firewall
DEPENDS:=+libubox +libubus +libuci +libip4tc +IPV6:libip6tc +libxtables +kmod-ipt-core +kmod-ipt-conntrack +IPV6:kmod-nf-conntrack6 +kmod-ipt-nat
PROVIDES:=uci-firewall
CONFLICTS:=firewall4
endef

define Package/firewall/description
Expand Down
2 changes: 1 addition & 1 deletion package/network/config/firewall4/Makefile
Expand Up @@ -26,7 +26,7 @@ define Package/firewall4
+kmod-nft-nat +kmod-nft-nat6 \
+nftables-json \
+ucode +ucode-mod-fs +ucode-mod-ubus +ucode-mod-uci
CONFLICTS:=firewall
PROVIDES:=uci-firewall
endef

define Package/firewall4/description
Expand Down

6 comments on commit 53b87a7

@zxlhhyccc
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

@stintel
Copy link
Member Author

@stintel stintel commented on 53b87a7 Jan 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use copy-paste next time. Now I cannot copy-paste the error in the commit message. Also this is not the commit that introduced the problem.

@kuanyili
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

firewall \

This line should probably change also from firewall to uci-firewall as in openwrt/luci@48599d8

Otherwise, if we try to build an LuCI-included image with image builder, package luci will resolve to firewall4, conflicting with firewall which is listed as one of the default router packages.

@PolynomialDivision
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kuanyili Thanks. I can approve.

@stintel
Copy link
Member Author

@stintel stintel commented on 53b87a7 Jan 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

firewall \

This line should probably change also from firewall to uci-firewall as in openwrt/luci@48599d8

Otherwise, if we try to build an LuCI-included image with image builder, package luci will resolve to firewall4, conflicting with firewall which is listed as one of the default router packages.

I doubt this is the correct solution, as it would make the default package selection unpredictable. Surely we don't want that.

@kuanyili
Copy link
Contributor

@kuanyili kuanyili commented on 53b87a7 Jan 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

firewall \

This line should probably change also from firewall to uci-firewall as in openwrt/luci@48599d8
Otherwise, if we try to build an LuCI-included image with image builder, package luci will resolve to firewall4, conflicting with firewall which is listed as one of the default router packages.

I doubt this is the correct solution, as it would make the default package selection unpredictable. Surely we don't want that.

You're right. This was also my concern. There is currently no way to set package precedence.

Not sure if there is a better approach but for those who are interested, I'm working around the conflict by explicitly specifying the desired firewall package before any package that triggers a uci-firewall dependency resolution.

More specifically, I'm doing something like

  • PACKAGES="firewall luci" to select firewall (firewall must be placed before luci)
  • PACKAGES="-firewall firewall4 luci" to select firewall4

while running image builder for the time being.

Please sign in to comment.