Subject: Re: [bug#66935] [PATCH 4/4] services: dovecot: Fix incorrect type for ssl? field.

From: Clément Lassieur <[email-masked]>
Date: Mon, 06 Nov 2023 16:43:19 +0100

On Mon, Nov 06 2023, Bruno Victal wrote:

>> By the way, I see that the "required" value can be used for the
>> ssl dovecot core setting, but for the inet-listener this is not
>> specified: https://doc.dovecot.org/configuration_manual/service_configuration/?highlight=inet_listener#ssl.
>
> According to the dovecot link you provided, it isn't clear whether this
> 'ssl' in the context of inet-listener is a dovecot boolean [1] or is a
> string in the same manner like the core ssl setting [2].
> I'm afraid the dovecot documentation isn't the clearest here.
>
>> I think changing the ssl? option type from boolean to string should
>> be done for all boolean valued options simultaneously in a separate
>> patch, if at all.
>
> Some of the boolean options are really dovecot boolean, it's only
> this ssl? field in the inet-listener that is strange.
> I think it might be better to leave it for a separate patch as you
> have suggested, in that case feel free to discard the 4/4 patch.

Indeed the inet_listener ssl is a BOOL, as code says, whereas the master
one is an ENUM.  So the actual guix service looks correct.

The code for the master setting:

--8<---------------cut here---------------start------------->8---
static const struct setting_define master_setting_defines[] = {
	DEF(STR, base_dir),
	DEF(STR, state_dir),
	DEF(STR, libexec_dir),
	DEF(STR, instance_name),
	DEF(STR, protocols),
	DEF(STR, listen),
	DEF(ENUM, ssl),
	DEF(STR, default_internal_user),
	DEF(STR, default_internal_group),
	DEF(STR, default_login_user),
	DEF(UINT, default_process_limit),
	DEF(UINT, default_client_limit),
	DEF(TIME, default_idle_kill),
	DEF(SIZE, default_vsz_limit),

	DEF(BOOL, version_ignore),

	DEF(UINT, first_valid_uid),
	DEF(UINT, last_valid_uid),
	DEF(UINT, first_valid_gid),
	DEF(UINT, last_valid_gid),

	DEFLIST_UNIQUE(services, "service", &service_setting_parser_info),

	SETTING_DEFINE_LIST_END
};

static const struct master_settings master_default_settings = {
	.base_dir = PKG_RUNDIR,
	.state_dir = PKG_STATEDIR,
	.libexec_dir = PKG_LIBEXECDIR,
	.instance_name = PACKAGE,
	.protocols = "imap pop3 lmtp",
	.listen = "*, ::",
	.ssl = "yes:no:required",
	.default_internal_user = "dovecot",
	.default_internal_group = "dovecot",
	.default_login_user = "dovenull",
	.default_process_limit = 100,
	.default_client_limit = 1000,
	.default_idle_kill = 60,
	.default_vsz_limit = 256*1024*1024,

	.version_ignore = FALSE,

	.first_valid_uid = 500,
	.last_valid_uid = 0,
	.first_valid_gid = 1,
	.last_valid_gid = 0,

#ifndef CONFIG_BINARY
	.services = ARRAY_INIT
#else
	.services = { { &config_all_services_buf,
			     sizeof(struct service_settings *) } },
#endif
};
--8<---------------cut here---------------end--------------->8---

The code for the inet_listener setting:

--8<---------------cut here---------------start------------->8---
static const struct setting_define inet_listener_setting_defines[] = {
	DEF(STR, name),
	DEF(STR, address),
	DEF(IN_PORT, port),
	DEF(BOOL, ssl),
	DEF(BOOL, reuse_port),
	DEF(BOOL, haproxy),

	SETTING_DEFINE_LIST_END
};
--8<---------------cut here---------------end--------------->8---

Cheers,
Clément