Re: [Buildroot] [PATCH 1/1] package/postfix: add postfix 3.11.4 package

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello Love,

Thanks a lot for your patch and your contribution. There is however
quite a lot to fix before it can be accepted in Buildroot. See below.

On Wed, Jul 01, 2026 at 07:16:20PM +0200, Love Fogelström wrote:
> From: ljfogelstrom <ljfogelstrom@protonmail.com>
> 
> Signed-off-by: ljfogelstrom <ljfogelstrom@protonmail.com>

We will need your git configuration to be fixed so that your first
name/last name show up here.

The commit title should be just:

    package/postfix: new package

> diff --git a/package/postfix/0001-makedefs.patch b/package/postfix/0001-makedefs.patch
> new file mode 100644
> index 0000000000..f54e98d982
> --- /dev/null
> +++ b/package/postfix/0001-makedefs.patch
> @@ -0,0 +1,203 @@
> +From a0cf1c49eb846a55d014ae91c24810c340e4e85a Mon Sep 17 00:00:00 2001
> +From: ljfogelstrom <ljfogelstrom@protonmail.com>
> +Date: Thu, 25 Jun 2026 11:56:40 +0200
> +Subject: [PATCH] makedefs+install
> +
> +Signed-off-by: ljfogelstrom <ljfogelstrom@protonmail.com>

Same: first name/last name.

But more important, we need:

1. A proper commit log that explains what is going on

2. The patch must be submitted upstream. We no longer accept "hacks"
   in Buildroot that are not submitted upstream. Each patch must have
   an Upstream: tag that points to the upstream submission.

> diff --git a/package/postfix/Config.in b/package/postfix/Config.in
> new file mode 100644
> index 0000000000..9305ce09cc
> --- /dev/null
> +++ b/package/postfix/Config.in
> @@ -0,0 +1,27 @@
> +config BR2_PACKAGE_POSTFIX
> +	bool "postfix"
> +	depends on BR2_USE_MMU
> +	select BR2_PACKAGE_BERKELEYDB
> +	select BR2_PACKAGE_PCRE2
> +	help
> +	  Postfix Mail Transfer Agent
> +
> +	   http://postfix.org

Indentation for this line should be one tab + 2 spaces.

 +
> +if BR2_PACKAGE_POSTFIX
> +
> +	config BR2_PACKAGE_POSTFIX_ICU

Do not intend sub-options.

> +		bool "UTF-8 support"
> +		select BR2_PACKAGE_ICU

You need to replicate the depends on from ICU here:

        depends on BR2_INSTALL_LIBSTDCPP # icu
        depends on BR2_USE_WCHAR # icu
        depends on BR2_TOOLCHAIN_GCC_AT_LEAST_7 # icu
        depends on BR2_HOST_GCC_AT_LEAST_7 # icu
        depends on BR2_TOOLCHAIN_HAS_THREADS # icu
	depends on !BR2_BINFMT_FLAT # icu

> +		help
> +		  Enable support for SMTPUTF8 with the ICU library
> +		  which allows international characters
> +		  in email addresses and headers.
> +	comment "icu needs a toolchain w/ C++, wchar, threads, gcc >= 7, host gcc >= 7"

And this should be:

comment "UTF-8 support needs ..."

> diff --git a/package/postfix/init/systemd/postfix.service b/package/postfix/init/systemd/postfix.service

File should be directly in package/postfix, the init/systemd subdirs
are not needed.

> new file mode 100644
> index 0000000000..7a72aaa8e5
> --- /dev/null
> +++ b/package/postfix/init/systemd/postfix.service

postfix doesn't provide upstream a proper systemd unit file?
> diff --git a/package/postfix/init/sysv/S51postfix b/package/postfix/init/sysv/S51postfix
> new file mode 100644
> index 0000000000..d3e6d61648
> --- /dev/null
> +++ b/package/postfix/init/sysv/S51postfix

No need for package/postfix/init/sysv/S51postfix, put it directly in
package/postfix.

Make sure to have a look at package/busybox/S01syslogd and use it as a
template for init scripts. You can also check out the Buildroot
manual. We try to have consistency between our initscripts.

> diff --git a/package/postfix/postfix.mk b/package/postfix/postfix.mk
> new file mode 100644
> index 0000000000..e2cda4e85f
> --- /dev/null
> +++ b/package/postfix/postfix.mk
> @@ -0,0 +1,94 @@
> +################################################################################
> +#
> +# postfix
> +#
> +################################################################################
> +
> +POSTFIX_VERSION = 3.11.4
> +POSTFIX_SITE = https://www.artfiles.org/postfix.org/postfix-release/official

POSTFIX_LICENSE and POSTFIX_LICENSE_FILES must be added.

> +POSTFIX_DEPENDENCIES = berkeleydb pcre2
> +
> +ifdef BR2_PACKAGE_POSTFIX_ICU

ifeq ($(BR2_PACKAGE_POSTFIX_ICU),y)

> +POSTFIX_DEPENDENCIES += icu
> +endif
> +
> +ifdef BR2_LINUX_KERNEL_VERSION
> +export pf_linux_release != echo $(call qstrip,$(BR2_LINUX_KERNEL_VERSION)) | grep -Eo \
> +	"[1-7]\.[0-9]{1,3}(\.[0-9]{1,3})?"
> +else
> +export pf_linux_release = 6.12 # assuming we're building for linux, default version shouldn't matter
> +endif

Hu? What are you trying to do here?

