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