# qiBuild documentation

# Python coding guide

## General

### Pure coding style

The code should follow the Python coding style expressed in PEP8 with the following exceptions:

- Soft limit for line length is 80 characters. Hard limit is 100 characters. You should only exceed 80 characters if it doesn’t hurt readability.
- Indentation is made of four spaces. No exception.
- No trailing whitespace are allowed.
- Every text file must be pushed using UNIX line endings. (On windows, you are advised to set `core.autocrlf` to true).
- Please use a spell checker when you write comments. Typos in comments are annoying and distractive, typos in doc strings are bad because we generate public documentation from some of those doc strings.
- There must be two lines between top-level definitions. One line between methods.
- Do not initialize several variables on the same line, unless they come from a tuple (for instance the return of a function, or a iteration on a directory)

    # Yes:
    ok, mess  = run_command()

    for test_result in test_results:
        outcome, message = res

    # No:
    ok, mess = False, ""

    class Foo:
        self.bar, self.baz = None, True

- The Google Python Style Guide and Pocoo Team Style Guide contain lots of useful things, please read them.

### Use the standard library

There are lots of good modules in the Python standard library. Please have a look here before re-inventing the wheel.

Use built-in types a maximum.

Some examples of useful built-in functions:

- range to generate a list (range).
- min and max instead of writing a loop to compute the lowest/biggest element of the list.
- enumerate, reversed and sorted for list manipulation.

Some examples of useful modules:

- argparse for command line arguments parsing.
- itertools instead of writing for loops.
- pprint, instead of trying to rewrite complex `__str__` functions
- re for regular expression operations.

Some string functions you will always use:

- startswith and endswith instead of `foo[:5] == 'aldeb'`.
- join, split and splitlines to join multiple strings with the same separator or split a string into a list on a separator.
- ljust and rjust instead of writing custom padding code
- Always precise object when creating it. Use `foo = list()` instead of `foo = []`. `bar = dict()` instead of `bar = {}`, etc.
- When copying a list, use `my_copy = list(my_list)` instead of `my_copy = my_list[:]`
- You can compute max/min/join on any iterator, so no need to create a list, a generator is enough: `max(len(x) for x in myiterable)`

## Some more specific rules

### Do not just copy/paste code from the Internet

It’s perfectly fine to look on the Internet for a solution of your problem. If you find a snippet of code you want to copy/paste, please:

- check the license if any (qiBuild must stay BSD compatible, so no GPL code)
- fix the style if necessary
- cite the origin of your code and their authors, either in a comment, or for bigger code, in the README.rst

### Always specify optional arguments explicitly

When you have a function that looks like this:

    def foo(project, config=None):
        """run foo on the given project"""

Python will let you use `foo(my_proj, "linux32")`, automatically converting the regular argument `linux32` into the optional argument named `config`

Please don’t do that and use the explicit form instead:

    # in bar.py

    # BAD : second argument is in fact an optional argument.
    foo(my_proj, "linux32")

    # OK: the optional argument is explicit:
    foo(my_proj, config="linux32")

This can cause problems if someone ever changes the `foo` function and adds a new optional argument before `config`:

    def foo(project, clean=False, config=None):
        """run foo on the given project

        :param clean: ...
        """

The line in `bar.py` will call `foo()` with `clean="linux32"` and `config=None`, leading to interesting bugs.

### Doc strings

Right now the state of the docstrings inside qiBuild is quite a mess. But you should try to write docstrings as if all of them were going to be used with sphinx autodoc extension.

Follow PEP257.

So the canonical docstring should look like:

    def foo(bar, baz):
        """Does this and that
        :param bar: ...
        :param baz: ...

        :raise: MyError if ...
        :return: True if ...
        """

But please do not put too much in the doc string, we want to keep the code readable.

    # Bad: too much stuff here

    def foo(bar, baz):
        """ Does this and that
        :param bar: ...
        :param baz: ...

        :raise: MyError if ...
        :return: True if ...

        .. seealso:

            * :ref:`this-other-topic`

        Example ::

          bar = Bar()
          baz = Baz()
          f = foo(bar, baz)
        """

