public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Michael D Kinney" <michael.d.kinney@intel.com>
To: Laszlo Ersek <lersek@redhat.com>,
	Leif Lindholm <leif.lindholm@linaro.org>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: "devel@edk2.groups.io" <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
Date: Thu, 4 Apr 2019 17:00:36 +0000	[thread overview]
Message-ID: <E92EE9817A31E24EB0585FDF735412F5B9C79280@ORSMSX112.amr.corp.intel.com> (raw)
In-Reply-To: <983adf0b-4a30-4a48-b015-502157c712f3@redhat.com>

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: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
> >>


  reply	other threads:[~2019-04-04 17:00 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-03 22:00 [Patch 0/4] Normalize line endings to CRLF in ARM packages Michael D Kinney
2019-04-03 22:00 ` [Patch 1/4] ArmPkg/ArmScmiDxe: Remove non-ASCII character in comment Michael D Kinney
2019-04-03 22:00 ` [Patch 2/4] ArmPkg: Normalize line endings to CRLF Michael D Kinney
2019-04-03 22:00 ` [Patch 3/4] ArmVirtPkg: " Michael D Kinney
2019-04-04 12:52   ` [edk2-devel] " Laszlo Ersek
2019-04-04 16:55     ` Michael D Kinney
2019-04-03 22:00 ` [Patch 4/4] ArmPlatformPkg: " Michael D Kinney
2019-04-04  3:54 ` [Patch 0/4] Normalize line endings to CRLF in ARM packages Leif Lindholm
2019-04-04 10:39   ` Laszlo Ersek
2019-04-04 17:00     ` Michael D Kinney [this message]
2019-04-04 17:17       ` [edk2-devel] " Kevin@Insyde
2019-04-05  3:24       ` Leif Lindholm
2019-04-05 18:11         ` Michael D Kinney

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=E92EE9817A31E24EB0585FDF735412F5B9C79280@ORSMSX112.amr.corp.intel.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