[libvirt-dbus PATCH 0/9] Convert documentation to reStructuredText, other cleanups

I think it's high time that we have a new libvirt-dbus release, since the last one was more than a year ago: since then, there has been at least one bug fix necessary to keep it building on modern platforms, an additional API has been implemented, and of course we've switched the build system from autotools to Meson. I validated the status of what's in master by importing a snapshot into the Debian package and everything seems to be in order, so I'm fairly confident we could basically just tag a release right away; there are, however, a few straightforward improvements that I would like to get into the new release, so here we are :) Andrea Bolognani (9): git: Minimize ignore patterns spec: Install fewer documentation files README: Convert to reStructuredText HACKING: Convert to reStructuredText NEWS: Convert to reStructuredText AUTHORS: Convert to reStructuredText man: Convert to reStructuredText meson: Install documentation spec: Pick documentation from installation directory .gitignore | 47 --------- AUTHORS.in | 16 ---- AUTHORS.rst.in | 17 ++++ HACKING.md | 205 --------------------------------------- HACKING.rst | 207 ++++++++++++++++++++++++++++++++++++++++ NEWS | 88 ----------------- NEWS.rst | 94 ++++++++++++++++++ README.md => README.rst | 37 +++---- docs/libvirt-dbus.pod | 66 ------------- docs/libvirt-dbus.rst | 71 ++++++++++++++ docs/meson.build | 6 +- libvirt-dbus.spec.in | 7 +- meson.build | 24 ++++- tools/gen-authors.sh | 2 +- 14 files changed, 437 insertions(+), 450 deletions(-) delete mode 100644 AUTHORS.in create mode 100644 AUTHORS.rst.in delete mode 100644 HACKING.md create mode 100644 HACKING.rst delete mode 100644 NEWS create mode 100644 NEWS.rst rename README.md => README.rst (68%) delete mode 100644 docs/libvirt-dbus.pod create mode 100644 docs/libvirt-dbus.rst -- 2.25.3

