[Openembedded-architecture] Consistent white-space indentation in metadata layers

Thu May 4 11:31:26 UTC 2017

On Thu, May 04, 2017 at 12:27:43AM +0100, Richard Purdie wrote:
> On Wed, 2017-05-03 at 21:50 +0100, Phil Blundell wrote:
> > I also agree that making this change to oe-core doesn't seem like it
> > would be a dramatic upheaval.  There seem to be 302 .bb files and 75
> > classes containing tabs in their shell fragments (which is a bit
> > under half the total of each) and I can't see any reason why they
> > couldn't be safely replaced in an automated way.  And it probably is
> > true that oe-core has undergone more disruptive changes than this in
> > the last five years without any severe consequences.  No doubt any
> > reformatting of this kind would cause some short-term hassle for
> > people backporting patches to older branches or maintaining their own
> > patches on top of upstream oe-core, but both these activities are
> > already fairly miserable occupations and I doubt this extra grief
> > would really move the needle in terms of level of overall suffering.
> >  And even if it did, they'd get over it.
> 
> Speaking as someone who has had to deal with this before, and handle
> backports over various changes, I do believe the whitespace changes
> will be more of a pain than normal due to the size of the codeblock
> that changes, making it harder to isolate the 'real' differences.
> 
> I have done this for the python whitespace changes, things like the
> getVar improvements and with others and I think the shell changes would
> be by far the most disruptive.
> 
> It certainly can be done (as can most things), I've not felt a
> particular need to put the stable maintainers through that pain though.
> They do a pretty thankless task already without adding to the issues.

There is 179 commits in morty branch since morty was released
(2a59d0fa7bda78927435603e3049ce373cf6a198..123962018251dfb1d6ca5aa5c0d02534007de3ab)
git diff --stat shows:
 181 files changed, 3869 insertions(+), 1104 deletions(-)

git diff --stat without modified .patch files:
 115 files changed, 559 insertions(+), 394 deletions(-)

With only 184 tabs in unified diff (not counting modified .patch files)
or 87 tabs in modified lines (with git diff -U0)

Lets see how many more changes will be merged to morty after pyro is released.
Less then 1/5 of what was merged so far?
e.g. with krogoth there was 338 changes in total since krogoth release and only
49 since morty release
OE @ ~/openembedded-core $ git log --oneline 9838f8d077d16e52ad592879d65a9e8350b93075..origin/krogoth | wc -l
338
OE @ ~/openembedded-core $ git log --oneline 0900fed3fb6eec62e9e25f6d03af934f9776d105..origin/krogoth | wc -l
49

Yes, it's not my decision to make, but there is no need to exaggerate how
many conflicts will this cause for backports.

> > On the other hand, I can't help feeling that anybody who finds it all
> > that difficult to cope with the difference in whitespace conventions
> > between oe-core and meta-oe is possibly in the wrong job.  These two
> > trees are, after all, distinct codebases, maintained by different
> > individuals, and the fact that they happen to use the same tools
> > should not necessarily oblige them to use the same indentation
> > style. 
> > 
> > But on the third hand, given that the arguments for preferring spaces
> > over tabs are fairly weak, given also that meta-oe has fewer users
> > than oe-core, and given finally that meta-oe was the project which
> > originally went off-piste by disregarding the TSC recommendation on
> > formatting, overall I think my view is that oe-core should probably
> > stay as it is and meta-oe should either adapt itself to match or just
> > resign itself to being different.
> 
> One of the reasons I've not pushed this for as long is that I still
> worry about the precedent that meta-oe set in doing this. A TSC
> decision was ignored and no action was taken. If we now decided to
> change/overrule/replace the TSC decision, it does continue a dangerous
> precedent.

And what about the interpretation of that TSC decision I had?

That it's not worth the pain it would cause to the people
handling the patches, but not strictly preventing anyone from doing it
in his own layers and self-inflict the pain.

I've read the minutes few times and I still don't see anything about
forbidding any layer maintainer to do it if he doesn't mind the extra
work (or new layers which are starting from scratch so don't have this
backporting issue). 2 from 5 TSC members approving the change in meta-oe
might indicate that they interpreted it the same or similarly and there
was no dangerous precedent.

The whole whitespace discussion from
http://lists.openembedded.org/pipermail/tsc/2012-August/000352.html
follows:

(9:08:27 AM) RP: Martin has asked we consider shell whitespace
(9:08:42 AM) RP: Personally, I'm not keen to rock the boat on the
shell whitespace issues
(9:08:58 AM) fray: ya.. I'm definitely more hesitent there..
(9:09:02 AM) RP: The tabs vs spaces in python were important as python
is whitespace delimited
(9:09:07 AM) RP: shell is a different question
(9:09:17 AM) fray: mind you, as a janitorial task, the whitespace for
shell functions in classes and such should still be "fixed"
(9:09:27 AM) fray: but I see that purely as janitorial vs functional defect
(9:09:38 AM) RP: I think the downsides of breaking patch application
and patch backporting to 1.2 outweigh any nice feelings about using
spaces
(9:10:00 AM) RP: So I'm not in favour of changing the current
situation and think we should maintain the current policy
(9:10:21 AM) fray: ya.. cleanup whitespace as (shell) functionals are revised..
(9:10:37 AM) fray: many can be safely done.. but it really is a
cleanup as appropriate task..
(9:10:44 AM) RP: fray: well, I don't even want to do that, just
continue with the current policy, tabs n shell functions
(9:10:50 AM) RP: and change anything that doesn't match over time
(9:10:54 AM) bluelightning: RP: are there any options for patch
application that ignore whitespace differences?
(9:11:09 AM) fray: I'm more referring to the tab, 3 space, 4 space,
all intermingled..
(9:11:18 AM) RP: bluelightning: it can be done and some commands
accept the options but git am doesn't like it for example
(9:11:27 AM) fray: makes it really hard to work on some of the files..
that's the cleanup (as the functions are worked on) that I'm
suggestign
(9:11:28 AM) RP: It would create a world of pain for people handling the patches
(9:11:32 AM) bluelightning: hmm ok
(9:11:47 AM) fray: but it's purely cosmetic vs functional
(9:12:28 AM) bluelightning: seems to me we have to look at this from a
cost/benefit perspective
(9:13:26 AM) RP: agreed, and I don't see a massive benefit
(9:13:36 AM) bluelightning: benefits here are only consistency with
python functions; costs are making the change and ongoing significant
patch backporting hassle for a number of people
(9:14:04 AM) RP: and I do feel strongly about this as one of the
people who would be significantly impacted by this
(9:14:20 AM) RP: its also significantly more of the codebase than the
python changes affected
(9:14:36 AM) RP: python functions were mainly in a few core classes
(9:14:48 AM) khem: so tab for shell and 4 spaces for python ?
(9:14:53 AM) RP: khem: yes
(9:15:01 AM) RP: at least that is my vote
(9:15:17 AM) khem: lets document it in bold letters and may be modify
th syntax checker
(9:15:34 AM) RP: I understand why people would like to do differently
but I don't think the benefits worth it
(9:15:47 AM) fray: no objections from me..
(9:15:52 AM) RP: khem: agreed, we should publicise that more
(9:15:55 AM) khem: RP: its a hard thing
(9:16:05 AM) khem: to use two styles in same metadata
(9:16:20 AM) bluelightning: I'm agreed, it's not worth changing atm
(9:16:27 AM) RP: ok, I think we have a decision there

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: Digital signature
URL: <http://lists.openembedded.org/pipermail/openembedded-architecture/attachments/20170504/699545ab/attachment-0002.sig>