Re: [Patch 0/4] Normalize line endings to CRLF in ARM packages

[Leif Lindholm <leif.lindholm@linaro.org>]

On Thu, Apr 04, 2019 at 05:00:36PM +0000, Kinney, Michael D wrote:
> Leif,
> 
> There were a few files when the license change patches
> were created that failed PatchCheck.py.
> 
> I am trying to resolve those before the license change
> so the license change patches do not have any PatchCheck.py
> failures.
> 
> If I do these as part of the license change, then the
> change will look larger than just the license change in
> these files.
> 
> I prefer to have a separate patch to resolve the line 
> endings.

Oh, it needs to be separate patches. Apologies if I was unclear on
that. It should not even form part of the same set.

What I meant was that since a substantial bit of churn is about to hit
the repository, we might as well go about doing the line ending
conversion shortly before or shortly after the license change.

If we do it shortly before, we don't need *this* set. If we do it
shortly after, we do.

> There may be other files in edk2 repo that have mixed
> line endings in lines not associated with the license
> change itself.  We can consider cleaning those up in
> a separate series.
> 
> The other option is to abandon these patches to normalize
> line endings and allow the license change patches to not
> pass PatchCheck.py.
> 
> Which do you prefer?

I think it would be completely valid for the license change patches
to not pass PatchCheck.py.

But why are they not passing? Is it complaining about context lines?

Regards,

Leif

> Thanks,
> 
> Mike
> 
> 
> 
> > -----Original Message-----
> > From: Laszlo Ersek [mailto:lersek@redhat.com]
> > Sent: Thursday, April 4, 2019 3:39 AM
> > To: Leif Lindholm <leif.lindholm@linaro.org>; Kinney,
> > Michael D <michael.d.kinney@intel.com>
> > Cc: devel@edk2.groups.io; Ard Biesheuvel
> > <ard.biesheuvel@linaro.org>; Andrew Fish
> > <afish@apple.com>
> > Subject: Re: [Patch 0/4] Normalize line endings to CRLF
> > in ARM packages
> > 
> > On 04/04/19 05:54, Leif Lindholm wrote:
> > > Hi Mike,
> > >
> > > This looks fine. But I did have one thought:
> > > If we're just about to touch pretty much every file in
> > the tree anyway
> > > - can we consider doing the full conversion to native
> > line endings at
> > > the same time?
> > >
> > > (This doesn't help the 'git blame' problem, it just
> > reminded me.)
> > 
> > The git-blame issue that such a conversion introduces is
> > not terrible.
> > Let's say you run
> > 
> >   git blame -- ArmPkg/Library/GccLto/liblto-arm.s
> > 
> > and it gives you a commit for a specific line you care
> > about. Let's call
> > that commit C1.
> > 
> > Then you do
> > 
> >   git show --color C1 | less
> > 
> > and expect to see the commit message & patch that
> > introduced the *logic*
> > on that line. But, you only see a "useless" CRLF
> > conversion, in that
> > commit. So what do you do? Simple, repeat the process as
> > follows:
> > 
> >   git blame C1^ -- ArmPkg/Library/GccLto/liblto-arm.s
> > 
> > This will assign blame on the lines of the file as the
> > file was right
> > before C1 changed the line terminators.
> > 
> > 
> > The problem that I think *is* worse is that, jumping
> > about in the git
> > history (with git checkout / git bisect), around the
> > large conversion
> > commit, will expose the user in a brief time to files
> > that are both LF
> > and CRLF encoded, internally. Therefore, whatever their
> > local
> > auto-conversion-on-checkout settings are, some commits
> > in such a work
> > session will not match those settings.
> > 
> > For example, assuming you are on Windows, and set up
> > LF-to-CRLF-on-checkout (as you should), then git-
> > checkout a commit from
> > before the "LF only" conversion commit, you might see
> > double CRs. I
> > guess. I never tested it.
> > 
> > Thanks
> > Laszlo
> > 
> > 
> > 
> > >
> > > /
> > >     Leif
> > >
> > > On Wed, Apr 03, 2019 at 03:00:10PM -0700, Kinney,
> > Michael D wrote:
> > >> https://bugzilla.tianocore.org/show_bug.cgi?id=1659
> > >>
> > >> Normalize line endings to use CRLF to pass
> > PatchCheck.py
> > >>
> > >> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > >> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > >> Cc: Laszlo Ersek <lersek@redhat.com>
> > >> Contributed-under: TianoCore Contribution Agreement
> > 1.1
> > >> Signed-off-by: Michael D Kinney
> > <michael.d.kinney@intel.com>
> > >>
> > >> Kinney, Michael D (4):
> > >>   ArmPkg/ArmScmiDxe: Remove non-ASCII character in
> > comment
> > >>   ArmPkg: Normalize line endings to CRLF
> > >>   ArmVirtPkg: Normalize line endings to CRLF
> > >>   ArmPlatformPkg: Normalize line endings to CRLF
> > >>
> > >>  .../ArmScmiPerformanceProtocolPrivate.h       |   2
> > +-
> > >>  ArmPkg/Library/ArmSmcLibNull/ArmSmcLibNull.c  |  44
> > +-
> > >>  .../ArmSmcPsciResetSystemLib/AArch64/Reset.S  |  60
> > +-
> > >>  .../AArch64/Reset.asm                         |  70
> > +-
> > >>  .../ArmSmcPsciResetSystemLib/Arm/Reset.S      |  58
> > +-
> > >>  .../ArmSmcPsciResetSystemLib/Arm/Reset.asm    |  68
> > +-
> > >>  ArmPkg/Library/CompilerIntrinsicsLib/memset.c | 124
> > ++--
> > >>  ArmPkg/Library/GccLto/liblto-aarch64.s        |  54
> > +-
> > >>  ArmPkg/Library/GccLto/liblto-arm.s            | 122
> > ++--
> > >>  ArmPlatformPkg/Scripts/Ds5/profile.py         | 668
> > +++++++++---------
> > >>  ArmVirtPkg/Include/Platform/Hidden.h          |  56
> > +-
> > >>  11 files changed, 663 insertions(+), 663 deletions(-
> > )
> > >>
> > >> --
> > >> 2.21.0.windows.1
> > >>
>