Meson enforces out-of-tree builds, which means we no longer need to have patterns for most generated files. Python, unfortunately, is still very inconsiderate and litters all over the source directory. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- .gitignore | 47 ----------------------------------------------- 1 file changed, 47 deletions(-) diff --git a/.gitignore b/.gitignore index 7ed7554..362e476 100644 --- a/.gitignore +++ b/.gitignore @@ -1,49 +1,2 @@ -*.la -*.lo -*.log -*.o -*.pyc -*.swp -*.trs -*Makefile -*Makefile.in -*~ -.deps/ -.libs/ -.pytest_cache/ __pycache__/ -vgcore.* - -/AUTHORS -/INSTALL -/aclocal.m4 -/autom4te.cache/ -/build-aux/ /build/ -/config.h -/config.h.in -/config.log -/config.status -/configure -/libtool -/libvirt-dbus-*.tar.* -/libvirt-dbus.spec -/m4/libtool.m4 -/m4/ltoptions.m4 -/m4/ltsugar.m4 -/m4/ltversion.m4 -/m4/lt~obsolete.m4 -/run -/stamp-h1 - -/data/session/org.libvirt.service -/data/system/org.libvirt.service -/data/system/libvirt-dbus.rules -/data/system/org.libvirt.conf - -/docs/*.8 - -/src/libvirt-dbus -/src/org.libvirt.service - -/tests/test_util -- 2.25.3

The information contained in README is either redundant or already covered by the package metadata, so it's pointless to include this file. Similarly, HACKING only makes sense as a companion to the source code, and can thus be left out of the binary package. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- libvirt-dbus.spec.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt-dbus.spec.in b/libvirt-dbus.spec.in index 3425e9e..4be17f2 100644 --- a/libvirt-dbus.spec.in +++ b/libvirt-dbus.spec.in @@ -50,7 +50,7 @@ getent passwd %{system_user} >/dev/null || \ exit 0 %files -%doc README.md HACKING.md AUTHORS NEWS +%doc AUTHORS NEWS %license COPYING %{_sbindir}/libvirt-dbus %{_datadir}/dbus-1/services/org.libvirt.service -- 2.25.3

Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- README.md => README.rst | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) rename README.md => README.rst (68%) diff --git a/README.md b/README.rst similarity index 68% rename from README.md rename to README.rst index 25534e4..eacbbf2 100644 --- a/README.md +++ b/README.rst @@ -1,3 +1,4 @@ +============ libvirt-dbus ============ @@ -12,40 +13,40 @@ API better suited for dbus-based applications. libvirt-dbus is Free Software and licenced under LGPLv2+. - * [https://libvirt.org/libvirt-dbus.html](https://libvirt.org/dbus.html) +* https://libvirt.org/libvirt-dbus.html The latest official releases can be found at: - * [https://libvirt.org/sources/dbus/](https://libvirt.org/sources/dbus/) +* https://libvirt.org/sources/dbus Dependencies / supported platforms ----------------------------------- +================================== The packages required to build libvirt-dbus are - - libvirt - - libvirt-glib - - glib2 +* libvirt +* libvirt-glib +* glib2 Installation ------------- +============ libvirt-dbus uses Meson build system, so the build & install process is fairly simple. For example, to install as root user: -``` -# meson build --prefix=/usr --sysconfdir=/etc --localstatedir=/var -# ninja -C build install -``` +:: + + # meson build --prefix=/usr --sysconfdir=/etc --localstatedir=/var + # ninja -C build install or to install as unprivileged user: -``` -$ meson build --prefix=$HOME/usr -$ ninja -C build install -``` +:: + + $ meson build --prefix=$HOME/usr + $ ninja -C build install Patches submissions @@ -54,14 +55,14 @@ Patches submissions Patch submissions are welcomed from any interested contributor. Please send them to the main libvir-list mailing list - * libvir-list@redhat.com +* libvir-list@redhat.com Questions about usage / deployment can be send to the end users mailing list - * libvirt-users@redhat.com +* libvirt-users@redhat.com For further information about mailing lists & contacting the developers, please consult -[https://libvirt.org/contact.html](https://libvirt.org/contact.html) +https://libvirt.org/contact.html -- 2.25.3

Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- HACKING.md | 205 --------------------------------------------------- HACKING.rst | 207 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 207 insertions(+), 205 deletions(-) delete mode 100644 HACKING.md create mode 100644 HACKING.rst diff --git a/HACKING.md b/HACKING.md deleted file mode 100644 index e90f136..0000000 --- a/HACKING.md +++ /dev/null @@ -1,205 +0,0 @@ -Tips for hacking on libvirt-dbus -================================ - -Here is where to get code: - -``` -$ git clone https://libvirt.org/git/libvirt-dbus.git -``` - -Alternatively you can use one of the mirrors: - -[https://github.com/libvirt/libvirt-dbus](https://github.com/libvirt/libvirt-dbus) -[https://gitlab.com/libvirt/libvirt-dbus](https://gitlab.com/libvirt/libvirt-dbus) - - -Running from git repository ---------------------------- - - * The first step is to run meson to create build directory: - - ``` - meson build - ``` - - Now you can compile libvirt-dbus: - - ``` - ninja -C build - ``` - - - * Before posting a patch, you should run tests: - - ``` - ninja -C build test - ``` - - The test tool requires python3, python3-pytest, python3-dbus and flake8. - - It is possible to run only specific test using: - - ``` - meson test -C build $test-name - ``` - - or a group of tests: - - ``` - meson test -C build --suite $label - ``` - - For more information see [https://mesonbuild.com/Unit-tests.html#testing-tool](https://mesonbuild.com/Unit-tests.html#testing-tool). - - - * To run libvirt-dbus directly from the build dir without installing it - use the run script: - - ``` - ./run ./src/libvirt-dbus - ``` - - -Coding style rules ------------------- - - * Opening & closing braces for functions should be at start of line: - - ``` - int - foo(int bar) - { - ... - } - ``` - - Not - - ``` - int - foo(int bar) { - ... - } - ``` - - * Opening brace for if/while/for loops should be at the end of line: - - ``` - if (foo) { - bar; - wizz; - } - ``` - - Not - - ``` - if (foo) - { - bar; - wizz; - } - ``` - - Rationale: putting every if/while/for opening brace on a new line - expands function length too much. - - - * If a brace needs to be used for one clause in an if/else statement, - it should be used for both clauses, even if the other clauses are - only single statements. eg: - - ``` - if (foo) { - bar; - wizz; - } else { - eek; - } - ``` - - Not - - ``` - if (foo) { - bar; - wizz; - } else - eek; - ``` - - - * Function parameter attribute annotations should follow the parameter - name, eg: - - ``` - int - foo(int bar G_GNUC_UNUSED) - { - } - ``` - - Not - - ``` - int - foo(G_GNUC_UNUSED int bar) - { - } - ``` - - Rationale: Adding / removing G_GNUC_UNUSED should not cause the - rest of the line to move around since that obscures diffs. - - - * There should be no space between function names & open brackets eg: - - ``` - int - foo(int bar) - { - } - ``` - - Not - - ``` - int - foo (int bar) - { - } - ``` - - - * To keep lines under 80 characters (where practical), multiple parameters - should be on new lines. Do not attempt to line up parameters vertically eg: - - ``` - int - foo(int bar, - unsigned long wizz) - { - } - ``` - - Not - - ``` - int - foo(int bar, unsigned long wizz) - { - } - ``` - - Not - - ``` - int - foo(int bar, - unsigned long wizz) - { - } - ``` - - Rationale: attempting vertical alignment causes bigger diffs when - modifying code if type names change causing whitespace re-alignment. diff --git a/HACKING.rst b/HACKING.rst new file mode 100644 index 0000000..6f2bca1 --- /dev/null +++ b/HACKING.rst @@ -0,0 +1,207 @@ +================================ +Tips for hacking on libvirt-dbus +================================ + +Here is where to get code: + +:: + + $ git clone https://libvirt.org/git/libvirt-dbus.git + +Alternatively you can use one of the mirrors: + +https://github.com/libvirt/libvirt-dbus +https://gitlab.com/libvirt/libvirt-dbus + + +Running from git repository +=========================== + +* The first step is to run meson to create build directory: + + :: + + meson build + + Now you can compile libvirt-dbus: + + :: + + ninja -C build + + +* Before posting a patch, you should run tests: + + :: + + ninja -C build test + + The test tool requires python3, python3-pytest, python3-dbus and flake8. + + It is possible to run only specific test using: + + :: + + meson test -C build $test-name + + or a group of tests: + + :: + + meson test -C build --suite $label + + For more information see https://mesonbuild.com/Unit-tests.html#testing-tool. + + +* To run libvirt-dbus directly from the build dir without installing it + use the run script: + + :: + + ./run ./src/libvirt-dbus + + +Coding style rules +================== + +* Opening & closing braces for functions should be at start of line: + + :: + + int + foo(int bar) + { + ... + } + + Not + + :: + + int + foo(int bar) { + ... + } + + +* Opening brace for if/while/for loops should be at the end of line: + + :: + + if (foo) { + bar; + wizz; + } + + Not + + :: + + if (foo) + { + bar; + wizz; + } + + Rationale: putting every if/while/for opening brace on a new line + expands function length too much. + + +* If a brace needs to be used for one clause in an if/else statement, + it should be used for both clauses, even if the other clauses are + only single statements. eg: + + :: + + if (foo) { + bar; + wizz; + } else { + eek; + } + + Not + + :: + + if (foo) { + bar; + wizz; + } else + eek; + + +* Function parameter attribute annotations should follow the parameter + name, eg: + + :: + + int + foo(int bar G_GNUC_UNUSED) + { + } + + Not + + :: + + int + foo(G_GNUC_UNUSED int bar) + { + } + + Rationale: Adding / removing G_GNUC_UNUSED should not cause the + rest of the line to move around since that obscures diffs. + + +* There should be no space between function names & open brackets eg: + + :: + + int + foo(int bar) + { + } + + Not + + :: + + int + foo (int bar) + { + } + + +* To keep lines under 80 characters (where practical), multiple parameters + should be on new lines. Do not attempt to line up parameters vertically eg: + + :: + + int + foo(int bar, + unsigned long wizz) + { + } + + Not + + :: + + int + foo(int bar, unsigned long wizz) + { + } + + Not + + :: + + int + foo(int bar, + unsigned long wizz) + { + } + + Rationale: attempting vertical alignment causes bigger diffs when + modifying code if type names change causing whitespace re-alignment. -- 2.25.3

Note that, while it didn't have a file extension, it was already basically a Markdown document: now that the file extension is present and matching the contents, GitLab and friends will render it nicely. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- NEWS | 88 ----------------------------------------- NEWS.rst | 94 ++++++++++++++++++++++++++++++++++++++++++++ libvirt-dbus.spec.in | 2 +- 3 files changed, 95 insertions(+), 89 deletions(-) delete mode 100644 NEWS create mode 100644 NEWS.rst diff --git a/NEWS b/NEWS deleted file mode 100644 index 4a24206..0000000 --- a/NEWS +++ /dev/null @@ -1,88 +0,0 @@ -libvirt-dbus releases -===================== - -# v1.3.0 (2019-01-18) - - * New features - - - Support for all relevant interface APIs up to libvirt 3.0.0 - - * Bug fixes - - - Storage volumes and with special characters in the name are now correctly listed - - * Build-system improvements - - - Detect flake8 automatically - - - Install daemon under @sbindir@ - - - Fix version check for dependencies - - -# v1.2.0 (2018-07-04) - - * Bug fixes - - - Properly deregister NodeDevice event callback - - * Improvements - - - Allow system access to users in the libvirt group - - - Switch to xz compression for release archives - - * Build-system fixes - - - Fix default polkit rule directory and make it configurable - - - Fix typo to properly build binaries with PIE - - -# v1.1.0 (2018-06-21) - - * New features - - - Support for all relevant nwfilter APIs up to libvirt 3.0.0 - - - Support for all relevant storage volume APIs up to libvirt 3.0.0 - - - Support for all relevant node device APIs up to libvirt 3.0.0 - - * Bug fixes - - - Don't report error for GetAll on properties if some property is not accessible - - -# v1.0.0 (2018-05-16) - - * Fist stable release, from now on we will not change existing APIs - - * New features - - - Support for almost all relevant domain APIs and events up to libvirt 3.0.0 - - - Support for all relevant connect APIs up to libvirt 3.0.0 - - - Support for all relevant network APIs and events up to libvirt 3.0.0 - - - Support for all relevant storage pool APIs and events up to libvirt 3.0.0 - - - Support for all relevant secret APIs and evetns up to libvirt 3.0.0 - - * Improvements - - - Added org.gtk.GDBus.DocString annotation for D-Bus APIs - - - Added man page for libvirt-dbus daemon - - -# v0.0.1 (2018-03-22) - - * First unstable release - - - Basic support for starting and destroying domains - - - Basic support for getting some information about domains - - - Basic support for listing existing and creating new domains diff --git a/NEWS.rst b/NEWS.rst new file mode 100644 index 0000000..64f17e8 --- /dev/null +++ b/NEWS.rst @@ -0,0 +1,94 @@ +===================== +libvirt-dbus releases +===================== + +v1.3.0 (2019-01-18) +=================== + +* New features + + - Support for all relevant interface APIs up to libvirt 3.0.0 + +* Bug fixes + + - Storage volumes and with special characters in the name are now correctly listed + +* Build-system improvements + + - Detect flake8 automatically + + - Install daemon under @sbindir@ + + - Fix version check for dependencies + + +v1.2.0 (2018-07-04) +=================== + +* Bug fixes + + - Properly deregister NodeDevice event callback + +* Improvements + + - Allow system access to users in the libvirt group + + - Switch to xz compression for release archives + +* Build-system fixes + + - Fix default polkit rule directory and make it configurable + + - Fix typo to properly build binaries with PIE + + +v1.1.0 (2018-06-21) +=================== + +* New features + + - Support for all relevant nwfilter APIs up to libvirt 3.0.0 + + - Support for all relevant storage volume APIs up to libvirt 3.0.0 + + - Support for all relevant node device APIs up to libvirt 3.0.0 + +* Bug fixes + + - Don't report error for GetAll on properties if some property is not accessible + + +v1.0.0 (2018-05-16) +=================== + +* Fist stable release, from now on we will not change existing APIs + +* New features + + - Support for almost all relevant domain APIs and events up to libvirt 3.0.0 + + - Support for all relevant connect APIs up to libvirt 3.0.0 + + - Support for all relevant network APIs and events up to libvirt 3.0.0 + + - Support for all relevant storage pool APIs and events up to libvirt 3.0.0 + + - Support for all relevant secret APIs and evetns up to libvirt 3.0.0 + +* Improvements + + - Added org.gtk.GDBus.DocString annotation for D-Bus APIs + + - Added man page for libvirt-dbus daemon + + +v0.0.1 (2018-03-22) +=================== + +* First unstable release + + - Basic support for starting and destroying domains + + - Basic support for getting some information about domains + + - Basic support for listing existing and creating new domains diff --git a/libvirt-dbus.spec.in b/libvirt-dbus.spec.in index 4be17f2..a9fdd0c 100644 --- a/libvirt-dbus.spec.in +++ b/libvirt-dbus.spec.in @@ -50,7 +50,7 @@ getent passwd %{system_user} >/dev/null || \ exit 0 %files -%doc AUTHORS NEWS +%doc AUTHORS NEWS.rst %license COPYING %{_sbindir}/libvirt-dbus %{_datadir}/dbus-1/services/org.libvirt.service -- 2.25.3

Like NEWS, this was very close to being a valid Markdown document before, and now it's a valid reStructuredText one. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- AUTHORS.in | 16 ---------------- AUTHORS.rst.in | 17 +++++++++++++++++ libvirt-dbus.spec.in | 2 +- meson.build | 6 +++--- tools/gen-authors.sh | 2 +- 5 files changed, 22 insertions(+), 21 deletions(-) delete mode 100644 AUTHORS.in create mode 100644 AUTHORS.rst.in diff --git a/AUTHORS.in b/AUTHORS.in deleted file mode 100644 index d5a486e..0000000 --- a/AUTHORS.in +++ /dev/null @@ -1,16 +0,0 @@ - libvirt-dbus Authors - ==================== - -The libvirt-dbus project was started by: - - Lars Karlitski <lars@karlitski.net> - Pavel Hrdina <phrdina@redhat.com> - -The primary maintainers of libvirt-dbus are: - - Katerina Koukiou <kkoukiou@redhat.com> - Pavel Hrdina <phrdina@redhat.com> - -Patches have been received from: - -@contributorslist@ diff --git a/AUTHORS.rst.in b/AUTHORS.rst.in new file mode 100644 index 0000000..1087b73 --- /dev/null +++ b/AUTHORS.rst.in @@ -0,0 +1,17 @@ +==================== +libvirt-dbus Authors +==================== + +The libvirt-dbus project was started by: + +* Lars Karlitski <lars@karlitski.net> +* Pavel Hrdina <phrdina@redhat.com> + +The primary maintainers of libvirt-dbus are: + +* Katerina Koukiou <kkoukiou@redhat.com> +* Pavel Hrdina <phrdina@redhat.com> + +Patches have been received from: + +@contributorslist@ diff --git a/libvirt-dbus.spec.in b/libvirt-dbus.spec.in index a9fdd0c..175910e 100644 --- a/libvirt-dbus.spec.in +++ b/libvirt-dbus.spec.in @@ -50,7 +50,7 @@ getent passwd %{system_user} >/dev/null || \ exit 0 %files -%doc AUTHORS NEWS.rst +%doc AUTHORS.rst NEWS.rst %license COPYING %{_sbindir}/libvirt-dbus %{_datadir}/dbus-1/services/org.libvirt.service diff --git a/meson.build b/meson.build index b293b8c..7518e94 100644 --- a/meson.build +++ b/meson.build @@ -256,11 +256,11 @@ if git authors = run_command('tools/gen-authors.sh') configure_file( configuration: { 'contributorslist': authors.stdout() }, - input: 'AUTHORS.in', - output: 'AUTHORS', + input: 'AUTHORS.rst.in', + output: 'AUTHORS.rst', ) - foreach file : [ 'libvirt-dbus.spec', 'AUTHORS' ] + foreach file : [ 'libvirt-dbus.spec', 'AUTHORS.rst' ] meson.add_dist_script('tools/dist.sh', meson.build_root(), file) endforeach endif diff --git a/tools/gen-authors.sh b/tools/gen-authors.sh index eb39215..bf0a3b6 100755 --- a/tools/gen-authors.sh +++ b/tools/gen-authors.sh @@ -1,4 +1,4 @@ #!/bin/sh cd $MESON_SOURCE_ROOT -git log --pretty=format:' %aN <%aE>' | sort -u +git log --pretty=format:'* %aN <%aE>' | sort -u -- 2.25.3

All the documentation for libvirt-dbus is now written in the same language. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/libvirt-dbus.pod | 66 ---------------------------------------- docs/libvirt-dbus.rst | 71 +++++++++++++++++++++++++++++++++++++++++++ docs/meson.build | 6 ++-- libvirt-dbus.spec.in | 2 +- 4 files changed, 75 insertions(+), 70 deletions(-) delete mode 100644 docs/libvirt-dbus.pod create mode 100644 docs/libvirt-dbus.rst diff --git a/docs/libvirt-dbus.pod b/docs/libvirt-dbus.pod deleted file mode 100644 index 49de044..0000000 --- a/docs/libvirt-dbus.pod +++ /dev/null @@ -1,66 +0,0 @@ -=head1 NAME - -libvirt-dbus - D-Bus daemon exporting libvirt API - -=head1 SYNOPSIS - -B<libvirt-dbus> [I<OPTION>]... - -=head1 DESCRIPTION - -libvirt-dbus wraps libvirt API to provide a high-level object-oriented -API better suited for dbus-based applications. - -Normally libvirt-dbus is started by D-Bus daemon on demand. - -=head1 OPTIONS - -=over - -=item B<-h --help> - -Display command line help usage then exit. - -=item B<--system> - -Connect to the system bus. - -=item B<--session> - -Connect to the session bus. - -=item B<-t --threads> I<NUM> - -Configure maximal number of worker threads. - -=back - -=head1 BUGS - -Please report all bugs you discover. This should be done via either: - -=over - -=item a) the mailing list - -L<https://libvirt.org/contact.html> - -=item b) the bug tracker - -L<https://libvirt.org/bugs.html> - -=back - -Alternatively, you may report bugs to your software distribution / vendor. - -=head1 AUTHORS - -Please refer to the AUTHORS file distributed with libvirt-dbus. - -=head1 LICENSE - -libvirt-dbus is Free Software and licenced under LGPLv2+. - -=head1 SEE ALSO - -L<https://libvirt.org/dbus.html>, L<https://libvirt.org/> diff --git a/docs/libvirt-dbus.rst b/docs/libvirt-dbus.rst new file mode 100644 index 0000000..cb01fac --- /dev/null +++ b/docs/libvirt-dbus.rst @@ -0,0 +1,71 @@ +============ +libvirt-dbus +============ + +---------------------------------- +D-Bus daemon exporting libvirt API +---------------------------------- + +:Manual section: 8 + +SYNOPSIS +======== + +``libvirt-dbus`` [*OPTION*]... + +DESCRIPTION +=========== + +libvirt-dbus wraps libvirt API to provide a high-level object-oriented +API better suited for dbus-based applications. + +Normally libvirt-dbus is started by D-Bus daemon on demand. + +OPTIONS +======= + +**-h --help** + + Display command line help usage then exit. + +**--system** + + Connect to the system bus. + +**--session** + + Connect to the session bus. + +**-t --threads** *NUM* + + Configure maximal number of worker threads. + +BUGS +==== + +Please report all bugs you discover. This should be done via either: + +a) the mailing list + + https://libvirt.org/contact.html + +b) the bug tracker + + https://libvirt.org/bugs.html + +Alternatively, you may report bugs to your software distribution / vendor. + +AUTHORS +======= + +Please refer to the AUTHORS file distributed with libvirt-dbus. + +LICENSE +======= + +libvirt-dbus is Free Software and licenced under LGPLv2+. + +SEE ALSO +======== + +https://libvirt.org/dbus.html, https://libvirt.org/ diff --git a/docs/meson.build b/docs/meson.build index e62e71a..fbeaf12 100644 --- a/docs/meson.build +++ b/docs/meson.build @@ -1,8 +1,8 @@ -prog_pod2man = find_program('pod2man') +prog_rst2man = find_program('rst2man') configure_file( - command: [prog_pod2man.path(), '-s 8', '@INPUT@', '@OUTPUT@'], - input: 'libvirt-dbus.pod', + command: [prog_rst2man.path(), '@INPUT@', '@OUTPUT@'], + input: 'libvirt-dbus.rst', output: 'libvirt-dbus.8', install_dir: join_paths (get_option('mandir'), 'man8'), ) diff --git a/libvirt-dbus.spec.in b/libvirt-dbus.spec.in index 175910e..84172fa 100644 --- a/libvirt-dbus.spec.in +++ b/libvirt-dbus.spec.in @@ -19,7 +19,7 @@ BuildRequires: meson >= %{meson_version} BuildRequires: glib2-devel >= %{glib2_version} BuildRequires: libvirt-devel >= %{libvirt_version} BuildRequires: libvirt-glib-devel >= %{libvirt_glib_version} -BuildRequires: /usr/bin/pod2man +BuildRequires: /usr/bin/rst2man Requires: dbus Requires: glib2 >= %{glib2_version} -- 2.25.3

On Sat, Apr 25, 2020 at 10:59:44PM +0200, Andrea Bolognani wrote:
diff --git a/docs/meson.build b/docs/meson.build index e62e71a..fbeaf12 100644 --- a/docs/meson.build +++ b/docs/meson.build @@ -1,8 +1,8 @@ -prog_pod2man = find_program('pod2man') +prog_rst2man = find_program('rst2man')
This broke on RHEL-7 CI since it needs a -3 suffix, and IIRC will break other distros which need a .py suffix In libvirt configure.ac we pick from rst2man rst2man.py rst2man-3 Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Mon, Apr 27, 2020 at 04:27:38PM +0100, Daniel P. Berrangé wrote:
On Sat, Apr 25, 2020 at 10:59:44PM +0200, Andrea Bolognani wrote:
diff --git a/docs/meson.build b/docs/meson.build index e62e71a..fbeaf12 100644 --- a/docs/meson.build +++ b/docs/meson.build @@ -1,8 +1,8 @@ -prog_pod2man = find_program('pod2man') +prog_rst2man = find_program('rst2man')
This broke on RHEL-7 CI since it needs a -3 suffix, and IIRC will break other distros which need a .py suffix
In libvirt configure.ac we pick from
rst2man rst2man.py rst2man-3
Technically the rst2man is correct for libvirt-dbus as that's the same thing we have in libvirt-dbus.spec file, but we install python3 version into our CI images. I guess it would be ideal to follow what libvirt is doing so I'll post a patch that requires python3-docutils or python36-docutils and add all of the mentioned names into the meson.build file. Pavel

On Mon, 2020-04-27 at 16:27 +0100, Daniel P. Berrangé wrote:
On Sat, Apr 25, 2020 at 10:59:44PM +0200, Andrea Bolognani wrote:
-prog_pod2man = find_program('pod2man') +prog_rst2man = find_program('rst2man')
This broke on RHEL-7 CI since it needs a -3 suffix, and IIRC will break other distros which need a .py suffix
In libvirt configure.ac we pick from
rst2man rst2man.py rst2man-3
Right. I'll commit the obvious fix. Thanks for the heads-up! -- Andrea Bolognani / Red Hat / Virtualization

On Mon, 2020-04-27 at 18:37 +0200, Andrea Bolognani wrote:
On Mon, 2020-04-27 at 16:27 +0100, Daniel P. Berrangé wrote:
On Sat, Apr 25, 2020 at 10:59:44PM +0200, Andrea Bolognani wrote:
-prog_pod2man = find_program('pod2man') +prog_rst2man = find_program('rst2man')
This broke on RHEL-7 CI since it needs a -3 suffix, and IIRC will break other distros which need a .py suffix
In libvirt configure.ac we pick from
rst2man rst2man.py rst2man-3
Right. I'll commit the obvious fix. Thanks for the heads-up!
... except Pavel already did :) -- Andrea Bolognani / Red Hat / Virtualization

People who install the RPM packages already get this, but it's good practice so let's make it happen for those who install from source as well. The process is straightforward, but we have to be a bit careful with AUTHORS because this specific file is found in different directories depending on whether we're building from git or from a release. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- meson.build | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/meson.build b/meson.build index 7518e94..9395bc4 100644 --- a/meson.build +++ b/meson.build @@ -12,6 +12,7 @@ project( prefix = get_option('prefix') datadir = prefix / get_option('datadir') sbindir = prefix / get_option('sbindir') +set_variable('docdir', datadir / 'doc/libvirt-dbus') opt_dirs = [ 'dbus_interfaces', @@ -258,6 +259,7 @@ if git configuration: { 'contributorslist': authors.stdout() }, input: 'AUTHORS.rst.in', output: 'AUTHORS.rst', + install_dir: docdir, ) foreach file : [ 'libvirt-dbus.spec', 'AUTHORS.rst' ] @@ -266,6 +268,22 @@ if git endif +# Install documentation + +docs = [ + 'COPYING', + 'NEWS.rst', +] +if not git + docs += ['AUTHORS.rst'] +endif + +install_data( + docs, + install_dir: docdir, +) + + # Include sub-directories subdir('data') -- 2.25.3

On Sat, Apr 25, 2020 at 10:59:45PM +0200, Andrea Bolognani wrote:
People who install the RPM packages already get this, but it's good practice so let's make it happen for those who install from source as well.
The process is straightforward, but we have to be a bit careful with AUTHORS because this specific file is found in different directories depending on whether we're building from git or from a release.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- meson.build | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/meson.build b/meson.build index 7518e94..9395bc4 100644 --- a/meson.build +++ b/meson.build @@ -12,6 +12,7 @@ project( prefix = get_option('prefix') datadir = prefix / get_option('datadir') sbindir = prefix / get_option('sbindir') +set_variable('docdir', datadir / 'doc/libvirt-dbus')
No need to use set_variable(), you can use directly: docdir = datadir / 'doc' / 'libvirt-dbus' The set_variable() function is usually used in places where you need to dynamically generate the variable name. Pavel

On Mon, 2020-04-27 at 14:20 +0200, Pavel Hrdina wrote:
On Sat, Apr 25, 2020 at 10:59:45PM +0200, Andrea Bolognani wrote:
prefix = get_option('prefix') datadir = prefix / get_option('datadir') sbindir = prefix / get_option('sbindir') +set_variable('docdir', datadir / 'doc/libvirt-dbus')
No need to use set_variable(), you can use directly:
docdir = datadir / 'doc' / 'libvirt-dbus'
The set_variable() function is usually used in places where you need to dynamically generate the variable name.
I see! This is my first time touching a Meson-based build system, so I mostly copy-pasted my way through O:-) I'll adopt your version. -- Andrea Bolognani / Red Hat / Virtualization

On Mon, 2020-04-27 at 14:48 +0200, Andrea Bolognani wrote:
On Mon, 2020-04-27 at 14:20 +0200, Pavel Hrdina wrote:
On Sat, Apr 25, 2020 at 10:59:45PM +0200, Andrea Bolognani wrote:
prefix = get_option('prefix') datadir = prefix / get_option('datadir') sbindir = prefix / get_option('sbindir') +set_variable('docdir', datadir / 'doc/libvirt-dbus')
No need to use set_variable(), you can use directly:
docdir = datadir / 'doc' / 'libvirt-dbus'
The set_variable() function is usually used in places where you need to dynamically generate the variable name.
I see! This is my first time touching a Meson-based build system, so I mostly copy-pasted my way through O:-)
I'll adopt your version.
I'm actually going to use a slight variation, docdir = datadir / 'doc' / meson.project_name() to avoid repetition. -- Andrea Bolognani / Red Hat / Virtualization

On Sat, Apr 25, 2020 at 10:59:45PM +0200, Andrea Bolognani wrote:
People who install the RPM packages already get this, but it's good practice so let's make it happen for those who install from source as well.
Do we really want this ? I've not noticed apps doing this in general - these files are usually non-installed with autotools and meson AFAIK.
The process is straightforward, but we have to be a bit careful with AUTHORS because this specific file is found in different directories depending on whether we're building from git or from a release.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- meson.build | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/meson.build b/meson.build index 7518e94..9395bc4 100644 --- a/meson.build +++ b/meson.build @@ -12,6 +12,7 @@ project( prefix = get_option('prefix') datadir = prefix / get_option('datadir') sbindir = prefix / get_option('sbindir') +set_variable('docdir', datadir / 'doc/libvirt-dbus')
opt_dirs = [ 'dbus_interfaces', @@ -258,6 +259,7 @@ if git configuration: { 'contributorslist': authors.stdout() }, input: 'AUTHORS.rst.in', output: 'AUTHORS.rst', + install_dir: docdir, )
foreach file : [ 'libvirt-dbus.spec', 'AUTHORS.rst' ] @@ -266,6 +268,22 @@ if git endif
+# Install documentation + +docs = [ + 'COPYING', + 'NEWS.rst', +] +if not git + docs += ['AUTHORS.rst'] +endif + +install_data( + docs, + install_dir: docdir, +) + + # Include sub-directories
subdir('data') -- 2.25.3
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Mon, Apr 27, 2020 at 01:52:13PM +0100, Daniel P. Berrangé wrote:
On Sat, Apr 25, 2020 at 10:59:45PM +0200, Andrea Bolognani wrote:
People who install the RPM packages already get this, but it's good practice so let's make it happen for those who install from source as well.
Do we really want this ? I've not noticed apps doing this in general - these files are usually non-installed with autotools and meson AFAIK.
It was probably my motivation not to install them in the first place as well because the user already has the files from sources so there is usually no need to install them. I've checked systemd and they are installing these files using meson but GLib for example is not installing them. After checking other libvirt projects we are not installing this files using autotools or other build systems. Taking all of that into account I'm changing my mind and we should not install these files from sources as well. Pavel

Sorry, I only see these messages now, after having pushed already :( On Mon, 2020-04-27 at 15:38 +0200, Pavel Hrdina wrote:
On Mon, Apr 27, 2020 at 01:52:13PM +0100, Daniel P. Berrangé wrote:
On Sat, Apr 25, 2020 at 10:59:45PM +0200, Andrea Bolognani wrote:
People who install the RPM packages already get this, but it's good practice so let's make it happen for those who install from source as well.
Do we really want this ? I've not noticed apps doing this in general - these files are usually non-installed with autotools and meson AFAIK.
It was probably my motivation not to install them in the first place as well because the user already has the files from sources so there is usually no need to install them.
One doesn't necessarily keep the source directory around after installing a piece or software, and the person installing it might be different from the one interested in looking at these files: think the user on a managed server curious about what features a newly-deployed version of some software introduces.
I've checked systemd and they are installing these files using meson but GLib for example is not installing them.
After checking other libvirt projects we are not installing this files using autotools or other build systems.
I can bring up another counter-example that's close to us: QEMU installs these files, along with a bunch of other documentation.
Taking all of that into account I'm changing my mind and we should not install these files from sources as well.
I still think it's a good idea. The packaging for both Fedora and Debian go out of their way to include them, and at least for the latter it's strongly recommended by policy to include NEWS while including COPYING is mandatory. Clearly both projects consider this information important to users, and I don't see any disadvantage in installing them even for people who build from sources directly instead of using a package, only advantages really. That said, if you guys feel strongly against installing them, we can revert my last two commits quite easily :) -- Andrea Bolognani / Red Hat / Virtualization

On Mon, Apr 27, 2020 at 04:16:57PM +0200, Andrea Bolognani wrote:
Sorry, I only see these messages now, after having pushed already :(
On Mon, 2020-04-27 at 15:38 +0200, Pavel Hrdina wrote:
On Mon, Apr 27, 2020 at 01:52:13PM +0100, Daniel P. Berrangé wrote:
On Sat, Apr 25, 2020 at 10:59:45PM +0200, Andrea Bolognani wrote:
People who install the RPM packages already get this, but it's good practice so let's make it happen for those who install from source as well.
Do we really want this ? I've not noticed apps doing this in general - these files are usually non-installed with autotools and meson AFAIK.
It was probably my motivation not to install them in the first place as well because the user already has the files from sources so there is usually no need to install them.
One doesn't necessarily keep the source directory around after installing a piece or software, and the person installing it might be different from the one interested in looking at these files: think the user on a managed server curious about what features a newly-deployed version of some software introduces.
I've checked systemd and they are installing these files using meson but GLib for example is not installing them.
After checking other libvirt projects we are not installing this files using autotools or other build systems.
I can bring up another counter-example that's close to us: QEMU installs these files, along with a bunch of other documentation.
QEMU isn't a good example of best practice as it is a completely custom written set of build system rules.
Taking all of that into account I'm changing my mind and we should not install these files from sources as well.
I still think it's a good idea.
The packaging for both Fedora and Debian go out of their way to include them, and at least for the latter it's strongly recommended by policy to include NEWS while including COPYING is mandatory. Clearly both projects consider this information important to users, and I don't see any disadvantage in installing them even for people who build from sources directly instead of using a package, only advantages really.
I think these are different situations though. Distro vendors include the files because the end users don't have any direct visibility of the source tarball. So the only way users would see the files is if the binary packages included them. I don't think that extends to building from source in general & don't see a need for us to do this specially in the libvirt-dbus module, or indeed our other modules in general. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Mon, 2020-04-27 at 15:30 +0100, Daniel P. Berrangé wrote:
On Mon, Apr 27, 2020 at 04:16:57PM +0200, Andrea Bolognani wrote:
On Mon, 2020-04-27 at 15:38 +0200, Pavel Hrdina wrote:
I've checked systemd and they are installing these files using meson but GLib for example is not installing them.
After checking other libvirt projects we are not installing this files using autotools or other build systems.
I can bring up another counter-example that's close to us: QEMU installs these files, along with a bunch of other documentation.
QEMU isn't a good example of best practice as it is a completely custom written set of build system rules.
The fact that it accomplishes something using a custom set of tools doesn't automatically make the result not a good idea.
The packaging for both Fedora and Debian go out of their way to include them, and at least for the latter it's strongly recommended by policy to include NEWS while including COPYING is mandatory. Clearly both projects consider this information important to users, and I don't see any disadvantage in installing them even for people who build from sources directly instead of using a package, only advantages really.
I think these are different situations though. Distro vendors include the files because the end users don't have any direct visibility of the source tarball. So the only way users would see the files is if the binary packages included them. I don't think that extends to building from source in general & don't see a need for us to do this specially in the libvirt-dbus module, or indeed our other modules in general.
The fact that something is not strictly needed doesn't automatically make it not worth pursuing. Anyway, I don't care enough about this to keep the discussion going any further, so I'm just going to go ahead and revert the change. Can we have a libvirt-dbus release now? :) -- Andrea Bolognani / Red Hat / Virtualization

On Mon, Apr 27, 2020 at 04:54:33PM +0200, Andrea Bolognani wrote:
On Mon, 2020-04-27 at 15:30 +0100, Daniel P. Berrangé wrote:
On Mon, Apr 27, 2020 at 04:16:57PM +0200, Andrea Bolognani wrote:
On Mon, 2020-04-27 at 15:38 +0200, Pavel Hrdina wrote:
I've checked systemd and they are installing these files using meson but GLib for example is not installing them.
After checking other libvirt projects we are not installing this files using autotools or other build systems.
I can bring up another counter-example that's close to us: QEMU installs these files, along with a bunch of other documentation.
QEMU isn't a good example of best practice as it is a completely custom written set of build system rules.
The fact that it accomplishes something using a custom set of tools doesn't automatically make the result not a good idea.
The packaging for both Fedora and Debian go out of their way to include them, and at least for the latter it's strongly recommended by policy to include NEWS while including COPYING is mandatory. Clearly both projects consider this information important to users, and I don't see any disadvantage in installing them even for people who build from sources directly instead of using a package, only advantages really.
I think these are different situations though. Distro vendors include the files because the end users don't have any direct visibility of the source tarball. So the only way users would see the files is if the binary packages included them. I don't think that extends to building from source in general & don't see a need for us to do this specially in the libvirt-dbus module, or indeed our other modules in general.
The fact that something is not strictly needed doesn't automatically make it not worth pursuing.
Anyway, I don't care enough about this to keep the discussion going any further, so I'm just going to go ahead and revert the change.
Can we have a libvirt-dbus release now? :)
Yes and no :) when I was testing your changes the test suite failed for me as it was missing dbus-daemon package which is no longer installed by default on Fedora. I'm already looking into dbus-broker and how to use it instead but it looks like it might not be that ease :/. I'll continue with the investigation to see if there is some way how we can use dbus-broker for our testing or I'll document somewhere that we require dbus-daemon and why. Pavel

Now that the documentation is installed just like other files, we can also pick it from the installation directory instead of the source directory. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- libvirt-dbus.spec.in | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libvirt-dbus.spec.in b/libvirt-dbus.spec.in index 84172fa..2230ffc 100644 --- a/libvirt-dbus.spec.in +++ b/libvirt-dbus.spec.in @@ -50,8 +50,9 @@ getent passwd %{system_user} >/dev/null || \ exit 0 %files -%doc AUTHORS.rst NEWS.rst -%license COPYING +%doc %{_docdir}/%{name}/AUTHORS.rst +%doc %{_docdir}/%{name}/NEWS.rst +%license %{_docdir}/%{name}/COPYING %{_sbindir}/libvirt-dbus %{_datadir}/dbus-1/services/org.libvirt.service %{_datadir}/dbus-1/system-services/org.libvirt.service -- 2.25.3

On Sat, Apr 25, 2020 at 10:59:46PM +0200, Andrea Bolognani wrote:
Now that the documentation is installed just like other files, we can also pick it from the installation directory instead of the source directory.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- libvirt-dbus.spec.in | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/libvirt-dbus.spec.in b/libvirt-dbus.spec.in index 84172fa..2230ffc 100644 --- a/libvirt-dbus.spec.in +++ b/libvirt-dbus.spec.in @@ -50,8 +50,9 @@ getent passwd %{system_user} >/dev/null || \ exit 0
%files -%doc AUTHORS.rst NEWS.rst -%license COPYING +%doc %{_docdir}/%{name}/AUTHORS.rst +%doc %{_docdir}/%{name}/NEWS.rst +%license %{_docdir}/%{name}/COPYING %{_sbindir}/libvirt-dbus %{_datadir}/dbus-1/services/org.libvirt.service %{_datadir}/dbus-1/system-services/org.libvirt.service
This patch will change the location of COPYING file from /usr/share/licenses/libvirt-dbus/COPYING to /usr/share/doc/libvirt-dbus/COPYING Which is probably OK but the preferred location is the licenses directory. I would like to keep this behavior for RPM packages which means move the file in the %install phase or using this: %exclude %{_docdir}/%{name}/COPYING %license COPYING Pavel

On Mon, 2020-04-27 at 14:39 +0200, Pavel Hrdina wrote:
On Sat, Apr 25, 2020 at 10:59:46PM +0200, Andrea Bolognani wrote:
%files -%doc AUTHORS.rst NEWS.rst -%license COPYING +%doc %{_docdir}/%{name}/AUTHORS.rst +%doc %{_docdir}/%{name}/NEWS.rst +%license %{_docdir}/%{name}/COPYING
This patch will change the location of COPYING file from
/usr/share/licenses/libvirt-dbus/COPYING
to
/usr/share/doc/libvirt-dbus/COPYING
Which is probably OK but the preferred location is the licenses directory. I would like to keep this behavior for RPM packages which means move the file in the %install phase or using this:
%exclude %{_docdir}/%{name}/COPYING %license COPYING
That's a nuance of RPM packaging that I completely missed. I'll squash in the %exclude trick you suggest and verify that the file actually ends up in the expected location before pushing. Thanks for the speedy review! :) -- Andrea Bolognani / Red Hat / Virtualization

On Sat, Apr 25, 2020 at 10:59:37PM +0200, Andrea Bolognani wrote:
I think it's high time that we have a new libvirt-dbus release, since the last one was more than a year ago: since then, there has been at least one bug fix necessary to keep it building on modern platforms, an additional API has been implemented, and of course we've switched the build system from autotools to Meson.
I validated the status of what's in master by importing a snapshot into the Debian package and everything seems to be in order, so I'm fairly confident we could basically just tag a release right away; there are, however, a few straightforward improvements that I would like to get into the new release, so here we are :)
Apart from patch 8/9 which I can't review confidently, the rest looks good, provided you've tested this thoroughly, for the whole series: Reviewed-by: Erik Skultety <eskultet@redhat.com>

On Mon, 2020-04-27 at 10:17 +0200, Erik Skultety wrote:
On Sat, Apr 25, 2020 at 10:59:37PM +0200, Andrea Bolognani wrote:
I think it's high time that we have a new libvirt-dbus release, since the last one was more than a year ago: since then, there has been at least one bug fix necessary to keep it building on modern platforms, an additional API has been implemented, and of course we've switched the build system from autotools to Meson.
I validated the status of what's in master by importing a snapshot into the Debian package and everything seems to be in order, so I'm fairly confident we could basically just tag a release right away; there are, however, a few straightforward improvements that I would like to get into the new release, so here we are :)
Apart from patch 8/9 which I can't review confidently, the rest looks good, provided you've tested this thoroughly, for the whole series:
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Thanks! Pavel promised he'd look at this series, and consider making a new release, soon :) -- Andrea Bolognani / Red Hat / Virtualization

On Sat, Apr 25, 2020 at 10:59:37PM +0200, Andrea Bolognani wrote:
I think it's high time that we have a new libvirt-dbus release, since the last one was more than a year ago: since then, there has been at least one bug fix necessary to keep it building on modern platforms, an additional API has been implemented, and of course we've switched the build system from autotools to Meson.
I validated the status of what's in master by importing a snapshot into the Debian package and everything seems to be in order, so I'm fairly confident we could basically just tag a release right away; there are, however, a few straightforward improvements that I would like to get into the new release, so here we are :)
Andrea Bolognani (9): git: Minimize ignore patterns spec: Install fewer documentation files README: Convert to reStructuredText HACKING: Convert to reStructuredText NEWS: Convert to reStructuredText AUTHORS: Convert to reStructuredText man: Convert to reStructuredText meson: Install documentation spec: Pick documentation from installation directory
With the issues in last two patches fixed Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
participants (4)
-
Andrea Bolognani
-
Daniel P. Berrangé
-
Erik Skultety
-
Pavel Hrdina