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.

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?

Thanks,

Mike



> -----Original Message-----
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Thursday, April 4, 2019 3:39 AM
> To: Leif Lindholm <leif.lindh...@linaro.org>; Kinney,
> Michael D <michael.d.kin...@intel.com>
> Cc: devel@edk2.groups.io; Ard Biesheuvel
> <ard.biesheu...@linaro.org>; Andrew Fish
> <af...@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.lindh...@linaro.org>
> >> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
> >> Cc: Laszlo Ersek <ler...@redhat.com>
> >> Contributed-under: TianoCore Contribution Agreement
> 1.1
> >> Signed-off-by: Michael D Kinney
> <michael.d.kin...@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
> >>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#75): https://edk2.groups.io/g/devel/message/75
Mute This Topic: https://groups.io/mt/30886235/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to