]>
Commit | Line | Data |
---|---|---|
24aec6db | 1 | # Libgit2 Conventions |
7335ffc3 | 2 | |
1631147c RB |
3 | We like to keep the source consistent and readable. Herein are some |
4 | guidelines that should help with that. | |
7335ffc3 | 5 | |
1f8cb02f CMN |
6 | ## External API |
7 | ||
8 | We have a few rules to avoid surprising ways of calling functions and | |
9 | some rules for consumers of the library to avoid stepping on each | |
10 | other's toes. | |
11 | ||
12 | - Property accessors return the value directly (e.g. an `int` or | |
13 | `const char *`) but if a function can fail, we return a `int` value | |
14 | and the output parameters go first in the parameter list, followed | |
15 | by the object that a function is operating on, and then any other | |
16 | arguments the function may need. | |
17 | ||
18 | - If a function returns an object as a return value, that function is | |
19 | a getter and the object's lifetime is tied to the parent | |
20 | object. Objects which are returned as the first argument as a | |
21 | pointer-to-pointer are owned by the caller and it is repsponsible | |
22 | for freeing it. Strings are returned via `git_buf` in order to | |
23 | allow for re-use and safe freeing. | |
24 | ||
25 | - Most of what libgit2 does relates to I/O so you as a general rule | |
26 | you should assume that any function can fail due to errors as even | |
27 | getting data from the filesystem can result in all sorts of errors | |
28 | and complex failure cases. | |
29 | ||
30 | - Paths inside the Git system are separated by a slash (0x2F). If a | |
31 | function accepts a path on disk, then backslashes (0x5C) are also | |
32 | accepted on Windows. | |
33 | ||
34 | - Do not mix allocators. If something has been allocated by libgit2, | |
35 | you do not know which is the right free function in the general | |
36 | case. Use the free functions provided for each object type. | |
37 | ||
1631147c RB |
38 | ## Compatibility |
39 | ||
40 | `libgit2` runs on many different platforms with many different compilers. | |
1631147c | 41 | |
68a19ca9 RB |
42 | The public API of `libgit2` is [ANSI C](http://en.wikipedia.org/wiki/ANSI_C) |
43 | (a.k.a. C89) compatible. | |
44 | ||
45 | Internally, `libgit2` is written using a portable subset of C99 - in order | |
46 | to maximize compatibility (e.g. with MSVC) we avoid certain C99 | |
47 | extensions. Specifically, we keep local variable declarations at the tops | |
48 | of blocks only and we avoid `//` style comments. | |
49 | ||
50 | Also, to the greatest extent possible, we try to avoid lots of `#ifdef`s | |
51 | inside the core code base. This is somewhat unavoidable, but since it can | |
52 | really hamper maintainability, we keep it to a minimum. | |
1631147c RB |
53 | |
54 | ## Match Surrounding Code | |
55 | ||
56 | If there is one rule to take away from this document, it is *new code should | |
57 | match the surrounding code in a way that makes it impossible to distinguish | |
58 | the new from the old.* Consistency is more important to us than anyone's | |
59 | personal opinion about where braces should be placed or spaces vs. tabs. | |
60 | ||
61 | If a section of code is being completely rewritten, it is okay to bring it | |
62 | in line with the standards that are laid out here, but we will not accept | |
63 | submissions that contain a large number of changes that are merely | |
64 | reformatting. | |
7335ffc3 | 65 | |
24aec6db | 66 | ## Naming Things |
7335ffc3 | 67 | |
1631147c RB |
68 | All external types and functions start with `git_` and all `#define` macros |
69 | start with `GIT_`. The `libgit2` API is mostly broken into related | |
70 | functional modules each with a corresponding header. All functions in a | |
71 | module should be named like `git_modulename_functioname()` | |
72 | (e.g. `git_repository_open()`). | |
7335ffc3 | 73 | |
24aec6db BS |
74 | Functions with a single output parameter should name that parameter `out`. |
75 | Multiple outputs should be named `foo_out`, `bar_out`, etc. | |
7335ffc3 | 76 | |
24aec6db BS |
77 | Parameters of type `git_oid` should be named `id`, or `foo_id`. Calls that |
78 | return an OID should be named `git_foo_id`. | |
6533aadc | 79 | |
1631147c RB |
80 | Where a callback function is used, the function should also include a |
81 | user-supplied extra input that is a `void *` named "payload" that will be | |
82 | passed through to the callback at each invocation. | |
6533aadc | 83 | |
1631147c | 84 | ## Typedefs |
7335ffc3 | 85 | |
1631147c RB |
86 | Wherever possible, use `typedef`. In some cases, if a structure is just a |
87 | collection of function pointers, the pointer types don't need to be | |
88 | separately typedef'd, but loose function pointer types should be. | |
7335ffc3 | 89 | |
24aec6db | 90 | ## Exports |
7335ffc3 SP |
91 | |
92 | All exported functions must be declared as: | |
93 | ||
1631147c | 94 | ```c |
24aec6db | 95 | GIT_EXTERN(result_type) git_modulename_functionname(arg_list); |
ee72ffd0 | 96 | ``` |
7335ffc3 | 97 | |
24aec6db | 98 | ## Internals |
7335ffc3 | 99 | |
1631147c | 100 | Functions whose *modulename* is followed by two underscores, |
24aec6db | 101 | for example `git_odb__read_packed`, are semi-private functions. |
7335ffc3 SP |
102 | They are primarily intended for use within the library itself, |
103 | and may disappear or change their signature in a future release. | |
104 | ||
24aec6db BS |
105 | ## Parameters |
106 | ||
107 | Out parameters come first. | |
108 | ||
109 | Whenever possible, pass argument pointers as `const`. Some structures (such | |
1631147c RB |
110 | as `git_repository` and `git_index`) have mutable internal structure that |
111 | prevents this. | |
24aec6db BS |
112 | |
113 | Callbacks should always take a `void *` payload as their last parameter. | |
1631147c RB |
114 | Callback pointers are grouped with their payloads, and typically come last |
115 | when passed as arguments: | |
24aec6db | 116 | |
1631147c RB |
117 | ```c |
118 | int git_foo(git_repository *repo, git_foo_cb callback, void *payload); | |
24aec6db BS |
119 | ``` |
120 | ||
24aec6db BS |
121 | ## Memory Ownership |
122 | ||
123 | Some APIs allocate memory which the caller is responsible for freeing; others | |
124 | return a pointer into a buffer that's owned by some other object. Make this | |
125 | explicit in the documentation. | |
126 | ||
24aec6db BS |
127 | ## Return codes |
128 | ||
1631147c RB |
129 | Most public APIs should return an `int` error code. As is typical with most |
130 | C library functions, a zero value indicates success and a negative value | |
131 | indicates failure. | |
132 | ||
133 | Some bindings will transform these returned error codes into exception | |
134 | types, so returning a semantically appropriate error code is important. | |
135 | Check | |
136 | [`include/git2/errors.h`](https://github.com/libgit2/libgit2/blob/development/include/git2/errors.h) | |
24aec6db BS |
137 | for the return codes already defined. |
138 | ||
1631147c RB |
139 | In your implementation, use `giterr_set()` to provide extended error |
140 | information to callers. | |
7335ffc3 | 141 | |
1631147c RB |
142 | If a `libgit2` function internally invokes another function that reports an |
143 | error, but the error is not propagated up, use `giterr_clear()` to prevent | |
144 | callers from getting the wrong error message later on. | |
7335ffc3 | 145 | |
7335ffc3 | 146 | |
1631147c | 147 | ## Structs |
24aec6db | 148 | |
1631147c | 149 | Most public types should be opaque, e.g.: |
7335ffc3 | 150 | |
ee72ffd0 | 151 | ```C |
24aec6db | 152 | typedef struct git_odb git_odb; |
ee72ffd0 | 153 | ``` |
7335ffc3 | 154 | |
24aec6db BS |
155 | ...with allocation functions returning an "instance" created within |
156 | the library, and not within the application. This allows the type | |
157 | to grow (or shrink) in size without rebuilding client code. | |
158 | ||
159 | To preserve ABI compatibility, include an `int version` field in all opaque | |
160 | structures, and initialize to the latest version in the construction call. | |
161 | Increment the "latest" version whenever the structure changes, and try to only | |
162 | append to the end of the structure. | |
6dafd056 | 163 | |
24aec6db | 164 | ## Option Structures |
6dafd056 | 165 | |
1631147c RB |
166 | If a function's parameter count is too high, it may be desirable to package |
167 | up the options in a structure. Make them transparent, include a version | |
168 | field, and provide an initializer constant or constructor. Using these | |
169 | structures should be this easy: | |
7335ffc3 | 170 | |
24aec6db BS |
171 | ```C |
172 | git_foo_options opts = GIT_FOO_OPTIONS_INIT; | |
173 | opts.baz = BAZ_OPTION_ONE; | |
174 | git_foo(&opts); | |
175 | ``` | |
0e7fa1fe | 176 | |
24aec6db | 177 | ## Enumerations |
0e7fa1fe | 178 | |
1631147c RB |
179 | Typedef all enumerated types. If each option stands alone, use the enum |
180 | type for passing them as parameters; if they are flags to be OR'ed together, | |
181 | pass them as `unsigned int` or `uint32_t` or some appropriate type. | |
7335ffc3 | 182 | |
24aec6db | 183 | ## Code Layout |
7335ffc3 | 184 | |
1631147c RB |
185 | Try to keep lines less than 80 characters long. This is a loose |
186 | requirement, but going significantly over 80 columns is not nice. | |
7335ffc3 | 187 | |
1631147c RB |
188 | Use common sense to wrap most code lines; public function declarations |
189 | can use a couple of different styles: | |
190 | ||
191 | ```c | |
192 | /** All on one line is okay if it fits */ | |
193 | GIT_EXTERN(int) git_foo_simple(git_oid *id); | |
194 | ||
195 | /** Otherwise one argument per line is a good next step */ | |
24aec6db | 196 | GIT_EXTERN(int) git_foo_id( |
1631147c RB |
197 | git_oid **out, |
198 | int a, | |
199 | int b); | |
24aec6db BS |
200 | ``` |
201 | ||
1631147c | 202 | Indent with tabs; set your editor's tab width to 4 for best effect. |
24aec6db | 203 | |
1631147c RB |
204 | Avoid trailing whitespace and only commit Unix-style newlines (i.e. no CRLF |
205 | in the repository - just set `core.autocrlf` to true if you are writing code | |
206 | on a Windows machine). | |
7335ffc3 | 207 | |
24aec6db | 208 | ## Documentation |
7335ffc3 | 209 | |
24aec6db | 210 | All comments should conform to Doxygen "javadoc" style conventions for |
1631147c RB |
211 | formatting the public API documentation. Try to document every parameter, |
212 | and keep the comments up to date if you change the parameter list. | |
7335ffc3 | 213 | |
24aec6db BS |
214 | ## Public Header Template |
215 | ||
216 | Use this template when creating a new public header. | |
217 | ||
218 | ```C | |
219 | #ifndef INCLUDE_git_${filename}_h__ | |
220 | #define INCLUDE_git_${filename}_h__ | |
221 | ||
222 | #include "git/common.h" | |
223 | ||
224 | /** | |
225 | * @file git/${filename}.h | |
226 | * @brief Git some description | |
227 | * @defgroup git_${filename} some description routines | |
228 | * @ingroup Git | |
229 | * @{ | |
230 | */ | |
231 | GIT_BEGIN_DECL | |
232 | ||
233 | /* ... definitions ... */ | |
234 | ||
235 | /** @} */ | |
236 | GIT_END_DECL | |
237 | #endif | |
ee72ffd0 | 238 | ``` |
24aec6db | 239 | |
394711ff TN |
240 | ## Inlined functions |
241 | ||
242 | All inlined functions must be declared as: | |
243 | ||
244 | ```C | |
245 | GIT_INLINE(result_type) git_modulename_functionname(arg_list); | |
246 | ``` | |
247 | ||
68a19ca9 RB |
248 | `GIT_INLINE` (or `inline`) should not be used in public headers in order |
249 | to preserve ANSI C compatibility. | |
250 | ||
1631147c RB |
251 | ## Tests |
252 | ||
a313de0d | 253 | `libgit2` uses the [clar](https://github.com/vmg/clar) testing framework. |
1631147c RB |
254 | |
255 | All PRs should have corresponding tests. | |
256 | ||
257 | * If the PR fixes an existing issue, the test should fail prior to applying | |
258 | the PR and succeed after applying it. | |
259 | * If the PR is for new functionality, then the tests should exercise that | |
260 | new functionality to a certain extent. We don't require 100% coverage | |
261 | right now (although we are getting stricter over time). | |
262 | ||
263 | When adding new tests, we prefer if you attempt to reuse existing test data | |
264 | (in `tests-clar/resources/`) if possible. If you are going to add new test | |
265 | repositories, please try to strip them of unnecessary files (e.g. sample | |
266 | hooks, etc). |