Blame CONVENTIONS.md

Packit ae9e2a
# Libgit2 Conventions
Packit ae9e2a
Packit ae9e2a
We like to keep the source consistent and readable.  Herein are some
Packit ae9e2a
guidelines that should help with that.
Packit ae9e2a
Packit ae9e2a
## External API
Packit ae9e2a
Packit ae9e2a
We have a few rules to avoid surprising ways of calling functions and
Packit ae9e2a
some rules for consumers of the library to avoid stepping on each
Packit ae9e2a
other's toes.
Packit ae9e2a
Packit ae9e2a
 - Property accessors return the value directly (e.g. an `int` or
Packit ae9e2a
   `const char *`) but if a function can fail, we return a `int` value
Packit ae9e2a
   and the output parameters go first in the parameter list, followed
Packit ae9e2a
   by the object that a function is operating on, and then any other
Packit ae9e2a
   arguments the function may need.
Packit ae9e2a
Packit ae9e2a
 - If a function returns an object as a return value, that function is
Packit ae9e2a
   a getter and the object's lifetime is tied to the parent
Packit ae9e2a
   object. Objects which are returned as the first argument as a
Packit ae9e2a
   pointer-to-pointer are owned by the caller and it is responsible
Packit ae9e2a
   for freeing it. Strings are returned via `git_buf` in order to
Packit ae9e2a
   allow for re-use and safe freeing.
Packit ae9e2a
Packit ae9e2a
 - Most of what libgit2 does relates to I/O so as a general rule
Packit ae9e2a
   you should assume that any function can fail due to errors as even
Packit ae9e2a
   getting data from the filesystem can result in all sorts of errors
Packit ae9e2a
   and complex failure cases.
Packit ae9e2a
Packit ae9e2a
 - Paths inside the Git system are separated by a slash (0x2F). If a
Packit ae9e2a
   function accepts a path on disk, then backslashes (0x5C) are also
Packit ae9e2a
   accepted on Windows.
Packit ae9e2a
Packit ae9e2a
 - Do not mix allocators. If something has been allocated by libgit2,
Packit ae9e2a
   you do not know which is the right free function in the general
Packit ae9e2a
   case. Use the free functions provided for each object type.
