Yocto style guide change proposal

Yocto style guide change proposal

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 2385 bytes --]

in "PLEASE READ: Major change landing shortly (python whitespace)"

RP said:
It's become clear we have a horrible mixture of whitespace (tabs and
space) in python functions.

That was resolved in following patches for bitbake, oe-core and other
layers.

Now we have horrible mixture of whitespaces (tabs and space) only in 
recipe files, because yocto style guide recommends tabs in shell
functions. So if recipe has e.g. do_install_append as well as
populate_packages_prepend (not so uncommon combination as tabs fixing
patches show), then according to yocto style guide it should look like
this:

do_install_append() {
	foo
}
python populate_packages_prepend () {
    libdir = bb.data.expand('${libdir}', d)
    do_split_packages(d, libdir, '^lib(.*)\.so\.*', 'lib%s', 'ORC %s library', extra_depends='', allow_links=True)
}

especially with default tab width 8 spaces it's ugly and because it
is inconsistent, many devs used spaces in shell functions too. Now when
someone accidentaly use tab also in python function it will show warning
or fail to parse. Some devs are using mix of tabs and spaces even on the
same line (e.g. to indent SRC_URI multiline entries).

Maybe this inconsistence in recipe files is because yocto style guide is
harder to find then openembedded style guide..

First google hit for "openembedded style guide":
http://www.openembedded.org/wiki/Styleguide

says about tabs only this:
- Use spaces for indentation as developers tends to use different amount
  of spaces per one tab.

So only in yocto wiki there are 2 more bullets after that
https://wiki.yoctoproject.org/wiki/Recipe_%26_Patch_Style_Guide
- Use spaces for indentation as developers tends to use different amount
  of spaces per one tab.
- Shell functions should use tabs
- Python functions should use spaces (4 spaces per indent).

And yocto style guide is not in first 5 pages of result when searching
for "openembedded style guide".

If we change the style guild to use 4 spaces per indent also for shell
functions then we can easily update bitbake/contrib/vim to highlight
tabs, so that every tab would be easily spotted in recipe/bbclass.

FWIW: I've prepared patch for whole meta-smartphone to unify that and
I've sent RFC for few bbclasses in meta-oe too.

Cheers,

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 205 bytes --]

Re: Yocto style guide change proposal

[Richard Purdie]

On Fri, 2012-07-20 at 11:32 +0200, Martin Jansa wrote:
> Now we have horrible mixture of whitespaces (tabs and space) only in 
> recipe files, because yocto style guide recommends tabs in shell
> functions. So if recipe has e.g. do_install_append as well as
> populate_packages_prepend (not so uncommon combination as tabs fixing
> patches show), then according to yocto style guide it should look like
> this:
> 
> do_install_append() {
> 	foo
> }
> python populate_packages_prepend () {
>     libdir = bb.data.expand('${libdir}', d)
>     do_split_packages(d, libdir, '^lib(.*)\.so\.*', 'lib%s', 'ORC %s library', extra_depends='', allow_links=True)
> }
> 
> especially with default tab width 8 spaces it's ugly and because it
> is inconsistent, many devs used spaces in shell functions too. Now when
> someone accidentaly use tab also in python function it will show warning
> or fail to parse. Some devs are using mix of tabs and spaces even on the
> same line (e.g. to indent SRC_URI multiline entries).

We've said tabs for shell functions for *years*. I'm sure if I were to
look at the mailing list archives, that would be clear.

In summary, I agree we need to make the style guides consistent and have
one version of them. I disagree with spaces for everything though,m
particularly as we have said to use tabs for as long and many of the
recipes do this (certainly more than use spaces).

Cheers,

Richard

Re: Yocto style guide change proposal

[Chris Larson]

