Recipe & Patch Style Guide: Difference between revisions

From Yocto Project
Jump to navigationJump to search
(Add note about package variable grouping)
Line 238: Line 238:
= Format Guidelines =
= Format Guidelines =


* No spaces are allowed behind the line continuation symbol
* No spaces are allowed after the line continuation symbol
* The correct spacing for a variable is FOO = "BAR".
* The correct spacing for a variable is FOO = "BAR".
* Use quotes on the right hand side of assignments: FOO = "BAR"
* Use quotes on the right hand side of assignments: FOO = "BAR"
* Comments inside bb files are allowed using the '#' character at the beginning of a line.
* Comments inside bb files are allowed using the '#' character at the beginning of a line.
* Use spaces for indentation as developers tends to use different amount of spaces per one tab.
* Use spaces for indentation as developers tends to use different amount of spaces per one tab.
* Indentation of multiline variables such as SRC_URI is desireable.
* Indentation of multiline variables such as SRC_URI is desirable.


= Style Checking tools =
= Style Checking tools =

Revision as of 17:20, 16 May 2011

Motivation

As with most style guides, the idea here is to have a consistent format and look so that when someone new comes to the scene they can learn quickly and get involved. This will also help the reviewers and maintainers understand what changes are being made by contributors.

Recipes file format and style

  • Consistent whitespace throughout the file
  • File follows a roughly standard variable order
  • Patches are all documented
  • No legacy staging
  • Use BBCLASSEXTEND where possible instead of -native versions
  • No -sdk or -nativesdk packages, use BBCLASSEXTEND
  • pkgconfig .pc files are correct and don't need manual mangling
  • No custom do_configure for autotooled projects
  • Uses "make install" where at all possible
    • if recipe installs can we patch make install and push upstream

Additional MetaData

Metadata to add into Recipes, when we start the update process or for non-changing packages we should add this data now.

  • DESCRIPTION
  • HOMEPAGE
  • BUGTRACKER
  • LICENSE
  • LIC_FILES_CHKSUM
  • DISTRO_PN_ALIAS

White Space Management

  • Most variables such as SRC_URI should use spaces.
  • Shell functions should use tabs
  • Python functions should use spaces (4 spaces per indent).

Patches

Patches should contain information about where they came from and why they are required, as the project moves forward, we want to increase the quality of the data that is made available to the community. This can be done in the recipes and also in the patch files in order to understand why the patch exists and to help future maintainer to understand why it is required.

Patch Comments

Patch comments should contains information about why the patch is required, who created the patch and when, if that information is not currently know the the last information about where the patch came from (such as from OpenEmbedded) is very important to maintain in the patch header. Having a Signed-off-by: tag is good to acknowledge that someone looked at the patch and added the comments to ensure it is still valid and current.

To recap, patches should contain:

  • Why
  • Date Created
  • Who Created it
  • Any Upstream information
    • link to upstream mailing list
    • link to upstream bug tracking
    • IRC info
  • Signed-off-by:
    • This is most important when importing the patch and not the original author

Patch Upstreaming

In order to keep track of patches and ultimate reduce the number of patches that the Yotco Project maintains, we need to track the status of the patches that are created. As mentioned above, we the patches to contain comments about why they are created, by whom and when, we also want to track if it's appropriate to get this particular patch into the upstream project. Since we want to track this information, we need a consistent tag and set of status that can be searched. Be sure to include any URL to bug tracking, mailing list or other reference material pertaining to the patch.

Upstream-Status:

Pending
No determination has been made yet or not yet submitted to upstream
Submitted
Submitted to upstream, waiting approval
Accepted
Accepted in upstream, expect it to be removed at next update, include expected version info
Backport
Backported from new upstream version, because we are at a fixed version, include upstream version info
Denied
Not accepted by upstream, include reason in patch
Inappropriate [reason]
The patch is not appropriate for upstream, include a brief reason on the same line enclosed with []
reason can be:
native
licensing
configuration
enable feature
disable feature
bugfix (add bug url here)
embedded specific
other (give details in comments)

All patches and diff files should include this information.

New style patch application

The patch and pnum parameters have been renamed to the more logical apply and striplevel. The apply parameter takes either "yes" or "no" and the striplevel parameter takes an integer (0, 1, etc).

Both parameters are now optional with "sane" defaults.

The apply parameter is optional for SRC_URI lines with patch or diff extensions, which will default to being applied.

The striplevel parameter is also optional with a default striplevel of 1.

Old style parameters (patch and pnum) will continue to work for some time but it would be useful to move to the new style syntax as people are updating other parts of their recipes.

Therefore a patch line would be changed from:

file://some.patch;patch=1;pnum=2

to:

file://some.patch;striplevel=2 

and a patch line:

file://another.diff;patch=1;pnum=1

could be changed to:

file://another.diff 

Recipe Metadata Updates

DESCRIPTION and SUMMARY

