new var DIST_CONTRIB adds useful things for packagers from contrib

nomnombtc commented at 3:29 am on August 24, 2016: contributor

This creates a new variable DIST_CONTRIB in Makefile.am and adds it to EXTRA_DIST.

I included a few useful things like contrib/debian subdir (contains manpages, .desktop file), contrib/rpm subdir (contains .spec file to build rpms), contrib/init subdir (contains things like systemd startup files) and the *.bash-completion files.

Reason is that many packagers just use the (unsigned) tarball from the github release page, as it contains the full contrib folder including manpages. With this change the release tarball created by make dist contains most of those files too, so packagers can use this one instead. See also #6753

new var DIST_CONTRIB adds useful things for packagers from contrib/ to EXTRA_DIST 1f9c6be496

jonasschnelli added the label Build system on Aug 24, 2016

laanwj commented at 8:51 am on August 25, 2016: member

This reminds me, we should move the manpages from contrib/debian to a more general directory, and make make install install them, like a proper unix package.

nomnombtc commented at 4:52 am on August 26, 2016: contributor

Indeed this would make sense. I looked into how other projects handle the manpages. And it seems most of the stuff gets automatically handled by automake, just need to define a few things.

So I got inspired by how gnome does it: https://git.gnome.org/browse/gnome-shell/tree/Makefile.am and played a bit around in trial&error mode. And came up with this: https://github.com/nomnombtc/bitcoin/commits/man_automake

But I wasn’t sure about the license of the debian manpages, only one is MIT the others are GPL. Then I found this program called help2man, which creates manpages from running a binary with -help option and so I just created a few manpages on my own.

A positive side effect of all of this is that those manpages now also end up in the tarball that gets created by make dist - which was my goal with this PR we are commenting in here …

If you think this is the correct way of doing it and you can clarify if the debian manpages can be included because of the GPL license, we could use this for 7626.

laanwj commented at 8:03 am on August 26, 2016: member

Full diff can be seen here: https://github.com/bitcoin/bitcoin/compare/master...nomnombtc:man_automake

Looks good to me, concept ACK.

The man page licensing issue came up before: #1040 (comment) . I don’t have an answer there. I do not know what the licensing rules are for documentation, but I’d feel better to have them under the same license as Bitcoin Core itself.

We could ask the authors if they agree with re-licensing.

Good idea to auto-generate the man pages! that will prevent them from being eternally out of date. The installation step looks ok to me.

As gen_manpages.sh is a tool to be manually executed by developers in a suitable environment, please put it in contrib/devtools, the scripts to update translations and such are there too.

MarcoFalke commented at 8:14 am on August 26, 2016: member

I don’t understand why licensing is an issue with auto generated man pages through help2man. Our help text is MIT licensed and changing the formatting with a program is not a copyrightable action. So changing from MIT to GPL exclusively is not even possible.

MarcoFalke commented at 8:14 am on August 26, 2016: member

Concept ACK

laanwj commented at 10:03 am on August 26, 2016: member

I don’t understand why licensing is an issue with auto generated man pages through help2man. Our help text is MIT licensed and changing the formatting with a program is not a copyrightable action

As I understood it’s a problem with the current manpages in contrib/debian, not the proposed auto-generated ones.

MarcoFalke commented at 10:56 am on August 26, 2016: member

Ok, but we would just delete the current manpages in contrib/debian and (maybe) replace them with the autogenerated ones? GPL doesn’t forbid deleting.

laanwj commented at 11:03 am on August 26, 2016: member

Sure, if there is nothing in those man pages that we want to keep, then we can save ourselves the work of trying to get them re-licensed. That’s a valid option.

remove debian folder from DIST_CONTRIB 1da2e16a99

in Makefile.am: in 1f9c6be496 outdated

31@@ -32,6 +32,10 @@ OSX_PLIST=$(top_builddir)/share/qt/Info.plist #not installed
32 OSX_QT_TRANSLATIONS = da,de,es,hu,ru,uk,zh_CN,zh_TW
33 
34 DIST_DOCS = $(wildcard doc/*.md) $(wildcard doc/release-notes/*.md)
35+DIST_CONTRIB = $(wildcard $(top_srcdir)/contrib/*.bash-completion) \
36+	       $(top_srcdir)/contrib/debian \

laanwj commented at 12:46 pm on September 26, 2016:

Now that the manpages have been moved out of debian, is there still a reason to include the debian directory here?

nomnombtc commented at 0:34 am on September 27, 2016:

There is still a bitcoin-qt.desktop file and a bitcoin-qt.protocol file in there (makes it open bitcoin-qt when clicking a bitcoin: url in kde apps like kmail for example) but I could live without it… Problem is still apparently everything in debian/ is GPL2 according to the copyright file in that folder:

Files: debian/* Copyright: 2010-2011, Jonas Smedegaard dr@jones.dk 2011, Matt Corallo matt@bluematt.me License: GPL-2+

So I guess it makes sense to remove it for now, until someone has a better idea because of the licensing.

laanwj commented at 6:36 am on September 27, 2016:

Thanks, yes, the licensing is still annoying.

Would be easy enough to write our own desktop and protocol file and install them, though OTOH I suppose those things are quite distro specific, at least where to install them.

Out of scope for this pull at least.

laanwj assigned theuni on Sep 29, 2016

theuni commented at 5:25 pm on September 29, 2016: member

Concept ACK. Please list them rather than using wildcards, though. That ensures that local files don’t cause unexpected results.

I know doc already uses it above, but let’s not make it worse here.

do not use wildcard for .bash-completion files f6c6c6b30e

nomnombtc commented at 5:27 pm on September 30, 2016: contributor

@theuni

Ok no problem, I’ve added a commit that removes the wildcard.

fanquake commented at 12:26 pm on November 6, 2016: member

Concept ACK

MarcoFalke commented at 3:50 pm on November 6, 2016: member

Hmm, maybe squash the three commits into one?

Concept ACK f6c6c6b

laanwj referenced this in commit 078900df75 on Nov 7, 2016

laanwj commented at 12:07 pm on November 7, 2016: member

Merged (and squashed) via 078900d

laanwj closed this on Nov 7, 2016

nomnombtc deleted the branch on Dec 13, 2016

codablock referenced this in commit 05b34f49b4 on Jan 13, 2018

andvgal referenced this in commit 61c314b123 on Jan 6, 2019

CryptoCentric referenced this in commit 94a8e284f1 on Feb 15, 2019

MarcoFalke locked this on Sep 8, 2021

new var DIST_CONTRIB adds useful things for packagers from contrib #8568