just looked at the packaging, mostly related to clean building, but not
only.
On April 2, 2024 7:16 pm, Stefan Hanreich wrote:
> Signed-off-by: Stefan Hanreich <s.hanre...@proxmox.com>
> ---
> Makefile | 93 +++++++++++++++++++++++++++++++++
> debian/changelog | 5 ++
> debian/control | 31 +++++++++++
> debian/copyright | 16 ++++++
> debian/proxmox-firewall.service | 16 ++++++
> debian/proxmox-firewall.timer | 11 ++++
> debian/rules | 14 +++++
> debian/source/format | 1 +
> defines.mk | 13 +++++
> 9 files changed, 200 insertions(+)
> create mode 100644 Makefile
> create mode 100644 debian/changelog
> create mode 100644 debian/control
> create mode 100644 debian/copyright
> create mode 100644 debian/proxmox-firewall.service
> create mode 100644 debian/proxmox-firewall.timer
> create mode 100644 debian/rules
> create mode 100644 debian/source/format
> create mode 100644 defines.mk
>
> diff --git a/Makefile b/Makefile
> new file mode 100644
> index 0000000..984c318
> --- /dev/null
> +++ b/Makefile
> @@ -0,0 +1,93 @@
> +include /usr/share/dpkg/pkg-info.mk
> +include /usr/share/dpkg/architecture.mk
> +include defines.mk
> +
> +PACKAGE=proxmox-firewall
> +BUILDDIR ?= $(PACKAGE)-$(DEB_VERSION_UPSTREAM)
> +
> +
> +DEB=$(PACKAGE)_$(DEB_VERSION_UPSTREAM_REVISION)_$(DEB_HOST_ARCH).deb
> +DBG_DEB=$(PACKAGE)-dbgsym_$(DEB_VERSION_UPSTREAM_REVISION)_$(DEB_HOST_ARCH).deb
> +DSC=rust-$(PACKAGE)_$(DEB_VERSION_UPSTREAM_REVISION).dsc
this doesn't match d/control ;)
> +
> +DEBS = $(DEB) $(DBG_DEB)
> +
> +ifeq ($(BUILD_MODE), release)
you need to set/export this in d/rules, else the package will contain a
debug build..
> +CARGO_BUILD_ARGS += --release
> +COMPILEDIR := target/release
> +else
> +COMPILEDIR := target/debug
> +endif
> +
> +USR_BIN := \
> + proxmox-firewall
> +
> +COMPILED_BINS := \
> + $(addprefix $(COMPILEDIR)/,$(USR_BIN))
> +
> +all: cargo-build
> +
> +.PHONY: cargo-build
> +cargo-build:
> + cargo build $(CARGO_BUILD_ARGS)
> +
> +$(COMPILED_BINS): cargo-build
> +
> +install: $(COMPILED_BINS)
> + install -dm755 $(DESTDIR)$(SBINDIR)
> + $(foreach i,$(USR_BIN), \
> + install -m755 $(COMPILEDIR)/$(i) $(DESTDIR)$(SBINDIR)/ ;)
I am not sure this (and all the helper stuff above that only exists for
it) make much sense. maybe we could simply get `$(CARGO) install` to
work here, and then let the packging put it into /usr/sbin ?
> +
> +update-dcontrol: #$(BUILDDIR)
> + debcargo package \
> + --config debian/debcargo.toml \
> + --changelog-ready \
> + --no-overlay-write-back \
> + --directory $(BUILDDIR) \
> + $(PACKAGE) \
> + $(shell dpkg-parsechangelog -l debian/changelog -SVersion | sed -e
> 's/-.*//')
> + cat $(BUILDDIR)/debian/control debian/control.extra > debian/control
> + wrap-and-sort -t -k -f debian/control
this doesn't work because there is no debian/debcargo.toml and also no
debian/control.extra ;) since debcargo doesn't work with workspaces
(yet), it probably doesn't make sense to leave this in.. one just has to
remember to update d/control when updating any of the Cargo.toml files
w.r.t. dependencies or features.
> +
> +.PHONY: build
> +build: $(BUILDDIR)
> +$(BUILDDIR):
> + rm -rf $@ $@.tmp; mkdir $@.tmp
> + cp -a proxmox-firewall proxmox-nftables proxmox-ve-config debian
> Cargo.toml Makefile defines.mk $@.tmp/
this is missing the .cargo dir, which doesn't matter (much) for building
directly via `make deb` (since cargo will look in parent dirs), but
completely breaks for `make dsc` or `make sbuild`. but I suggest
handling that in d/rules (see comment there)
> + mv $@.tmp $@
> +
> +.PHONY: deb
> +deb: $(DEB)
> +$(HELPER_DEB) $(DBG_DEB) $(HELPER_DBG_DEB) $(DOC_DEB): $(DEB)
> +$(DEB): $(BUILDDIR)
> + cd $(BUILDDIR); dpkg-buildpackage -b -us -uc --no-pre-clean
> + lintian $(DEB) $(DOC_DEB) $(HELPER_DEB)
> +
> +.PHONY: test
> +test:
> + cargo test
> +
> +.PHONY: dsc
> +dsc:
> + rm -rf $(BUILDDIR) $(DSC)
> + $(MAKE) $(DSC)
> + lintian $(DSC)
> +$(DSC): $(BUILDDIR)
> + cd $(BUILDDIR); dpkg-buildpackage -S -us -uc -d -nc
> +
> +sbuild: $(DSC)
> + sbuild $<
> +
> +.PHONY: dinstall
> +dinstall: $(DEB)
> + dpkg -i $(DEB) $(DBG_DEB) $(DOC_DEB)
> +
> +.PHONY: distclean
> +distclean: clean
> +
> +.PHONY: clean
> +clean:
> + cargo clean
> + rm -f *.deb *.build *.buildinfo *.changes *.dsc rust-$(PACKAGE)*.tar*
> + rm -rf $(PACKAGE)-[0-9]*/
> + find . -name '*~' -exec rm {} ';'
> diff --git a/debian/changelog b/debian/changelog
> new file mode 100644
> index 0000000..7918ec9
> --- /dev/null
> +++ b/debian/changelog
> @@ -0,0 +1,5 @@
> +proxmox-firewall (0.1-1) UNRELEASED; urgency=medium
the `-1` and `debian/source/format` disagree (again, only a problem when
attempting building via sbuild)
> +
> + * Initial release.
> +
> + -- Stefan Hanreich <s.hanre...@proxmox.com> Thu, 07 Mar 2024 10:15:10 +0100
> diff --git a/debian/control b/debian/control
> new file mode 100644
> index 0000000..e04ce68
> --- /dev/null
> +++ b/debian/control
> @@ -0,0 +1,31 @@
> +Source: proxmox-firewall
> +Section: admin
> +Priority: optional
> +Maintainer: Proxmox Support Team <supp...@proxmox.com>
> +Build-Depends: cargo:native,
> + debhelper-compat (= 13),
> + dh-cargo (>= 25),
you aren't actually using that though? (and probably shouldn't, it's
mainly for "simple" crate <-> package situations)
> + librust-anyhow-1+default-dev,
> + librust-env-logger-0.10+default-dev,
> + librust-log-0.4+default-dev (>= 0.4.17-~~),
> + librust-nix-0.26+default-dev (>= 0.26.1-~~),
> + librust-serde-1+default-dev,
> + librust-serde-1+derive-dev,
> + librust-serde-json-1+default-dev,
> + librust-serde-plain-1+default-dev,
> + librust-serde-plain-1+default-dev,
> + librust-serde-with+default-dev,
> + librust-libc-0.2+default-dev,
> + librust-proxmox-schema-3+default-dev,
this is missing at least libnftables-dev and netbase
> +Standards-Version: 4.6.2
> +Homepage: https://www.proxmox.com
> +
> +Package: proxmox-firewall
> +Architecture: any
> +Conflicts: ulogd,
> +Depends: ${misc:Depends}, ${shlibs:Depends},
> + pve-firewall,
> + nftables,
this is missing at least netbase ;)
> +Description: Proxmox VE nft Firewall
> + This package contains a nftables-based implementation of the Proxmox VE
> + Firewall
> diff --git a/debian/copyright b/debian/copyright
> new file mode 100644
> index 0000000..fe09a1b
> --- /dev/null
> +++ b/debian/copyright
> @@ -0,0 +1,16 @@
> +Copyright (C) 2018-2024 Proxmox Server Solutions GmbH
> +
> +This software is written by Proxmox Server Solutions GmbH
> <supp...@proxmox.com>
> +
> +This program is free software: you can redistribute it and/or modify
> +it under the terms of the GNU Affero General Public License as published by
> +the Free Software Foundation, either version 3 of the License, or
> +(at your option) any later version.
> +
> +This program is distributed in the hope that it will be useful,
> +but WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +GNU Affero General Public License for more details.
> +
> +You should have received a copy of the GNU Affero General Public License
> +along with this program. If not, see <http://www.gnu.org/licenses/>.
> diff --git a/debian/proxmox-firewall.service b/debian/proxmox-firewall.service
> new file mode 100644
> index 0000000..5f9bf4b
> --- /dev/null
> +++ b/debian/proxmox-firewall.service
> @@ -0,0 +1,16 @@
> +[Unit]
> +Description=Proxmox VE nftables firewall
> +ConditionPathExists=/usr/sbin/proxmox-firewall
when would this path not exist?
> +Wants=pve-cluster.service pvefw-logger.service
> +After=pvefw-logger.service pve-cluster.service network.target
> systemd-modules-load.service
should the timer also get this to avoid premature starting? or am I
misremembering how timers and their services interact :)
> +DefaultDependencies=no
> +Before=shutdown.target
> +Conflicts=shutdown.target
> +
> +[Service]
> +ExecStart=/usr/sbin/proxmox-firewall
> +Type=oneshot
> +
> +[Install]
> +WantedBy=multi-user.target
> +
so this is started by the timer below (on boot and then every 5s) but
also enabled and thus started once on boot? that seems confusing ;)
> diff --git a/debian/proxmox-firewall.timer b/debian/proxmox-firewall.timer
> new file mode 100644
> index 0000000..d051102
> --- /dev/null
> +++ b/debian/proxmox-firewall.timer
> @@ -0,0 +1,11 @@
> +[Unit]
> +Description=Proxmox VE nft Firewall timer
capitalisation compared to the service above (and also, nft vs nftables)
> +
> +[Timer]
> +OnBootSec=1s
> +OnUnitInactiveSec=5s
> +Unit=proxmox-firewall.service
> +
> +[Install]
> +WantedBy=timers.target
> +
> diff --git a/debian/rules b/debian/rules
> new file mode 100644
d/rules should be executable :)
> index 0000000..5539a00
> --- /dev/null
> +++ b/debian/rules
> @@ -0,0 +1,14 @@
> +#!/usr/bin/make -f
> +
> +# Uncomment this to turn on verbose mode.
> +#export DH_VERBOSE=1
> +
> +%:
> + dh $@
> +
I would suggest calling the cargo wrapper here to create a proper
.cargo/config file for a clean build. you can look at proxmox-backup for
inspiration ;)
here's a starting point:
diff --git a/debian/rules b/debian/rules
index 5539a00..bbb4d0a 100644
--- a/debian/rules
+++ b/debian/rules
@@ -1,14 +1,31 @@
#!/usr/bin/make -f
# Uncomment this to turn on verbose mode.
-#export DH_VERBOSE=1
+export DH_VERBOSE=1
+
+include /usr/share/dpkg/pkg-info.mk
+include /usr/share/rustc/architecture.mk
+
+export BUILD_MODE=release
+
+CARGO=/usr/share/cargo/bin/cargo
+
+export CFLAGS CXXFLAGS CPPFLAGS LDFLAGS
+export DEB_HOST_RUST_TYPE DEB_HOST_GNU_TYPE
+export CARGO_HOME = $(CURDIR)/debian/cargo_home
+
+export DEB_CARGO_CRATE=proxmox-firewall_$(DEB_VERSION_UPSTREAM)
+export DEB_CARGO_PACKAGE=proxmox-firewall
%:
dh $@
+override_dh_auto_configure:
+ @perl -ne 'if (/^version\s*=\s*"(\d+(?:\.\d+)+)"/) { my $$v_cargo =
$$1; my $$v_deb = "$(DEB_VERSION_UPSTREAM)"; \
+ die "ERROR: d/changelog <-> Cargo.toml version mismatch: $$v_cargo
!= $$v_deb\n" if $$v_cargo ne $$v_deb; exit(0); }' Cargo.toml
+ $(CARGO) prepare-debian $(CURDIR)/debian/cargo_registry
--link-from-system
+ dh_auto_configure
+
override_dh_installsystemd:
dh_installsystemd --no-start proxmox-firewall.service
dh_installsystemd proxmox-firewall.timer
we probably also want `cargo` in the Makefile to be $(CARGO)
(defaulting to `cargo` there if not set). that way, `cargo build` and
`cargo test` also go through the wrapper and pick up any special sauce
> +override_dh_installsystemd:
> + dh_installsystemd --no-start proxmox-firewall.service
> + dh_installsystemd proxmox-firewall.timer
> +
> +override_dh_installinit:
> +
don't think this dh_installinit override is needed?
> diff --git a/debian/source/format b/debian/source/format
> new file mode 100644
> index 0000000..89ae9db
> --- /dev/null
> +++ b/debian/source/format
> @@ -0,0 +1 @@
> +3.0 (native)
native means no Debian revision in the version, see above w.r.t.
d/changelog
> diff --git a/defines.mk b/defines.mk
> new file mode 100644
> index 0000000..e01164d
> --- /dev/null
> +++ b/defines.mk
> @@ -0,0 +1,13 @@
> +PREFIX = /usr
> +BINDIR = $(PREFIX)/bin
> +SBINDIR = $(PREFIX)/sbin
> +LIBDIR = $(PREFIX)/lib
> +LIBEXECDIR = $(LIBDIR)
> +DATAROOTDIR = $(PREFIX)/share
> +MAN1DIR = $(PREFIX)/share/man/man1
> +MAN5DIR = $(PREFIX)/share/man/man5
> +SYSCONFDIR = /etc
> +
> +# For local overrides
> +-include local.mak
> +
> --
> 2.39.2
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