Rather use the modularity of `autodoc`:

    # OK: still readable

    def foo(bar, baz):
        """ Does this and that
        :param bar: ...
        :param baz: ...

        :raise: MyError if ...
        :return: True if ...
        """

    .. autofunction:: qisy.sh.mkdir

    .. seealso:

      * :ref:`this-other-topic`

     Example

     .. code-block:: python

          bar = Bar()
          baz = Baz()
          f = foo(bar, baz)

### Module/packages organization

- Every file that ends with the python extension must support to be imported, without side effects.

- `import foo` must never fail, unless there is a necessary module that could not be found. Do not catch the ImportError unless it is necessary, for instance to deal with optional dependencies:

      import required_module

      HAS_NICE_FEATURE = True
      try:
          import nicefeature
      except ImportError:
          HAS_NICE_FEATURE = False

      #...

      if HAS_NICE_FEATURE:
          #....

- Even if you are sure you code is standalone, and is only supposed to be used as a script, please follow the following skeleton:

      """The foo script adds spam to the eggs """

      def add_eggs(spam, eggs):
          """Add some spam to the eggs """

          #...

      def main():
          """Parse command line """

          #...

          add_eggs(spam, eggs)

      if __name__ == "__main__":
          main()

Note that the `main()` function does nothing but parsing command line, the real work being done by a nicely named `add_eggs()` function.

Unless you have a good reason too, please do not call `sys.exit()` outside of the `main()` function.

You will be glad to have written your `foo.py` script this way if you want to add some spam to the eggs somewhere else :)

- Keep all the imports at the beginning of the file. Separate imports from your package and imports from dependencies/standard library. Also separate normal imports and “from” imports.

  Example (bad):

      import foo
      from bar import toto
      import sys

      # Some code here (100 lines)

      import tata

      # Some other code here.

  Example (good):

      import sys

      import foo
      import tata

      from bar import toto

      # Some code here.

- If you want to shorten the name of a module, you can use `as alias_name` to rename it, but then you must keep it consistent across your whole project.

### Classes

- Use new-style classes. We don’t care of the overhead and this is the default in Python3. This means you should inherit from `object` or a new-style class.

- Avoid inheritance when you can and favor composition. With the dynamic nature of Python and the fact that every method is “virtual”, it can quickly become a nightmare to follow the code flow. Also, using composition makes it easier to test.

- When you want to make sure a class follows an interface, use `abc.ABCMeta` instead of `raise NotImplementedError`. This way you get the error when the class is instantiated instead of when the method is called

      # Yes
      class AbstractFoo(object):
          __metaclass__ = abc.ABCMeta

          @abc.abstractmethod
          def foo(self):
              pass

          @abc.abstractmethod
          def bar(self):
              pass

      # No:
      class AbstractFoo:

          def foo(self):
              raise NotImplementedError()

          def bar(self):
              raise NotImplementedError()

- The `__init__` method should only initialize the attributes. When an attribute of a class is computed from an other attribute, use a property instead:

      # Yes
      class Foo(object):

          def __init__(self, root, src):
                self.root = root
                self.src = src

          @property
          def path(self):
              return os.path.join(self.root, self.src)

      # No:
      class Foo(object):

          def __init__(self, root, src):
                self.root = root
                self.src = src
                self.path = os.path.join(self.root, self.src)

  - You will get an error if someone tries to set the `path` attribute
  - You are sure that `path` will be updated when someone changes `src` after the object is initialized.

### Variable naming

Without going to the extend of using Polish notation, it is useful to have a convention for variable naming, especially since Python has a dynamic type system, and to keep the code base consistent

- Do not name variables you do not intend to use later:

  >     foo, _ = run_foobar()