Packit ae9e2a
Packit ae9e2a
## Compatibility
Packit ae9e2a
Packit ae9e2a
`libgit2` runs on many different platforms with many different compilers.
Packit ae9e2a
Packit ae9e2a
The public API of `libgit2` is [ANSI C](http://en.wikipedia.org/wiki/ANSI_C)
Packit ae9e2a
(a.k.a. C89) compatible.
Packit ae9e2a
Packit ae9e2a
Internally, `libgit2` is written using a portable subset of C99 - in order
Packit ae9e2a
to maximize compatibility (e.g. with MSVC) we avoid certain C99
Packit ae9e2a
extensions.  Specifically, we keep local variable declarations at the tops
Packit ae9e2a
of blocks only and we avoid `//` style comments.
Packit ae9e2a
Packit ae9e2a
Also, to the greatest extent possible, we try to avoid lots of `#ifdef`s
Packit ae9e2a
inside the core code base.  This is somewhat unavoidable, but since it can
Packit ae9e2a
really hamper maintainability, we keep it to a minimum.
Packit ae9e2a
Packit ae9e2a
## Match Surrounding Code
Packit ae9e2a
Packit ae9e2a
If there is one rule to take away from this document, it is *new code should
Packit ae9e2a
match the surrounding code in a way that makes it impossible to distinguish
Packit ae9e2a
the new from the old.* Consistency is more important to us than anyone's
Packit ae9e2a
personal opinion about where braces should be placed or spaces vs. tabs.
Packit ae9e2a
Packit ae9e2a
If a section of code is being completely rewritten, it is okay to bring it
Packit ae9e2a
in line with the standards that are laid out here, but we will not accept
Packit ae9e2a
submissions that contain a large number of changes that are merely
Packit ae9e2a
reformatting.
Packit ae9e2a
Packit ae9e2a
## Naming Things
Packit ae9e2a
Packit ae9e2a
All external types and functions start with `git_` and all `#define` macros
Packit ae9e2a
start with `GIT_`.  The `libgit2` API is mostly broken into related
Packit ae9e2a
functional modules each with a corresponding header.  All functions in a
Packit ae9e2a
module should be named like `git_modulename_functioname()`
Packit ae9e2a
(e.g. `git_repository_open()`).
Packit ae9e2a
Packit ae9e2a
Functions with a single output parameter should name that parameter `out`.
Packit ae9e2a
Multiple outputs should be named `foo_out`, `bar_out`, etc.
Packit ae9e2a
Packit ae9e2a
Parameters of type `git_oid` should be named `id`, or `foo_id`.  Calls that
Packit ae9e2a
return an OID should be named `git_foo_id`.
Packit ae9e2a
Packit ae9e2a
Where a callback function is used, the function should also include a
Packit ae9e2a
user-supplied extra input that is a `void *` named "payload" that will be
Packit ae9e2a
passed through to the callback at each invocation.
Packit ae9e2a
Packit ae9e2a
## Typedefs
Packit ae9e2a
Packit ae9e2a
Wherever possible, use `typedef`.  In some cases, if a structure is just a
Packit ae9e2a
collection of function pointers, the pointer types don't need to be
Packit ae9e2a
separately typedef'd, but loose function pointer types should be.
Packit ae9e2a
Packit ae9e2a
## Exports
Packit ae9e2a
Packit ae9e2a
All exported functions must be declared as:
Packit ae9e2a
Packit ae9e2a
```c
Packit ae9e2a
GIT_EXTERN(result_type) git_modulename_functionname(arg_list);
Packit ae9e2a
```
Packit ae9e2a
Packit ae9e2a
## Internals
Packit ae9e2a
Packit ae9e2a
Functions whose *modulename* is followed by two underscores,
Packit ae9e2a
for example `git_odb__read_packed`, are semi-private functions.
Packit ae9e2a
They are primarily intended for use within the library itself,
Packit ae9e2a
and may disappear or change their signature in a future release.
Packit ae9e2a
Packit ae9e2a
## Parameters
Packit ae9e2a
Packit ae9e2a
Out parameters come first.
Packit ae9e2a
Packit ae9e2a
Whenever possible, pass argument pointers as `const`.  Some structures (such
Packit ae9e2a
as `git_repository` and `git_index`) have mutable internal structure that
Packit ae9e2a
prevents this.
Packit ae9e2a
Packit ae9e2a
Callbacks should always take a `void *` payload as their last parameter.
Packit ae9e2a
Callback pointers are grouped with their payloads, and typically come last
Packit ae9e2a
when passed as arguments:
Packit ae9e2a
Packit ae9e2a
```c
Packit ae9e2a
int git_foo(git_repository *repo, git_foo_cb callback, void *payload);
Packit ae9e2a
```
Packit ae9e2a
Packit ae9e2a
## Memory Ownership
Packit ae9e2a
Packit ae9e2a
Some APIs allocate memory which the caller is responsible for freeing; others
Packit ae9e2a
return a pointer into a buffer that's owned by some other object.  Make this
Packit ae9e2a
explicit in the documentation.
Packit ae9e2a
Packit ae9e2a
## Return codes
Packit ae9e2a
Packit ae9e2a
Most public APIs should return an `int` error code.  As is typical with most
Packit ae9e2a
C library functions, a zero value indicates success and a negative value
Packit ae9e2a
indicates failure.
Packit ae9e2a
Packit ae9e2a
Some bindings will transform these returned error codes into exception
Packit ae9e2a
types, so returning a semantically appropriate error code is important.
Packit ae9e2a
Check
Packit ae9e2a
[`include/git2/errors.h`](https://github.com/libgit2/libgit2/blob/development/include/git2/errors.h)
Packit ae9e2a
for the return codes already defined.
Packit ae9e2a
Packit ae9e2a
In your implementation, use `giterr_set()` to provide extended error
Packit ae9e2a
information to callers.
Packit ae9e2a
Packit ae9e2a
If a `libgit2` function internally invokes another function that reports an
Packit ae9e2a
error, but the error is not propagated up, use `giterr_clear()` to prevent
Packit ae9e2a
callers from getting the wrong error message later on.
Packit ae9e2a
Packit ae9e2a
Packit ae9e2a
## Structs
Packit ae9e2a
Packit ae9e2a
Most public types should be opaque, e.g.:
Packit ae9e2a
Packit ae9e2a
```C
Packit ae9e2a
typedef struct git_odb git_odb;
Packit ae9e2a
```
Packit ae9e2a
Packit ae9e2a
...with allocation functions returning an "instance" created within
Packit ae9e2a
the library, and not within the application.  This allows the type
Packit ae9e2a
to grow (or shrink) in size without rebuilding client code.
Packit ae9e2a
Packit ae9e2a
To preserve ABI compatibility, include an `int version` field in all opaque
Packit ae9e2a
structures, and initialize to the latest version in the construction call.
Packit ae9e2a
Increment the "latest" version whenever the structure changes, and try to only
Packit ae9e2a
append to the end of the structure.
Packit ae9e2a
Packit ae9e2a
## Option Structures
Packit ae9e2a
Packit ae9e2a
If a function's parameter count is too high, it may be desirable to package
Packit ae9e2a
up the options in a structure.  Make them transparent, include a version
Packit ae9e2a
field, and provide an initializer constant or constructor.  Using these
Packit ae9e2a
structures should be this easy:
Packit ae9e2a
Packit ae9e2a
```C
Packit ae9e2a
git_foo_options opts = GIT_FOO_OPTIONS_INIT;
Packit ae9e2a
opts.baz = BAZ_OPTION_ONE;
Packit ae9e2a
git_foo(&opts);
Packit ae9e2a
```
Packit ae9e2a
Packit ae9e2a
## Enumerations
Packit ae9e2a
Packit ae9e2a
Typedef all enumerated types.  If each option stands alone, use the enum
Packit ae9e2a
type for passing them as parameters; if they are flags to be OR'ed together,
Packit ae9e2a
pass them as `unsigned int` or `uint32_t` or some appropriate type.
Packit ae9e2a
Packit ae9e2a
## Code Layout
Packit ae9e2a
Packit ae9e2a
Try to keep lines less than 80 characters long.  This is a loose
Packit ae9e2a
requirement, but going significantly over 80 columns is not nice.
Packit ae9e2a
Packit ae9e2a
Use common sense to wrap most code lines; public function declarations
Packit ae9e2a
can use a couple of different styles:
Packit ae9e2a
Packit ae9e2a
```c
Packit ae9e2a
/** All on one line is okay if it fits */
Packit ae9e2a
GIT_EXTERN(int) git_foo_simple(git_oid *id);
Packit ae9e2a
Packit ae9e2a
/** Otherwise one argument per line is a good next step */
Packit ae9e2a
GIT_EXTERN(int) git_foo_id(
Packit ae9e2a
	git_oid **out,
Packit ae9e2a
	int a,
Packit ae9e2a
	int b);
Packit ae9e2a
```
Packit ae9e2a
Packit ae9e2a
Indent with tabs; set your editor's tab width to 4 for best effect.
Packit ae9e2a
Packit ae9e2a
Avoid trailing whitespace and only commit Unix-style newlines (i.e. no CRLF
Packit ae9e2a
in the repository - just set `core.autocrlf` to true if you are writing code
Packit ae9e2a
on a Windows machine).
Packit ae9e2a
Packit ae9e2a
## Documentation
Packit ae9e2a
Packit ae9e2a
All comments should conform to Doxygen "javadoc" style conventions for
Packit ae9e2a
formatting the public API documentation.  Try to document every parameter,
Packit ae9e2a
and keep the comments up to date if you change the parameter list.
Packit ae9e2a
Packit ae9e2a
## Public Header Template
Packit ae9e2a
Packit ae9e2a
Use this template when creating a new public header.
Packit ae9e2a
Packit ae9e2a
```C
Packit ae9e2a
#ifndef INCLUDE_git_${filename}_h__
Packit ae9e2a
#define INCLUDE_git_${filename}_h__
Packit ae9e2a
Packit ae9e2a
#include "git/common.h"
Packit ae9e2a
Packit ae9e2a
/**
Packit ae9e2a
 * @file git/${filename}.h
Packit ae9e2a
 * @brief Git some description
Packit ae9e2a
 * @defgroup git_${filename} some description routines
Packit ae9e2a
 * @ingroup Git
Packit ae9e2a
 * @{
Packit ae9e2a
 */
Packit ae9e2a
GIT_BEGIN_DECL
Packit ae9e2a
Packit ae9e2a
/* ... definitions ... */
Packit ae9e2a
Packit ae9e2a
/** @} */
Packit ae9e2a
GIT_END_DECL
Packit ae9e2a
#endif
Packit ae9e2a
```
Packit ae9e2a
Packit ae9e2a
## Inlined functions
Packit ae9e2a
Packit ae9e2a
All inlined functions must be declared as:
Packit ae9e2a
Packit ae9e2a
```C
Packit ae9e2a
GIT_INLINE(result_type) git_modulename_functionname(arg_list);
Packit ae9e2a
```
Packit ae9e2a
Packit ae9e2a
`GIT_INLINE` (or `inline`) should not be used in public headers in order
Packit ae9e2a
to preserve ANSI C compatibility.
Packit ae9e2a
Packit ae9e2a
## Tests
Packit ae9e2a
Packit ae9e2a
`libgit2` uses the [clar](https://github.com/vmg/clar) testing framework.
Packit ae9e2a
Packit ae9e2a
All PRs should have corresponding tests.
Packit ae9e2a
Packit ae9e2a
* If the PR fixes an existing issue, the test should fail prior to applying
Packit ae9e2a
  the PR and succeed after applying it.
Packit ae9e2a
* If the PR is for new functionality, then the tests should exercise that
Packit ae9e2a
  new functionality to a certain extent.  We don't require 100% coverage
Packit ae9e2a
  right now (although we are getting stricter over time).
Packit ae9e2a
Packit ae9e2a
When adding new tests, we prefer if you attempt to reuse existing test data
Packit ae9e2a
(in `tests-clar/resources/`) if possible.  If you are going to add new test
Packit ae9e2a
repositories, please try to strip them of unnecessary files (e.g. sample
Packit ae9e2a
hooks, etc).