> On Sep 19, 2019, at 12:24 PM, Laszlo Ersek wrote: > > 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.) > Laszlo, I agree with your logic around (1) and (2). I'd also point out we can do the review using (1) and squash into a single commit if we need to take path (2). Thanks, Andrew Fish > Thanks, > Laszlo > >