We should write our CONTRIBUTING files in markdown syntax, because
it's nice to read a plain text and gets nicely rendered.
However, if the file doesn't have a ".md" extension, gitlab's
web interface shows it as plain text file.
Rename the file.
This possibly breaks links like [1], but referring to a branch name
(and not a commit ID or a tag) is anyway fragile. Hence, I don't try
to fix that by adding a symlink or similar, because I think that just
makes it more confusing.
[1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/blob/master/CONTRIBUTING
"--with test" does two things:
(1) it enables "-Werror" compiler option. We always enable all
compiler warnings we care about, but this option makes all
warnings fatal.
Compiler warnings depend on compiler version and build options.
It's hard to build without any compiler warnings, in particular
for *future* compiler versions which we don't know yet. It
is desirable that a SRPM from yesterday can also be build
tomorrow.
(2) it fails build if any unit tests fail. We always run all
unit tests, but "--with test" makes it fatal. Again, we
have many unit tests that interact with the system (that is,
make system calls, like creating IP addresses or write files).
It is surprisingly hard to get them pass 100% on all the systems
we care. For example, on copr a test setup randomly fails during
ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
nm_utils_ifname_cpy(ifr.ifr_name, TEST_IFNAME);
r = ioctl(fd, TUNSETIFF, &ifr);
It's not clear why, nor is it at all clear that there is a bug
in NetworkManager. Making tests fatal basically means that a build
on copr infrastructure fails with a probability from a few percent.
Enough to be seriously annoying.
Note that on copr we actually build "--with test", because we want to catch these
issues. Likewise for our CI builds we explicitly specify "--with test".
In general, we build with various build configurations (compiler warnings)
and run unit tests on a source package many times. Starting on the
developer machine (`make check`), gitlab-ci, copr builds,
NetworkManager-ci. If you build an SRPM with such sources, a failure
of the unit tests is much more likely a glitch than an actual issue.
This is about changing the default if you build a Fedora/RHEL package.
That is with the Fedora/RHEL packages that are build in koji/brew.
Well, at least usually. In practice, we don't build frequently on non
x64_86 archs, so what I said there is less true. But the package build
is not there to replace CI/testing. The package build is there to get
a (mostly) working binary.
Note that RHEL packages anyway go through rpmdiff too, and rpmdiff
parses the build log and complain if `make check` fails.
This reverts commit e68e5c0a4c.
Currently "src/" mostly contains the source code of the daemon.
I say mostly, because that is not true, there are also the device,
settings, wwan, ppp plugins, the initrd generator, the pppd and dhcp
helper, and probably more.
Also we have source code under libnm-core/, libnm/, clients/, and
shared/ directories. That is all confusing.
We should have one "src" directory, that contains subdirectories. Those
subdirectories should contain individual parts (libraries or
applications), that possibly have dependencies on other subdirectories.
There should be a flat hierarchy of directories under src/, which
contains individual modules.
As the name "src/" is already taken, that prevents any sensible
restructuring of the code.
As a first step, move "src/" to "src/core/". This gives space to
reorganize the code better by moving individual components into "src/".
For inspiration, look at systemd's "src/" directory.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/743
nm-cloud-setup is provided by sub-package "NetworkManager-cloud-setup",
which also has the manual page. The main package "NetworkManager" should
not also contain the manual page.
- accept directory names in the command line. In that case,
still honor the excluded files. That is a major improvement
for me, because I usually only want to reformat a directory
that I know has changed and it is fast to only process some
directories.
- pass all files at once to clang-format. For me that gives
a significant speed improvement (about 3 times faster), although
clang-format is only single threaded. Possibly clang-format could
even be faster by checking files in parallel.
In case of a style error, the script still falls back to
iterate over all files to find the first bad file and print
the full diff. But that is considered an unusual case.
- make it correctly work from calling it from a subdirectory.
In that case, we only check files inside that directory --
but still correctly honor the excluded files.
"shared/nm-std-aux/unaligned.h" is taken from systemd and frequently
re-imported via the "systemd" branch.
It is not our code, and should not be formatted with our clang-format.
Alpine is especially interesting because it uses musl as libc.
The build does not yet succeed. There are several issues that
need to be fixed.
However, it will be simpler to fix things, if we have tests
in place -- even if at the moment they are known to be broken.
See-also: https://git.alpinelinux.org/aports/tree/community/networkmanager?h=master
With Fedora 33+ and RHEL 9+, the default plugins are
"plugins=keyfile,ifcfg-rh", instead of "plugins=ifcfg-rh,keyfile".
Update our "NetworkManager.conf" file to reflect that.
"build_clean.sh" (and "build.sh") scripts can both create a source
tarball (via `make dist`/`make distcheck`), an SRPM (and a spec file),
or build RPMs from the SRPM.
Note that the generated spec file has various options, like
%bcond_without nmtui
%bcond_without debug
%bcond_without test
When building an RPM from the SRPM, you can specify the "--with" or
"--without" option for rpmbuild. This is also what the "-w" / "-W" options
for "build_clean.sh" do.
However, the SRPM still has the intrinsic defaults, and if you later
build an RPM from it, you would have to pass "--with" / "--without"
to rpmbuild.
Often that is not conveniently possible, for example, when you build the
SRPM in koji.
Extend the scripts so that also the defaults for "-w debug" and "-w
test" can be specified when generating the SRPM. You can do that with
the new options "--default-for-{debug,test}" to "build_clean.sh".
Alternatively, it suffices to specify the previously supported
"-w" / "-W" options. That way, we will pass those options to rpmbuild,
but also set them as defaults in the generate spec file. The new
options "--default-for-{debug,test}" are only needed if you want
the default in the spec file to be different then what you use
when creating the SRPM.
By default, "build_clean.sh" script likes to automatically add "-w test"
-- unless the user specified "-w test" or "-W test" on the command line.
That is mostly fine. However, the spec file has an internal default for the
"test" option. So if you want to use the default that gets determined
by the spec file, then we should suppress that automatism.
We always run the unit tests during package build and also enable all compiler
warnings. However, by default we used to ignore failures. That is, because
rebuilding a package on another, future distro led to frequent, annoying build
failures. Especially compiler warnings appear easily when using a
different compiler version.
The default mostly matters here when you want to build the package in
brew/koji, where you don't have a possibility to explicitly select the
build option.
Note that rpmdiff detects failures in the build log, and thus we usually
would not miss failures for builds we add to errata. Also, all our CI
tests build packages with a manner where they would not allow a failure
of the unit tests. So, we run these unit tests frequently and in a
manner where we notice a failure.
For rhel-9 builds, change the default here and let test failures and
compiler warnings be fatal to the build.
"find-backports" searches commit messages of upstream branches for
"Fixes:" comments. Those will then be highlighted to be backported,
if the script determines that to be necessary.
"find-backports" also honors the "cherry picked from" comments, to detect when
a patch was already backported. That is thus a way to suppress reporting a
commit to be backported.
Add another way to flag commits so they don't need backporting. Via
"Ignore-Backport:" tag.
As "find-backports" also honors "refs/notes/bugs" notes, this can be used
like:
git notes \
--ref refs/notes/bugs \
append \
-m "Ignore-Backport: e""29f00fa0c69 ('NEWS: fix entry that is targeted for 1.30 instead of 1.28')" \
2''3364aa8f3bd6b11e2ac9e30117eaabfe1f3a9f2
The checkpatch test tests the patches on the merg-request, as they
branch off from master (or one of the stable branches).
It thus need the full git history, but the git repository might be a
shallow clone. Fix it.
"debian/REQUIRED_PACKAGES" is used by gitlab-ci to prepare the image. We require
"udev" package, if only to install "/usr/share/pkgconfig/udev.pc" to get the
udev directory.
Otherwise build fails with:
Run-time dependency udev found: NO (tried pkgconfig)
meson.build:371:2: ERROR: Dependency "udev" not found, tried pkgconfig
These packages are also used during CI tests (in the checkpatch
stage). They are also used for formatting C/python code, and thus
useful for developing.
Install them as part of REQUIRED_PACKAGES script.
We already build a large variety of configurations in gitlab-ci,
we don't need yet another configuration to run tests on travis-ci.
Also, because the travis-ci setup is outdated and we don't look
at it. Let's focus on gitlab-ci instead.
We use clang-format to format our code, and the exact format depends on
the clang version. Currently we use clang-11, as packaged in Fedora 33.
Add a script that runs a Fedora 33 container with podman and reformats
the current working directory.
Usage:
./contrib/scripts/nm-code-format-container.sh
Our new format gets enforced by clang-format, and we now only use
four space indentation, instead of tabs.
Adjust the checkpatch script to account for that.
Also, now there are probably no cases left where we want to see any
tabs in our sources. Complain about any tabs we find.
Add a new `main.rc-manager=auto` setting, that favours to use
systemd-resolved (and not touch "/etc/resolv.conf" but configure
it via D-Bus), or falls back to `resolvconf`/`netconfig` binaries
if they are installed and enabled at compile time.
As final fallback use "symlink", like before.
Note that on Fedora there is no "openresolv" package ([1]). Instead, "systemd"
package provides "/usr/sbin/resolvconf" as a wrapper for systemd-resolved's
"resolvectl". On such a system the fallback to resolvconf is always
wrong, because NetworkManager should either talk to systemd-resolved
directly or not but never call "/usr/sbin/resolvconf". So, the special handling
for resolvconf and netconfig is only done if NetworkManager was build with these
applications explicitly enabled.
Note that SUSE builds NetworkManager with
--with-netconfig=yes
--with-config-dns-rc-manager-default=netconfig
and the new option won't be used there either. But of course, netconfig
already does all the right things on SUSE.
[1] https://bugzilla.redhat.com/show_bug.cgi?id=668153
Suggested-by: Jason A. Donenfeld <Jason@zx2c4.com>
With Fedora 33, LTO will be enabled by default via CFLAGS in
redhat-rpm-config ([1]).
That basically sets "CFLAGS=-flto -ffat-lto-objects".
Note that we have our own configure/meson option to enable LTO.
With "--with-lto" we set CFLAGS="-flto -flto-partition=none". This
is necessary due ([2], [3]).
So, disable Fedora's automatism, but turn on the suitable configure
option to get working LTO.
[1] 5baaf4a99c?branch=master
[2] e6cf4213a7 ('build: fix building with LTO')
[3] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48200#c28
This defaults to $DO_RELEASE. In that case, the script will also GPG sign
the source tarball.
The purpose is that when we do a release we want to ensure that the
published tarball is really the one that we generated. In that case,
the SHA sum would suffice, however that requires you to manually note
it down and compare the result. With the gpg signature, that
verification can be better automated.
Currently only "minor" and "devel" releases are implement. It's also not yet
tested with --no-dry-run, because that would actually create a release.
Test it when using it the next time.
On CentOS 8, many devel packages are not available. Even after
# dnf config-manager --set-enabled PowerTools
certain devel packages are missing. Some of these (libndp-devel,
mobile-broadband-provider-info-devel, teamd-devel) we build in copr
([1]), but libpsl-devel and qt-devel are still missing.
Only install them optionally and allow failure for them not being
present.
[1] https://copr.fedorainfracloud.org/coprs/nmstate/nm-build-deps/repo/epel-8/nmstate-nm-build-deps-epel-8.repo
When doing a release, we should care about the checksum of the tarball.
Log all of them... also, because fedpkg uses sha512, ftpadmin@gnome uses
sha256, etc.
error: bare words are no longer supported, please use "...": no != "yes"
error: ^
error: /builds/NetworkManager/NetworkManager/contrib/fedora/rpm/NetworkManager.20200418-170120.dp5cp5/SPECS/NetworkManager.spec:596: bad %if condition: no != "yes"
error: bare words are no longer supported, please use "...": no != yes
error: /builds/NetworkManager/NetworkManager/contrib/fedora/rpm/NetworkManager.20200418-163008.VM582H/SPECS/NetworkManager.spec:596: bad %if condition: no != yes
error: bare words are no longer supported, please use "...": "x" != x
error: ^
error: /root/nm-build/NetworkManager/contrib/fedora/rpm/NetworkManager.20200402-030113.Hk7EGs/SPECS/NetworkManager.spec:32: bad %if condition: "x" != x
ERROR: rpmbuild FAILED
We always build PolicyKit support, because it merely depends on some
D-Bus calls. However, there are two things to configure:
- the default value for main.auth-polkit in NetworkManager.conf. This
is now called "-Dconfig_auth_polkit_default=$VAL".
- whether to install the policy file. This is called "-Dpolkit=$VAL".
These settings are mostly independent, so add "config_auth_polkit_default" to
make the default explicitly configurable.
(cherry picked from commit c21c6bc0be)
The bluetooth plugin (with BlueZ5/NAP support) always gets
build, but DUN support requires a library.
When enabling build of the bluetooth subpackage, then always
enable DUN support. And enable it explicitly, especially meson
would not autodetect support and disable it by default.
(cherry picked from commit 30f6a5dd21)
The "ibft" plugin is no more. The default on RHEL/Fedora is now "ifcfg-rh[,keyfile]".
Adjust the configuration, because a wrong comment is confusing here.
Modifying configuration snippets is potentially annoying, because the user might
have edited the file, so on upgrade a "NetworkManager.conf.rpmnew" file
will be created. Still do it.
warning: extra tokens at the end of %endif directive in line 717: %endif # end autotools
warning: extra tokens at the end of %endif directive in line 775: %endif # end autotools
When opening a merge request from a fork of NetworkManager, then the
pipeline runs with the a checkout of the fork. That means, checkpatch
would compare the branch against "master" (or "nm-x-y" stable branches)
of the fork, instead of upstream.
That doesn't seem too useful. Instead, also add upstream NetworkManager
as git remote, fetch the branches, and use the branches from there as
base for checkpatch.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/255
The preferred logo and logotype are the ones in the main "logo" folder.
There are also few variants available in the "alternate" folder that are
allowed.
Main color for the logo is blue #32557dff.
The alternatei logo red is #cc0000ff.
The font is Montserrat.
Thanks to Máirín Duffy for all the help and support!
REQUIRED_PACKAGES has two uses:
- to setup a system for developing NetworkManager. This installs
convenience packages like "cscope".
- to install the packages required for unit testing in gitlab-ci.
For gitlab-ci we should only install the packages that we actually
need.
Previously, dnf/yum used to ignore packages that didn't exist.
In Fedora 32, dnf starts to fail the entire command:
No match for argument: python-gobject-base
Error: Unable to find a match: python-gobject-base
Since this script is supposed to work with different RHEL/Fedora
versions, it's expected that not all packages are available everywhere.
Fix that, by installing packages that we know that they might be missing
one by one (and ignore the error).
"NetworkManager-wifi" package requires either wpa_supplicant or iwd.
When installing the package without explicitly installing supplicant
or iwd (and not having it installed yet), then we want to drag in
wpa_supplicant by default. That is accomplished by suggesting wpa_supplicant
package.
Otherwise, the user installing NetworkManager-wifi might get iwd,
which is only functioning if the user explicitly enables the backend
in "NetworkManager.conf".
https://bugzilla.redhat.com/show_bug.cgi?id=1743585
This is a one-off hacky tool that we'll use to convert the long license
boilerplates to SPDX headers that are more friendly to automated tools.
Then we can drop it and forget it existed.
Nowadays, we should prefer "/run" over "/var/run". When not specifying
during ./configure, autotools however still defaults to "/var/run".
This default is also visible in the pre-generated documenation, for
example `man NetworkManager.conf` says
Unless the symlink points to the internal file /run/NetworkManager/resolv.conf,
in which case the ...
Let's enable the option to use IWD as an alternative to wpa_supplicant
for Wi-Fi support. People have been asking for this, it works, and is well
maintained.
Completely rework how settings plugin handle connections and how
NMSettings tracks the list of connections.
Previously, settings plugins would return objects of (a subtype of) type
NMSettingsConnection. The NMSettingsConnection was tightly coupled with
the settings plugin. That has a lot of downsides.
Change that. When changing this basic relation how settings connections
are tracked, everything falls appart. That's why this is a huge change.
Also, since I have to largely rewrite the settings plugins, I also
added support for multiple keyfile directories, handle in-memory
connections only by keyfile plugin and (partly) use copy-on-write NMConnection
instances. I don't want to spend effort rewriting large parts while
preserving the old way, that anyway should change. E.g. while rewriting ifcfg-rh,
I don't want to let it handle in-memory connections because that's not right
long-term.
--
If the settings plugins themself create subtypes of NMSettingsConnection
instances, then a lot of knowledge about tracking connections moves
to the plugins.
Just try to follow the code what happend during nm_settings_add_connection().
Note how the logic is spread out:
- nm_settings_add_connection() calls plugin's add_connection()
- add_connection() creates a NMSettingsConnection subtype
- the plugin has to know that it's called during add-connection and
not emit NM_SETTINGS_PLUGIN_CONNECTION_ADDED signal
- NMSettings calls claim_connection() which hocks up the new
NMSettingsConnection instance and configures the instance
(like calling nm_settings_connection_added()).
This summary does not sound like a lot, but try to follow that code. The logic
is all over the place.
Instead, settings plugins should have a very simple API for adding, modifying,
deleting, loading and reloading connections. All the plugin does is to return a
NMSettingsStorage handle. The storage instance is a handle to identify a profile
in storage (e.g. a particular file). The settings plugin is free to subtype
NMSettingsStorage, but it's not necessary.
There are no more events raised, and the settings plugin implements the small
API in a straightforward manner.
NMSettings now drives all of this. Even NMSettingsConnection has now
very little concern about how it's tracked and delegates only to NMSettings.
This should make settings plugins simpler. Currently settings plugins
are so cumbersome to implement, that we avoid having them. It should not be
like that and it should be easy, beneficial and lightweight to create a new
settings plugin.
Note also how the settings plugins no longer care about duplicate UUIDs.
Duplicated UUIDs are a fact of life and NMSettings must handle them. No
need to overly concern settings plugins with that.
--
NMSettingsConnection is exposed directly on D-Bus (being a subtype of
NMDBusObject) but it was also a GObject type provided by the settings
plugin. Hence, it was not possible to migrate a profile from one plugin to
another.
However that would be useful when one profile does not support a
connection type (like ifcfg-rh not supporting VPN). Currently such
migration is not implemented except for migrating them to/from keyfile's
run directory. The problem is that migrating profiles in general is
complicated but in some cases it is important to do.
For example checkpoint rollback should recreate the profile in the right
settings plugin, not just add it to persistent storage. This is not yet
properly implemented.
--
Previously, both keyfile and ifcfg-rh plugin implemented in-memory (unsaved)
profiles, while ifupdown plugin cannot handle them. That meant duplication of code
and a ifupdown profile could not be modified or made unsaved.
This is now unified and only keyfile plugin handles in-memory profiles (bgo #744711).
Also, NMSettings is aware of such profiles and treats them specially.
In particular, NMSettings drives the migration between persistent and non-persistent
storage.
Note that a settings plugins may create truly generated, in-memory profiles.
The settings plugin is free to generate and persist the profiles in any way it
wishes. But the concept of "unsaved" profiles is now something explicitly handled
by keyfile plugin. Also, these "unsaved" keyfile profiles are persisted to file system
too, to the /run directory. This is great for two reasons: first of all, all
profiles from keyfile storage in fact have a backing file -- even the
unsaved ones. It also means you can create "unsaved" profiles in /run
and load them with `nmcli connection load`, meaning there is a file
based API for creating unsaved profiles.
The other advantage is that these profiles now survive restarting
NetworkManager. It's paramount that restarting the daemon is as
non-disruptive as possible. Persisting unsaved files to /run improves
here significantly.
--
In the past, NMSettingsConnection also implemented NMConnection interface.
That was already changed a while ago and instead users call now
nm_settings_connection_get_connection() to delegate to a
NMSimpleConnection. What however still happened was that the NMConnection
instance gets never swapped but instead the instance was modified with
nm_connection_replace_settings_from_connection(), clear-secrets, etc.
Change that and treat the NMConnection instance immutable. Instead of modifying
it, reference/clone a new instance. This changes that previously when somebody
wanted to keep a reference to an NMConnection, then the profile would be cloned.
Now, it is supposed to be safe to reference the instance directly and everybody
must ensure not to modify the instance. nmtst_connection_assert_unchanging()
should help with that.
The point is that the settings plugins may keep references to the
NMConnection instance, and so does the NMSettingsConnection. We want
to avoid cloning the instances as long as they are the same.
Likewise, the device's applied connection can now also be referenced
instead of cloning it. This is not yet done, and possibly there are
further improvements possible.
--
Also implement multiple keyfile directores /usr/lib, /etc, /run (rh #1674545,
bgo #772414).
It was always the case that multiple files could provide the same UUID
(both in case of keyfile and ifcfg-rh). For keyfile plugin, if a profile in
read-only storage in /usr/lib gets modified, then it gets actually stored in
/etc (or /run, if the profile is unsaved).
--
While at it, make /etc/network/interfaces profiles for ifupdown plugin reloadable.
--
https://bugzilla.gnome.org/show_bug.cgi?id=772414https://bugzilla.gnome.org/show_bug.cgi?id=744711https://bugzilla.redhat.com/show_bug.cgi?id=1674545
This is inspired by the existing systemd integration, with a few differences:
* This parses the WPAD option, which systemd requested, but did not use.
* We hook into the DAD handling, only making use of the configured address
once DAD has completed successfully, and declining the lease if it fails.
There are still many areas of possible improvement. In particular, we need
to ensure the parsing of all options are compliant, as n-dhcp4 treats all
options as opaque, unlike sd-dhcp4. We probably also need to look at how
to handle failures and retries (in particular if we decline a lease).
We need to query the current MTU at client startu, as well as the hardware
broadcast address. Both these are provided by the kernel over netlink, so
it should simply be a matter of hooking that up with NM's netlink layer.
Contribution under LGPL2.0+, in addition to stated licenses.
In continations (that use spaces for alignment), don't allow the number
of leading tabs to change. Previously only removal of tabs was
disallowed, but addition doesn't make sense either, as only spaces
should be used for further alignemnt.
This catches situations like this:
|<-tab->all_work_and_no_play (makes,
|<-tab-> jack,
|<-tab-><-tab-> a dull boy);
The functionality of the ibft settings plugin is now handled by
nm-initrd-generator. There is no need for it anymore, drop it.
Note that ibft called iscsiadm, which requires CAP_SYS_ADMIN to work
([1]). We really want to drop this capability, so the current solution
of a settings plugin (as it is implemented) is wrong. The solution
instead is nm-initrd-generator.
Also, on Fedora the ibft was disabled and probably on most other
distributions as well. This was only used on RHEL.
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1371201#c7
We no longer add these. If you use Emacs, configure it yourself.
Also, due to our "smart-tab" usage the editor anyway does a subpar
job handling our tabs. However, on the upside every user can choose
whatever tab-width he/she prefers. If "smart-tabs" are used properly
(like we do), every tab-width will work.
No manual changes, just ran commands:
F=($(git grep -l -e '-\*-'))
sed '1 { /\/\* *-\*- *[mM]ode.*\*\/$/d }' -i "${F[@]}"
sed '1,4 { /^\(#\|--\|dnl\) *-\*- [mM]ode/d }' -i "${F[@]}"
Check remaining lines with:
git grep -e '-\*-'
The ultimate purpose of this is to cleanup our files and eventually use
SPDX license identifiers. For that, first get rid of the boilerplate lines.
It doesn't have to be at the end of line, there may be more words
following.
Fixes: d66a1ace23 ('contrib/checkpatch: avoid command injection in checkpatch.pl script')
For the most part, we only have one main .gitignore file.
There were a few nested files, merge them into the main file.
I find it better to have only one gitignore file, otherwise the
list of ignored files is spread out through the working directory.
On Ubuntu 16.04 (trusty) valgrind fails due to rdrand being advertised
but not implemented.
Work around that by installing valgrind from Ubuntu 18.04 (bionic) via
the "contrib/scripts/nm-ci-install-valgrind-in-ubuntu1604.sh" script.
This change is a bit annoying, because we package "NetworkManager.conf" file in
our RPM. So, upon package upgrade, rpm will note that a new config file should
be installed and thus will leave ".rpmnew" files.
Also, don't mention "/var/run". It should really be just "/run" because
"/var" might not be mounted in early boot/initrd or in rescue environment.
This is a provide packages that install dispatcher scripts should depend
on. It will make it easier to keep track of them and possibly split out
the dispatcher into an optional package if not needed.
We also generate a source tarball and artifact it.
Hence, we need proper gtk-doc links. This requires files in
/usr/share/gtk-doc/html for adding cross links. Install glib2-doc
package.
Note that in containers dnf is configured to not install documentation
files. We need to override that.
This removes libnm-glib, libnm-glib-vpn, and libnm-util for good.
The it has been replaced with libnm since NetworkManager 1.0, disabled
by default since 1.12 and no up-to-date distributions ship it for years
now.
Removing the libraries allows us to:
* Remove the horrible hacks that were in place to deal with accidental use
of both the new and old library in a single process.
* Relief the translators of maintenance burden of similar yet different
strings.
* Get rid of known bad code without chances of ever getting fixed
(libnm-glib/nm-object.c and libnm-glib/nm-object-cache.c)
* Generally lower the footprint of the releases and our workspace
If there are some really really legacy users; they can just build
libnm-glib and friends from the NetworkManager-1.16 distribution. The
D-Bus API is stable and old libnm-glib will keep working forever.
https://github.com/NetworkManager/NetworkManager/pull/308
Enabling eBPF causes src/devices/tests/test-acd to fail:
strace: bpf(BPF_MAP_CREATE, {map_type=BPF_MAP_TYPE_HASH, key_size=4, value_size=1, max_entries=8, map_flags=0, inner_map_fd=0, map_name="", map_ifindex=0, btf_fd=0, btf_key_type_id=0, btf_value_type_id=0}, 112) = -1 EPERM (Operation not permitted)
NetworkManager-Message: 10:07:04.404: <warn> [1554631624.4046] acd[0xa2b400,10]: couldn't init ACD for announcing addresses on interface 'nm-test-veth0': Operation not permitted
Interestingly it does not always fail. Seems to depend on the kernel
which is used in the containerized test environments of gitlab-ci.
For now, just disable eBPF and use the fallback implementation.
For better or worse, our release builds commonly do not disable assertions.
That means,
- NDEBUG is not set, and assert() is in effect
- G_DISABLE_ASSERT is not set, and g_assert() is in effect
- G_DISABLE_CHECKS is not set, and g_return*() is in effect.
On the other hand, NM_MORE_ASSERTS is not enabled by default and nm_assert()
is stripped away. That is the actual purpose of nm_assert(): it is
commonly disabled on release builds, while all other assertions are
enabled.
Note that it is fully supported to build NetworkManager with all kind of
assertions disabled. However, such a configuration is not much tested
and I would not recommend it for that reason.
%meson expands to
$ /usr/bin/meson --buildtype=plain --prefix=/usr --libdir=/usr/lib64 --libexecdir=/usr/libexec --bindir=/usr/bin --sbindir=/usr/sbin --includedir=/usr/include --datadir=/usr/share --mandir=/usr/share/man --infodir=/usr/share/info --localedir=/usr/share/locale --sysconfdir=/etc --localstatedir=/var --sharedstatedir=/var/lib --wrap-mode=nodownload --auto-features=enabled -Db_ndebug=true . x86_64-redhat-linux-gnu $OTHER_ARGS
thus passing -DNDEBUG to the meson build. Override that.
This removes libnm-glib, libnm-glib-vpn, and libnm-util for good.
The it has been replaced with libnm since NetworkManager 1.0, disabled
by default since 1.12 and no up-to-date distributions ship it for years
now.
Removing the libraries allows us to:
* Remove the horrible hacks that were in place to deal with accidental use
of both the new and old library in a single process.
* Relief the translators of maintenance burden of similar yet different
strings.
* Get rid of known bad code without chances of ever getting fixed
(libnm-glib/nm-object.c and libnm-glib/nm-object-cache.c)
* Generally lower the footprint of the releases and our workspace
If there are some really really legacy users; they can just build
libnm-glib and friends from the NetworkManager-1.16 distribution. The
D-Bus API is stable and old libnm-glib will keep working forever.
https://github.com/NetworkManager/NetworkManager/pull/308
The capture variables, $1, etc, are not valid unless the match
succeeded, and they're not cleared, either.
$ git checkout -B C origin/master && \
echo XXXXX > f.txt && \
git add f.txt && \
git commit -m 'this commit does something()'
Branch 'C' set up to track remote branch 'master' from 'origin'.
Reset branch 'C'
Your branch is up to date with 'origin/master'.
sh: -c: line 0: syntax error near unexpected token `('
sh: -c: line 0: `git log --abbrev=12 --pretty=format:"%h ('%s')" -1 does something() 2>/dev/null'
>>> VALIDATE "a169a98e14 this commit does something()"
(commit message):4: Commit 'does something()' does not seem to exist:
> Subject: [PATCH] this commit does something()
(commit message):4: Refer to the commit id properly: :
> Subject: [PATCH] this commit does something()
The patch does not validate.
In newer RPM file triggers in glibc package take care of this. While
these scriptlets whould do no harm there, removing them yields a tiny
theoretical performance improvement.
Group tag is not required, though is harmless. We could either remove it,
or keep it, but there's absolutely no excuse for conditionalizing it.
Let's keep it for now, because rpm -i still prints it.
This reverts commit 1feeba6f1a.
The "vala-tools" package was merged into "vala" [1]. While "vala"
now "Provides: vala-tools", update the build requirements for
Fedora 30 and newer.
[1] 82b21cc302f6c878a04a
This enables -Werror for meson builds on gitlab-ci and semaphore.
Not on Travis, the compiler there is too old, giving too many bogus
warnings.
This reverts commit 928d68d04a ("m4:
disable -Wmissing-braces for newer clang").
Also, let one docker image do multiple builds. We fetch a fedora docker
image, and then install 250 MB of packages. That alone takes a lot of
time and resources. Instead of running a large number of docker images
that only do one build, let one image do several builds.
Also, install ccache. Hopefully this way we can benefit from
building the same sources multiple times.
Also note that building docs does not work currently with clang,
due to g-ir-scanner. See commit 05568860cce5332977d92b85f7c25b8ed646cd58.
g-ir-scanner does not support building with clang, due to [1], [2], [3].
It triggers
checking if /usr/bin/g-ir-scanner works... no (compiler failure -- check config.log)
configure: error: introspection enabled but can't be used
with
clang-7: error: unknown argument: '-fstack-clash-protection'
See also commit 99b92fd992, which adds this configure
check.
Honor the environment variable WITH_DOCS to allow the caller to overwrite
the automatic detection that the script does.
[1] https://bugzilla.gnome.org/show_bug.cgi?id=757934
[2] https://gitlab.gnome.org/GNOME/gobject-introspection/issues/150
[3] c14d037228
To run the tests with python3, we need python3-gobject.
Note that "contrib/fedora/REQUIRED_PACKAGES" is called by
"contrib/scripts/nm-ci-run.sh" script to install the packages
in Fedora.
Otherwise the following fails:
$ ./contrib/fedora/rpm/build_clean.sh -g -s x.1
...
error: parse error in expression
error: /data/src/_NetworkManager/contrib/fedora/rpm/NetworkManager.20190207-165257.XOkW4i/SPECS/NetworkManager.spec:35: bad %if condition
ERROR: rpmbuild FAILED
Even with the fix, not all characters are allowed:
$ ./contrib/fedora/rpm/build_clean.sh -g -s x-1
...
error: line 112: Illegal char '-' (0x2d) in: Release: 22165.x-1.25b13e2053.fc29
ERROR: rpmbuild FAILED
Now that the default for the internal client is "mac", we don't need
this snippet anymore. Drop it.
Don't renumber the source files but leave the gap at Source3. Everytime
we add config snippets the numbers need to be shuffled, so don't fill
the gap and maybe use it in the future.
https://bugzilla.redhat.com/show_bug.cgi?id=1661165
This allows us to add a file "TODO.txt" in the top level directory.
This file is not intended to be merged to master, but keep track of
stuff that is still to do before merging a branch.
Let checkpatch.pl warn about the presence of such a file.
meson is able to get variables defined in pkg-config files such as
directory paths. PolicyKit defines in its pkg-config file the path to
the directory where `policy` files are present.
This removes the `polkit_dir` option to ease the move to start using
those variables. The `polkit` variable has also been converted to
boolean.
Fedora spec script has also been updated accordingly.
For older NetworkManager versions, a match spec that only contained except:
specifiers could never yield a positive match. That is not very useful and
got fixed by commit 242de347adbf653a709607979d36a0da1ca3ff0b (core: fix
device spec matching for a list of "except:").
Still, adjust the configuration snippet so that it also works with
configurations that predate the fix.
For one, it's not unreasonable that we want to run the same
tests both for gitlab and travis.
Move the actual tests into a script, which is called by both
CI environments.
We still can do something different, based on the environment.
The advantage here is, that the common part will be shared, and
the places where we differ can easily be spot.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/44
The ebpf syscall doesn't work on RHEL even if the linux/bpf.h header
is available: let's explicitly disable it.
On Fedora explicitly enable eBPF instead of autodetecting it.
This allows us to somehow work with no PCI (or USB for that matter), such
as some virtual machines. Old ModemManager doesn't like this though, but
there doesn't seem to be a way around it.
https://github.com/NetworkManager/NetworkManager/pull/246
While this is packaged in "NetworkManager-config-server.rpm"
sub-package, it's not in "00-server.conf" file. The reason
is that a convenient way to disable configuration from
"/usr/lib/NetworkManager/conf.d", is by putting a (possibly empty)
file into /etc directory with the same name. If the sub-package
only provides one large "00-server.conf" file, this is no longer
possible at a granular level.
We have a few source code tags like "TODO" and "FIXME".
"XXX" is not intended to be merged, it is for marking
places in code while still working on it.
autotools build has/had a bug, where ibft test files would only be disted
if the ibft plugin was enabled.
Regardless of that, `build_clean.sh --release` is our suggested way to
create a release tarball. It should always enable the ibft plugin.
It didn't do so, due to a bug.
On RHEL, openvswitch package is not in the base set of packages. Hence,
we cannot depend NetworkManager-ovs package on openvswitch.
This isn't really a problem, because NetworkManager's OVS plugin must
anyway behave graceful when openvswich service is not running or not
available. It only means, that a user who wants to use the OVS plugin
needs to explicitly install the openvswitch package.
https://bugzilla.redhat.com/show_bug.cgi?id=1629178https://bugzilla.redhat.com/show_bug.cgi?id=1633190
The main purpose of "checkpatch-feature-branch.sh" is to test all
patches of a feature branch. When we run the script against master
(or nm-1-*), then there is no feature branch.
Previously, the script would just error out.
That is not very useful, in particular as we call this from gitlab-ci,
which also runs on master.
Instead, in that case, test the HEAD.
This takes current HEAD branch, and finds all the commits what
are not on master or one of the nm-1-* branches, and runs
checkpatch.pl on each.
The use is to run checkpatch.pl on all patches of a feature
branch.
If the NetworkManager daemon has been stopped manually we don't want it
to be autostarted by a client request.
[lkundrak@v3.sk: The auto-activation is probably more surprising than useful.
Services that need NetworkManager API should depend on NetworkManager service
directly.
I have no idea what purpose does the D-Bus service file serve nowadays,
but it looks rather hacky (really, activating /bin/false) and the comment
in it suggests that the autoactivating behavior was not intended anyway.
Debian has been shipping this for quite some time and no complains have been
heard.]
https://github.com/NetworkManager/NetworkManager/pull/230
Add support for building with meson, enabled by '--with meson' so that
we can regularly test the whole build+test+install procedure with
meson. I compared the RPM contents of NM, NM-libnm, NM-libnm-devel
packages and they match the autotools ones. It's also faster:
$ time contrib/fedora/rpm/build_clean.sh -g -Q -f
real 3m54.239s
user 11m15.000s
sys 1m28.456s
$ time contrib/fedora/rpm/build_clean.sh -g -Q -f -w meson
real 3m9.938s
user 9m5.225s
sys 1m4.392s
In NetworkManager-libnm-devel we ship the same documentation in two
different places:
/usr/share/doc/NetworkManager-libnm-devel
/usr/share/gtk-doc/html/NetworkManager
Remove the former, which was added in commit e01c17523a.
Also, remove the same documentation from NetworkManager-glib-devel
since it's already present in NetworkManager-libnm-devel.
nm-initrd-generator scans the command line for options relevant to network
configuration and creates configuration files for an early instance of
NetworkManager run from the initial ramdisk during early boot.
Set the execution bit on /usr/sbin/{ifup,ifdown} ghost files to match
the mode of same files installed by initscripts.
Otherwise, they will appear as changed according to rpm verify:
.M....... g /usr/sbin/ifdown
.M....... g /usr/sbin/ifup
when the alternatives mechanism is not in place.
# ll /usr/sbin/if{up,down}
-rwxr-xr-x. 1 root root 1651 Aug 24 06:23 /usr/sbin/ifdown
-rwxr-xr-x. 1 root root 5010 Aug 24 06:23 /usr/sbin/ifup
https://bugzilla.redhat.com/show_bug.cgi?id=1626517
(cherry picked from commit d8a972c575)
Seems rpmbuild does not honor the latest occurance with
--with test --without test
to disable tests. Work around that.
Fixes: ad850c4f03
(cherry picked from commit cc8c207120)
In general, when we build a package, we want no compiler warnings
and all unit tests to pass.
That is in particular true when building a package for the distribution
in koji. When builing in koji, we (rightly) cannot pass rpmbuild options, so
the default whether tests/compiler-warnings are fatal matter very much.
One could argue: let's have the tests/compiler-warnings fatal and fail the build.
During a build in koji for a Fedora release, we want them all pass. And if somebody
does a manual build, the person can patch the spec file (or use rpmbuild
flags).
However, note how commit "f7b5e48cdb contrib/rpm: don't force fatal warnings
with tests" already disabled fatal compiler warnings. Why? It seems
compiler warnings should be even more stable than our unit tests, as long
as you target a particular Fedora release and compiler version. So this
was done to support rebuilding an SRPM for a different Fedora release,
or to be more graceful during early development phase of a Fedora
release, where things are not as stable yet.
The exactly same reasoning applies to treating unit-tests failures as fatal.
For example, a recent iproute2 issue broke unit tests. That meant, with
that iproute2 release in build root, the NetworkManager RPM could not be built.
Very annoying.
Now:
- if "test" is enabled, that means both `make check` and compiler warnings
are treated fatal. If "test" is disabled, `make check` and compiler
warnings are still done, just not fatal.
- "test" is now disabled by default via the spec file. They are not fatal
when building in koji or when rebuilding the package manually.
- tests can be enabled optionally. Note that the "build_clean.sh"
script enables them by default. So, a user using this script would
need to explicitly "--without test".
(cherry picked from commit ad850c4f03)
- always enable more compiler warnings. They are not marked as breaking
the build anyway.
- also, always build with '--with-tests=yes'. Note that our autotools is
actually very nice. Even if you build '--with-tests=no', you still can
run `make check` and the tests are build on demand. The only
difference here is whether the tests are build during `make` or during
`make check`. While little difference, build everything during the
`make` step.
- when running tests, use `make -k check`. Even if they fail, we want to
run the entire test suite.
- also running tests are disabled, still run them. But don't let them
fail the build.
(cherry picked from commit 58b030f39a)
The correct way to create a tarball for release is
./contrib/fedora/rpm/build_clean.sh -r
Just ensure to issue this from a clean shell environment.
(cherry picked from commit 5894da67dc)
The NetworkManager spec file used to determine devel builds as those that
have an odd minor version number. In that case, the built package would
enable more-asserts.
-- By the way, why is '1.13.3-dev' considered a delopment version worthy of more
asserts, but a build from the development phase of the next minor release on
'nm-1-12' branch not?
Note that during the development phase of Fedora (and sometimes even afterwards),
we commonly package development versions from 'master'. For example '1.12.0-0.1',
which is some snapshot with version number '1.11.x-dev' (or '1.12-rc1' in this case),
but before the actual '1.12.0' release.
It's problematic that for part of the devel phase we compile the
package for the distribution with more assertions. This package is
significanly different and rpmdiff and coverity give different results
for them.
For example, the binary size of debug packages is larger, so first
rpmdiff will complain that the binary sized increased (compare to the
previous version) and then later it decreases again.
Likewise, coverity finds significantly different issues on a debug
build. For example, it sees assertions against NULL and takes that
as a hint as to whether the parameter can/shall be NULL. Keeping
coverity warnings low is already high effort to sort out false
positives. We should not invest time in checking debug builds with
coverity, at least not as long as there are more important issues.
But more importantly, the --with-more-asserts configure option governs whether
nm_assert() is enabled. The only point of existance of nm_assert() -- compared to
g_assert(), g_return_*() and assert() -- is that this variant is disabled by default.
It's only used for checks that are really really not supposed to fail and/or
which may be expensive to do. This is useful for developing and CI,
but it's not right to put into the distribution. It really enables
assertions that you don't want in such a scenario. Enabling them even
for distribution builds defeats their purpose. If you care about an
assertion to be usually/always enabled, you should use g_assert() or
g_return_*() instead.
What this changes, that "devel" builds in koji/brew do not have more-asserts
enabled. When manually building the SRPM one still can enable it,
for example via
$ ./contrib/fedora/rpm/build_clean.sh -w debug
Also our CI has an option to build packages with or without more-asserts
(defaulting to more asserts already).
(cherry picked from commit b4e2f83403)
Fedora is removing gcc from the default build-root [1], hence
require it.
Actually, we already have a "BuildRequires: libtool", which has a
dependancy on gcc and we already got it implicitly. Just make it
explicit.
[1] https://fedoraproject.org/wiki/Changes/Remove_GCC_from_BuildRoot
Don't use the integer type before signed/unsigned, but the
other way around. That is,
unsigned long var;
instead of
long unsigned var;
Also, just use "unsigned" instead of "unsigned int".
Tabs are not only wrong after a space, they are always
wrong if they don't appear at the beginning of a line.
That would happen usually, when trying to align multiple
lines like
enum {
VALUE1 = 1;
OTHER_VALUE = 2;
};
When doing that, the alignment will only be correct, if the
reader later uses the same tab-width. Note that in NetworkManager
we recommend the tab-width to be 4 characters, but with our "smart
tab" indentation style, it wouldn't actually matter and the reader
is free to choose any other tab-width -- as long as we don't use
non-leading tabs.
Don't allow non-leading tabs.
We should not use glib typedefs for basic C types char, short, int,
long, float or double. We commonly do not use them, so enforce
consistency.
That is not true for typedefs like guint, which we commonly use
because it's shorter typing than "unsigned int" (or "int unsigned"
or "unsigned"). Whether or not to use guint is left undecided at this
point.
A naive code compliance checker. Invoke directly:
contrib/scripts/checkpatch.pl 0001-switch-comments-to-klingon.patch
contrib/scripts/checkpatch.pl hello.[ch] world.c
Use from a commit hook:
echo 'git format-patch --stdout -1 |contrib/scripts/checkpatch.pl || :>' \
>.git/hooks/post-commit
Or view the documentation with "perldoc contrib/scripts/checkpatch.pl"
Unless the initscripts package too old to allow alternatives is present,
install nmcli as an alternative implementation of ifup and ifdown.
The triggerin scriptlet allow us to do the right thing regardless of
which initscripts version is installed or even when it's upgraded.
The initscripts patch was included in Fedora 29:
https://github.com/fedora-sysv/initscripts/pull/197
From Fedora 28 on we can build without Python 2. That is good,
because it's eventually going to be removed.
Based on a change in Fedora dist-git by Iryna Shcherbina.
This makes package updates more robust, avoiding in-place replaces of
the plugins.
Previously, if an upgrade transaction was terminated, NetworkManager
library could end up being of a different version than the plugins.
If the user was unfortunate enough to connect using a connection that
required a plugin (say, Wi-Fi), he would be left without a network
connection making it somewhat inconvenient to recover from the botched
upgrade.
This makes the whole situation a little bit less sad.
The VPN plugins are kept where they always have been -- the path is not
qualified with a version number.
When built with iwd support, add an option to use iwd in place of
wpa_supplicant.
The "boolean dependencies" are only supported since RPM 4.13, with older
versions just keep things the way they were before.
This makes package updates more robust, avoiding in-place replaces of
the plugins.
Previously, if an upgrade transaction was terminated, NetworkManager
library could end up being of a different version than the plugins.
If the user was unfortunate enough to connect using a connection that
required a plugin (say, Wi-Fi), he would be left without a network
connection making it somewhat inconvenient to recover from the botched
upgrade.
This makes the whole situation a little bit less sad.
On RHEL, we don't have NetworkManager-config-connectivity-fedora package.
Hence, the spec files for RHEL differ from upstream in this regard.
The aim is that contrib/rpm's spec file can be used almost as-is for
RHEL, Fedora and possibly other distros. Hence, build the subpackage
conditionally to minimize the difference.
They were not (notably) touched in more than 3 years.
I doubt anybody is using them.
Also, nowadays we have contrib/rpm to build NetworkManager
packages for Fedora/RPM. We have copr, we have automated CI
in CentOS CI and beaker.
Also, nowadays it should be easy to spawn a a fedora image
in a container or tools like vagrant.
I think there are better alternatives. Drop the scripts.
From libnl3, we only used the helper function to parse/generate netlink
messages and the socket functions to send/receive messages. We don't
need an external dependency to do that, it is simple enough.
Drop the libnl3 dependency, and replace all missing code by directly
copying it from libnl3 sources. At this point, I mostly tried to
import the required bits to make it working with few modifications.
Note that this increases the binary size of NetworkManager by 4736 bytes
for contrib/rpm build on x86_64. In the future, we can simplify the code
further.
A few modifications from libnl3 are:
- netlink errors NLE_* are now in the domain or regular errno.
The distinction of having to bother with two kinds of error
number domains was annoying.
- parts of the callback handling is copied partially and unused parts
are dropped. Especially, the verbose/debug handlers are not used.
In following commits, the callback handling will be significantly
simplified.
- the complex handling of seleting ports was simplified. We now always
let kernel choose the right port automatically.
Disable undefined sanitizer on RHEL since it's not supported. Also,
enable address sanitizer only for executables, as having it enabled in
libraries causes problems when applications built without asan load
them.
Will be used by CI trigger to name packages that are build during testing
of a github pull request with the corresponding pull request ID.
"build_clean.sh" now supports a command line option -s|--snapshot. But the
same paramter can also be set via $NM_BUILD_SNAPSHOT environment
variable. Using the environment variable is useful to support older versions
and new versions of "build_clean.sh", so that the script can just ignore the
snapshot setting if it doesn't understand it yet.
Even Gentoo disables this plugin since before 0.9.8 release
of NetworkManager. Time to say goodbye.
If somebody happens to show up to maintain it, we may resurrect it
later.
If "$distro_plugins=ifnet" was set, configure.ac would use that
to autodetect --with-hostname-persist=gentoo. Replace that autodetect
part by checking for /etc/gentoo-release file.
It only makes sense to call delete() with NMPObjects that
we obtained from the platform cache. Otherwise, if we didn't
get it from the cache in the first place, we wouldn't know
what to delete.
Hence, the input argument is (almost) always an NMPObject
in the first place. That is different from add(), where
we might create a new specific NMPlatform* instance on the
stack. For add() it makes slightly more sense to have different
functions depending on the type. For delete(), it doesn't.
I don't think we should do this.
- renamining/dropping configure options is still an annoyance,
because it requires to different ./configure options depending
on the version. The rename from --enable-teamctl to --enable-team
might be theoretically nice, but more annoying then helpful.
- There is no strict dependency between --enable-team and
--enable-json-validation. At most, one could argue that
when enabling the team plugin (--enable-teamctl), then
libnm must also be build with --enable-json-validation.
But in fact, the team plugin will happily work with a
libnm that doesn't link against libjansson.
That is --enable-teamctl --disable-json-validation will work
in practice just fine.
On the other hand, libnm is a client library to create connection
profiles, fully supporting team profiles also makes sense if the
actual plugin is not installed (or build). Thus, --disable-teamctl
--enable-json-validation certainly makes sense.
At this point, one might ask whether libnm is even still complete without
libjansson. Maybe libnm should *require* --enable-json-validation.
But that is not what the patch was doing, and it would also need
some careful consideration before doing so.
This reverts commit 9d5cd7eae8.
Rename the team functionality enablement from 'teamdctl' to 'team'.
Force jansson lib requirement for team functionality: NetworkManager
requires the teamd daemon to manage team. As teamd depends upon jansson
lib, adding jansson requirement for teaming support in NetworkManager
seems reasonable.
Remove the jansson_validation flag, as the only generic json function in
nmcli (not related to team) was the one to check if a string was in json
format. Anyway, that function is used for team checks only. So, move
also json validation functions under the WITH_TEAM flag.
GNU less supports filters. That makes it nice to use instead of cat.
Also, less is well suited for output to a pipe.
With this, `NM-log nm-log.txt.gz` works as you would expect
Don't compile ovs support when the RPM is built --without=ovs, to fix
the following error:
error: Installed (but unpackaged) file(s) found:
/usr/lib/systemd/system/NetworkManager.service.d/NetworkManager-ovs.conf
/usr/lib64/NetworkManager/libnm-device-plugin-ovs.so
/usr/share/man/man7/nm-openvswitch.7.gz
Fixes: 830a5a14cb
Add dhclient and iptables packages to build dependencies to satisfy
rpmbuild complaints:
```
error: Failed build dependencies:
dhclient is needed by NetworkManager-1:1.9.2-18653.43dba57439.fc28.x86_64
iptables is needed by NetworkManager-1:1.9.2-18653.43dba57439.fc28.x86_64
ERROR: rpmbuild FAILED
```
https://github.com/NetworkManager/NetworkManager/pull/33
Since commit d61eaf2545 ("service: don't
install dependency for "NetworkManager-wait-online.service" to
"network-online.target.wants") we no longer install NM-w-o.service
in "network-online.target.wants" directory.
Obviously, for previous RPM versions NM-w-o.service was always enabled.
For current versions, it depends now on the preset. Most importantly,
this allows the user to disable the service, without masking it.
Previously NM-w-o.service was always implicitly enabled.
But presets are not applied during package upgrade, so it means that
after upgrade the service will be disabled. Hack around that via an RPM
scriptlet.
https://bugzilla.redhat.com/show_bug.cgi?id=1455704
This isn't useful for contrib/fedora/rpm itself because here
the __SOURCE__ gets set by the build scripts.
But this spec file is copied to Fedora downstream where the
SOURCE URL is used.
A newer compiler version might emit some warnings and break the build
of the RPM. Of course, such warnings must be fixed. But it is still very
inconvenient to break the build of an old RPM version without easy workaround.
When building without "test" (which is on by default), don't use fatal warnings
for compilation.