LICENSE Metadata

  • The LICENSE information in the .bb file needs to be practical.
  • if there's "or any later version" in GPL related copyright, append "+" then which effectively means below:
GPLv2, GPLv2+, GPLv3, GPLv3+, LGPLv2, LGPLv2+, LGPLv2.1, LGPLv2.1+, LGPLv3, LGPLv3+
  • Scripts generated by autotools are not counted for licensing (they are always under GPL)
  • Dual license: GPLv2 | BSD
  • Multiple licenses: GPLv3+ & LGPLv2.1+
  • GPLv3 (correction may be required!)
anti-tivoization in GPLv3 only applies to User Products, which per definition is “either 
(1) a “consumer product”, which means any tangible personal property which is normally 
used for personal, family, or household purposes, or (2) anything designed or sold for 
incorporation into a dwelling."
  • For package changing its license, better to keep new license in .inc file with old license in corresponding .bb file. Take readline for example:
readline.inc: LICENSE = "GPLv3+"
readline_5.2.bb: LICENSE = "GPLv2"
  • we can treat MIT-style license as "MIT", meaning that any lawyer can tell it derivatives from standard form, such as below one:
Permission to use, copy, modify, distribute, and sell this software and its
documentation for any purpose is hereby granted without fee, provided that
the above copyright notice appear in all copies and that both that copyright
notice and this permission notice appear in supporting documentation, and
that the name of the copyright holders not be used in advertising or
publicity pertaining to distribution of the software without specific,
written prior permission.  The copyright holders make no representations
about the suitability of this software for any purpose.  It is provided "as
is" without express or implied warranty.
  • some package may have complex license, such as wireless-tool:
most of files are GPLv2; 
one part in file is GPLv2+; 
some of them are dual licensed, such as sample_enc.c under LGPL | MPL | BSD.

In such case, first ignore the GPLv2+ bit since there is no way you could ever ship the package under say GPLv3 due to many headers being v2 only. Since there are files that are GPLv2 only, the answer is no. The LICENSE field is therefore primarily GPLv2 and we can ignore the 2+ bits. If they're a key part, the recipe becomes "GPLv2 & (LGPL | MPL | BSD)"

  • automake may generate COPYING automatically if there's no such one existing (e.g. Xsettings-client-0.10). A short answer is to add a MIT-style COPYING in poky and then install it before autotools work. See last section for detail description
  • all .bb files require LICENSE fields, even for those Poky specific (which are MIT).
  • ask on the ML for license information for those local files we don't know their origins
  • Name Sub-Packages with different Licenses
    • LICENSE = "LGPLv2.1 & GPLv3+"
    • LICENSE_libidn = "LGPLv2.1"
    • LICENSE_libidn-tests = "GPLv3+"
  • when listing sub-package license, remember to use names included in PACKAGES instead of source directories, e.g:
LICENSE = "GPLv2 & LGPLv2 & BSD & MIT"
LICENSE_lib/ext2fs = "LGPLv2"

Better to use:
LICENSE_e2fsprogs-mke2fs = "LGPLv2"
because mke2fs is a package name

Autotools adds wrong COPYING

Autotools add the wrong COPYING license to source code with out COPYING, to ensure that we have the correct and consistent license, add the correct license file to the SRC_URI List and a do_config_prepend().

SRC_URI = "... \
        ...
        file://XXX-license"

do_configure_prepend() {
        # This package doesn't ship with its own COPYING file and 
        # autotools will install a GPLv2 one instead of MIT. Add the
        # correct license here to avoid confusion.
        cp ${WORKDIR}/MIT-style-license ${S}/COPYING
}


Licence Updates

There are 2 parts to the licensing update that needs to happen in the recipe files, first is the LICENSE metadata, and the second is the License Verification with LIC_FILE_CHKSUM

LICENSE

  • GPLv2, GPLv2+, GPLv3, GPLv3+, LGPLv2,
  • LGPLv2+, LGPLv2.1, LGPLv2.1+, LGPLv3, LGPLv3+
  • Dual license: GPLv2 | BSD
  • Multiple licenses: GPLv3+ & LGPLv2.1+
  • Name Sub-Packages with different Licenses
    • LICENSE = "LGPLv2.1 & GPLv3+"
    • LICENSE_libidn = "LGPLv2.1"
    • LICENSE_libidn-tests = "GPLv3+"

LIC_FILES_CHKSUM

If a LICENSE or COPYING file exists, use it along with the license text from one source (header file prefered), if there is inconsistencies (multi versions, different licenses) report this information and then we can determine what the next steps should be to reconcile. Next steps include internal review and working with upstream community to reconclie licenses.

More information for using the LIC_FILES_CHKSUM variable
  • How to specify the LIC_FILES_CHKSUM variable:
LIC_FILES_CHKSUM = “file://COPYING;md5=xxxx \
                    file://licfile1.txt;beginline=5;endline=29;md5=yyyy \
                    file://licfile2.txt;endline=50;md5=zzzz \
                    ...“
  • Explanation:

This file lists all the files with the text of licenses for the source code. It is also possible to specify on which line the license text starts and on which line it ends within that file using the “beginline” & “endline” parameters. If the “beginline” parameter is not specified then license text begins from the 1st line is assumed. Similarly if “endline” parameter is not specified then the license text ends at the last line in the file is assumed. So if a file contains only licensing information, then there is no need to specify “beginline” & “endline” parameters.

The “md5” parameter stores the md5 checksum of the license text. So if the license text changes in any way, it’s md5 sum will differ and will not match with the previously stored md5 checksum. This mismatch will trigger build failure, notifying developer about the license text md5 mismatch, and allowing the developer to review the license text changes. Also note that if md5 checksum is not matched while building, the correct md5 checksum is printed in the build log.

There is no limit on how many files can be specified. But generally every project would need specifying of just one or two files for license tracking. Many projects would have a “COPYING” file which will store all the license information for all the source code files. If the “COPYING” file is valid then tracking only that file would be enough.

  • Tips on using the LIC_FILE_CHKSUM variable:

1. if you specify empty or invalid “md5” parameter; then while building the package, bitbake will give md5 not matched error, and also show the correct “md5” parameter value in the build log

2. If the whole file contains only license text, then there is no need to specify “beginline” and “endline” parameters.


Shamelessly taken from OE

Naming Conventions

Use $packagename_$version.bb

Format Guidelines

  • No spaces are allowed after the line continuation symbol
  • The correct spacing for a variable is FOO = "BAR".
  • Use quotes on the right hand side of assignments: FOO = "BAR"
  • Comments inside bb files are allowed using the '#' character at the beginning of a line.
  • Use spaces for indentation as developers tends to use different amount of spaces per one tab.
  • Indentation of multiline variables such as SRC_URI is desirable.

Style Checking tools

Please run ./contrib/oe-stylize.py on your recipes before submitting them.

Style Guidelines

  • Put the inherit declaration after the initial variables are set up and before any custom do_ routines. This is flexible as ordering is often important.
  • If you define custom do_ routines, keep them in the order of the tasks being executed, that is:
    • do_fetch
    • do_unpack
    • do_patch
    • do_configure
    • do_compile
    • do_install
    • do_package
    • do_stage
  • Don't use cp to put files into staging or destination directories, use install instead.
  • Don't use mkdir to create destination directories, use install -d instead.
  • There is a standard set of variables often found in a .bb file and the preferred order (to make the file easily readable to seasoned developers) is
    • DESCRIPTION
    • AUTHOR
    • HOMEPAGE
    • SECTION
    • PRIORITY
    • LICENSE
    • DEPENDS
    • SRCDATE
    • SRCREV
    • PV
    • PR
    • SRC_URI
    • S
    • inherit ...
    • build class specific variables, i.e. EXTRA_QMAKEVARS_POST
    • task overrides, i.e. do_configure
    • PACKAGE_ARCH
    • PACKAGES
    • FILES
    • RDEPENDS
    • RRECOMMENDS
    • RSUGGESTS
    • PROVIDES
    • RPROVIDES
    • RCONFLICTS
  • Variables related to the same package (*_{PN} and similar) should be kept together and after any PACKAGES definition

Example Recipe

DESCRIPTION = "X11 Code Viewer"
AUTHOR = "John Bazz <john.bazz@example.org>"
HOMEPAGE = "http://www.example.org/xcv/"
SECTION = "x11/applications"
PRIORITY = "optional"
LICENSE = "GPLv2"
DEPENDS = "libsm libx11 libxext libxaw"
RDEPENDS = "shared-mime-info"
RRECOMMENDS = "ctags"
RCONFLICTS = "xcv2"
SRCDATE = "20060815"
PV = "0.0+cvs${SRCDATE}"
PR = "r5"

# upstream does not yet publish any release so we have to fetch last working version from CVS
SRC_URI = "cvs://anonymous@xcv.example.org/cvsroot/xcv;module=xcv \
           file://toolbar-resize-fix.patch;patch=1"

S = "${WORKDIR}/xcv/"

inherit autotools

do_configure_prepend() {
    rm ${S}/aclocal.m4
}

do_install() {
    install -d ${D}${bindir}
    install -d ${D}${mandir}/man1

    install -m 0755 xcv ${D}${bindir}/   
    install -m 0644 xcv.1.gz ${D}${mandir}/man1/
}

PR variables with recipes that use INC files

When recipe include files are used, the PR handling gets kind of messy. Its a pain to have to audit the PR in all the dependent recipes when you change something in an INC file. We usually use the following solution:

recipe: PR = "${INC_PR}.1"

inc file: INC_PR = "r1"

When converting existing recipes to use INC_PR, set the initial INC_PR to the maximum of the current PRs.