public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Laszlo Ersek" <lersek@redhat.com>
To: devel@edk2.groups.io, leif.lindholm@linaro.org, "Gao,
	Liming" <liming.gao@intel.com>
Cc: Baptiste Gerondeau <baptiste.gerondeau@linaro.org>,
	"ard.biesheuvel@linaro.org" <ard.biesheuvel@linaro.org>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>,
	"Zhang, Shenglei" <shenglei.zhang@intel.com>
Subject: Re: [edk2-devel] [PATCH 0/3] Arm builds on Visual Studio
Date: Thu, 19 Sep 2019 21:24:15 +0200	[thread overview]
Message-ID: <ee6c3912-e8d9-8ab6-018b-7720becd9122@redhat.com> (raw)
In-Reply-To: <20190919094450.GN28454@bivouac.eciton.net>

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
>>> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhang,
>>> Shenglei <shenglei.zhang@intel.com>; Baptiste Gerondeau
>>> <baptiste.gerondeau@linaro.org>
>>> 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

  parent reply	other threads:[~2019-09-19 19:24 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-18 12:25 [PATCH 0/3] Arm builds on Visual Studio Baptiste Gerondeau
2019-09-18 12:25 ` [PATCH 1/3] ArmPkg/MdePkg : Unify INF files format Baptiste Gerondeau
2019-09-19  9:29   ` Ard Biesheuvel
2019-09-18 12:25 ` [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT Baptiste Gerondeau
2019-09-19  9:32   ` Ard Biesheuvel
2019-09-19  9:48     ` Leif Lindholm
2019-09-19 10:01       ` Ard Biesheuvel
2019-09-19 10:09         ` Leif Lindholm
2019-09-19 10:25           ` Ard Biesheuvel
2019-09-19 10:34             ` Baptiste Gerondeau
2019-09-19 10:37               ` Ard Biesheuvel
2019-09-19 10:47                 ` Leif Lindholm
2019-09-19 10:53                   ` Ard Biesheuvel
2019-09-19 11:25                     ` Leif Lindholm
2019-09-19 12:36                       ` Ard Biesheuvel
2019-09-19 14:31                         ` Leif Lindholm
2019-09-19 14:44                           ` Ard Biesheuvel
2019-09-19 11:07                 ` Baptiste Gerondeau
2019-09-19 10:37             ` Leif Lindholm
2019-09-18 12:25 ` [PATCH 3/3] ARM/Assembler: Reuse RVCT assembler for MSFT build Baptiste Gerondeau
2019-09-19  9:38   ` Ard Biesheuvel
2019-09-19  9:52     ` Leif Lindholm
2019-09-19  9:59       ` Ard Biesheuvel
2019-09-18 16:43 ` [PATCH 0/3] Arm builds on Visual Studio Leif Lindholm
2019-09-19  6:19 ` Liming Gao
2019-09-19  9:44   ` Leif Lindholm
2019-09-19 14:53     ` Liming Gao
2019-09-19 19:24     ` Laszlo Ersek [this message]
2019-09-19 19:57       ` [edk2-devel] " Andrew Fish
2019-09-19 20:27       ` Leif Lindholm
2019-09-24  1:28         ` Liming Gao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ee6c3912-e8d9-8ab6-018b-7720becd9122@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox