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

[Andreas Müller]

On Wed, May 3, 2017 at 5:24 PM, Martin Jansa <martin.jansa at gmail.com> wrote:
> 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?
Using spaces for python has technical reasons - tabs for shell does
not. Different indention makes things unnecessary complicated - and
even worse using different style-guides for different layers.

Let's use 4 spaces for all indention - I would love to see it also for
multiline.

My 2ct

Andreas