summary refs log tree commit diff
path: root/CONTRIBUTING.md
diff options
context:
space:
mode:
authorMichael Biebl <biebl@debian.org>2021-10-01 23:05:04 +0200
committerMichael Biebl <biebl@debian.org>2021-10-01 23:05:04 +0200
commite74c568b07b50b97873fb4ee1d776dedefbd54d6 (patch)
tree3469f17ea9af91f7ff169b890633bda68b0cf76e /CONTRIBUTING.md
parentbfe522304da217296e2a61040f58e35ec5d6f3f2 (diff)
New upstream version 1.32.12 upstream/1.32.12
Diffstat (limited to 'CONTRIBUTING.md')
-rw-r--r--CONTRIBUTING.md232
1 files changed, 232 insertions, 0 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
new file mode 100644
index 00000000..2ee06fd2
--- /dev/null
+++ b/CONTRIBUTING.md
@@ -0,0 +1,232 @@
+Guidelines for Contributing
+===========================
+
+
+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/)).
+
+Find us on IRC channel `#nm` on Libera.Chat.
+
+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).
+
+
+Coding Standard
+---------------
+
+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.
+
+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'
+```
+
+Since our coding style is entirely automated, the following are just
+some details of the style we use:
+
+* Indent with 4 spaces. (_no_ tabs).
+
+* Have no space between the function name and the opening '('.
+  - GOOD: `g_strdup(x)`
+  - BAD:  `g_strdup (x)`
+
+* C-style comments
+  - GOOD: `f(x);  /* comment */`
+  - BAD:  `f(x);  // comment`
+
+* Keep assignments in the variable declaration area pretty short.
+  - GOOD: `MyObject *object;`
+  - BAD:  `MyObject *object = complex_and_long_init_function(arg1, arg2, arg3);`
+
+* 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.
+  - GOOD: `#define MY_CONSTANT 42`
+  - BAD:  `static const unsigned myConstant = 42;`
+
+
+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.
+
+
+Assertions in NetworkManager code
+---------------------------------
+
+There are different kind of assertions. Use the one that is appropriate.
+
+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,
+  this is usually the preferred way for assertions that are supposed to be
+  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.
+  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.
+
+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`
+  define. This is preferred for assertions that are expensive to check or
+  nor necessary to check frequently. It's also for conditions that can easily
+  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.
+
+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).
+
+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,
+  it's basically like g_assert().
+
+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
+  is really wrong) or debug messages. But better don't. If it's important,
+  find a way to report the condition via the API to the caller. If it's
+  not important, keep silent.
+  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`.
+
+6) `g_warn_if_*()` from glib. These are always compiled in and log a `G_LOG_LEVEL_WARNING`
+  warning. Don't use this.
+
+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)`
+  translate to plain C pointer casts. Use such cast macros deliberately, in production
+  code they are cheap, with more asserts enabled they check that the pointer type is
+  suitable.
+
+Of course, every assertion failure is a bug, and calling it must have no side effects.
+
+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.
+
+For testing, you also want to run NetworkManager with environment variable
+`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
+but for assertions.
+
+
+Git Notes (refs/notes/bugs)
+---------------------------
+
+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.
+
+The git notes reference is called "refs/notes/bugs".
+
+So configure:
+
+```
+$ git config --add 'remote.origin.fetch' 'refs/notes/bugs:refs/notes/bugs'
+$ git config --add 'notes.displayref' 'refs/notes/bugs'
+```
+
+For example, set notes with
+
+```
+$ git notes --ref refs/notes/bugs add -m "(cherry picked from $COMMIT_SHA)" HEAD
+```
+
+You should see the notes in git-log output as well.
+
+To resync our local notes use:
+
+```
+$ git fetch origin refs/notes/bugs:refs/notes/bugs -f
+```
+
+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