On Fri, Jul 20, 2012 at 6:56 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Fri, 2012-07-20 at 11:32 +0200, Martin Jansa wrote:
>> Now we have horrible mixture of whitespaces (tabs and space) only in
>> recipe files, because yocto style guide recommends tabs in shell
>> functions. So if recipe has e.g. do_install_append as well as
>> populate_packages_prepend (not so uncommon combination as tabs fixing
>> patches show), then according to yocto style guide it should look like
>> this:
>>
>> do_install_append() {
>>       foo
>> }
>> python populate_packages_prepend () {
>>     libdir = bb.data.expand('${libdir}', d)
>>     do_split_packages(d, libdir, '^lib(.*)\.so\.*', 'lib%s', 'ORC %s library', extra_depends='', allow_links=True)
>> }
>>
>> especially with default tab width 8 spaces it's ugly and because it
>> is inconsistent, many devs used spaces in shell functions too. Now when
>> someone accidentaly use tab also in python function it will show warning
>> or fail to parse. Some devs are using mix of tabs and spaces even on the
>> same line (e.g. to indent SRC_URI multiline entries).
>
> We've said tabs for shell functions for *years*. I'm sure if I were to
> look at the mailing list archives, that would be clear.
>
> In summary, I agree we need to make the style guides consistent and have
> one version of them. I disagree with spaces for everything though,m
> particularly as we have said to use tabs for as long and many of the
> recipes do this (certainly more than use spaces).

I don't think history is a particularly good reason to keep our
metadata inconsistent, especially with bitbake becoming picky about
it.
-- 
Christopher Larson

Re: Yocto style guide change proposal

[Richard Purdie]

On Fri, 2012-07-20 at 06:57 -0700, Chris Larson wrote:
> On Fri, Jul 20, 2012 at 6:56 AM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > On Fri, 2012-07-20 at 11:32 +0200, Martin Jansa wrote:
> >> Now we have horrible mixture of whitespaces (tabs and space) only in
> >> recipe files, because yocto style guide recommends tabs in shell
> >> functions. So if recipe has e.g. do_install_append as well as
> >> populate_packages_prepend (not so uncommon combination as tabs fixing
> >> patches show), then according to yocto style guide it should look like
> >> this:
> >>
> >> do_install_append() {
> >>       foo
> >> }
> >> python populate_packages_prepend () {
> >>     libdir = bb.data.expand('${libdir}', d)
> >>     do_split_packages(d, libdir, '^lib(.*)\.so\.*', 'lib%s', 'ORC %s library', extra_depends='', allow_links=True)
> >> }
> >>
> >> especially with default tab width 8 spaces it's ugly and because it
> >> is inconsistent, many devs used spaces in shell functions too. Now when
> >> someone accidentaly use tab also in python function it will show warning
> >> or fail to parse. Some devs are using mix of tabs and spaces even on the
> >> same line (e.g. to indent SRC_URI multiline entries).
> >
> > We've said tabs for shell functions for *years*. I'm sure if I were to
> > look at the mailing list archives, that would be clear.
> >
> > In summary, I agree we need to make the style guides consistent and have
> > one version of them. I disagree with spaces for everything though,m
> > particularly as we have said to use tabs for as long and many of the
> > recipes do this (certainly more than use spaces).
> 
> I don't think history is a particularly good reason to keep our
> metadata inconsistent, especially with bitbake becoming picky about
> it.

I don't see it as inconsistent. I think python needed some special
handling since the language itself uses whitespace for structure and
we're now following standard python practise and have a way to ensure
some kinds of bugs don't creep in in future. It was already the agreed
convention, it just wasn't getting well followed, or we had some issues
migrating old mixed code to any one format without enforcing one.

The shell functions are not whitespace sensitive, its mainly an
atheistic issue. There is no issue with mixing tabs and spaces in the
same file. Sure some editor macro files are slightly harder to write to
handle that but I don't see that as a pressing reason to change what has
been the agreed convention.

What I really don't want is a ton of patches changing 80% of the shell
functions. The python changes at least were limited to a smaller subset
of files, mostly core class ones with the majority being
package.bbclass. If we change the shell convention like this, we're in
for orders of mangitiude more changes for no good reason as far as I can
see. The number of functions out there which are whitespace indented
right now is small as far as I can see.

Cheers,

Richard

Re: Yocto style guide change proposal

[Chris Larson]

On Fri, Jul 20, 2012 at 7:12 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Fri, 2012-07-20 at 06:57 -0700, Chris Larson wrote:
>> On Fri, Jul 20, 2012 at 6:56 AM, Richard Purdie
>> <richard.purdie@linuxfoundation.org> wrote:
>> > On Fri, 2012-07-20 at 11:32 +0200, Martin Jansa wrote:
>> >> Now we have horrible mixture of whitespaces (tabs and space) only in
>> >> recipe files, because yocto style guide recommends tabs in shell
>> >> functions. So if recipe has e.g. do_install_append as well as
>> >> populate_packages_prepend (not so uncommon combination as tabs fixing
>> >> patches show), then according to yocto style guide it should look like
>> >> this:
>> >>
>> >> do_install_append() {
>> >>       foo
>> >> }
>> >> python populate_packages_prepend () {
>> >>     libdir = bb.data.expand('${libdir}', d)
>> >>     do_split_packages(d, libdir, '^lib(.*)\.so\.*', 'lib%s', 'ORC %s library', extra_depends='', allow_links=True)
>> >> }
>> >>
>> >> especially with default tab width 8 spaces it's ugly and because it
>> >> is inconsistent, many devs used spaces in shell functions too. Now when
>> >> someone accidentaly use tab also in python function it will show warning
>> >> or fail to parse. Some devs are using mix of tabs and spaces even on the
>> >> same line (e.g. to indent SRC_URI multiline entries).
>> >
>> > We've said tabs for shell functions for *years*. I'm sure if I were to
>> > look at the mailing list archives, that would be clear.
>> >
>> > In summary, I agree we need to make the style guides consistent and have
>> > one version of them. I disagree with spaces for everything though,m
>> > particularly as we have said to use tabs for as long and many of the
>> > recipes do this (certainly more than use spaces).
>>
>> I don't think history is a particularly good reason to keep our
>> metadata inconsistent, especially with bitbake becoming picky about
>> it.
>
> I don't see it as inconsistent. I think python needed some special
> handling since the language itself uses whitespace for structure and
> we're now following standard python practise and have a way to ensure
> some kinds of bugs don't creep in in future. It was already the agreed
> convention, it just wasn't getting well followed, or we had some issues
> migrating old mixed code to any one format without enforcing one.


If you don't see two different types of indentation in one file as
inconsistent, I think you need to look up consistency in a dictionary.
-- 
Christopher Larson

Re: Yocto style guide change proposal

[Paul Eggleton]

On Friday 20 July 2012 07:15:28 Chris Larson wrote:
> If you don't see two different types of indentation in one file as
> inconsistent, I think you need to look up consistency in a dictionary.

You're not wrong - but that's not the point. Personally I'd really prefer we 
had four spaces everywhere; I'm not entirely sure why we chose this convention 
in the first place. However, it's the convention we have now and changing it 
has serious implications - almost every recipe will have to be changed; 
backporting changes to stable releases will be made more difficult; submitted 
patches crossing the switch will be difficult to apply, history will be 
polluted, etc.

Changing this is not a small undertaking.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: Yocto style guide change proposal

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 1540 bytes --]

On Fri, Jul 20, 2012 at 03:31:04PM +0100, Paul Eggleton wrote:
> On Friday 20 July 2012 07:15:28 Chris Larson wrote:
> > If you don't see two different types of indentation in one file as
> > inconsistent, I think you need to look up consistency in a dictionary.
> 
> You're not wrong - but that's not the point. Personally I'd really prefer we 
> had four spaces everywhere; I'm not entirely sure why we chose this convention 
> in the first place. However, it's the convention we have now and changing it 
> has serious implications - almost every recipe will have to be changed; 

It doesn't need to be changed now everywhere, recipe maintainers can
change it if they want e.g. when upgrading recipe (preferably in
separate commit). Tabs highlight will help with that..

> backporting changes to stable releases will be made more difficult; submitted 
> patches crossing the switch will be difficult to apply, history will be 
> polluted, etc.
> 
> Changing this is not a small undertaking.

meta-efl + meta-smartphone size is about 1/2 of oe-core (counting .bb +
.inc) and it took only about 1 hour to replace all tabs with 8 spaces 
(when it was used to indent e.g. SRC_URI) or 4 spaces (when it was used 
in shell functions).

In many cases I had to replace 8 spaces with 4 when those shell
functions were indented with spaces already or with tabs/spaces mix.

Yes it's only aesthetics, but that's what "style guide" usually does.

