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"
