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

Wed May 3 15:24:27 UTC 2017

Hello,

many people are confused by the Styleguide used for OE metadata layers,

You can read the current state on:
https://www.openembedded.org/wiki/Styleguide
* Use spaces for indentation as developers tends to use different amount 
  of spaces per one tab.
* Python functions must be four space indented - no tabs.
* Shell functions in OE-Core usually use tabs for indentation, but other
  layers usually use consistent indentation with 4 spaces (in shell
  functions, python functions and for indentation of multi-line variables)

There are 2 major confusions
1) Why we're using different indentation for shell and python code in oe-core
2) Why we're not using this different indentation in many other layers
   most notably in meta-oe layers

This all started long time ago, basically started by this change:
http://lists.openembedded.org/pipermail/bitbake-devel/2012-July/002454.html
this change unified all the python code in bitbake and other layers to
use 4 spaces instead of previous mix of tabs and spaces.

There were good technical reasons for doing this, but because of how
python uses indentation to define blocks of code it was also dangerous.

I like consistency, so a bit later I've proposed to use the same 4-space
indentation also for shell code, the main reasons as listed in the
commit:
* This change is only aesthetic (unlike indentation in Python
  tasks).
* Some recipes were using tabs.
* Some were using 8 spaces.
* Some were using mix or different number of spaces.
* Make them consistently use 4 spaces everywhere.
* Yocto styleguide advises to use tabs (but the only reason to keep
  tabs is the need to update a lot of recipes). Lately this advice
  was also merged into the styleguide on the OE wiki.
* Using 4 spaces in both types of tasks is better because it's less
  error prone when someone is not sure if e.g.
  do_generate_toolchain_file() is Python or shell task and also allows
  to highlight every tab used in .bb, .inc, .bbappend, .bbclass as
  potentially bad (shouldn't be used for indenting of multiline
  variable assignments and cannot be used for Python tasks).

It was proposed in the thread about that python indentation change:
http://lists.openembedded.org/pipermail/bitbake-devel/2012-July/002461.html
and a bit later I've sent a patch for meta-oe layers:
http://lists.openembedded.org/pipermail/openembedded-devel/2013-April/090147.html
which was approved and merged by meta-oe maintainer at that time Koen
(also memeber of TSC at that time):
http://lists.openembedded.org/pipermail/openembedded-devel/2013-April/090162.html
and Joe maintainer of meta-networking (also inside meta-oe respository):
http://lists.openembedded.org/pipermail/openembedded-devel/2013-April/090181.html
another TSC member also approved it after the fact:
http://lists.openembedded.org/pipermail/openembedded-devel/2013-April/090184.html
and another TSC member was at least aware of this change:
http://lists.openembedded.org/pipermail/openembedded-devel/2013-April/090184.html

I'm writing all this because some people are still very upset about this,
because this topic was also discussed on TSC meeting:
http://lists.openembedded.org/pipermail/tsc/2012-August/000352.html
and mentioned in few follow up TSC meetings, e.g.
http://lists.openembedded.org/pipermail/openembedded-core/2012-October/069817.html
http://lists.openembedded.org/pipermail/openembedded-core/2013-April/077605.html
as lingering issue with the result:
-> Reluctant conclusion: tabs for shell, 4 spaces for python.
=> Need to ensure well documented & then remove

Please read the minutes yourself I don't want to influence your interpretation
of it.

Since then it was discussed few more times, e.g.
http://lists.openembedded.org/pipermail/openembedded-core/2013-April/077609.html
http://lists.openembedded.org/pipermail/openembedded-core/2013-April/077610.html
http://lists.openembedded.org/pipermail/openembedded-core/2013-April/077617.html
https://www.yoctoproject.org/irc/%23yocto.2013-10-31.log.html

Then I gave up and stopped trying to open this topic again, until today when
I mentioned it on OE meeting and RP suggested to discuss it on
openembedded-architecture ML.

There are 2 possible solutions
1) Revert the change in meta-oe (by creating completely new change which will
   change all shell code to use tabs instead of 4 spaces).
2) Re-evaluate TSC decision and if possible allow updating shell code in
   oe-core to use spaces instead of tabs.

I know which solution I prefer and which would be preferred by RP.
But it would to hear opinion of other Yocto/OE users and developers.

Why I still think it's worth using 4 spaces and how to mitigate the
bad side effects of such big change in metadata?

People shouldn't be confused by something as stupid as preferred white
space indentation - using the same 4 spaces for everything in every layer
is obviously the simplest styleguide everybody can follow.
I think everybody agreed on that, the main differences are if it's worth
the effort.

Merging the whitespace change just before the release will limit how many changes are
backported between branches with different indentation.
e.g. doing it in pyro and master branch now, will not complicate backports from
master to pyro and once pyro is released there will be much fewer changes backported
from master (or pyro) to morty (or even older) branch.

It's easy to resolve the conflicts when backporting the change with wrong indentation
and in worst case you'll backport some shell with 4 spaces indentation to e.g. morty
branch where other recipes will still use tabs - no problem, it will still parse and
build correctly, not like the python changes in 2012 where you had to be careful
or the code was behaving differently when you backported few lines of python with
wrong indentation. Or newer example:
https://patchwork.openembedded.org/patch/134327/
This change alone caused me couple parsing issues already, it's enough to backport
newer tzdata recipe to your morty based builds (because everybody needs latest tzdata
in products, right?) and BAM your bitbake won't like that recipe, because it doesn't
have 2nd parameter in d.getVar() call. It's still easy to resolve you can use this
patchwork link to find out that you should return ", True" as in most cases.

Reverting the change in meta-oe (and it's not only meta-oe, all layers I contribute
to are using 4 spaces everywhere as well) would cause the same kind of backporting and
patch handling issues as using the consistent indentation in oe-core.

Now when you've read the minutes :) I can say that my interpretation of
TSC meeting was 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. From my experience there was only
a pain to prepare the patch and then only a bit of pain in 1-2 weeks after
it was merged.

The pain to create such patch for oe-core should be lower than for other
layers, because oe-core is using tabs a bit more consistently, almost
every tab can be globally replaced with 4 spaces and only the tabs used
(already incorrectly) to indent multiline variables should be replaced
with 8 spaces instead of 4. Also "git diff -w" does great job to make
sure that nothing gets "broken". And best thing is we have a volunteer to
prepare such patch if we get the agreement that it will be merged when
it's ready.

This is much longer than my 2c, but I was trying to show whole picture to get most
accurate opinions from others.

Opinions?
-- 
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/20170503/3ac724c0/attachment-0001.sig>