[thaller@redhat.com: I introduced this bug when taking the original patch].
Fixes: 820f6f3a4a ('contrib/rpm: sync obsoletes_{initscripts_updown,ifcfg_rh} version')
Initially, when we obsoleted {initscripts_updown,ifcfg_rh}, the package
versions that we build from this upstream spec file differed from what
we put into Fedora/RHEL.
By now, this is long gone, and for upstream package builds we don't care
to accurately track the version, when using upstream/copr builds. Just
sync the version to what they are in Fedora/RHEL.
On the main branch, we commonly rebase our WIP branches to latest HEAD,
before merging them with "--no-ff". The effect is to have a merge commit
that acts as a parentheses around the set of patches.
When backporting such a branch, we should preserve that structure and
take the merge commit too. We should must use `git cherry-pick -x` to
record the commit IDs of the original patch.
This script helps with that.
Also hook it up in "contrib/scripts/nm-setup-git.sh" to create an alias
for it. This alias has the advantage, of fetching the latest version of
the script from "main" or "origin/main", so it also works on older
branches.
It seems that `meson test` is preferred over `ninja test`. Also, pass
"--print-errorlogs" to meson, and pass "-v" to the build steps.
Note that `ninja test` already ends up calling `meson test
--print-errorlogs`, but it doesn't use "-v", so the logs are truncated.
Like done for autotools. First we run the test without debugging option.
If it fails, we run it again to possibly trigger the failure again and
get better logs.
"debug" is implied when setting NMTST_DEBUG, but not specifying
"no-debug". This change has thus no effect, but it seems clearer to be
explicit.
The "debug" flag affects nmtst_is_debug(). Note that tests *must* not
result in different code paths based on debug, they may only
1) print more debug logging
2) do more assertion checks.
Having more assertion checks can result in different outcome of the
test, that is, that the additional assertion fails first. That is
acceptable, because failing earlier is possibly closer to the issue and
helps debugging. Also, when the additional failure is fixed and passes,
we still will fail at the assertion we are trying to debug.
In particular, an access to nmtst_get_rand*()/nmtst_rand*() must not
depend on nmtst_is_debug(), because then different randomized paths
are taken based on whether debugging is enabled.
The code was just wrong. Usually in gitlab-ci, NMTST_SEED_RANDOM is
unset, so the previous code would not have set it. Which means that our
tests run with NMTST_SEED_RANDOM="0".
Fuzzing (or randomizing tests) is very useful, we should do that for the
unit tests that run in gitlab-ci. Fix this.
But don't let the test choose a random number. Instead, let the calling
script choose it. That is, because we might run the tests more than once
(without debugging and no valgrind; in case of failure return with
debugging; with valgrind). Those runs should use the same seed.
This fixes commit 70487d9ff8 ('ci: randomize tests during our CI'),
but as fixing randomization can break previously running tests, we may
only want to backport this commit after careful evaluation.
Fixes: 096b9955d6 ('contrib/fedora: make "lto" in the spec file configurable')
Fixes: 7a62845424 ('contrib/rpm: fix condition in "NetworkManager.spec"')
Fixes: 096b9955d6 ('contrib/fedora: make "lto" in the spec file configurable')
Fixes: 7a62845424 ('contrib/rpm: fix condition in "NetworkManager.spec"')
When we build a copr image, we run the "nm-copr-build.sh" script.
That script, should honor "LTO=0|1|" to explicitly enable/disable
LTO. Since the copr script only builds a SRPM, which then gets build
we need that the default LTO flag in the SRPM is templated.
Fixes: 0566e9dc63 ('contrib: support disabling "LTO" in "nm-copr-build.sh"')
The "nm-copr-build.sh" script is run by our copr to generate the SRPM of
NetworkManager (via `curl ... | bash`).
Building with LTO takes a long time, for testing it can be nice to disable
that. Add an environment variable for that. It can be used when manually
building an RPM in copr.
With the meson build configuration, there is obviously python3 installed
and in the path. The build script will pick that up as preferred python.
However, we will also need working pygobject to build the documentation.
But we only have that for python2 installed. Fix that, by installing
"python36-gobject".
We have "BuildRequires: ppp-devel". While in Fedora 37 that has a
dependency on "ppp" package, that is not the case on Centos7. I didn't
test others, but the point is it's not always there.
"/usr/sbin/pppd" is provided by "ppp" package, and we might not have it
installed via the build requirements. But we only need it to detect the
path, which is not necessary on RHEL/Fedora. Just set the path
explicitly with the respective configure option.
NM-ci wants to install a lot of packages when running the first test.
In particular, NM-ci has no nice script that lists all the dependencies,
so it's not immediately clear which packages are required.
Still, install some of those packages so that they are already present
when running the first NM-ci test.
python-setuptools is now gone from debian:testing ([1], [2]):
Package python-setuptools is not available, but is referred to by another package.
This may mean that the package is missing, has been obsoleted, or
is only available from another source
E: Package 'python-setuptools' has no installation candidate
This package is entirely optional. Fix the failure by ignoring any failure to
install the package.
[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=938168
[2] https://tracker.debian.org/news/1391360/python-setuptools-removed-from-testing/
This is the version shipped in Fedora 37. As Fedora 37 is now out, the
core developers switch to it. Our gitlab-ci will also use that as base
image for the check-{patch.tree} tests and to generate the pages. There
is a need that everybody agrees on which clang-format version to use,
and that version should be the one of the currently used Fedora release.
Also update the used Fedora image in "contrib/scripts/nm-code-format-container.sh"
script.
The gitlab-ci still needs update in the following commit. The change
in isolation will break the "check-tree" test.
Cleanup the handling of close().
First of all, closing an invalid (non-negative) file descriptor (EBADF) is
always a serious bug. We want to catch that. Hence, we should use nm_close()
(or nm_close_with_error()) which asserts against such bugs. Don't ever use
close() directly, to get that additional assertion.
Also, our nm_close() handles EINTR internally and correctly. Recent
POSIX defines that on EINTR the close should be retried. On Linux,
that is never correct. After close() returns, the file descriptor is
always closed (or invalid). nm_close() gets this right, and pretends
that EINTR is a success (without retrying).
The majority of our file descriptors are sockets, etc. That means,
often an error from close isn't something that we want to handle. Adjust
nm_close() to return no error and preserve the caller's errno. That is
the appropriate reaction to error (ignoring it) in most of our cases.
And error from close may mean that there was an IO error (except EINTR
and EBADF). In a few cases, we may want to handle that. For those
cases we have nm_close_with_error().
TL;DR: use almost always nm_close(). Unless you want to handle the error
code, then use nm_close_with_error(). Never use close() directly.
There is much reading on the internet about handling errors of close and
in particular EINTR. See the following links:
https://lwn.net/Articles/576478/https://askcodes.net/coding/what-to-do-if-a-posix-close-call-fails-https://www.austingroupbugs.net/view.php?id=529https://sourceware.org/bugzilla/show_bug.cgi?id=14627https://news.ycombinator.com/item?id=3363819https://peps.python.org/pep-0475/
Our GObject structs should be internal API. In which case, we should
embed the private data in the struct themselves (`_priv`) and use the
_NM_GET_PRIVATE() macro. The advantage is better debugability because
following G_TYPE_INSTANCE_GET_PRIVATE() in the debugger is very
cumbersome. Another (less relevant) advantage is better performance.
Thus, warn about uses of g_type_class_add_private() and
G_TYPE_INSTANCE_GET_PRIVATE().
Note that if the struct and is in a header file (which is usually only
necessary when subclassing the type), then the private data should be
an opaque pointer `_priv` instead, and we should use the _NM_GET_PRIVATE_PTR()
macro. In that case, the use of g_type_class_add_private() and
G_TYPE_INSTANCE_GET_PRIVATE() is correct and the warning is false. But
this is only a warning, for the unusual case where we have deep object
hierarchies.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1396
"-u" calls `git diff -name-only $UPSTREAM` to get a list of files.
However, if $UPSTREAM contains a file that doesn't exist in the current
checkout, we get a non-existing file name and clang-format will fail.
Avoid that, by filtering only files that exist.
Also, pass "--no-renames" option to git-diff.
Reformatting the entire source tree takes quite long. For fast
development it's useful to only check the files that changes on the
current checkout.
For that there was already the "-F|--fast" option, but that only
compared the files that changed compared to HEAD^.
What actually would be useful is to check the files that changed on the
current branch, compared to some upstream commit. Add "-u|--upstream"
option to specify the upstream commit (usually "main").
As a special twist,
./contrib/scripts/nm-code-format.sh -u
is the same as
./contrib/scripts/nm-code-format.sh -u main
We need to mount sysfs, so that `ip netns exec` works.
Do that automatically when starting the system container, via rc.local.
While at it, use `podman build --squash-all` to speedup the building of
the container image.
It's between "stop" and "clean". It removes the container,
but keeps the container images. This is to fast restart without
rebuilding the container (image).
- instead of g_str_hash()/g_direct_hash(), use our own functions
nm_str_hash()/nm_direct_hash(). Those use siphash24 with a random
seed.
- don't pass g_direct_equal() to GHashTable. When omitting the equal
function, it falls back to direct pointer comparison, which is likely
faster. In any case, it's consistent to not use g_direct_hash()
when using pointer equality.
- instead of g_int_hash()/g_int64_hash()/g_double_hash(), use
our nm_pint_hash()/nm_pint64_hash()/nm_pdouble_hash(). The latter
two don't exist yet.
The reason is that we want to use siphash24.
Yes, our name differs from glib's. Our naming seems to make sense
to me however, because we also have nm_pstr_hash(), nm_pdirect_hash()
and even nm_ppdirect_hash() for following the pointers. Naming is hard.
- instead of g_int_equal()/g_int64_equal()/g_double_equal() use
our nm_pint_equal()/nm_pint64_equal()/nm_pdouble_equal(). The latter
two don't exist yet. The reason is purely naming consistency since
our hash variants follow the other name.
Leave a hint about core-dumps.
Also, now we have `contrib/fedora/rpm/configure-for-system.sh` script,
which can configure the build in a way similar to what we get
when doing an RPM build.
That means, inside "contrib/scripts/nm-in-container.sh" we
can just type `make install`, and it will replace the pre-installed
NetworkManager.
The main advantage is that it becomes convenient to run NetworkManager
as a systemd service. Previously, the suggested was to to install
NetworkManager inside another prefix, and run it in the terminal.
Running NetworkManager as systemd service is also necessary for NM-ci,
which restarts the NetworkManager service, and you couldn't run a test,
if you just started NetworkManager in a terminal.
Previously, you had to build a complete RPM, which takes a lot of time.
Yes, it's a large dependency. But on your outer host you
probably configured NetworkManager with QT enabled (for the
example scripts). We want to compile the same work tree inside
the container. So install qt-devel.
This will use the same option as when we do an RPM build.
The purpose is that you could type `make install` with such
a build, and it would replace the files that you'd get by installing
the NetworkManager RPMs.
Of course, you would not want to do that on your work station, but it
will be useful in a container, where we don't mind messing up the
installation.
autotools accepts both --with-$OPTION/--without-$OPTION and
--with-$OPTION=yes|no. Consistently use the latter.
The advantage is that whether it's enabled becomes an argument, so in a
script you could do
"--with-$OPTION=$VALUE"
Same for enable/disable option.
The test bcond is used to make test suite failure terminate the %check
phase. That should generally be done for distro builds.
What the distro builds shouldn't do is terminate the build on compiler
warnings, because they're fairly likely to appear on toolchain updates
without indicating actual bugs.
However, currently that's precisely what do we do now.
Let's use -Werror on debug builds instead. They are intentionally made
more prone to fail (e.g. trip more runtime assertions) because failures
can be more rapidly addressed and don't affect distro builds.
Resolve the defaults in build.sh instead of RPM macros. This looks less
terrible maintaining the same defaults as well as options to override it
upstream.
Moving it to the block that downstreams (Fedora, Red Hat) keep
customized makes it possible for them to also maintain customized
defaults here.
In particular, the downstreams should be able to enable bcond_test
at least for their production release (otherwise there's little point in
actually running tests at package build time).
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1286
Otherwise, the path "src/c-stdaux/src/docs/conf.py" matches for
formatting. But this file is imported via git-subtree, we don't
want to format it.
Filter out such paths.
Recent gettext version can extract and merge back strings from and to
various file formats, no need for intltool anymore.
https://wiki.gnome.org/Initiatives/GnomeGoals/GettextMigrationhttps://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/133https://github.com/NetworkManager/NetworkManager/pull/303https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/96
Clarification about the use of AM_GNU_GETTEXT_REQUIRE_VERSION:
In configure.ac, specify the minimum gettext version we require, rather
than the exact one. This fixes a situation where the autoconf macros
used for gettext will be the latest available on the system (for
example, 0.20); but the copied-in Makefile.in.in will be for the exact
version specified in configure.ac (in this case, 0.19).
In that situation, the gettext build rules will error out at `make` time
with the message:
*** error: gettext infrastructure mismatch: using a Makefile.in.in
from gettext version 0.19 but the autoconf macros are from gettext
version 0.20
Avoid that by specifying a minimum version dependency rather than an
exact one. This should not cause problems as we haven’t committed any
generated or external gettext files into git, so each developer will end
up regenerating the build system for their system’s version of gettext,
as expected.
See the subsection of
https://www.gnu.org/software/gettext/manual/html_node/Version-Control-Issues.html
for more information.
Note that autoreconf currently doesn’t recognise
AM_GNU_GETTEXT_REQUIRE_VERSION, so we must continue also using
AM_GNU_GETTEXT_VERSION. autopoint will ignore the latter if the former
is present. See
https://lists.gnu.org/archive/html/autoconf-patches/2015-10/msg00000.html.
[lkundrak@v3.sk: Fixed the meson build, adjusted autogen.sh:
droped "|| exit 1", dropped call to aclocal,
dropped --copy from gtkdocize.]
Apt is run for each package separately and errors are ignored. This is
not great -- it's slow and ignores errors. Therefore we sometimes end
up without packages we need.
Let's tolerate errors only for packages that we are know can fail to
install safely.
It's just convenient to have some tools around, not only
for testing, but also for (some limited) development.
In particular, because we bind-mount .vimrc inside the container, and
if I use vim, black/clang-format is just one key binding away.
You can of course just clone NetworkManager repository and start hacking
as you like. However, there are a few things like git-notest which are
interesting to setup.
Add a script to do this.
The script is supposed to be idempotent and do nothing, unless
necessary. By default it also only prints what it would do.
NetworkManager does not support by default legacy ifcfg configuration
files anymore, this support is now provided in a separate package
(https://fedoramagazine.org/converting-networkmanager-from-ifcfg-to-keyfiles/).
ifcfg directory (/etc/sysconfig/network-scripts/) should always be present,
regardless of NetworkManager support for network scripts. This change makes the
directory always present, not only when the recently splitted ifcfg subpackage
is installed, and also make it persistent after the package removal.
Fixes: 50a6627fd7 ('rpm: split ifcfg-rh settings plugin into a separate package')
It doesn't work anymore:
$ git clone git://github.com/thom311/libnl.git
Cloning into 'libnl'...
fatal: remote error:
The unauthenticated git protocol on port 9418 is no longer supported.
Please see https://github.blog/2021-09-01-improving-git-protocol-security-github/ for more information.
On recent Fedora and RHEL we no longer have differing "rpm_version"
and "real_version". So usually "rpm_version" is just the same as
"real_version".
Update the template spec file to reflect that. For the "build_clean.sh"
script, we anyway always set them both to "__VERSION__".
With "rc1" mode, we install more than one tarballs (the one for 1.37.90
and 1.39.0). If we reach this point, we already pushed the git tags.
There is no way back.
Ignore errors at first and try to release all tarballs. Only signal the error
at the end.
"find-backports" script parses the commit messages to figure out which
patches to backport. We use "refs/notes/bugs" notes to extend the
meta data after the commit was merged. If you don't setup the
notes, the output is likely incomplete or wrong.
Yes, this is annoying. It requires you to setup the notes as described
in "CONTRIBUTING.md". Also because the "release.sh" script runs "find-backports",
so that means you cannot do releases without setting up the notes
(unless you manually disable running "find-backports"). But you really shouldn't
make a release based on incomplete information.
"Ignore-Backport:" is already in use. For the find-backports script it
has the same meaning as a "cherry picked from" line, that means, we
assume that the referenced patch was backported already and the fix
applied.
This is of course useful to make the script shut up about backports that
we don't want to do. However, it requires us to tag the old branch
with this, so that the script thinks that the patch is already there.
Imaging we have a wrong commit on "next" branch with a Fixes line. We
don't want to backport it, so we would have to tag the "old" branch with
"Ignore-Backport:". That is cumbersome.
Instead, now also support that if a commit contains a "Fixes:" line any
an "Ignore-Fixes:" for the same fixed commit, then this let's the
"Fixes:" line be ignored.
This is more for completeness, to go along "nm-code-format.sh"
script.
Usually it's very simple to run black directly (you may still do that).
However, black by default only reformats files with ".py" extension.
So to get all our python files, you'd need to know and explicitly
select them... or use this script.
Also, `black .` scans the entire source tree, and is rather slow.
This script knows which files to select and is thus faster.
By having a ".md" extension, gitlab renders a nice page instead of
showing as plain text.
Currently our README is pretty bad. Partly, because it doesn't get
shown nicely.
Rename. The file effectively was already markdown. The old file is
gone.
For this we also need to change the automake flavor to "foreign"
(See [1]).
[1] https://autotools.info/automake/options.html#automake.options.flavors
