From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Thu, 19 Sep 2019 12:24:18 -0700 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 11D43811AB; Thu, 19 Sep 2019 19:24:18 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-121-45.rdu2.redhat.com [10.10.121.45]) by smtp.corp.redhat.com (Postfix) with ESMTP id 53CF919C5B; Thu, 19 Sep 2019 19:24:16 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 0/3] Arm builds on Visual Studio To: devel@edk2.groups.io, leif.lindholm@linaro.org, "Gao, Liming" Cc: Baptiste Gerondeau , "ard.biesheuvel@linaro.org" , "Kinney, Michael D" , "Zhang, Shenglei" References: <4A89E2EF3DFEDB4C8BFDE51014F606A14E4FE630@SHSMSX104.ccr.corp.intel.com> <20190919094450.GN28454@bivouac.eciton.net> From: "Laszlo Ersek" Message-ID: Date: Thu, 19 Sep 2019 21:24:15 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190919094450.GN28454@bivouac.eciton.net> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Thu, 19 Sep 2019 19:24:18 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 09/19/19 11:44, Leif Lindholm wrote: > Hi Liming, > > On Thu, Sep 19, 2019 at 06:19:42AM +0000, Gao, Liming wrote: >> I add my comments. >> >>> -----Original Message----- >>> From: Baptiste Gerondeau [mailto:baptiste.gerondeau@linaro.org] >>> Sent: Thursday, September 19, 2019 12:05 AM >>> To: devel@edk2.groups.io >>> Cc: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; Kinney, Michael D >>> ; Gao, Liming ; Zhang, >>> Shenglei ; Baptiste Gerondeau >>> >>> Subject: [PATCH 0/3] Arm builds on Visual Studio >>> >>> EDIT: Resending the series since I mistakenly used the wrong email, >>> sorry ! >>> >>> We are currently making an effort to make ARM (and AARCH64 eventually) >>> builds using Microsoft's Visual Studio Compiler (aka MSVC/MSFT). >>> >>> These 3 patches correspond to an effort to make the assembler work with >>> MSFT, which entails : >>> - Feeding MSFT the RVCT .asm files, since they share syntax >>> requirements. >> >> Please separate the patch. Each patch is for each package, can't cross packages. >> If so, the package maintainer can easy review the change. > > I agree with this as a general rule, but for this (hopefully never to > be repeated) operation, it makes sense to me to keep each change in > this set as one patch. > > For the simple reason that the alternative leaves several unusable > commits in sequence in the repository. There is simply no way to > bisect through this change on a per-package basis. > > This is after all a horrible horrible hack that lets us keep using the > .asm files provided for one toolchain family (RVCT) in a different > toolchain family (MSFT), without having to delete and re-add, losing > history in the process. > > Would you be OK with an exception for this extremely unusual > situation? (The question was posed to Liming, but I'm going to follow up here with my own thoughts, after getting CC'd by Liming. Thanks for that BTW.) Let's assume the changes are split up with a fine granularity. Under that assumption, consider two cases: (1) ARM builds on Visual Studio are broken until the last patch is applied, but all other toolchains continue working fine throughout the series. versus (2) ARM builds are broken on Visual Studio *and* on at least one other -- preexistent -- toolchain, until the last patch in the series is applied. Which case reflects reality? If it's case (1), then I prefer the fine-grained patch series structure. Nothing regresses with existent toolchains (so bisection works with them), we just have to advertise the last patch in the series as the one that enables Visual Studio. If it's case (2), then I agree with the larger (multi-package) patch, as a rare exception. (We can also put it like this: if it's *possible* to write this series such that it enables (1), then we should strive for that.) Thanks, Laszlo