CONTRIBUTING.md 9.66 KB
Newer Older
Thomas Haller's avatar
Thomas Haller committed
1
2
Guidelines for Contributing
===========================
3

4

5
6
7
8
9
10
11
12
13
Community
---------

Check out website https://networkmanager.dev and our [GNOME page](https://wiki.gnome.org/Projects/NetworkManager).

The release tarballs can be found at [download.gnome.org](https://download.gnome.org/sources/NetworkManager/).

Our mailing list is networkmanager-list@gnome.org ([archive](https://mail.gnome.org/archives/networkmanager-list/)).

14
Find us on IRC channel `#nm` on Libera.Chat.
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35

Report issues and send patches via [gitlab.freedesktop.org](https://gitlab.freedesktop.org/NetworkManager/NetworkManager/)
or our mailing list.


Legal
-----

NetworkManager is partly licensed under terms of GNU Lesser General Public License
version 2 or later ([LGPL-2.1-or-later](COPYING.LGPL)). That is for example the case for libnm.
For historical reasons, the daemon itself is licensed under terms of GNU General
Public License, version 2 or later ([GPL-2.0-or-later](COPYING)). See the SPDX license comment
in the source files.

Note that all new contributions to NetworkManager **MUST** be made under terms of
LGPL-2.1-or-later, that is also the case for files that are currently licensed GPL-2.0-or-later.
The reason is that we might one day use the code under terms of LGPL-2.1-or-later and all
new contributions already must already agree to that.
For more details see [RELICENSE.md](RELICENSE.md).


Thomas Haller's avatar
Thomas Haller committed
36
37
Coding Standard
---------------
38

39
40
41
42
43
44
45
46
47
The formatting uses clang-format with clang 11.0. Run
`./contrib/scripts/nm-code-format.sh -i` to reformat the code
or call `clang-format` yourself.
You may also call `./contrib/scripts/nm-code-format-container.sh`
which runs a Fedora 33 container using podman.
You are welcome to not bother and open a merge request with
wrong formatting, but note that we then will automatically adjust
your contribution before merging.

48
49
50
51
52
53
54
55
The automatic reformatting was done by commit 328fb90f3e0d4e35975aff63944ac0412d7893a5.
Use `--ignore-rev` option or `--ignore-revs-file .git-blame-ignore-revs` to ignore
the reformatting commit with git-blame:

```
$ git config --add 'blame.ignoreRevsFile' '.git-blame-ignore-revs'
```

56
57
Since our coding style is entirely automated, the following are just
some details of the style we use:
58
59
60
61

* Indent with 4 spaces. (_no_ tabs).

* Have no space between the function name and the opening '('.
62
  - GOOD: `g_strdup(x)`
63
  - BAD:  `g_strdup (x)`
Adam Langley's avatar
Adam Langley committed
64

65
* C-style comments
66
67
  - GOOD: `f(x);  /* comment */`
  - BAD:  `f(x);  // comment`
Adam Langley's avatar
Adam Langley committed
68
69

* Keep assignments in the variable declaration area pretty short.
70
71
  - GOOD: `MyObject *object;`
  - BAD:  `MyObject *object = complex_and_long_init_function(arg1, arg2, arg3);`
Adam Langley's avatar
Adam Langley committed
72
73
74
75
76

* 80-cols is a guideline, don't make the code uncomfortable in order to fit in
  less than 80 cols.

* Constants are CAPS_WITH_UNDERSCORES and use the preprocessor.
77
78
79
  - GOOD: `#define MY_CONSTANT 42`
  - BAD:  `static const unsigned myConstant = 42;`

Adam Langley's avatar
Adam Langley committed
80

81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
Unit Tests
----------

We have plenty of unit tests. Run them with `make check` or
`meson -C "$BUILD_DIR" test`.

Note that some files in the source tree are both generated and commited
to git. That means, certain changes to the code also affect these generated
files. The unit test fail in that case, to indicate that the generated
files no longer match what is commited to git.
You can also automatically regenerate the files by running `NM_TEST_REGENERATE=1 make check`.
Note that test-client requires working translation.
See the [comment](https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/blob/eee4332e8facfa5ff5940fa1655575d76ca143ea/src/tests/client/test-client.py#L19)
for how to configure it.


97
Assertions in NetworkManager code
Thomas Haller's avatar
Thomas Haller committed
98
---------------------------------
99

100
There are different kind of assertions. Use the one that is appropriate.
101

102
103
104
105
1) `g_return_*()` from glib. This is usually enabled in release builds and
  can be disabled with `G_DISABLE_CHECKS` define. This uses `g_log()` with
  `G_LOG_LEVEL_CRITICAL` level (which allows the program to continue,
  unless `G_DEBUG=fatal-criticals` or `G_DEBUG=fatal-warnings` is set). As such,
106
  this is usually the preferred way for assertions that are supposed to be
107
108
109
110
111
112
113
  enabled by default. \
  \
  Optimally, after a `g_return_*()` failure the program can still continue. This is
  also the reason why `g_return_*()` is preferable over `g_assert()`.
  For example, that is often not the case for functions that return a `GError`, because
  `g_return_*()` will return failure without setting the error output. That often leads
  to a crash immediately after, because the caller requires the `GError` to be set.
114
115
116
  Make a reasonable effort so that an assertion failure may allow the process
  to proceed. But don't put too much effort in it. After all, it's an assertion
  failure that is not supposed to happen either way.
117

118
119
2) `nm_assert()` from NetworkManager. This is disabled by default in release
  builds, but enabled if you build `--with-more-assertions`. See the `WITH_MORE_ASSERTS`
120
  define. This is preferred for assertions that are expensive to check or
121
  nor necessary to check frequently. It's also for conditions that can easily
122
123
124
  be verified to be true and where future refactoring is unlikely to break the
  invariant.
  Use such asserts deliberately and assume they are removed from production builds.
125

126
127
128
129
3) `g_assert()` from glib. This is used in unit tests and commonly enabled
  in release builds. It can be disabled with `G_DISABLE_ASSERT` define.
  Since such an assertion failure results in a hard crash, you
  should almost always prefer `g_return_*()` over `g_assert()` (except in unit tests).
130

131
132
4) `assert()` from C89's `<assert.h>`. It is usually enabled in release builds and
  can be disabled with `NDEBUG` define. Don't use it in NetworkManager,
133
134
  it's basically like g_assert().

135
136
137
138
139
140
141
142
143
144
5) `g_log()` from glib. These are always compiled in, depending on the logging level
  they act as assertions too. `G_LOG_LEVEL_ERROR` messages abort the program, `G_LOG_LEVEL_CRITICAL`
  log a critical warning (like `g_return_*()`, see `G_DEBUG=fatal-criticals`)
  and `G_LOG_LEVEL_WARNING` logs a warning (see `G_DEBUG=fatal-warnings`).
  `G_LOG_LEVEL_DEBUG` level is usually not printed, unless `G_MESSAGES_DEBUG` environment
  variable enables it. \
  \
  In general, avoid using `g_log()` in NetworkManager. We have nm-logging instead
  which logs to syslog or systemd-journald.
  From a library like libnm it might make sense to log warnings (if something
145
  is really wrong) or debug messages. But better don't. If it's important,