Cheers,

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 205 bytes --]

Re: Yocto style guide change proposal

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 2261 bytes --]

On Fri, Jul 20, 2012 at 02:56:01PM +0100, Richard Purdie wrote:
> On Fri, 2012-07-20 at 11:32 +0200, Martin Jansa wrote:
> > Now we have horrible mixture of whitespaces (tabs and space) only in 
> > recipe files, because yocto style guide recommends tabs in shell
> > functions. So if recipe has e.g. do_install_append as well as
> > populate_packages_prepend (not so uncommon combination as tabs fixing
> > patches show), then according to yocto style guide it should look like
> > this:
> > 
> > do_install_append() {
> > 	foo
> > }
> > python populate_packages_prepend () {
> >     libdir = bb.data.expand('${libdir}', d)
> >     do_split_packages(d, libdir, '^lib(.*)\.so\.*', 'lib%s', 'ORC %s library', extra_depends='', allow_links=True)
> > }
> > 
> > especially with default tab width 8 spaces it's ugly and because it
> > is inconsistent, many devs used spaces in shell functions too. Now when
> > someone accidentaly use tab also in python function it will show warning
> > or fail to parse. Some devs are using mix of tabs and spaces even on the
> > same line (e.g. to indent SRC_URI multiline entries).
> 
> We've said tabs for shell functions for *years*. I'm sure if I were to
> look at the mailing list archives, that would be clear.

Is new developer supposed to read all mailing list archives few years
back? Or can we assume that he will google for style guide? Note about
tabs was added to yocto style guide by Saul 21:05, 7 April 2011 and even
now it says:
"Use spaces for indentation as developers tends to use different amount
of spaces per one tab."
So tabs should be used already only in shell tasks, not for multiline
variables etc..

Not sure if it's worth your time trying to find it in archive..

> In summary, I agree we need to make the style guides consistent and have
> one version of them. I disagree with spaces for everything though,m
> particularly as we have said to use tabs for as long and many of the
> recipes do this (certainly more than use spaces).

If someone is using 4 spaces everywhere, then he wont probably make
mistake by using anything else then 4 spaces in python tasks where it
matters.

Regards,

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 205 bytes --]

Re: Yocto style guide change proposal

[Richard Purdie]

On Fri, 2012-07-20 at 17:17 +0200, Martin Jansa wrote:
> On Fri, Jul 20, 2012 at 02:56:01PM +0100, Richard Purdie wrote:
> > On Fri, 2012-07-20 at 11:32 +0200, Martin Jansa wrote:
> > > Now we have horrible mixture of whitespaces (tabs and space) only in 
> > > recipe files, because yocto style guide recommends tabs in shell
> > > functions. So if recipe has e.g. do_install_append as well as
> > > populate_packages_prepend (not so uncommon combination as tabs fixing
> > > patches show), then according to yocto style guide it should look like
> > > this:
> > > 
> > > do_install_append() {
> > > 	foo
> > > }
> > > python populate_packages_prepend () {
> > >     libdir = bb.data.expand('${libdir}', d)
> > >     do_split_packages(d, libdir, '^lib(.*)\.so\.*', 'lib%s', 'ORC %s library', extra_depends='', allow_links=True)
> > > }
> > > 
> > > especially with default tab width 8 spaces it's ugly and because it
> > > is inconsistent, many devs used spaces in shell functions too. Now when
> > > someone accidentaly use tab also in python function it will show warning
> > > or fail to parse. Some devs are using mix of tabs and spaces even on the
> > > same line (e.g. to indent SRC_URI multiline entries).
> > 
> > We've said tabs for shell functions for *years*. I'm sure if I were to
> > look at the mailing list archives, that would be clear.
> 
> Is new developer supposed to read all mailing list archives few years
> back? Or can we assume that he will google for style guide? Note about
> tabs was added to yocto style guide by Saul 21:05, 7 April 2011 and even
> now it says:
> "Use spaces for indentation as developers tends to use different amount
> of spaces per one tab."
> So tabs should be used already only in shell tasks, not for multiline
> variables etc..

Hence my proposal we sort this out and effectively remove the Yocto
style guide...

Cheers,

Richard