public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: "Jin, Eric" <eric.jin@intel.com>,
	Supreeth Venkatesh <supreeth.venkatesh@arm.com>,
	Leif Lindholm <leif.lindholm@linaro.org>,
	Andrew Fish <afish@apple.com>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Subject: Re: Line endings: Was "Re: [edk2-test][Patch] uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test"
Date: Mon, 17 Dec 2018 10:34:40 +0100	[thread overview]
Message-ID: <7af16613-cc1b-e658-8a55-4f299f88c5d0@redhat.com> (raw)
In-Reply-To: <DA72DC7456565B47808A57108259571F63725320@SHSMSX103.ccr.corp.intel.com>

On 12/15/18 04:59, Jin, Eric wrote:
> Currently, the git am fail with " --ignore-space-change" switch on/off. It is better if we can solve it with "core.whitespace", "am.keepcr" and "core.autocrlf" in the meantime.
> 
> Should all existing patches hold until the conversion done or the conversion is tried on the branch until it is validated? 

If you just want to apply a patch from the mailing list, while sticking
with the current line endings (=CRLF), then please use the git settings
described e.g. at:

https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-05


Note that applying an edk2[-*] patch from the mailing list with git-am
might fail for some reasons even if your git settings are fine (as
described above).

(1) One is that the submitter might have messed up the patch, and added
LF-terminated lines to a CRLF context.

(2) Another is that, for a good while, "git-am" would reject
"/dev/null\r\n" header lines -- /dev/null lines are used when a patch
adds or removes a file --, and only accept "/dev/null\n" ones. This is a
problem because "am.keepcr" is required for edk2 patches (for now), and
that results in \r *not* being stripped from "/dev/null\r\n" either.

In other words, without am.keepcr, the patch context wouldn't match, and
with am.keepcr, git would reject /dev/null lines (i.e., patches that
added or removed files).


In order to remedy (2), I had submitted a trivial (one-liner) patch for
the git project in the year 2014:

https://public-inbox.org/git/1411434583-27692-1-git-send-email-lersek@redhat.com/

The patch was rejected. Thus I've had to carry it in my own personal git
build, until quite recently, in order to more easily work with edk2 patches.

Thankfully, the issue was finally fixed in the following two git.git
commits:

* 8bc172e5f298 ("apply: use starts_with() in gitdiff_verify_name()",
2017-07-01)
* e454ad4becff ("apply: handle Subversion diffs with /dev/null
gracefully", 2018-02-15)

(In terms of git releases, this means v2.17.0.)

... If you asked me why my patch wasn't originally accepted, but was
accepted from another contributor, approx. 3.5+ years later (such that
the new commit message would even reference "git-for-windows", i.e. it'd
target a use case that was *very similar* to mine), then my answer would
be: because that's how open source works; your success depends on what
mood you catch a maintainer in.

--------

Anyway, regarding the conversion, for the long term:

- All text files should first be converted to LF only. (With a series of
commits.)

- For people on Windows, they should set "core.autocrlf" in their local
git configs.

- "am.keepcr" should be cleared by everyone, in their respective git
configs.

- "core.whitespace" might need more tweaking, but it's less important.

Thanks
Laszlo


      reply	other threads:[~2018-12-17  9:34 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-12  3:32 [edk2-test][Patch] uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test Eric Jin
2018-12-12 15:06 ` Leif Lindholm
2018-12-12 21:07 ` Supreeth Venkatesh
2018-12-14  7:12   ` Jin, Eric
2018-12-14 10:59     ` Line endings: Was "Re: [edk2-test][Patch] uefi-sct/SctPkg:Correct macro name style in HwErrRecVariable Test" Leif Lindholm
2018-12-14 13:24       ` Laszlo Ersek
2018-12-14 17:12         ` Supreeth Venkatesh
2018-12-14 19:57           ` Laszlo Ersek
2018-12-14 23:54             ` Supreeth Venkatesh
2018-12-15  3:59               ` Jin, Eric
2018-12-17  9:34                 ` Laszlo Ersek [this message]

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=7af16613-cc1b-e658-8a55-4f299f88c5d0@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