- Use plural for containers:

  >     # No:
  >     result = set()

  >     # Yes:
  >     results = set()

  This has the nice benefit of allowing you to have meaningful “loop names”:

  >     for result in results:
  >       # ...

- Use _name suffix when your are using a .name attribute

  >     # No:
  >     my_projects = [x.name for x in projects]

  >     # Yes:
  >     my_project_names = [x.name for x in projects]

- Use `path` when you have an absolute path, and `src` when you have a relative, posix path

### File Paths

- Never use strings to manipulate file paths. Use built-in `os.path` module which will handle all the nasty stuff for you:

      # BAD : you are doomed if you ever want to
      # generate a .bat file with bar_path
      bar_path = spam_path + "/" + "bar"

      # OK:
      bar_path = os.path.join(spam_path, "bar")

- When using `os.path.join()`, use one argument per file/directory:

      # BAD: you can end up with an ugly path like c:\path\to/foo/bar
      my_path = os.path.join(base_dir, "foo/bar")

      # OK:
      my_path = os.path.join(base_dir, "foo", "bar")

- Always convert files coming from the user to native, absolute path:

      user_input = #...
      my_path = qibuild.sh.to_native_path(user_input)

- Always store and manipulate native paths (using `os.path`), and if needed convert to POSIX or Windows format at the last moment.

  Note

  If you need to build POSIX paths, don’t use string operations either, use `posixpath.join` (This works really well to build URL, for instance)

- Pro-tip: to hard-code paths on Windows:

  Use `r""` rather than ugly `"\\"`:

      # UGLY:
      WIN_PATH = "c:\\windows\\spam\\eggs"

      # NICE:
      WIN_PATH = r"c:\windows\spam\eggs"

### Environment Variables

Please make sure to never modify `os.environ`

Remember that `os.environ` is in fact a huge global variable, and we all know it’s a bad idea to use global variables ...

Instead, use a copy of `os.environ`, for instance:

    import qibuild

    # Notice the .copy() !
    # If you forget it, build_env is a *reference* to
    # os.environ, so os.environ will be modified ...
    cmd_env = os.environ.copy()
    cmd_env["SPAM"] = "eggs"
    # Assuming foobar need SPAM environment variable set to 'eggs'
    cmd = ["foobar"]
    qisys.command.call(foobar, env=cmd_env)

In more complex cases, especially when handling the `%PATH%` environment variable, you can use `qibuild.envsetter.EnvSetter`.

A small example:

    import qibuild

    envsetter = qibuild.envsetter.EnvSetter()
    envsetter.prepend_to_path(r"c:\Program Files\Foobar\bin")
    build_env = envsetter.get_build_env()
    cmd = ["foobar", "/spam:eggs"]
    qisys.command.call(cmd, env=build_env)

### Platform-dependent code

Please use:

    # Windows vs everything else:
    import os
    if os.name == "posix":
        do_posix() # mac, linux
    if os.name == 'nt':
        do_windows()

    # Discriminate platform per platform:
    import sys

    if sys.platform.startswith("win"):
        # win32 or win64
        do_win()
    else if sys.platform.startswith("linux"):
        # linux, linux2 or linux3
        do_linux()
    else if sys.platform == "darwin":
        # mac
        do_mac()

### Output messages to the user

- Please use `qisys.ui` to print nice message to the user and not just `print`. This makes it easier to distinguish between real messages and the quick `print` you add for debugging.
- Speaking of debug, the tricky parts of qibuild contains some calls to `qisys.ui.debug` that are only triggered when using `-v, --verbose`. Don’t hesitate to use that, especially when something tricky is going on but you do not want to tell the user about it.

### Debugging

When something goes wrong, you will just have the last error message printed, with no other information. (Which is nice for the end user!)

If it’s an unexpected error message, here is what you can do:

- run qibuild with `-v` flag to display debug messages
- run qibuild with `--backtrace` to print the full backtrace
- run qibuild with `--pdb` to drop to a pdb session when an uncaught exception is raised.

### Error messages