146
  find a way to report the condition via the API to the caller. If it's
147
  not important, keep silent.
148
149
  In particular, don't use levels `G_LOG_LEVEL_CRITICAL` and `G_LOG_LEVEL_WARNING` because
  we treat them as assertions and we want to run all out tests with `G_DEBUG=fatal-warnings`.
150

151
6) `g_warn_if_*()` from glib. These are always compiled in and log a `G_LOG_LEVEL_WARNING`
152
153
  warning. Don't use this.

154
155
7) `G_TYPE_CHECK_INSTANCE_CAST()` from glib. Unless building with `WITH_MORE_ASSERTS`,
  we set `G_DISABLE_CAST_CHECKS`. This means, cast macros like `NM_DEVICE(ptr)`
156
  translate to plain C pointer casts. Use such cast macros deliberately, in production
157
  code they are cheap, with more asserts enabled they check that the pointer type is
158
159
160
  suitable.

Of course, every assertion failure is a bug, and calling it must have no side effects.
161

162
163
Theoretically, you are welcome to set `G_DISABLE_CHECKS`, `G_DISABLE_ASSERT` and
`NDEBUG` in production builds. In practice, nobody tests such a configuration, so beware.
164

165
For testing, you also want to run NetworkManager with environment variable
166
167
`G_DEBUG=fatal-warnings` to crash upon `G_LOG_LEVEL_CRITICAL` and `G_LOG_LEVEL_WARNING`
`g_log()` message. NetworkManager won't use these levels for regular logging
168
but for assertions.
169
170
171
172
173


Git Notes (refs/notes/bugs)
---------------------------

174
175
176
177
We use special tags in commit messages like "Fixes", "cherry picked from" and "Ignore-Backport".
The [find-backports](contrib/scripts/find-backports) script uses these to find patches that
should be backported to older branches. Sometimes we don't know a-priory to mark a commit
with these tags so we can instead use the `bugs` notes.
178

179
The git notes reference is called "refs/notes/bugs".
180
181
182

So configure:

183
184
185
186
```
$ git config --add 'remote.origin.fetch' 'refs/notes/bugs:refs/notes/bugs'
$ git config --add 'notes.displayref' 'refs/notes/bugs'
```
187
188
189

For example, set notes with

190
191
192
```
$ git notes --ref refs/notes/bugs add -m "(cherry picked from $COMMIT_SHA)" HEAD
```
193
194
195
196
197

You should see the notes in git-log output as well.

To resync our local notes use:

198
199
200
```
$ git fetch origin refs/notes/bugs:refs/notes/bugs -f
```
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232

Code Structure
---------------------------

`./contrib`- Contains a lot of required package, configuration for different platform and environment, build NM from source tree.

`./data`- Contains some configurations and rules.

`./docs`- Contains the generated documentation for libnm and for the D-Bus API.

`./examples`- Some code examples for basic networking operations and status checking.

`./introspection`- XML docs describing various D-Bus interface and their properties.

`./m4`- Contains M4 macros source files for autoconf.

`./man`- NM manual files.

`./po`- contains text-based portable object file. These .PO files are referenced by GNU gettext as a property file and these files are human readable used for translating purpose.

`./src`- source code for libnm, nmcli, nm-cloud-setup, nmtui…

`./tools`- tools for generating the intermediate files or merging the file.

Cscope/ctags
---------------------------

NetworkManager's source code is large. It may be a good idea to use tools like cscope/ctags to index the
source code and navigate it. These tools can integrate with editors like `Vim` and `Emacs`. See:

- http://cscope.sourceforge.net/cscope_vim_tutorial.html
- https://www.emacswiki.org/emacs/CScopeAndEmacs