[PATCH 0/5] xfsdump, xfsprogs distro builds and DEBUG=

[Donald Douwsma <ddouwsma@redhat.com>]

tldr; Fedora builds with -DDEBUG, is that what folk want or expect?

We see cases from the field where xfsrestore aborts on an assert(3).
In each case there's a real problem that needs to be fixed, but these
problems are preventing folk from restoring their data. With the asserts
suppressed or removed xfsrestore usually does something sane enough, at
worst leaving restored content in the orphanage where a user can find
it.

This was notably seen with the original xfsdump bind mount fix, now
we're seeing asserts fire during cumulative restores in
noref_elim_recurse.

  xfsrestore: tree.c:1421: noref_elim_recurse: Assertion 'isrealpr' failed

This appears to be caused by a rename to within the tree between restore
levels, or a combination of modifications during occurring during the
dump. Fixing it will likely require changes in noref_elim_recurse, or
tree_post, to ensure elements of the tree are created for these edge
cases.

But why are we seeing assert(3) active in Fedora based distro builds?

xfsprogs and xfsdump m4/package_globals.m4 shows builds should default
to DEBUG and xfsdump doc/INSTALL talks about how to turn off asserts and
create an optimized build, but only Debian has its build rules in-tree,
where they set DEBUG=-DNDEBUG.

Building locally on Fedora or similar gets varied results with recent
Fedora versions building as if DEBUG=-DNDEBUG was set within the realm
of configure or autoconf, which may be misleading people.

But for Fedora release builds we can see that they're building with
DEBUG=-DDEBUG resulting in gcc called with -DDEBUG in the build logs.

  https://src.fedoraproject.org/rpms/xfsdump
  https://kojipkgs.fedoraproject.org//packages/xfsdump/3.2.0/4.fc44/data/logs/x86_64/build.log

  https://src.fedoraproject.org/rpms/xfsprogs
  https://kojipkgs.fedoraproject.org//packages/xfsprogs/6.18.0/2.fc44/data/logs/x86_64/build.log

I don't think the intention was to have these build as DEBUG, but that
seems to be the current result. But before I dig into this further can I
confirm what people think this should do?

The following series is illustrative of the changes to build xfsdump
cleanly with DEBUG=-DNDEBUG, or the type of downstream workarounds
required to help end users hitting problems for the status quo.

Don

Donald Douwsma (5):
  xfsrestore: remove unused variable strctxp
  annotate variables only used for assert
  xfsrestore: include TREE_DEBUG all builds
  xfsrestore: remove failing assert from noref_elim_recurse
  xfsrestore: assert suppression workaround

 common/drive_minrmt.c    |  5 +--
 common/drive_scsitape.c  |  5 +--
 common/drive_simple.c    |  9 ++---
 common/main.c            |  6 +--
 common/mlog.c            |  6 +--
 common/qlock.c           | 18 ++++-----
 common/ring.c            |  2 +-
 dump/content.c           |  8 ++--
 include/config.h.in      |  4 ++
 man/man8/xfsrestore.8    |  4 ++
 restore/Makefile         |  6 ++-
 restore/content.c        | 36 +++++++-----------
 restore/dirattr.c        |  3 +-
 restore/getopt.h         |  4 +-
 restore/inomap.c         |  9 ++---
 restore/restore_assert.c | 39 +++++++++++++++++++
 restore/restore_assert.h | 18 +++++++++
 restore/tree.c           | 82 ++++++++++++++--------------------------
 restore/win.c            | 24 ++++--------
 19 files changed, 148 insertions(+), 140 deletions(-)
 create mode 100644 restore/restore_assert.c
 create mode 100644 restore/restore_assert.h

-- 
2.47.3