Please do not overlook those. Often, when writing code you do something like:

    try:
        something_really_complicated()
    except SomeStrangeError, e:
        log.error("Error occurred: %s", e)

Because you are in an hurry, and just are thinking “Great, I’ve handled the exception, now I can go back to write some code...”

The problem is: the end user does not care you are glad you have handled the exception, he needs to understand what happens.

So you need to take a step back, think a little. “What path would lead to this exception? What was the end user probably doing? How can I help him understand what went wrong, and how he can fix this?”

So here is a short list of DO’s and DON’Ts when you are writing your error messages.

- Wording should look like:

      Could not < description of what went wrong >
      <Detailed explanation>
      Please < suggestion of a solution >

  For instance:

      Could not open configuration file
      'path/to/inexistant.cfg' does not exist
      Please check your configuration.

- Put filenames between quotes. For instance, if you are using a path given via a GUI, or via a prompt, it’s possible that you forgot to strip it before using it, thus trying to create `'/path/to/foo '` or `'path/to/foo\n'`.

  Unless you are putting the filename between quotes, this kind of error is hard to notice.

- Put commands to use like this:

      Please try running: `qibuild configure -c linux32 foo'

- Give information. Code like this makes little kitten cry:

      try:
          with open(config_file, "w") as fp:
              config = fp.read()
      except IOError, err:
          raise Exception("Could not open config file for writing")

  It’s not helpful at all! It does not answer those basic questions:

  - What was the config file?
  - What was the problem with opening the config file?
  - ...

  So the end user has no clue what to do... And the fix is so simple! Just add a few lines:

      try:
          with open(config_file, "w") as fp:
              config = fp.read()
      except IOError, err:
          mess = "Could not open config '%s' file for writing\n" % config_file
          mess += "Error was: %s" % err
          raise Exception(mess)

  So the error message would then be:

      Could not open '/etc/foo/bar.cfg' for writing
      Error was: [Errno 13] Permission denied

  Which is much more helpful.

- Suggest a solution. This is the hardest part, but it is nice if the user can figure out what to do next.

  Here are a few examples:

      $ qibuild configure -c foo

      Error: Invalid configuration foo
       * No toolchain named foo. Known toolchains are:
          ['linux32', 'linux64']
       * No custom cmake file for config foo found.
         (looked in /home/dmerejkowsky/work/tmp/qi/.qi/foo.cmake)

      $ qibuild install foo (when build dir does not exist)

      Error: Could not find build directory:
        /home/dmerejkowsky/work/tmp/qi/foo/build-linux64-release
      If you were trying to install the project, make sure that you have configured
      and built it first

      $ qibuild configure # when not in a worktree

      Error: Could not find a work tree. please try from a valid work tree,
      specify an existing work tree with '--work-tree {path}', or create a new
      work tree with 'qibuild init'

      $ qibuild configure # at the root for the worktree

      Error: Could not guess project name from the working tree. Please try
      from a subdirectory of a project or specify the name of the project.

### Interacting with the user

Make sure you only ask user when you have absolutely no way to do something smart by default.

(See for instance how `qibuild open` ask when it has absolutely no choice but to ask)

And when you ask, make sure the default action (pressing enter) will do the smartest thing.

Most people will not pay attention to the questions, (and they do not have to), so make the default obvious. (See for instance how `qibuild config --wizard` does it)

### Adding new tests

You should add your new test using `py.test`. For each Python module there should be a matching test module, containing unit tests:

    qisrc/foo.py
    qisrc/test/test_foo.py

And for each action there should be a matching test module, containing high-level integration tests

    qibuild/actions/foo.py
    qibuild/test/test_qibuild_foo.py

### Using external programs

To call external programs use the helpers in qisys.

And when possible use long options.

    # BAD
    grep -rniIEoC3 foo

    # GOOD
    grep --recursive --line-number --ignore-case --binary-files=without-match \
    --extended-regexp --only-matching --context=3 foo

It is a more readable script.

Changelog CMake index Python index Index