> +pf_ccargs = -DNO_NIS -DNO_NISPLUS -DHAS_DEV_URANDOM $(strip $(TARGET_CFLAGS))
> +ifneq "$(strip $(TARGET_LDFLAGS))" ""
> +pf_ccargs += -Wl,$(TARGET_LDFLAGS:$() $()=,)
> +endif

Variables should be in capital letters, and prefixed by the package
name, i.e POSTFIX_CFLAGS for example.

Why do you need to strip TARGET_CFLAGS?

What are you trying to do with TARGET_LDFLAGS?

> +pf_shared = $(if $(BR2_STATIC_LIBS),no,yes)

Variables should be in capital letters, and prefixed by the package
name, i.e POSTFIX_SHARED for example.

> +
> +pf_default_settings = config_directory=/etc/postfix \

Same comment.

> +			 command_directory=/usr/sbin \
> +			 daemon_directory=/usr/libexec/postfix \
> +			 data_directory=/var/lib/postfix \
> +			 html_directory=no \
> +			 mail_owner=postfix \
> +			 mailq_path=/usr/bin/mailq \
> +			 manpage_directory=/usr/local/man \

Really relevant?

> +			 newaliases_path=/usr/bin/newaliases \
> +			 queue_directory=/var/spool/postfix \
> +			 readme_directory=no \
> +			 sendmail_path=/usr/sbin/sendmail \
> +			 setgid_group=postdrop \
> +			 shlib_directory=/usr/lib/postfix \
> +			 meta_directory=/etc/postfix \
> +			 sample_directory=/etc/postfix \
> +			 mail_version=$(POSTFIX_VERSION)
> +
> +# Replace definitions in the 'makedefs' script to enable cross-compilation.
> +define POSTFIX_CONFIGURE_CMDS
> +	[ ! -f $(@D)/makedefs.orig ] && mv -v $(@D)/makedefs $(@D)/makedefs.orig;:

Why is this needed?

> +	/usr/bin/sed -E -e "s,/usr/,$(STAGING_DIR)/usr/,g" \

Use $(SED).

> +			 -e "s,\s/lib(32|64)?, $(STAGING_DIR)/lib\1,g" \
> +			 -e "s,(pcre2|icu|pkg)-config,$(STAGING_DIR)/bin/\1-config,g" \
> +			 -e "s,pkgconf,$(STAGING_DIR)/bin/pkgconf,g" \
> +			$(@D)/makedefs.orig >$(@D)/makedefs

Why aren't you doing this in place in $(@D)/makedefs ?

> +	$(MAKE) $(TARGET_CONFIGURE_OPTS) \

Please pass $(TARGET_MAKE_ENV) in the environment.

> +		CCARGS+="$(pf_ccargs)" \
> +		OPT=$(shell echo $(TARGET_CFLAGS) | grep -Eo "\-O([0-3gsz]|fast)?") \
> +		DEBUG=$(shell echo $(TARGET_CFLAGS) | grep -Eo "\-g[a-z0-9]*") \

This is a bit meh :/

> +		SHELL="/bin/sh" \
> +		shared=$(pf_shared) makefiles -C $(@D)
> +	/usr/bin/sed -Ei -e "s,bin/postconf,:,g" \
> +			$(@D)/Makefile;: just in case
> +endef
> +
> +define POSTFIX_BUILD_CMDS
> +	$(MAKE) -C $(@D)

$(TARGET_MAKE_ENV) in the environment.

> +endef
> +
> +define POSTFIX_INSTALL_TARGET_CMDS
> +	$(MAKE) POSTFIX_INSTALL_OPTS="install_root=$(TARGET_DIR) $(strip $(pf_default_settings))" \

Why do you need to strip $(pf_default_settings) ?

> +		non-interactive-package -C $(@D)
> +endef
> +
> +define POSTFIX_INSTALL_INIT_SYSV
> +	$(INSTALL) -m 755 $(POSTFIX_PKGDIR)/init/sysv/*postfix $(TARGET_DIR)/etc/init.d

Please make this:

       $(INSTALL) -D -m 755 $(POSTFIX_PKGDIR)/S51postfix $(TARGET_DIR)/etc/init.d/S51postfix

> +	if [ "$(BR2_INIT_OPENRC)" ] ; then \
> +		if ! find -P $(TARGET_DIR)/etc/init.d -type f -name "sysv-rcs" ;\
> +		then \
> +			ln -st $(TARGET_DIR)/etc/runlevels/default \
> +				$(TARGET_DIR)/etc/init.d/*postfix ;\
> +		fi ;\
> +	fi

What are you trying to do here?

If you really need to so something related to openrc, then:

ifeq ($(BR2_INIT_OPENRC),y)
define POSTFIX_OPENRC_FIXUPS
       ...
endef
endif

define POSTFIX_INSTALL_INIT_SYSV
       ...
	$(POSTFIX_OPENRC_FIXUPS)
endef


> +define POSTFIX_INSTALL_INIT_SYSTEMD
> +	$(INSTALL) -m 644 $(POSTFIX_PKGDIR)/init/systemd/postfix.service $(TARGET_DIR)/etc/systemd/system

Change to:

       $(INSTALL) -D -m 644 $(POSTFIX_PKGDIR)/postfix.service $(TARGET_DIR)/etc/systemd/system/postfix.service

> +endef
> +
> +define POSTFIX_USERS
> +	postfix -1 postfix -1 ! - - - Postfix Mail Transfer Agent
> +	postdrop -1 postdrop -1 ! - - -
> +endef
> +
> +$(eval $(generic-package))

Also, you need to add a .hash file.

And ideally, a runtime test in support/testing/ would be good for this
kind of package.

Could you try to work on a new iteration?

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot