From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 5FB9D7803CE for ; Mon, 13 Nov 2023 20:08:33 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=FZJnJsRh3fQyA0EV4eu1tmIHcRTtY+b2OooHSl19t9A=; c=relaxed/simple; d=groups.io; h=DKIM-Filter:Message-ID:Date:MIME-Version:User-Agent:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1699906112; v=1; b=olWSupIOc0H7dvLn1hWI+Dd+uIvQL1ul87tVEywV0BQz/w4f0VnoPqhHqriLbYaJzrLO6YfV l652wmn5WQjGyEkaypfrHCIQ0DOcgSj1tEJKhNFL5kVtKiuu5DzOWna4gEoXT8HQyhAwUtT7azA PTzwOG6W4HPTXt/BDI8q5y08= X-Received: by 127.0.0.2 with SMTP id LZ0oYY7687511xfWuv3YakTT; Mon, 13 Nov 2023 12:08:32 -0800 X-Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web10.1389.1699906111367540433 for ; Mon, 13 Nov 2023 12:08:31 -0800 X-Received: from [192.168.4.22] (unknown [47.201.241.95]) by linux.microsoft.com (Postfix) with ESMTPSA id 21DB520B74C1; Mon, 13 Nov 2023 12:08:30 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 21DB520B74C1 Message-ID: <23dd696a-52a1-4c26-bfb6-5b5587325c42@linux.microsoft.com> Date: Mon, 13 Nov 2023 15:08:28 -0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] edk2 uncrustify update (73.0.8)? To: Laszlo Ersek , edk2-devel-groups-io Cc: Michael Kinney , Pedro Falcato , Andrew Fish , Marcin Juszkiewicz , "Leif Lindholm (Quic)" References: <0107c96b-849a-db48-194b-1a4c1f3b0c78@redhat.com> From: "Michael Kubacki" In-Reply-To: <0107c96b-849a-db48-194b-1a4c1f3b0c78@redhat.com> Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,mikuback@linux.microsoft.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: rLixH5IPY0EJTfKe2KACJihAx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=olWSupIO; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=linux.microsoft.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On 11/13/2023 6:58 AM, Laszlo Ersek wrote: > Hi Michael, >=20 > recently I encountered an uncrustify failure on github. >=20 > The reason was that my local uncrustify was *more recent* (73.0.8) than > the one we use in edk2 CI (which is 73.0.3, per the edk2 file > ".pytool/Plugin/UncrustifyCheck/uncrustify_ext_dep.yaml"). >=20 > Updating the version number in the YAML file (i.e., advancing edk2 to > version 73.0.8) seems easy enough, but: >=20 > - Do you think 73.0.8 is mature enough for adoption in edk2? >=20 > This upstream uncrustify release was tagged in April (and I can't see > any more recent commits), so I assume it should be stable. >=20 Yes, it is stable. We've been using each new Uncrustify release against=20 edk2 code in Project Mu during that time. I updated our code to include=20 that change in September 2022 -=20 https://github.com/microsoft/mu_basecore/commit/6932526bee9a2f5f3af7588923b= eae5e5d8fd128. The changes since then have been additional build support for Linux and=20 Windows Arm and macOS. I originally did not bring this to edk2 right away to verify stability=20 over time and reduce thrash if any other changes came in to consolidate=20 overall disruption to edk2. > - Would the version update require a whole-tree re-uncrustification? >=20 Yes. I just did it. It is relatively minor and impacts expected code areas. https://github.com/tianocore/edk2/pull/5043/files I'm happy to send that to the list if desired. > The reason I'm not just ignoring this topic is that 73.0.8 actually > produces *better output* than 73.0.3, at least in the one instance I > encountered. Compare: >=20 >> diff --git a/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDevi= ceSupport.c b/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDevice= Support.c >> index 434cdca84b23..3a6f75988220 100644 >> --- a/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSuppo= rt.c >> +++ b/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSuppo= rt.c >> @@ -43,12 +43,12 @@ STATIC EFI_INCOMPATIBLE_PCI_DEVICE_SUPPORT_PROTOCOL >> STATIC CONST EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR mMmio64Configuration = =3D { >> ACPI_ADDRESS_SPACE_DESCRIPTOR, // Desc >> (UINT16)( // Len >> - sizeof (EFI_ACPI_ADD= RESS_SPACE_DESCRIPTOR) - >> - OFFSET_OF ( >> - EFI_ACPI_ADDRESS_S= PACE_DESCRIPTOR, >> - ResType >> - ) >> - ), >> + sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) - >> + OFFSET_OF ( >> + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR, >> + ResType >> + ) >> + ), >> ACPI_ADDRESS_SPACE_TYPE_MEM, // ResType >> 0, // GenFlag >> 0, // SpecificFlag >> @@ -77,12 +77,12 @@ STATIC CONST EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR mMmi= o64Configuration =3D { >> STATIC CONST EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR mOptionRomConfiguratio= n =3D { >> ACPI_ADDRESS_SPACE_DESCRIPTOR, // Desc >> (UINT16)( // Len >> - sizeof (EFI_ACPI_ADD= RESS_SPACE_DESCRIPTOR) - >> - OFFSET_OF ( >> - EFI_ACPI_ADDRESS_S= PACE_DESCRIPTOR, >> - ResType >> - ) >> - ), >> + sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) - >> + OFFSET_OF ( >> + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR, >> + ResType >> + ) >> + ), >> ACPI_ADDRESS_SPACE_TYPE_MEM, // ResType >> 0, // GenFlag >> 0, // Disable option r= oms SpecificFlag >=20 > Note that 73.0.3 indents the subexpression to the "//" comment on the > previous line, while 73.0.8 ignores the comment -- which I think is > justified here. >=20 > I believe this improvement may come from uncrustify commit 239c4fad745b > ("Prevent endless indentation scenario in struct assignment", > 2022-07-29). I think it's worth having in edk2. >=20 > CC: stewards, Pedro (commit 6ded9f50c3aa), Marcin (traditionally a big > fan of uncrustify :)) >=20 > Thanks > Laszlo -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#111177): https://edk2.groups.io/g/devel/message/111177 Mute This Topic: https://groups.io/mt/102559740/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-