Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 20 Feb 2023 23:18:16 +0100
From:      =?UTF-8?Q?Stefan_E=c3=9fer?= <se@FreeBSD.org>
To:        Jessica Clarke <jrtc27@freebsd.org>
Cc:        "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org>
Subject:   Re: git: c2bb66023fe3 - main - kbdcontrol: enable pre-Unicode dead key table compatibility
Message-ID:  <c10b51c3-2e86-be62-017b-e185859a7447@FreeBSD.org>
In-Reply-To: <B7AB527B-52E0-4590-9166-04092249AD84@freebsd.org>
References:  <202302141315.31EDFu9Q044384@gitrepo.freebsd.org> <B7AB527B-52E0-4590-9166-04092249AD84@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Am 14.02.23 um 15:05 schrieb Jessica Clarke:
> On 14 Feb 2023, at 13:15, Stefan Eßer <se@FreeBSD.org> wrote:
>>
>> The branch main has been updated by se:
>>
>> URL: https://cgit.FreeBSD.org/src/commit/?id=c2bb66023fe3e3617f56302b6960f59545a5535c
>>
>> commit c2bb66023fe3e3617f56302b6960f59545a5535c
>> Author:     Stefan Eßer <se@FreeBSD.org>
>> AuthorDate: 2023-02-14 12:49:06 +0000
>> Commit:     Stefan Eßer <se@FreeBSD.org>
>> CommitDate: 2023-02-14 12:49:06 +0000
>>
>>     kbdcontrol: enable pre-Unicode dead key table compatibility
>>
>>     The definition of pre-Unicode keymap ioctls will be made optional and
>>     dependent on COMPAT_FREEBSD13 in a follow-up commit to 14-CURRENT.
>>
>>     While we generally provide ABI compatibility for older binaries on
>>     a new kernel, but not functionally extended userland programs on an
>>     old kernel, it has been specifically requested to preserve ABI
>>     compatibility for the kbdcontrol program for both these cases.
>>
>>     Passing the kernel configuration option COMPAT_FREEBSD13 to the build
>>     of kbdcontrol will make ioctls visible to the build that are normally
>>     hidden, but required to implement compatibility with kernels that only
>>     support 8 bit characters in dead key maps.
>>
>>     This commit is not to be merged to any previous FreeBSD version and
>>     it shall be reverted as soon as this type of ABI compatibility is no
>>     longer deemed necessary (probably before 14-STABLE is branched).
>>
>>     This commit is a part of review D38465 and split off to allow it to be
>>     reverted using the commit ID.
>> ---
>> usr.sbin/kbdcontrol/Makefile | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/usr.sbin/kbdcontrol/Makefile b/usr.sbin/kbdcontrol/Makefile
>> index 960671a40fe7..d2107fb86290 100644
>> --- a/usr.sbin/kbdcontrol/Makefile
>> +++ b/usr.sbin/kbdcontrol/Makefile
>> @@ -9,4 +9,7 @@ SRCS=	kbdcontrol.c lex.l
>> WARNS?=	4
>> CFLAGS+= -I${.CURDIR}
>>
>> +# temporarily added for pre-Unicode accent key API/ABI compatibility
>> +CFLAGS+= -DCOMPAT_FREEBSD13
> 
> This doesn’t seem right... COMPAT_FREEBSD* is for kernel not userspace.
> Normally this would be a _WANT_FOO.

Yes, I know and have explained the reasoning in the commit message:

There has been a request for API compatibility of the new kbdcontrol command
on an old kernel, even though we do not provide that kind of compatibility
in general.

Defining COMPAT_FREEBSD13 has the side-effect of making definitions for the
API compatibility in the kernel (typically protected with #ifdef _KERNEL)
visible in the user-land. A _WANT_FOO definition would have needed to be
mapped to COMPAT_FREEBSD13 (and possibly _KERNEL) anyway to have the desired
effect.

The way this compatibility has been implemented and made conditional on the
compatibility ioctls being defined in kbio.h makes it possible to commit the
kbdcontrol sources from -CURRENT to -12 and -13 (but *without* this commit
to the Makefile!). This would allow to merge later commits applied to this
command in -CURRENT to merged back, without causing conflicts.

Since this commit (to he Makefile) is meant to be reverted as soon as possible
and since it is minimal code bloat when not needed (the binary size on amd64
is unchanged when rounded to 512 byte "sectors", for example), I do not see
any use in adding the complexity of a _WANT_FOO option.

I plan to revert this commit as soon as I get the information that this kind
of compatibility is no longer required (after EoL of 13.1 and release of 14.0,
if I understand the reasons for the request for this compatibility correctly).



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?c10b51c3-2e86-be62-017b-e185859a7447>