Re: [PATCH 1/7] xz: add fall-through comments to a switch statement

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.12.2021 17:45, Ian Jackson wrote:
> Julien Grall writes ("Re: [PATCH 1/7] xz: add fall-through comments to a 
> switch statement"):
>> On 06/12/2021 16:12, Jan Beulich wrote:
>>> Hmm, I did address both your and Ian's concerns in v2, admittedly by only
>>> going as far as minimally necessary. I therefore wouldn't call this an
>>> "open objection".
>>
>> I believe my objection is still open. I still have have no way to verify 
>> what you did is correct.
> 
> I can't believe this is still outstanding.

Same here, hardly surprising I suppose.

>  I think I understand
> Julien's position, and I agree with what I have understood.
> 
> In particular, I think I understand why Julien feels it necessary to
> make an issue of this.  The Signed-off-by lines are there to help
> provide assurance that we aren't making legal mistakes.  They need to
> be verifiable by a reviewer.  So that means that when a patch's own
> declaration of its origin is "this patch came from Linux commit XYZ"
> then all the S-o-b in that Linux git commit should be retained.
> 
> If the patch came from somewhere else, eg a mailing list post, I think
> it would be OK to say something like "this patch came from lmkl, [url
> to posting], and has since been committed to Linux as [commitid]~".
> In that case the S-o-b should match the mailing list posting, but the
> Xen patch being posted must then be identical to the mailing list
> posting.
> 
> IOW it should be a deterministic process to start with the patch's
> declaration of where it came from (or which sources it came from), and
> verify that 1. the patch really did come from there and 2. all of the
> approriate tags, especially S-o-b, are present.
> 
> By far the easiest way to achieve this is to take the patch from Linux
> git using (eg) git-cherry-pick.  git will automatically DTRT. [1]

It may be the easiest way, and I do understand Julien's replies in this
regard. This doesn't, however, mean that I agree the "easy" aspect
weighs higher than my view on stripping parts which aren't meaningful
in our tree (or, to be precise, which I think aren't meaningful; I will
leave it entirely open that I may be wrong with this).

As to git-cherry-pick: Just like I would have expected, even after
adding Linus'es tree as a remote this doesn't really work, due to the
different tree layouts:

$ git remote add --no-tags linus 
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

$ git cherry-pick 83d3c4f22a
warning: inexact rename detection was skipped due to too many files.
warning: you may want to set your merge.renamelimit variable to at least 73569 
and retry the command.
error: could not apply 83d3c4f22a36... lib/xz: Avoid overlapping memcpy() with 
invalid input with in-place decompression
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'

$ git status
On branch smoke
Your branch is up to date with 'origin/smoke'.

You are currently cherry-picking commit 83d3c4f22a36.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)

        deleted by us:   lib/decompress_unxz.c
        deleted by us:   lib/xz/xz_dec_lzma2.c

no changes added to commit (use "git add" and/or "git commit -a")

To adjust for our tree layout, manual intervention is necessary anyway
(unless there's a way to get "inexact rename detection" to actually
recognize the differences). What I take as a basis (git-format-patch
output or anything else) should be entirely up to me, I would say.

> I don't have as strong an opinion about other tags, eg ones indicating
> approval in Linux.  However, I think the overwhelming majority of
> people would think it conventional to transfer all of the tags from
> the original commit even if they are irrelevant in the new context.

Well, clearly I'm not part of this overwhelming majority then: I continue
to think that irrelevant things would better be omitted for clarity.

> I don't understand Jan's position.
> 
> Jan, why are you fighting so hard to delete these tags ?

Counter question: Why are you and Julien fighting so hard for the
retaining of what I consider inapplicable information? But to answer
your question: Just like I consider missing information a problem, I do
also consider meaningless data a problem. Plus of course there's now
the psychological effect resulting from already having invested far
more time here than I think any one of us should have invested: I now
absolutely want to understand whether I have been doing things wrong for
years. As said, me doing what I have done here hasn't been a problem
before.

It's still not clear to me whether I'm doing anything wrong in the first
place - my question as to why these tags are relevant in our trees has
remained unanswered. I've actually taken the time to dig out "Developer's
Certificate of Origin 1.1" - from all I can tell my submission matches (b)
(and that's imo true even for v1, i.e. before re-adding some of the tags
to match the submissions according to mailing list archives, as far as
entries are available).

In this context I'd also like to point out that unlike Linux we don't
normally make use of (c).

> What practical harm does its presence do ?

I did answer this one earlier on as well as above: I consider their
presence inapplicable. There's no severe "harm", but there's also no
reason I know to keep them. Unless I'm told of a reason, I view it as
the submitter's choice to keep or strip such. In fact, if I saw an
import which had such seemingly stray tags, I probably wouldn't insist
on stripping them, but I might question their presence / applicability.

> [1] Jan, I know that you don't use git very much.  I think this is a
> great shame.  I find it perplexing to see how anyone can work without
> it.  The git command line UI is indeed terrible, but by now almost
> everyone has either bitten the bullet of learning it, or adopted one
> of the overlay UI packages that now exist.  (Personally I did learn
> the cli but am starting to forget git cli nonsence since now I do
> almost everything with magit inside emacs.)
> 
> I think the time has long passed when it is reasonable for a key Xen
> developer to ask others to do additional work, or deal with anomalies,
> in order to accomodate an unwillingness to use git.  Obviously we all
> have our own workflows, but git has heavily influenced our shared
> norms (and data formats).  If someone chooses not to use git, they
> must at least be able to pretend.

I appreciate (yet don't share) your view, but primarily I'm afraid I
don't view this rant as relevant in this context. Even if I had found
a way to do the import entirely via git commands, I likely would have
edited the description to strip tags alongside adding my S-o-b (the
latter I understand I could have git do for me). Since git-cherry-pick
didn't work for me here, I also can't tell whether the usual
"(cherry-picked from ...)" that git can be told to add would actually
have identified the different tree, or whether I would have had to add
that to make the referenced hash meaningful.

Jan