Repository: bitcoin/bitcoin (language: cpp)

Issue:
depends: `sed` robustness
As seen in https://github.com/bitcoin/bitcoin/pull/16837, the `sed` calls in `depends` fail silently, and can result in silent, unwanted regressions when packages are updated.

It seems that a possible solution might be something like the following: https://askubuntu.com/a/1036918


Discussion / hints:
+1 for improving robustness here. Silently failing errors are horrible for troubleshooting.

Edit: https://github.com/bitcoin/bitcoin/pull/16837#discussion_r322842701  illustrates, again, why this is important
I spent some time looking at this today. Would it be ok if we required gnu `sed` in depends (there is some precedent as we require `glibtoolize` elsewhere in the build system)? Otherwise I'm not sure that the suggested approach would work out of the box on macOS (BSD sed).

I did have a look at the approach in https://stackoverflow.com/a/28966696, but that seemed a bit messy. Will revisit later on.
~~Doesn't `sed -i` already rely on GNU sed? I don't remember seeing it in BSDs.~~

Apparently it does, just checked on FreeBSD at least and
```
     -i extension
             Edit files in-place similarly to -I, but treat each file
             independently from other files.  In particular, line numbers in
             each file start at 1, the "$" address matches the last line of
             the current file, and address ranges are limited to the current
             file.  (See Sed Addresses.) The net result is as though each file
             were edited by a separate sed instance.
```
Couldn't we mock (or wrap) `sed` while doing depends? That way we can control the exit code.

A wrapper that does the following, should be good enough:
```
  - check that ${file}_BACKUP does not exist
  - run replacement with unmodified sed on $file, additionally passing `--in-place=_BACKUP`
  - assert that $file differs from $file_BACKUP
  - delete $file_BACKUP
> Couldn't we mock (or wrap) `sed` while doing depends? That way we can control the exit code.
> 
> A wrapper that does the following, should be good enough:
> 
> ```
>   - check that ${file}_BACKUP does not exist
>   - run replacement with unmodified sed on $file, additionally passing `--in-place=_BACKUP`
>   - assert that $file differs from $file_BACKUP
>   - delete $file_BACKUP
> ```

How would we make that wrapper accessible in the Makefile? Somethine like `SHELL = /usr/bin/env PATH=$(CURDIR)/wrappers:${PATH} sh`?
Yeah, something like that.
I've removed "good first issue" here, as the remaining `sed` usage is basically contained to qt (1 in libxcb that we ), and is non-trivial to remove.

Fix patch:
diff --git a/depends/packages/libxcb.mk b/depends/packages/libxcb.mk
index fa30e80f5c087..036eaf6560a11 100644
--- a/depends/packages/libxcb.mk
+++ b/depends/packages/libxcb.mk
@@ -4,6 +4,7 @@ $(package)_download_path=https://xcb.freedesktop.org/dist
 $(package)_file_name=$(package)-$($(package)_version).tar.xz
 $(package)_sha256_hash=a55ed6db98d43469801262d81dc2572ed124edc3db31059d4e9916eb9f844c34
 $(package)_dependencies=xcb_proto libXau
+$(package)_patches = remove_pthread_stubs.patch
 
 define $(package)_set_vars
 $(package)_config_opts=--disable-static --disable-devel-docs --without-doxygen --without-launchd
@@ -20,7 +21,7 @@ endef
 
 define $(package)_preprocess_cmds
   cp -f $(BASEDIR)/config.guess $(BASEDIR)/config.sub build-aux && \
-  sed "s/pthread-stubs//" -i configure
+  patch -p1 -i $($(package)_patch_dir)/remove_pthread_stubs.patch
 endef
 
 define $(package)_config_cmds
diff --git a/depends/patches/libxcb/remove_pthread_stubs.patch b/depends/patches/libxcb/remove_pthread_stubs.patch
new file mode 100644
index 0000000000000..1f32dea527e25
--- /dev/null
+++ b/depends/patches/libxcb/remove_pthread_stubs.patch
@@ -0,0 +1,12 @@
+Remove uneeded pthread-stubs dependency
+--- a/configure
++++ b/configure
+@@ -19695,7 +19695,7 @@ fi
+ NEEDED="xau >= 0.99.2"
+ case $host_os in
+ linux*) ;;
+-     *) NEEDED="$NEEDED pthread-stubs" ;;
++     *) NEEDED="$NEEDED" ;;
+ esac
+ 
+ pkg_failed=no


Test patch: