Skip to content

Commit

Permalink
build: improve sed robustness by not using sed
Browse files Browse the repository at this point in the history
Summary:
```
While using sed can be handy to use for a quick-fix, these instances
accumulate, and can become unmaintainable. Not only that, but using sed
isn't necessarily robust and it can fail silently. Most of our usage is
also missing any documentation explaining why something is being done,
when it should be updated/removed etc.

Rather than relying on sed going forward, where possible, I've converted
our sed usage into patches. These are easier to maintain, contain
documentation, and should fail loudly when they don't apply.

The remaining sed usage, (1 in miniupnpc, the rest in qt), are
non-trivial to remove, as they are using build-time variables, or some
input from the environment.
```

Backport of core [[bitcoin/bitcoin#19761 | PR19761]], slightly updated to match our depends.

Depends on D7760.

Test Plan:
  make build-all

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D7763
  • Loading branch information
fanquake authored and sickpig committed May 13, 2021
1 parent ed7706e commit 354e500
Show file tree
Hide file tree
Showing 14 changed files with 334 additions and 10 deletions.
7 changes: 4 additions & 3 deletions depends/packages/bdb.mk
Expand Up @@ -4,6 +4,8 @@ $(package)_download_path=https://download.oracle.com/berkeley-db
$(package)_file_name=db-$($(package)_version).NC.tar.gz
$(package)_sha256_hash=76a25560d9e52a198d37a31440fd07632b5f1f8f9f2b6d5438f4bc3e7c9013ef
$(package)_build_subdir=build_unix
$(package)_patches=clang_cxx_11.patch
$(package)_patches+=winioctl.patch

define $(package)_set_vars
$(package)_config_opts=--disable-shared --enable-cxx --disable-replication --enable-option-checking
Expand All @@ -15,9 +17,8 @@ endef

define $(package)_preprocess_cmds
cd src &&\
sed -i.old 's/__atomic_compare_exchange/__atomic_compare_exchange_db/' dbinc/atomic.h && \
sed -i.old 's/atomic_init/atomic_init_db/' dbinc/atomic.h mp/mp_region.c mp/mp_mvcc.c mp/mp_fget.c mutex/mut_method.c mutex/mut_tas.c && \
sed -i.old 's/WinIoCtl.h/winioctl.h/' dbinc/win_db.h &&\
patch -p1 < $($(package)_patch_dir)/clang_cxx_11.patch && \
patch -p1 < $($(package)_patch_dir)/winioctl.patch && \
cd .. &&\
cp -f $(BASEDIR)/config.guess $(BASEDIR)/config.sub dist
endef
Expand Down
2 changes: 2 additions & 0 deletions depends/packages/boost.mk
Expand Up @@ -3,6 +3,7 @@ $(package)_version=1_70_0
$(package)_download_path=https://dl.bintray.com/boostorg/release/1.70.0/source/
$(package)_file_name=$(package)_$($(package)_version).tar.bz2
$(package)_sha256_hash=430ae8354789de4fd19ee52f3b1f739e1fba576f0aded0897c3c2bc00fb38778
$(package)_patches=unused_var_in_process.patch

define $(package)_set_vars
$(package)_config_opts_release=variant=release
Expand All @@ -24,6 +25,7 @@ $(package)_cxxflags_linux=-fPIC
endef

define $(package)_preprocess_cmds
patch -p1 < $($(package)_patch_dir)/unused_var_in_process.patch && \
echo "using $($(package)_toolset_$(host_os)) : : $($(package)_cxx) : <cxxflags>\"$($(package)_cxxflags) $($(package)_cppflags)\" <linkflags>\"$($(package)_ldflags)\" <archiver>\"$($(package)_archiver_$(host_os))\" <striper>\"$(host_STRIP)\" <ranlib>\"$(host_RANLIB)\" <rc>\"$(host_WINDRES)\" : ;" > user-config.jam
endef

Expand Down
3 changes: 2 additions & 1 deletion depends/packages/miniupnpc.mk
Expand Up @@ -3,6 +3,7 @@ $(package)_version=2.0.20180203
$(package)_download_path=https://miniupnp.tuxfamily.org/files/
$(package)_file_name=$(package)-$($(package)_version).tar.gz
$(package)_sha256_hash=90dda8c7563ca6cd4a83e23b3c66dbbea89603a1675bfdb852897c2c9cc220b7
$(package)_patches=dont_use_wingen.patch

define $(package)_set_vars
$(package)_build_opts=CC="$($(package)_cc)"
Expand All @@ -14,7 +15,7 @@ endef
define $(package)_preprocess_cmds
mkdir dll && \
sed -e 's|MINIUPNPC_VERSION_STRING \"version\"|MINIUPNPC_VERSION_STRING \"$($(package)_version)\"|' -e 's|OS/version|$(host)|' miniupnpcstrings.h.in > miniupnpcstrings.h && \
sed -i.old "s|miniupnpcstrings.h: miniupnpcstrings.h.in wingenminiupnpcstrings|miniupnpcstrings.h: miniupnpcstrings.h.in|" Makefile.mingw
patch -p1 < $($(package)_patch_dir)/dont_use_wingen.patch
endef

define $(package)_build_cmds
Expand Down
4 changes: 3 additions & 1 deletion depends/packages/native_cctools.mk
Expand Up @@ -4,6 +4,8 @@ $(package)_download_path=https://github.com/tpoechtrager/cctools-port/archive
$(package)_file_name=$($(package)_version).tar.gz
$(package)_sha256_hash=3e35907bf376269a844df08e03cbb43e345c88125374f2228e03724b5f9a2a04
$(package)_build_subdir=cctools
$(package)_patches=ld64_disable_threading.patch

$(package)_clang_version=6.0.1
$(package)_clang_download_path=https://releases.llvm.org/$($(package)_clang_version)
$(package)_clang_download_file=clang+llvm-$($(package)_clang_version)-x86_64-linux-gnu-ubuntu-16.04.tar.xz
Expand Down Expand Up @@ -49,7 +51,7 @@ endef
define $(package)_preprocess_cmds
CC=$($(package)_cc) CXX=$($(package)_cxx) INSTALLPREFIX=$($(package)_extract_dir) ./libtapi/build.sh && \
CC=$($(package)_cc) CXX=$($(package)_cxx) INSTALLPREFIX=$($(package)_extract_dir) ./libtapi/install.sh && \
sed -i.old "/define HAVE_PTHREADS/d" $($(package)_build_subdir)/ld64/src/ld/InputFiles.h
patch -p1 < $($(package)_patch_dir)/ld64_disable_threading.patch
endef

define $(package)_config_cmds
Expand Down
10 changes: 6 additions & 4 deletions depends/packages/qt.mk
Expand Up @@ -8,7 +8,10 @@ $(package)_dependencies=openssl zlib
$(package)_linux_dependencies=freetype fontconfig libxcb
$(package)_build_subdir=qtbase
$(package)_qt_libs=corelib network widgets gui plugins testlib
$(package)_patches=fix_qt_pkgconfig.patch mac-qmake.conf fix_configure_mac.patch fix_no_printer.patch fix_rcc_determinism.patch xkb-default.patch no-xlib.patch
$(package)_patches=fix_qt_pkgconfig.patch mac-qmake.conf fix_configure_mac.patch fix_no_printer.patch
$(package)_patches+= fix_rcc_determinism.patch xkb-default.patch no-xlib.patch dont_hardcode_pwd.patch
$(package)_patches+= drop_lrelease_dependency.patch


$(package)_qttranslations_file_name=qttranslations-$($(package)_suffix)
$(package)_qttranslations_sha256_hash=b36da7d93c3ab6fca56b32053bb73bc619c8b192bb89b74e3bcde2705f1c2a14
Expand Down Expand Up @@ -138,9 +141,8 @@ endef

define $(package)_preprocess_cmds
sed -i.old "s|updateqm.commands = \$$$$\$$$$LRELEASE|updateqm.commands = $($(package)_extract_dir)/qttools/bin/lrelease|" qttranslations/translations/translations.pro && \
sed -i.old "/updateqm.depends =/d" qttranslations/translations/translations.pro && \
sed -i.old "s/src_plugins.depends = src_sql src_network/src_plugins.depends = src_network/" qtbase/src/src.pro && \
sed -i.old -e 's/if \[ "$$$$XPLATFORM_MAC" = "yes" \]; then xspecvals=$$$$(macSDKify/if \[ "$$$$BUILD_ON_MAC" = "yes" \]; then xspecvals=$$$$(macSDKify/' -e 's|/bin/pwd|pwd|' qtbase/configure && \
patch -p1 -i $($(package)_patch_dir)/drop_lrelease_dependency.patch && \
patch -p1 -i $($(package)_patch_dir)/dont_hardcode_pwd.patch &&\
mkdir -p qtbase/mkspecs/macx-clang-linux &&\
cp -f qtbase/mkspecs/macx-clang/Info.plist.lib qtbase/mkspecs/macx-clang-linux/ &&\
cp -f qtbase/mkspecs/macx-clang/Info.plist.app qtbase/mkspecs/macx-clang-linux/ &&\
Expand Down
6 changes: 5 additions & 1 deletion depends/packages/zeromq.mk
Expand Up @@ -3,6 +3,7 @@ $(package)_version=4.3.1
$(package)_download_path=https://github.com/zeromq/libzmq/releases/download/v$($(package)_version)/
$(package)_file_name=$(package)-$($(package)_version).tar.gz
$(package)_sha256_hash=bcbabe1e2c7d0eec4ed612e10b94b112dd5f06fcefa994a0c79a45d835cd21eb
$(package)_patches=remove_libstd_link.patch

define $(package)_set_vars
$(package)_config_opts=--without-docs --disable-shared --disable-curve --disable-curve-keygen --disable-perf
Expand All @@ -13,6 +14,10 @@ define $(package)_set_vars
$(package)_cxxflags=-std=c++11
endef

define $(package)_preprocess_cmds
patch -p1 < $($(package)_patch_dir)/remove_libstd_link.patch
endef

define $(package)_config_cmds
$($(package)_autoconf)
endef
Expand All @@ -26,6 +31,5 @@ define $(package)_stage_cmds
endef

define $(package)_postprocess_cmds
sed -i.old "s/ -lstdc++//" lib/pkgconfig/libzmq.pc && \
rm -rf bin share lib/*.la
endef
147 changes: 147 additions & 0 deletions depends/patches/bdb/clang_cxx_11.patch
@@ -0,0 +1,147 @@
commit 3311d68f11d1697565401eee6efc85c34f022ea7
Author: fanquake <fanquake@gmail.com>
Date: Mon Aug 17 20:03:56 2020 +0800

Fix C++11 compatibility

diff --git a/dbinc/atomic.h b/dbinc/atomic.h
index 0034dcc..7c11d4a 100644
--- a/dbinc/atomic.h
+++ b/dbinc/atomic.h
@@ -70,7 +70,7 @@ typedef struct {
* These have no memory barriers; the caller must include them when necessary.
*/
#define atomic_read(p) ((p)->value)
-#define atomic_init(p, val) ((p)->value = (val))
+#define atomic_init_db(p, val) ((p)->value = (val))

#ifdef HAVE_ATOMIC_SUPPORT

@@ -144,7 +144,7 @@ typedef LONG volatile *interlocked_val;
#define atomic_inc(env, p) __atomic_inc(p)
#define atomic_dec(env, p) __atomic_dec(p)
#define atomic_compare_exchange(env, p, o, n) \
- __atomic_compare_exchange((p), (o), (n))
+ __atomic_compare_exchange_db((p), (o), (n))
static inline int __atomic_inc(db_atomic_t *p)
{
int temp;
@@ -176,7 +176,7 @@ static inline int __atomic_dec(db_atomic_t *p)
* http://gcc.gnu.org/onlinedocs/gcc-4.1.0/gcc/Atomic-Builtins.html
* which configure could be changed to use.
*/
-static inline int __atomic_compare_exchange(
+static inline int __atomic_compare_exchange_db(
db_atomic_t *p, atomic_value_t oldval, atomic_value_t newval)
{
atomic_value_t was;
@@ -206,7 +206,7 @@ static inline int __atomic_compare_exchange(
#define atomic_dec(env, p) (--(p)->value)
#define atomic_compare_exchange(env, p, oldval, newval) \
(DB_ASSERT(env, atomic_read(p) == (oldval)), \
- atomic_init(p, (newval)), 1)
+ atomic_init_db(p, (newval)), 1)
#else
#define atomic_inc(env, p) __atomic_inc(env, p)
#define atomic_dec(env, p) __atomic_dec(env, p)
diff --git a/mp/mp_fget.c b/mp/mp_fget.c
index 5fdee5a..0b75f57 100644
--- a/mp/mp_fget.c
+++ b/mp/mp_fget.c
@@ -617,7 +617,7 @@ alloc: /* Allocate a new buffer header and data space. */

/* Initialize enough so we can call __memp_bhfree. */
alloc_bhp->flags = 0;
- atomic_init(&alloc_bhp->ref, 1);
+ atomic_init_db(&alloc_bhp->ref, 1);
#ifdef DIAGNOSTIC
if ((uintptr_t)alloc_bhp->buf & (sizeof(size_t) - 1)) {
__db_errx(env,
@@ -911,7 +911,7 @@ alloc: /* Allocate a new buffer header and data space. */
MVCC_MPROTECT(bhp->buf, mfp->stat.st_pagesize,
PROT_READ);

- atomic_init(&alloc_bhp->ref, 1);
+ atomic_init_db(&alloc_bhp->ref, 1);
MUTEX_LOCK(env, alloc_bhp->mtx_buf);
alloc_bhp->priority = bhp->priority;
alloc_bhp->pgno = bhp->pgno;
diff --git a/mp/mp_mvcc.c b/mp/mp_mvcc.c
index 34467d2..f05aa0c 100644
--- a/mp/mp_mvcc.c
+++ b/mp/mp_mvcc.c
@@ -276,7 +276,7 @@ __memp_bh_freeze(dbmp, infop, hp, bhp, need_frozenp)
#else
memcpy(frozen_bhp, bhp, SSZA(BH, buf));
#endif
- atomic_init(&frozen_bhp->ref, 0);
+ atomic_init_db(&frozen_bhp->ref, 0);
if (mutex != MUTEX_INVALID)
frozen_bhp->mtx_buf = mutex;
else if ((ret = __mutex_alloc(env, MTX_MPOOL_BH,
@@ -428,7 +428,7 @@ __memp_bh_thaw(dbmp, infop, hp, frozen_bhp, alloc_bhp)
#endif
alloc_bhp->mtx_buf = mutex;
MUTEX_LOCK(env, alloc_bhp->mtx_buf);
- atomic_init(&alloc_bhp->ref, 1);
+ atomic_init_db(&alloc_bhp->ref, 1);
F_CLR(alloc_bhp, BH_FROZEN);
}

diff --git a/mp/mp_region.c b/mp/mp_region.c
index e6cece9..ddbe906 100644
--- a/mp/mp_region.c
+++ b/mp/mp_region.c
@@ -224,7 +224,7 @@ __memp_init(env, dbmp, reginfo_off, htab_buckets, max_nreg)
MTX_MPOOL_FILE_BUCKET, 0, &htab[i].mtx_hash)) != 0)
return (ret);
SH_TAILQ_INIT(&htab[i].hash_bucket);
- atomic_init(&htab[i].hash_page_dirty, 0);
+ atomic_init_db(&htab[i].hash_page_dirty, 0);
}

/*
@@ -269,7 +269,7 @@ __memp_init(env, dbmp, reginfo_off, htab_buckets, max_nreg)
hp->mtx_hash = (mtx_base == MUTEX_INVALID) ? MUTEX_INVALID :
mtx_base + i;
SH_TAILQ_INIT(&hp->hash_bucket);
- atomic_init(&hp->hash_page_dirty, 0);
+ atomic_init_db(&hp->hash_page_dirty, 0);
#ifdef HAVE_STATISTICS
hp->hash_io_wait = 0;
hp->hash_frozen = hp->hash_thawed = hp->hash_frozen_freed = 0;
diff --git a/mutex/mut_method.c b/mutex/mut_method.c
index 2588763..5c6d516 100644
--- a/mutex/mut_method.c
+++ b/mutex/mut_method.c
@@ -426,7 +426,7 @@ atomic_compare_exchange(env, v, oldval, newval)
MUTEX_LOCK(env, mtx);
ret = atomic_read(v) == oldval;
if (ret)
- atomic_init(v, newval);
+ atomic_init_db(v, newval);
MUTEX_UNLOCK(env, mtx);

return (ret);
diff --git a/mutex/mut_tas.c b/mutex/mut_tas.c
index f3922e0..e40fcdf 100644
--- a/mutex/mut_tas.c
+++ b/mutex/mut_tas.c
@@ -46,7 +46,7 @@ __db_tas_mutex_init(env, mutex, flags)

#ifdef HAVE_SHARED_LATCHES
if (F_ISSET(mutexp, DB_MUTEX_SHARED))
- atomic_init(&mutexp->sharecount, 0);
+ atomic_init_db(&mutexp->sharecount, 0);
else
#endif
if (MUTEX_INIT(&mutexp->tas)) {
@@ -486,7 +486,7 @@ __db_tas_mutex_unlock(env, mutex)
F_CLR(mutexp, DB_MUTEX_LOCKED);
/* Flush flag update before zeroing count */
MEMBAR_EXIT();
- atomic_init(&mutexp->sharecount, 0);
+ atomic_init_db(&mutexp->sharecount, 0);
} else {
DB_ASSERT(env, sharecount > 0);
MEMBAR_EXIT();
19 changes: 19 additions & 0 deletions depends/patches/bdb/winioctl.patch
@@ -0,0 +1,19 @@
commit b2c9b538fed1aa1d65b85b372f7a49f1878e9e3b
Author: Fabien <fabcien@gmail.com>
Date: Mon Oct 5 11:45:28 2020 +0200

Fix winioctl.h case for MinGw

diff --git a/dbinc/win_db.h b/dbinc/win_db.h
index 6544558..6cb6878 100644
--- a/dbinc/win_db.h
+++ b/dbinc/win_db.h
@@ -46,7 +46,7 @@
#include <windows.h>
#include <winsock2.h>
#ifndef DB_WINCE
-#include <WinIoCtl.h>
+#include <winioctl.h>
#endif

#ifdef HAVE_GETADDRINFO
22 changes: 22 additions & 0 deletions depends/patches/boost/unused_var_in_process.patch
@@ -0,0 +1,22 @@
commit dbd95cdaefdea95307d004f019a1c394cf9389f0
Author: fanquake <fanquake@gmail.com>
Date: Mon Aug 17 20:15:17 2020 +0800

Remove unused variable in Boost Process

This causes issues with our linters / CI.

Can be removed once depends Boost is 1.71.0 or later.

diff --git a/boost/process/detail/posix/wait_group.hpp b/boost/process/detail/posix/wait_group.hpp
index 9dc249803..2502d9772 100644
--- a/boost/process/detail/posix/wait_group.hpp
+++ b/boost/process/detail/posix/wait_group.hpp
@@ -137,7 +137,6 @@ inline bool wait_until(

do
{
- int ret_sig = 0;
int status;
if ((::waitpid(timeout_pid, &status, WNOHANG) != 0)
&& (WIFEXITED(status) || WIFSIGNALED(status)))
26 changes: 26 additions & 0 deletions depends/patches/miniupnpc/dont_use_wingen.patch
@@ -0,0 +1,26 @@
commit e8077044df239bcf0d9e9980b0e1afb9f1f5c446
Author: fanquake <fanquake@gmail.com>
Date: Tue Aug 18 20:50:19 2020 +0800

Don't use wingenminiupnpcstrings when generating miniupnpcstrings.h

The wingenminiupnpcstrings tool is used on Windows to generate version
information. This information is irrelevant for us, and trying to use
wingenminiupnpcstrings would cause builds to fail, so just don't use it.

We should be able to drop this once we are using 2.1 or later. See
upstream commit: 9663c55c61408fdcc39a82987d2243f816b22932.

diff --git a/Makefile.mingw b/Makefile.mingw
index 574720e..fcc17bb 100644
--- a/Makefile.mingw
+++ b/Makefile.mingw
@@ -74,7 +74,7 @@ wingenminiupnpcstrings: wingenminiupnpcstrings.o

wingenminiupnpcstrings.o: wingenminiupnpcstrings.c

-miniupnpcstrings.h: miniupnpcstrings.h.in wingenminiupnpcstrings
+miniupnpcstrings.h: miniupnpcstrings.h.in
wingenminiupnpcstrings $< $@

minixml.o: minixml.c minixml.h
26 changes: 26 additions & 0 deletions depends/patches/native_cctools/ld64_disable_threading.patch
@@ -0,0 +1,26 @@
commit 584668415039adeed073decee7e04de28248afd3
Author: fanquake <fanquake@gmail.com>
Date: Tue Aug 18 01:20:24 2020 +0000

Disable threading to fix non-determinism

A bug in the file parser can cause dependencies to be calculated
differently based on which files have already been parsed. This is more
likely to occur on systems with more CPUs.

Just disable threading for now. There is no noticable slowdown.

See #9891.

diff --git a/cctools/ld64/src/ld/InputFiles.h b/cctools/ld64/src/ld/InputFiles.h
index ef9c756..90a70b6 100644
--- a/cctools/ld64/src/ld/InputFiles.h
+++ b/cctools/ld64/src/ld/InputFiles.h
@@ -25,7 +25,6 @@
#ifndef __INPUT_FILES_H__
#define __INPUT_FILES_H__

-#define HAVE_PTHREADS 1

#include <stdlib.h>
#include <sys/types.h>
27 changes: 27 additions & 0 deletions depends/patches/qt/dont_hardcode_pwd.patch
@@ -0,0 +1,27 @@
commit 0e953866fc4672486e29e1ba6d83b4207e7b2f0b
Author: fanquake <fanquake@gmail.com>
Date: Tue Aug 18 15:09:06 2020 +0800

Don't hardcode pwd path

Let a man use his builtins if he wants to! Also, removes the unnecessary
assumption that pwd lives under /bin/pwd.

See #15581.

diff --git a/qtbase/configure b/qtbase/configure
index 08b49a8d..faea5b55 100755
--- a/qtbase/configure
+++ b/qtbase/configure
@@ -36,9 +36,9 @@
relconf=`basename $0`
# the directory of this script is the "source tree"
relpath=`dirname $0`
-relpath=`(cd "$relpath"; /bin/pwd)`
+relpath=`(cd "$relpath"; pwd)`
# the current directory is the "build tree" or "object tree"
-outpath=`/bin/pwd`
+outpath=`pwd`

WHICH="which"

0 comments on commit 354e500

Please sign in to comment.