X-Original-To: alpine-devel@lists.alpinelinux.org Delivered-To: alpine-devel@mail.alpinelinux.org Received: from dal-a2.localdomain (unknown [74.117.189.115]) by mail.alpinelinux.org (Postfix) with ESMTP id D7389DC008A for ; Fri, 9 May 2014 11:30:35 +0000 (UTC) Received: from ncopa-desktop.alpinelinux.org (3.203.202.84.customer.cdi.no [84.202.203.3]) (using SSLv3 with cipher ECDHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) (Authenticated sender: ncopa@tanael.org) by dal-a2.localdomain (Postfix) with ESMTPSA id DCE76BC3973; Fri, 9 May 2014 11:30:34 +0000 (UTC) Date: Fri, 9 May 2014 13:30:30 +0200 From: Natanael Copa To: Stuart Cardall Cc: alpine-devel@lists.alpinelinux.org Subject: Re: [alpine-devel] [PATCH] testing/bitcoin: update to 0.9.1 (revised) Message-ID: <20140509133030.71a3fbc7@ncopa-desktop.alpinelinux.org> In-Reply-To: <1399610567-13489-1-git-send-email-developer@it-offshore.co.uk> References: <1399610567-13489-1-git-send-email-developer@it-offshore.co.uk> X-Mailer: Claws Mail 3.9.3 (GTK+ 2.24.23; x86_64-alpine-linux-musl) X-Mailinglist: alpine-devel Precedence: list List-Id: Alpine Development List-Unsubscribe: List-Post: List-Help: List-Subscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Fri, 9 May 2014 04:42:47 +0000 Stuart Cardall wrote: > This is a revised patch against a fresh aports clone. Thanks, but I doubt you payed attention to what I did to the apkbuild. > By default Bitcoin will listen for RPC connections on port 8332 > if no port is set. The post-install script sets a random / user/ pass > & port on a new install ONLY (& is no longer interactive). > > On musl bitcoin now builds the tests successfully. > > I've revised the init & added a pre-install script to now run > the daemon as a bitcoin user / group in /var/lib/bitcoin. Checkpath > enforces sensible permissions. > --- > testing/bitcoin/11-ssize_t.patch | 10 ++++ > testing/bitcoin/20-set-default-env.patch | 33 +++---------- > testing/bitcoin/APKBUILD | 82 ++++++++++++++++---------------- > testing/bitcoin/bitcoin.conf | 13 +++++ > testing/bitcoin/bitcoin.initd | 38 +++++++++++++++ > testing/bitcoin/bitcoin.post-install | 50 +++++++++++++++++++ > testing/bitcoin/bitcoin.pre-install | 5 ++ > testing/bitcoin/ssize_t.patch | 12 ----- > 8 files changed, 165 insertions(+), 78 deletions(-) > create mode 100644 testing/bitcoin/11-ssize_t.patch > create mode 100644 testing/bitcoin/bitcoin.conf > create mode 100644 testing/bitcoin/bitcoin.initd > create mode 100644 testing/bitcoin/bitcoin.post-install > create mode 100644 testing/bitcoin/bitcoin.pre-install > delete mode 100644 testing/bitcoin/ssize_t.patch Why do you replace ssize_t.patch with a version that has -p0 level for patch, which you then workaround with cd src/? Why not just keep the current patchm which is -p1 like 99% of the rest of our patches in aports tree? > > diff --git a/testing/bitcoin/11-ssize_t.patch b/testing/bitcoin/11-ssize_t.patch > new file mode 100644 > index 0000000..34df089 > --- /dev/null > +++ b/testing/bitcoin/11-ssize_t.patch > @@ -0,0 +1,10 @@ > +--- src/leveldb/db/db_iter.cc > ++++ src.new/leveldb/db/db_iter.cc > +@@ -2,6 +2,7 @@ > + // Use of this source code is governed by a BSD-style license that can be > + // found in the LICENSE file. See the AUTHORS file for names of contributors. > + > ++#include > + #include "db/db_iter.h" > + > + #include "db/filename.h" > diff --git a/testing/bitcoin/20-set-default-env.patch b/testing/bitcoin/20-set-default-env.patch > index b9717ab..fa9701d 100644 > --- a/testing/bitcoin/20-set-default-env.patch > +++ b/testing/bitcoin/20-set-default-env.patch > @@ -1,7 +1,7 @@ > ---- ./src/bitcoin-cli.cpp > -+++ ./src/bitcoin-cli.cpp > +--- src/bitcoin-cli.cpp > ++++ src.new/bitcoin-cli.cpp > @@ -53,6 +53,16 @@ > - > + > int main(int argc, char* argv[]) > { > + #ifndef WIN32 > @@ -17,10 +17,10 @@ > try > { > if(!AppInitRPC(argc, argv)) > ---- ./src/bitcoind.cpp > -+++ ./src/bitcoind.cpp > +--- src/bitcoind.cpp > ++++ src.new/bitcoind.cpp > @@ -166,6 +166,16 @@ > - > + > int main(int argc, char* argv[]) > { > + #ifndef WIN32 > @@ -34,24 +34,5 @@ > + #endif > + > bool fRet = false; > - > + > // Connect bitcoind signal handlers > ---- ./src/qt/bitcoin.cpp.orig > -+++ ./src/qt/bitcoin.cpp > -@@ -445,6 +445,16 @@ > - #ifndef BITCOIN_QT_TEST > - int main(int argc, char *argv[]) > - { > -+#ifndef WIN32 > -+ try > -+ { > -+ boost::filesystem::path::codecvt(); // Raises runtime error if current locale is invalid > -+ } catch(std::runtime_error &e) > -+ { > -+ setenv("LC_ALL", "C", 1); // Force C locale > -+ } > -+#endif > -+ > - /// 1. Parse command-line options. These take precedence over anything else. > - // Command-line options take precedence: > - ParseParameters(argc, argv); Why do you remove the above hunk? I doubt it was intentional. bitcoin-qt gave same locale error when i tried it here which is why I added it. http://git.alpinelinux.org/cgit/aports/commit/testing/bitcoin?id=2f4ba4fefd39abc22bdaba325a01df38f8414ad4 > diff --git a/testing/bitcoin/APKBUILD b/testing/bitcoin/APKBUILD > index 57a2752..8f729b6 100644 > --- a/testing/bitcoin/APKBUILD > +++ b/testing/bitcoin/APKBUILD > @@ -1,72 +1,74 @@ > -# Maintainer: Natanael Copa > +# Contributor: Natanael Copa > +# Maintainer: Stuart Cardall > pkgname=bitcoin > pkgver=0.9.1 > -pkgrel=2 > +pkgrel=0 We should only reset pkgrel to 0 when pkgver increases, so this is wrong. > pkgdesc="decentralized P2P electronic cash system" > url="http://www.bitcoin.org/" > arch="all" > license="MIT" > depends="" > -depends_dev="" > -makedepends="$depends_dev boost-dev openssl-dev db-dev miniupnpc-dev > - qt5-qtbase-dev qt5-qttools-dev protobuf-dev libqrencode-dev > - autoconf automake libtool" > -install="" > -subpackages="$pkgname-qt $pkgname-cli" > -source="$pkgname-$pkgver.tar.gz::https://github.com/bitcoin/bitcoin/archive/v$pkgver.tar.gz > - ssize_t.patch > +depends_dev="autoconf automake" > +makedepends="$depends_dev boost-dev openssl-dev db-dev miniupnpc-dev qt-dev protobuf-dev libqrencode-dev" > +install="$pkgname.post-install $pkgname.pre-install" Why do you pick qt4 instead of qt5? > +subpackages="$pkgname-qt" > +source="https://github.com/bitcoin/bitcoin/archive/v$pkgver.tar.gz this will case the tarball be named v0.9.1.tar.gz and be stored in the shared distfiles dir. Guess what will happen if more than one package does the same? We should always prefix the tarballs with $pkgname when using the github tags. > + 11-ssize_t.patch > 20-set-default-env.patch > + $pkgname.initd > + $pkgname.conf > " > > _builddir="$srcdir"/bitcoin-$pkgver > prepare() { > local i > - cd "$_builddir" > + cd "$_builddir"/src As mentioned, I'd like to fix the patches instead of cd src/ > for i in $source; do > case $i in > *.patch) msg $i; patch -p1 -i "$srcdir"/$i || return 1;; > esac > done > - ./autogen.sh > } > > build() { > cd "$_builddir" > + ./autogen.sh Any specific reason you want run autgen.sh from build() instead of prepare()? I think we normally run autoconf/automake from prepare. > ./configure \ > - --build=$CBUILD \ > - --host=$CHOST \ > - --prefix=/usr \ > - --mandir=/usr/share/man \ > - --with-incompatible-bdb \ > - --disable-ccache \ > - --with-gui=qt5 \ > - || return 1 > - make || return 1 > -} > - > -package() { > - cd "$_builddir" > - make install DESTDIR="$pkgdir" || return 1 > + --disable-tests \ > + --with-incompatible-bdb \ > + || return 1 > + make || return 1 I think we should keep the --build and --host options. The --disable-ccache is needed to use CC="ccache gcc", otherwise will the stupid confiure script set CC to "ccache ccache gcc" because it finds ccaches installed and build fails. I also think we should aim for qt5. > } > > qt() { > pkgdesc="Bitcoin with a Qt frontend & QR Code support" > + #depends="bitcoin=$pkgver-r$pkgrel" This is dead code an useless as comment. > + cd "$srcdir"/bitcoin-$pkgver/src/qt > + install -Dm755 bitcoin-qt "$pkgdir"/usr/bin/bitcoin-qt || return 1 > mkdir -p "$subpkgdir"/usr/bin we normally do make install from package() and move files to $subpkgdir. > - mv "$pkgdir"/usr/bin/bitcoin-qt "$subpkgdir"/usr/bin/ || return 1 > + mv "$pkgdir"/usr/bin/bitcoin-qt \ > + "$subpkgdir"/usr/bin/ > } > > -cli() { > - pkgdesc="Bitcoin CLI" > - mkdir -p "$subpkgdir"/usr/bin > - mv "$pkgdir"/usr/bin/bitcoin-cli "$subpkgdir"/usr/bin/ || return 1 Why do you remove the -cli subpackage? > +package() { > + cd "$_builddir" > + install -m755 -D "$srcdir"/$pkgname.initd "$pkgdir"/etc/init.d/$pkgname || return 1 > + install -m600 -D "$srcdir"/$pkgname.conf "$pkgdir"/etc/$pkgname.conf || return 1 > + install -Dm755 src/bitcoind "$pkgdir"/usr/bin/bitcoind || return 1 > } Why not make install DESTDIR="$pkgdir"? > > -md5sums="7a9c14c09b04e3e37d703fbfe5c3b1e2 bitcoin-0.9.1.tar.gz > -716a88e668729e89d3eb929da86f7f04 ssize_t.patch > -de733598adec12a7c0e8f2665a086f6d 20-set-default-env.patch" > -sha256sums="bf5021a426b5e38a741a5294a0ceb22daa74cda11c6dc0478c4aa48c55fdccb3 bitcoin-0.9.1.tar.gz > -f0a094c7b374338ad153ee06b7dd2369dad6b97b1f1bb0252da12c9aaace5321 ssize_t.patch > -d0763e09784cb09900be7e702e4bbadfc3230715d1f6afa6d70417ab64dde5ef 20-set-default-env.patch" > -sha512sums="75be422fc263f209783fc66d9fd99027d67c7460c599a23badafcf5546ce7cb21206fce04c516a1c101aeff13542b332249b3b70a70c483aa82a5048dbdc9d92 bitcoin-0.9.1.tar.gz > -98aa5ad81bdb4ae961b791bc978c39117cdf2d83c2181f92bebbb0db107d9b6e86eda265fb3f93ff8a5ca8a7754d7148818b98095d57201dff9363d60b97e7dd ssize_t.patch > -288934bea005cd6fdcf87525ce82fc0c031e0b86bba02c217c1e32536d3c51981ff187ee630f64787c6d7978d89802f8107b91fa8b2a86fbbb45cb79738e291b 20-set-default-env.patch" > +md5sums="7a9c14c09b04e3e37d703fbfe5c3b1e2 v0.9.1.tar.gz > +48ea824e0ac60c6cd5aa1b96ba9f9cca 11-ssize_t.patch > +f062bc0791c1db61c1a32bccb4bfb5cf 20-set-default-env.patch > +56edf10681b0cd7cc33de1fd24155108 bitcoin.initd > +3e9b4a37bb46e6cd83b6824870c58484 bitcoin.conf" > +sha256sums="bf5021a426b5e38a741a5294a0ceb22daa74cda11c6dc0478c4aa48c55fdccb3 v0.9.1.tar.gz > +09377eb957adf01dc9502437c945f61fc21a589cc808216c2bc42b563fb73a13 11-ssize_t.patch > +4b8058b6288e17eecd1b65a789f453a01247cb8034df2983bed65b3420e673d8 20-set-default-env.patch > +df060fcee7227a2c7c4a435e24dea25b6388a6d6a98f01975c466a2c770e976a bitcoin.initd > +b7a31bf251f1011ab6584b610acac8494612c2012e9771985228c36c08a315ac bitcoin.conf" > +sha512sums="75be422fc263f209783fc66d9fd99027d67c7460c599a23badafcf5546ce7cb21206fce04c516a1c101aeff13542b332249b3b70a70c483aa82a5048dbdc9d92 v0.9.1.tar.gz > +0c16a9103b889a600f7096a5a6eb4470feea3f996b07e7175f2dcc76b8fe3665070168c27f0fe60c0ca60314645bac22a7d01e54250f0ec280bc325546391c01 11-ssize_t.patch > +0550cebd88b25cbaced6bff5e471cb95f1177f9aaed71bd0d82b677b13c8ebb62d335945bb8f495ce7db702d2ae6e0f9c12391cb24095c9c3dd39cd429888a03 20-set-default-env.patch > +9bab044e3ddc229f8e2b2eb170361f92d234c78e21c6f1c24a5fe1d0b480db5b6728dda990752eb0820085ecdc41538e735d631ff5886c270475a5b634e3729f bitcoin.initd > +48abce0f0a5b088c957aa5cb2bcf9bb520593caa7a20019bdf785e43f7d2459968240d4529dfa30be2fca92891bf50c1253a513530511e99d8ee471d9ff6bd95 bitcoin.conf" > diff --git a/testing/bitcoin/bitcoin.conf b/testing/bitcoin/bitcoin.conf > new file mode 100644 > index 0000000..4f070b1 > --- /dev/null > +++ b/testing/bitcoin/bitcoin.conf > @@ -0,0 +1,13 @@ > +### Bitcoin configuration for Alpine Linux #### > +# # > +##### data directory is /var/lib/bitcoin ###### > +##### & set in /etc/init.d/bitcoin not here ### > +# > +#proxy=127.0.0.1:9050 #use TOR Socks Proxy > +rpcallowip=127.0.0.1 > +rpcuser=changeme > +rpcpassword=changeme > +rpcport=changeme > +daemon=1 > +#gen=1 #generate bitcoins > + Could we create a system user called 'bitcoin' in .pre-install and use that? > diff --git a/testing/bitcoin/bitcoin.initd b/testing/bitcoin/bitcoin.initd > new file mode 100644 > index 0000000..5f0ef76 > --- /dev/null > +++ b/testing/bitcoin/bitcoin.initd > @@ -0,0 +1,38 @@ > +#!/sbin/runscript > + > +# Bitcoin init.d file for Alpine Linux. > + > +name=bitcoind > +daemon=/usr/bin/$name > +config=/etc/bitcoin.conf > +user=bitcoin > +group=bitcoin > +## supercedes datadir set in $config ## > +datadir=/var/lib/bitcoin > +pidfile=/var/run/bitcoin/$name.pid > + > +depend() { > + need net > + after logger firewall > +} > + > +start() { > + ebegin "Starting ${name}" > + # enforce permissions > + checkpath -q -d ${pidfile%/*} -o ${user}:${group} > + checkpath -q -d ${datadir} -m 0700 -o ${user}:${group} > + checkpath -q -f ${config} -m 0600 -o ${user}:${group} > + start-stop-daemon --start --quiet \ > + --pidfile ${pidfile} \ > + --user ${user}:${group} \ > + --exec ${daemon} -- -conf=${config} -datadir=${datadir} -pid=${pidfile} > + eend $? > +} > + > +stop() { > + ebegin "Stopping ${name}" > + start-stop-daemon --stop --quiet \ > + --pidfile ${pidfile} \ > + --exec ${daemon} > + eend $? > +} > diff --git a/testing/bitcoin/bitcoin.post-install b/testing/bitcoin/bitcoin.post-install > new file mode 100644 > index 0000000..06d3af7 > --- /dev/null > +++ b/testing/bitcoin/bitcoin.post-install > @@ -0,0 +1,50 @@ > +#!/bin/sh > + > +NORMAL="\033[1;0m" > +STRONG="\033[1;1m" > +GREEN="\033[1;32m" > + > +config=$(grep -F 'config=' /etc/init.d/bitcoin |sed 's/config=//') > + > +randgen() { > + output=$(< /dev/urandom tr -dc '0-9a-zA-Z!@#$%^&*_+-' | head -c${1:-$1}) 2>/dev/null > + echo $output > +} > + > +# Generate random port > +randport() { > + n=$RANDOM > + while [ $n -gt 65536 ] && [ $n -lt 1024 ]; do > + n=$RANDOM > + done > + echo $n > +} That loop could theoretically by random loop for very long time. How about: randport=$(( 1024 + $(( $RANDOM % $(( 65534 - 1024 )) )) )) (I'm not sure it is a good idea to use 65535 as port number) > + > +# Find unused TCP port > +findRandomTcpPort(){ > + port=$(randport) > + while : > + do > + (echo >/dev/tcp/localhost/$port) &>/dev/null && port=$(randport) || break > + done > + echo $port > +} I don't have /dev/tcp/ here? > + > +GenPasswd(){ > + sed -i "/rpcuser=/ c \rpcuser=USER-"$(randgen 32)"" $config > + sed -i "/rpcpassword=/ c \rpcpassword=PW-"$(randgen 64)"" $config > + sed -i "/rpcport=/ c \rpcport="$(findRandomTcpPort)"" $config > + print_green "Generated random user / password / port in:" " $config\n" > +} > + > + > +print_green() { > + local prompt="${STRONG}$1${GREEN}$2${NORMAL}" > + printf "${prompt}%s" > +} > + > +if grep -F "changeme" $config 1>/dev/null; then > + GenPasswd > +fi > + > +exit 0 > diff --git a/testing/bitcoin/bitcoin.pre-install b/testing/bitcoin/bitcoin.pre-install > new file mode 100644 > index 0000000..f020a8b > --- /dev/null > +++ b/testing/bitcoin/bitcoin.pre-install > @@ -0,0 +1,5 @@ > +#!/bin/sh > +addgroup -S bitcoin 2>/dev/null > +adduser -S -H -h /var/lib/bitcoin -g bitcoin -G bitcoin -D -s /sbin/nologin bitcoin 2>/dev/null > +exit 0 > + > diff --git a/testing/bitcoin/ssize_t.patch b/testing/bitcoin/ssize_t.patch > deleted file mode 100644 > index 2c21854..0000000 > --- a/testing/bitcoin/ssize_t.patch > +++ /dev/null > @@ -1,12 +0,0 @@ > -diff --git a/src/leveldb/db/db_iter.cc b/src/leveldb/db/db_iter.cc > -index 071a54e..68a18f2 100644 > ---- a/src/leveldb/db/db_iter.cc > -+++ b/src/leveldb/db/db_iter.cc > -@@ -2,6 +2,7 @@ > - // Use of this source code is governed by a BSD-style license that can be > - // found in the LICENSE file. See the AUTHORS file for names of contributors. > - > -+#include > - #include "db/db_iter.h" > - > - #include "db/filename.h" -nc --- Unsubscribe: alpine-devel+unsubscribe@lists.alpinelinux.org Help: alpine-devel+help@lists.alpinelinux.org ---