public inbox for debugedit@sourceware.org
 help / color / mirror / Atom feed
* find-debuginfo: Allow for specifying output dir for all *.list files
@ 2025-10-29 15:11 Michal Domonkos
  2025-10-29 15:11 ` [PATCH 1/3] find-debuginfo: Refactor output directory Michal Domonkos
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Michal Domonkos @ 2025-10-29 15:11 UTC (permalink / raw)
  To: debugedit; +Cc: pmatilai

This patch series adds a new switch to the find-debuginfo script that allows
for overriding the output directory which otherwise defaults to the builddir
argument. A test is included as well. See the commit messages for details.

The primary motivation is for RPM to avoid polluting the unpacked source tree
in --build-in-place mode, see the following discussion for details:

    https://github.com/rpm-software-management/rpm/discussions/3644


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/3] find-debuginfo: Refactor output directory
  2025-10-29 15:11 find-debuginfo: Allow for specifying output dir for all *.list files Michal Domonkos
@ 2025-10-29 15:11 ` Michal Domonkos
  2026-01-14 22:18   ` Mark Wielaard
  2025-10-29 15:11 ` [PATCH 2/3] find-debuginfo: Add test for output files presence Michal Domonkos
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Michal Domonkos @ 2025-10-29 15:11 UTC (permalink / raw)
  To: debugedit; +Cc: pmatilai, Michal Domonkos

Separate the output directory path for the artifacts (*.list) from
$BUILDDIR so that we can make it configurable in the next commit.

No functional change.

Signed-off-by: Michal Domonkos <mdomonko@redhat.com>
---
 scripts/find-debuginfo.in | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/scripts/find-debuginfo.in b/scripts/find-debuginfo.in
index 40cd182..8bb688a 100755
--- a/scripts/find-debuginfo.in
+++ b/scripts/find-debuginfo.in
@@ -330,21 +330,23 @@ fi
 
 $quiet || echo "find-debuginfo: starting" 2>&1
 
+OUTDIR=$BUILDDIR
+
 i=0
 while ((i < nout)); do
-  outs[$i]="$BUILDDIR/${outs[$i]}"
+  outs[$i]="$OUTDIR/${outs[$i]}"
   l=''
   for f in ${lists[$i]}; do
-    l="$l $BUILDDIR/$f"
+    l="$l $OUTDIR/$f"
   done
   lists[$i]=$l
   ((++i))
 done
 
-LISTFILE="$BUILDDIR/$out"
-SOURCEFILE="$BUILDDIR/debugsources.list"
-LINKSFILE="$BUILDDIR/debuglinks.list"
-ELFBINSFILE="$BUILDDIR/elfbins.list"
+LISTFILE="$OUTDIR/$out"
+SOURCEFILE="$OUTDIR/debugsources.list"
+LINKSFILE="$OUTDIR/debuglinks.list"
+ELFBINSFILE="$OUTDIR/elfbins.list"
 
 > "$SOURCEFILE"
 > "$LISTFILE"
@@ -888,7 +890,7 @@ if [ -d "${RPM_BUILD_ROOT}/usr/lib" ] || [ -d "${RPM_BUILD_ROOT}/usr/src" ]; the
 fi
 
 if [ -n "$srcout" ]; then
-  srcout="$BUILDDIR/$srcout"
+  srcout="$OUTDIR/$srcout"
   > "$srcout"
   if [ -d "${RPM_BUILD_ROOT}/usr/src/debug" ]; then
     (cd "${RPM_BUILD_ROOT}/usr"
-- 
2.51.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/3] find-debuginfo: Add test for output files presence
  2025-10-29 15:11 find-debuginfo: Allow for specifying output dir for all *.list files Michal Domonkos
  2025-10-29 15:11 ` [PATCH 1/3] find-debuginfo: Refactor output directory Michal Domonkos
@ 2025-10-29 15:11 ` Michal Domonkos
  2026-01-14 22:33   ` Mark Wielaard
  2025-10-29 15:11 ` [PATCH 3/3] find-debuginfo: Add -O/--output-dir switch Michal Domonkos
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Michal Domonkos @ 2025-10-29 15:11 UTC (permalink / raw)
  To: debugedit; +Cc: pmatilai, Michal Domonkos

Signed-off-by: Michal Domonkos <mdomonko@redhat.com>
---
 tests/find-debuginfo.at | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/tests/find-debuginfo.at b/tests/find-debuginfo.at
index 26dcd2d..834f1af 100644
--- a/tests/find-debuginfo.at
+++ b/tests/find-debuginfo.at
@@ -99,6 +99,13 @@ $CC $CFLAGS -Wl,--build-id -g3 -I. -o baz baz.c foo.c bar.c
 cd ..
 ]])
 
+# Names of all output files produced by find-debuginfo
+m4_define([FIND_DEBUGINFO_OUTPUT_FILES],
+[debugfiles.list
+debuglinks.list
+debugsources.list
+elfbins.list])
+
 # Run find-debuginfo on a small build without any fancy options
 AT_SETUP([find-debuginfo sources])
 AT_KEYWORDS([find-debuginfo] [sources])
@@ -398,3 +405,26 @@ subdir_build/foobar.h
 AT_CHECK([cat subdir_build/debugsources.list | tr '\0' '\n' | sort -u],
          [0], [expout], [])
 AT_CLEANUP
+
+# Run find-debuginfo on a small build and check all output files (and only
+# those) were created
+AT_SETUP([find-debuginfo output files])
+AT_KEYWORDS([find-debuginfo] [sources] [debugdata] [gdb-index])
+FIND_DEBUGINFO_PKG_BUILD_SETUP
+# Make a snapshot of builddir, including the expected output files
+AT_CHECK([(ls subdir_build;
+           echo "FIND_DEBUGINFO_OUTPUT_FILES") | sort > expout],
+         [0], [], [])
+# We need to set some environment variables for running find-debuginfo
+# normally set by rpmbuild.
+AT_CHECK([env RPM_BUILD_DIR=${PWD} \
+              RPM_BUILD_ROOT=${PWD} \
+              RPM_PACKAGE_NAME=pkg \
+              RPM_PACKAGE_VERSION=ver \
+              RPM_PACKAGE_RELEASE=rel \
+              RPM_ARCH=arch \
+          find-debuginfo ${PWD}/subdir_build],
+         [0], [ignore], [])
+# Check builddir against the snapshot
+AT_CHECK([ls subdir_build | sort], [0], [expout], [])
+AT_CLEANUP
-- 
2.51.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 3/3] find-debuginfo: Add -O/--output-dir switch
  2025-10-29 15:11 find-debuginfo: Allow for specifying output dir for all *.list files Michal Domonkos
  2025-10-29 15:11 ` [PATCH 1/3] find-debuginfo: Refactor output directory Michal Domonkos
  2025-10-29 15:11 ` [PATCH 2/3] find-debuginfo: Add test for output files presence Michal Domonkos
@ 2025-10-29 15:11 ` Michal Domonkos
  2026-01-14 22:50   ` Mark Wielaard
  2025-10-30  8:00 ` find-debuginfo: Allow for specifying output dir for all *.list files Panu Matilainen
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Michal Domonkos @ 2025-10-29 15:11 UTC (permalink / raw)
  To: debugedit; +Cc: pmatilai, Michal Domonkos

Originally, as it seems, the meaning of the builddir argument was that
of an *output* directory (for the list files), since that's all it was
used for. The actual *build* directory is pointed to by $RPM_BUILD_DIR.

However, since commit a8c7efeae67856e144897efb7c40516588b811ee, we have
been passing the builddir argument to "debugedit --base-dir" when unique
dirs are requested, effectively making the argument live up to its name.
Thus, the output directory currently equals the builddir and cannot be
set independently when needed. Fix that by adding a new switch to allow
just that.

This switch will be useful in RPM's %{__find_debuginfo} macro where we
can prevent "rpmbuild --build-in-place" from polluting the source tree
(since builddir points to $PWD in that mode).

Use -p and -o in the included test to also cover that code path.

Signed-off-by: Michal Domonkos <mdomonko@redhat.com>
---
 scripts/find-debuginfo.in | 13 ++++++++++++-
 tests/find-debuginfo.at   | 26 ++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/scripts/find-debuginfo.in b/scripts/find-debuginfo.in
index 8bb688a..d061630 100755
--- a/scripts/find-debuginfo.in
+++ b/scripts/find-debuginfo.in
@@ -33,6 +33,7 @@ Options:
 [-j N] [--jobs N]
 [-o debugfiles.list]
 [-S debugsourcefiles.list]
+[-O PATH] [--output-dir PATH]
 [--run-dwz] [--dwz-low-mem-die-limit N]
 [--dwz-max-die-limit N]
 [--dwz-single-file-mode]
@@ -74,6 +75,10 @@ the -l filelist file, or whose names match the -p pattern.
 The -p argument is an grep -E -style regexp matching the a file name,
 and must not use anchors (^ or $).
 
+The -O or --output-dir PATH flag instructs find-debuginfo to store all output
+files in the PATH directory. If not given, all output files will be stored in
+builddir.
+
 The --run-dwz flag instructs find-debuginfo to run the dwz utility
 if available, and --dwz-low-mem-die-limit and --dwz-max-die-limit
 provide detailed limits.  See dwz(1) -l and -L option for details.
@@ -183,6 +188,7 @@ fi
 
 
 BUILDDIR=.
+OUTDIR=
 out=debugfiles.list
 srcout=
 nout=0
@@ -241,6 +247,10 @@ while [ $# -gt 0 ]; do
     fi
     shift
     ;;
+  -O|--output-dir)
+    OUTDIR=$2
+    shift
+    ;;
   -l)
     lists[$nout]="${lists[$nout]} $2"
     shift
@@ -330,7 +340,7 @@ fi
 
 $quiet || echo "find-debuginfo: starting" 2>&1
 
-OUTDIR=$BUILDDIR
+[ -z "$OUTDIR" ] && OUTDIR=$BUILDDIR
 
 i=0
 while ((i < nout)); do
@@ -348,6 +358,7 @@ SOURCEFILE="$OUTDIR/debugsources.list"
 LINKSFILE="$OUTDIR/debuglinks.list"
 ELFBINSFILE="$OUTDIR/elfbins.list"
 
+mkdir -p "$OUTDIR"
 > "$SOURCEFILE"
 > "$LISTFILE"
 > "$LINKSFILE"
diff --git a/tests/find-debuginfo.at b/tests/find-debuginfo.at
index 834f1af..59c15b9 100644
--- a/tests/find-debuginfo.at
+++ b/tests/find-debuginfo.at
@@ -428,3 +428,29 @@ AT_CHECK([env RPM_BUILD_DIR=${PWD} \
 # Check builddir against the snapshot
 AT_CHECK([ls subdir_build | sort], [0], [expout], [])
 AT_CLEANUP
+
+# Run find-debuginfo on a small build and check all output files (and only
+# those) were created in the supplied output directory
+AT_SETUP([find-debuginfo output files (custom dir)])
+AT_KEYWORDS([find-debuginfo] [sources] [debugdata] [gdb-index])
+FIND_DEBUGINFO_PKG_BUILD_SETUP
+# Make a snapshot of builddir
+AT_CHECK([ls subdir_build | sort > expout], [0], [], [])
+# We need to set some environment variables for running find-debuginfo
+# normally set by rpmbuild.
+AT_CHECK([env RPM_BUILD_DIR=${PWD} \
+              RPM_BUILD_ROOT=${PWD} \
+              RPM_PACKAGE_NAME=pkg \
+              RPM_PACKAGE_VERSION=ver \
+              RPM_PACKAGE_RELEASE=rel \
+              RPM_ARCH=arch \
+          find-debuginfo -p '.*' -o allfiles.list \
+                         -O ${PWD}/output ${PWD}/subdir_build],
+         [0], [ignore], [])
+# Check builddir against the snapshot
+AT_CHECK([ls subdir_build | sort], [0], [expout], [])
+# Check the output directory against the expected list
+AT_CHECK([(echo allfiles.list; echo "FIND_DEBUGINFO_OUTPUT_FILES") |
+          sort > expout], [0], [], [])
+AT_CHECK([ls output | sort], [0], [expout], [])
+AT_CLEANUP
-- 
2.51.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: find-debuginfo: Allow for specifying output dir for all *.list files
  2025-10-29 15:11 find-debuginfo: Allow for specifying output dir for all *.list files Michal Domonkos
                   ` (2 preceding siblings ...)
  2025-10-29 15:11 ` [PATCH 3/3] find-debuginfo: Add -O/--output-dir switch Michal Domonkos
@ 2025-10-30  8:00 ` Panu Matilainen
  2026-01-08  8:11 ` Panu Matilainen
  2026-01-14 22:54 ` Mark Wielaard
  5 siblings, 0 replies; 15+ messages in thread
From: Panu Matilainen @ 2025-10-30  8:00 UTC (permalink / raw)
  To: Michal Domonkos, debugedit

On 10/29/25 5:11 PM, Michal Domonkos wrote:
> This patch series adds a new switch to the find-debuginfo script that allows
> for overriding the output directory which otherwise defaults to the builddir
> argument. A test is included as well. See the commit messages for details.
> 
> The primary motivation is for RPM to avoid polluting the unpacked source tree
> in --build-in-place mode, see the following discussion for details:
 >
 >      https://github.com/rpm-software-management/rpm/discussions/3644
 >

Just FWIW, I wouldn't call that the primary motivation, it's just one of 
the beneficiaries.

In the past, rpm didn't have a good place to put its per-build temporary 
files, so it used RPM_BUILD_DIR. This has caused issues long before 
--build-in-place even existed, eg. 
https://bugzilla.redhat.com/show_bug.cgi?id=672538

I prefer to think of this as a generic build hygiene kind of thing. It's 
also a pre-requisite for a read-only source tree, which could be useful 
for enforcing clean vpath builds, and --build-in-place type usage where 
the source could even be on read-only media. Clearly this isn't 
something every build would/could use, but as an option.

	- Panu -




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: find-debuginfo: Allow for specifying output dir for all *.list files
  2025-10-29 15:11 find-debuginfo: Allow for specifying output dir for all *.list files Michal Domonkos
                   ` (3 preceding siblings ...)
  2025-10-30  8:00 ` find-debuginfo: Allow for specifying output dir for all *.list files Panu Matilainen
@ 2026-01-08  8:11 ` Panu Matilainen
  2026-01-08 10:16   ` Mark Wielaard
  2026-01-14 22:54 ` Mark Wielaard
  5 siblings, 1 reply; 15+ messages in thread
From: Panu Matilainen @ 2026-01-08  8:11 UTC (permalink / raw)
  To: Michal Domonkos, debugedit, Mark Wielaard

On 10/29/25 5:11 PM, Michal Domonkos wrote:
> This patch series adds a new switch to the find-debuginfo script that allows
> for overriding the output directory which otherwise defaults to the builddir
> argument. A test is included as well. See the commit messages for details.
> 
> The primary motivation is for RPM to avoid polluting the unpacked source tree
> in --build-in-place mode, see the following discussion for details:
> 
>      https://github.com/rpm-software-management/rpm/discussions/3644
> 

Mark, is there anything we can do to move this patch series forward?

We'd like to get this change done by RPM 6.1 alpha in this spring.

	- Panu -


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: find-debuginfo: Allow for specifying output dir for all *.list files
  2026-01-08  8:11 ` Panu Matilainen
@ 2026-01-08 10:16   ` Mark Wielaard
  2026-01-14  9:20     ` Panu Matilainen
  0 siblings, 1 reply; 15+ messages in thread
From: Mark Wielaard @ 2026-01-08 10:16 UTC (permalink / raw)
  To: Panu Matilainen; +Cc: Michal Domonkos, debugedit

Hi Panu, Hi Michal,

On Thu, Jan 08, 2026 at 10:11:38AM +0200, Panu Matilainen wrote:
> On 10/29/25 5:11 PM, Michal Domonkos wrote:
> >This patch series adds a new switch to the find-debuginfo script that allows
> >for overriding the output directory which otherwise defaults to the builddir
> >argument. A test is included as well. See the commit messages for details.
> >
> >The primary motivation is for RPM to avoid polluting the unpacked source tree
> >in --build-in-place mode, see the following discussion for details:
> >
> >     https://github.com/rpm-software-management/rpm/discussions/3644
> >
> 
> Mark, is there anything we can do to move this patch series forward?
> 
> We'd like to get this change done by RPM 6.1 alpha in this spring.

Oops, sorry. I totally forgot about this change.  I did look at it
before and thought it seemed sane. But apparently never replies or
followed up.  I'll try to go over it tomorrow.

Cheers,

Mark

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: find-debuginfo: Allow for specifying output dir for all *.list files
  2026-01-08 10:16   ` Mark Wielaard
@ 2026-01-14  9:20     ` Panu Matilainen
  0 siblings, 0 replies; 15+ messages in thread
From: Panu Matilainen @ 2026-01-14  9:20 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: Michal Domonkos, debugedit

On 1/8/26 12:16 PM, Mark Wielaard wrote:
> Hi Panu, Hi Michal,
> 
> On Thu, Jan 08, 2026 at 10:11:38AM +0200, Panu Matilainen wrote:
>> On 10/29/25 5:11 PM, Michal Domonkos wrote:
>>> This patch series adds a new switch to the find-debuginfo script that allows
>>> for overriding the output directory which otherwise defaults to the builddir
>>> argument. A test is included as well. See the commit messages for details.
>>>
>>> The primary motivation is for RPM to avoid polluting the unpacked source tree
>>> in --build-in-place mode, see the following discussion for details:
>>>
>>>      https://github.com/rpm-software-management/rpm/discussions/3644
>>>
>>
>> Mark, is there anything we can do to move this patch series forward?
>>
>> We'd like to get this change done by RPM 6.1 alpha in this spring.
> 
> Oops, sorry. I totally forgot about this change.  I did look at it
> before and thought it seemed sane. But apparently never replies or
> followed up.  I'll try to go over it tomorrow.

Thanks, and no worries. This isn't life-and-death timing critical to 
anything, more in the long term sanity deparment.

	- Panu -

> 
> Cheers,
> 
> Mark
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] find-debuginfo: Refactor output directory
  2025-10-29 15:11 ` [PATCH 1/3] find-debuginfo: Refactor output directory Michal Domonkos
@ 2026-01-14 22:18   ` Mark Wielaard
  0 siblings, 0 replies; 15+ messages in thread
From: Mark Wielaard @ 2026-01-14 22:18 UTC (permalink / raw)
  To: Michal Domonkos; +Cc: debugedit, pmatilai

Hi Michal,

Apologies I forgot about this patch set.

On Wed, Oct 29, 2025 at 04:11:53PM +0100, Michal Domonkos via Debugedit wrote:
> Separate the output directory path for the artifacts (*.list) from
> $BUILDDIR so that we can make it configurable in the next commit.
> 
> No functional change.

Thanks for splitting this out, it makes the patch series as a whole
easier.

> Signed-off-by: Michal Domonkos <mdomonko@redhat.com>
> ---
>  scripts/find-debuginfo.in | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/scripts/find-debuginfo.in b/scripts/find-debuginfo.in
> index 40cd182..8bb688a 100755
> --- a/scripts/find-debuginfo.in
> +++ b/scripts/find-debuginfo.in
> @@ -330,21 +330,23 @@ fi
>  
>  $quiet || echo "find-debuginfo: starting" 2>&1
>  
> +OUTDIR=$BUILDDIR
> +
>  i=0
>  while ((i < nout)); do
> -  outs[$i]="$BUILDDIR/${outs[$i]}"
> +  outs[$i]="$OUTDIR/${outs[$i]}"
>    l=''
>    for f in ${lists[$i]}; do
> -    l="$l $BUILDDIR/$f"
> +    l="$l $OUTDIR/$f"
>    done
>    lists[$i]=$l
>    ((++i))
>  done

OK, -o output files fo to OUTDIR.
  
> -LISTFILE="$BUILDDIR/$out"
> -SOURCEFILE="$BUILDDIR/debugsources.list"
> -LINKSFILE="$BUILDDIR/debuglinks.list"
> -ELFBINSFILE="$BUILDDIR/elfbins.list"
> +LISTFILE="$OUTDIR/$out"
> +SOURCEFILE="$OUTDIR/debugsources.list"
> +LINKSFILE="$OUTDIR/debuglinks.list"
> +ELFBINSFILE="$OUTDIR/elfbins.list"

OK, same for other file lists.

>  > "$SOURCEFILE"
>  > "$LISTFILE"
> @@ -888,7 +890,7 @@ if [ -d "${RPM_BUILD_ROOT}/usr/lib" ] || [ -d "${RPM_BUILD_ROOT}/usr/src" ]; the
>  fi
>  
>  if [ -n "$srcout" ]; then
> -  srcout="$BUILDDIR/$srcout"
> +  srcout="$OUTDIR/$srcout"
>    > "$srcout"

And for the debugsourcefiles.list (as given by -S)

All looks good.

Thanks,

Mark

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] find-debuginfo: Add test for output files presence
  2025-10-29 15:11 ` [PATCH 2/3] find-debuginfo: Add test for output files presence Michal Domonkos
@ 2026-01-14 22:33   ` Mark Wielaard
  2026-01-15  9:30     ` Michal Domonkos
  0 siblings, 1 reply; 15+ messages in thread
From: Mark Wielaard @ 2026-01-14 22:33 UTC (permalink / raw)
  To: Michal Domonkos; +Cc: debugedit, pmatilai

Hi Michal,

On Wed, Oct 29, 2025 at 04:11:54PM +0100, Michal Domonkos via Debugedit wrote:
> Signed-off-by: Michal Domonkos <mdomonko@redhat.com>
> ---
>  tests/find-debuginfo.at | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/tests/find-debuginfo.at b/tests/find-debuginfo.at
> index 26dcd2d..834f1af 100644
> --- a/tests/find-debuginfo.at
> +++ b/tests/find-debuginfo.at
> @@ -99,6 +99,13 @@ $CC $CFLAGS -Wl,--build-id -g3 -I. -o baz baz.c foo.c bar.c
>  cd ..
>  ]])
>  
> +# Names of all output files produced by find-debuginfo
> +m4_define([FIND_DEBUGINFO_OUTPUT_FILES],
> +[debugfiles.list
> +debuglinks.list
> +debugsources.list
> +elfbins.list])

Yes, when not -o or -S arguments are given.

>  # Run find-debuginfo on a small build without any fancy options
>  AT_SETUP([find-debuginfo sources])
>  AT_KEYWORDS([find-debuginfo] [sources])
> @@ -398,3 +405,26 @@ subdir_build/foobar.h
>  AT_CHECK([cat subdir_build/debugsources.list | tr '\0' '\n' | sort -u],
>           [0], [expout], [])
>  AT_CLEANUP
> +
> +# Run find-debuginfo on a small build and check all output files (and only
> +# those) were created
> +AT_SETUP([find-debuginfo output files])
> +AT_KEYWORDS([find-debuginfo] [sources] [debugdata] [gdb-index])

Lets just use the keywords [find-debuginfo] and [output] this check
isn't about sources, debugdata or gdb-index.

> +FIND_DEBUGINFO_PKG_BUILD_SETUP
> +# Make a snapshot of builddir, including the expected output files
> +AT_CHECK([(ls subdir_build;
> +           echo "FIND_DEBUGINFO_OUTPUT_FILES") | sort > expout],
> +         [0], [], [])

OK, sorted output of subdir_build plus expected output files.

> +# We need to set some environment variables for running find-debuginfo
> +# normally set by rpmbuild.
> +AT_CHECK([env RPM_BUILD_DIR=${PWD} \
> +              RPM_BUILD_ROOT=${PWD} \
> +              RPM_PACKAGE_NAME=pkg \
> +              RPM_PACKAGE_VERSION=ver \
> +              RPM_PACKAGE_RELEASE=rel \
> +              RPM_ARCH=arch \
> +          find-debuginfo ${PWD}/subdir_build],
> +         [0], [ignore], [])

OK, run on prepared builddir without any arguments.

> +# Check builddir against the snapshot
> +AT_CHECK([ls subdir_build | sort], [0], [expout], [])
> +AT_CLEANUP

And check the sorted subdir_build files match what we stored in
expout.

Looks good. There could also a check that uses -o and/or -S bit this
is probably enough.

Thanks,

Mark

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] find-debuginfo: Add -O/--output-dir switch
  2025-10-29 15:11 ` [PATCH 3/3] find-debuginfo: Add -O/--output-dir switch Michal Domonkos
@ 2026-01-14 22:50   ` Mark Wielaard
  2026-01-15  9:34     ` Michal Domonkos
  0 siblings, 1 reply; 15+ messages in thread
From: Mark Wielaard @ 2026-01-14 22:50 UTC (permalink / raw)
  To: Michal Domonkos; +Cc: debugedit, pmatilai

Hi Michal,

On Wed, Oct 29, 2025 at 04:11:55PM +0100, Michal Domonkos via Debugedit wrote:
> Originally, as it seems, the meaning of the builddir argument was that
> of an *output* directory (for the list files), since that's all it was
> used for. The actual *build* directory is pointed to by $RPM_BUILD_DIR.
> 
> However, since commit a8c7efeae67856e144897efb7c40516588b811ee, we have
> been passing the builddir argument to "debugedit --base-dir" when unique
> dirs are requested, effectively making the argument live up to its name.

That was my commit from 2017... I doubt I thought about this
implication back then.

> Thus, the output directory currently equals the builddir and cannot be
> set independently when needed. Fix that by adding a new switch to allow
> just that.
> 
> This switch will be useful in RPM's %{__find_debuginfo} macro where we
> can prevent "rpmbuild --build-in-place" from polluting the source tree
> (since builddir points to $PWD in that mode).
> 
> Use -p and -o in the included test to also cover that code path.

O, nice. That is what I was worried about in the previous test.

> Signed-off-by: Michal Domonkos <mdomonko@redhat.com>
> ---
>  scripts/find-debuginfo.in | 13 ++++++++++++-
>  tests/find-debuginfo.at   | 26 ++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/find-debuginfo.in b/scripts/find-debuginfo.in
> index 8bb688a..d061630 100755
> --- a/scripts/find-debuginfo.in
> +++ b/scripts/find-debuginfo.in
> @@ -33,6 +33,7 @@ Options:
>  [-j N] [--jobs N]
>  [-o debugfiles.list]
>  [-S debugsourcefiles.list]
> +[-O PATH] [--output-dir PATH]
>  [--run-dwz] [--dwz-low-mem-die-limit N]
>  [--dwz-max-die-limit N]
>  [--dwz-single-file-mode]
> @@ -74,6 +75,10 @@ the -l filelist file, or whose names match the -p pattern.
>  The -p argument is an grep -E -style regexp matching the a file name,
>  and must not use anchors (^ or $).
>  
> +The -O or --output-dir PATH flag instructs find-debuginfo to store all output
> +files in the PATH directory. If not given, all output files will be stored in
> +builddir.
> +

Thank for documenting. Now this is also covered in the man page.

>  The --run-dwz flag instructs find-debuginfo to run the dwz utility
>  if available, and --dwz-low-mem-die-limit and --dwz-max-die-limit
>  provide detailed limits.  See dwz(1) -l and -L option for details.
> @@ -183,6 +188,7 @@ fi
>  
>  
>  BUILDDIR=.
> +OUTDIR=
>  out=debugfiles.list
>  srcout=
>  nout=0
> @@ -241,6 +247,10 @@ while [ $# -gt 0 ]; do
>      fi
>      shift
>      ;;
> +  -O|--output-dir)
> +    OUTDIR=$2
> +    shift
> +    ;;
>    -l)
>      lists[$nout]="${lists[$nout]} $2"
>      shift
> @@ -330,7 +340,7 @@ fi
>  
>  $quiet || echo "find-debuginfo: starting" 2>&1
>  
> -OUTDIR=$BUILDDIR
> +[ -z "$OUTDIR" ] && OUTDIR=$BUILDDIR

OK. -O sets OUTDIR, otherwise OUTDIR is BUILDDIR.

>  
>  i=0
>  while ((i < nout)); do
> @@ -348,6 +358,7 @@ SOURCEFILE="$OUTDIR/debugsources.list"
>  LINKSFILE="$OUTDIR/debuglinks.list"
>  ELFBINSFILE="$OUTDIR/elfbins.list"
>  
> +mkdir -p "$OUTDIR"

OK. OUTDIR doesn't have to exist yet.

>  > "$SOURCEFILE"
>  > "$LISTFILE"
>  > "$LINKSFILE"
> diff --git a/tests/find-debuginfo.at b/tests/find-debuginfo.at
> index 834f1af..59c15b9 100644
> --- a/tests/find-debuginfo.at
> +++ b/tests/find-debuginfo.at
> @@ -428,3 +428,29 @@ AT_CHECK([env RPM_BUILD_DIR=${PWD} \
>  # Check builddir against the snapshot
>  AT_CHECK([ls subdir_build | sort], [0], [expout], [])
>  AT_CLEANUP
> +
> +# Run find-debuginfo on a small build and check all output files (and only
> +# those) were created in the supplied output directory
> +AT_SETUP([find-debuginfo output files (custom dir)])
> +AT_KEYWORDS([find-debuginfo] [sources] [debugdata] [gdb-index])

Same as previously, lets just use [find-debuginfo] and [output] as
keywords.

> +FIND_DEBUGINFO_PKG_BUILD_SETUP
> +# Make a snapshot of builddir
> +AT_CHECK([ls subdir_build | sort > expout], [0], [], [])

OK "snapshot" builddir.

> +# We need to set some environment variables for running find-debuginfo
> +# normally set by rpmbuild.
> +AT_CHECK([env RPM_BUILD_DIR=${PWD} \
> +              RPM_BUILD_ROOT=${PWD} \
> +              RPM_PACKAGE_NAME=pkg \
> +              RPM_PACKAGE_VERSION=ver \
> +              RPM_PACKAGE_RELEASE=rel \
> +              RPM_ARCH=arch \
> +          find-debuginfo -p '.*' -o allfiles.list \
> +                         -O ${PWD}/output ${PWD}/subdir_build],
> +         [0], [ignore], [])

Both -O and -o used (after -p so should create a new output file).

> +# Check builddir against the snapshot
> +AT_CHECK([ls subdir_build | sort], [0], [expout], [])

OK, shouldn't have changed.

> +# Check the output directory against the expected list
> +AT_CHECK([(echo allfiles.list; echo "FIND_DEBUGINFO_OUTPUT_FILES") |
> +          sort > expout], [0], [], [])
> +AT_CHECK([ls output | sort], [0], [expout], [])

OK, should now contain all expected files.

> +AT_CLEANUP

Looks good.

Thanks,

Mark

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: find-debuginfo: Allow for specifying output dir for all *.list files
  2025-10-29 15:11 find-debuginfo: Allow for specifying output dir for all *.list files Michal Domonkos
                   ` (4 preceding siblings ...)
  2026-01-08  8:11 ` Panu Matilainen
@ 2026-01-14 22:54 ` Mark Wielaard
  2026-01-15  9:57   ` Michal Domonkos
  5 siblings, 1 reply; 15+ messages in thread
From: Mark Wielaard @ 2026-01-14 22:54 UTC (permalink / raw)
  To: Michal Domonkos; +Cc: debugedit, pmatilai

Hi Michal,

On Wed, Oct 29, 2025 at 04:11:52PM +0100, Michal Domonkos via Debugedit wrote:
> This patch series adds a new switch to the find-debuginfo script that allows
> for overriding the output directory which otherwise defaults to the builddir
> argument. A test is included as well. See the commit messages for details.

Again apologies I forgot about this.  The patches look really
good. The only change I made was for the AT_KEYWORDS in the tests and
pushed all three with that small change.

Thanks,

Mark

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] find-debuginfo: Add test for output files presence
  2026-01-14 22:33   ` Mark Wielaard
@ 2026-01-15  9:30     ` Michal Domonkos
  0 siblings, 0 replies; 15+ messages in thread
From: Michal Domonkos @ 2026-01-15  9:30 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: debugedit, pmatilai

On Wed, Jan 14, 2026 at 11:33:02PM +0100, Mark Wielaard wrote:
> > +# Names of all output files produced by find-debuginfo
> > +m4_define([FIND_DEBUGINFO_OUTPUT_FILES],
> > +[debugfiles.list
> > +debuglinks.list
> > +debugsources.list
> > +elfbins.list])
> 
> Yes, when not -o or -S arguments are given.

Oh indeed, the comment actually could've said something like "... all *default*
output files ...", but nah :)

> > +AT_SETUP([find-debuginfo output files])
> > +AT_KEYWORDS([find-debuginfo] [sources] [debugdata] [gdb-index])
> 
> Lets just use the keywords [find-debuginfo] and [output] this check
> isn't about sources, debugdata or gdb-index.

Truly, thanks for catching. I apparently just copy-pasted this line from the
previous tests.

-- 
Michal Domonkos / RPM.org / Red Hat


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] find-debuginfo: Add -O/--output-dir switch
  2026-01-14 22:50   ` Mark Wielaard
@ 2026-01-15  9:34     ` Michal Domonkos
  0 siblings, 0 replies; 15+ messages in thread
From: Michal Domonkos @ 2026-01-15  9:34 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: debugedit, pmatilai

On Wed, Jan 14, 2026 at 11:50:31PM +0100, Mark Wielaard wrote:
> > However, since commit a8c7efeae67856e144897efb7c40516588b811ee, we have
> > been passing the builddir argument to "debugedit --base-dir" when unique
> > dirs are requested, effectively making the argument live up to its name.
> 
> That was my commit from 2017... I doubt I thought about this
> implication back then.

Yep, it was just waiting for someone to realize this and connect the dots, and
I happened to come across it now :D

-- 
Michal Domonkos / RPM.org / Red Hat


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: find-debuginfo: Allow for specifying output dir for all *.list files
  2026-01-14 22:54 ` Mark Wielaard
@ 2026-01-15  9:57   ` Michal Domonkos
  0 siblings, 0 replies; 15+ messages in thread
From: Michal Domonkos @ 2026-01-15  9:57 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: debugedit, pmatilai

On Wed, Jan 14, 2026 at 11:54:02PM +0100, Mark Wielaard wrote:
> Again apologies I forgot about this.  The patches look really
> good. The only change I made was for the AT_KEYWORDS in the tests and
> pushed all three with that small change.

No worries at all! Thanks for reviewing & merging. We will now proceed with the
RPM change to make use of this (to make --build-in-place more sane).

I should also say that I like this (your) review style where you comment on the
individual hunks with an "OK ..." line where you briefly summarize the change
as you understood it. This also serves as a double-check for the original
author and can reveal issues. There's a lot to like about this good old, email
based workflow, indeed.

Thanks!

-- 
Michal Domonkos / RPM.org / Red Hat


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2026-01-15  9:57 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-29 15:11 find-debuginfo: Allow for specifying output dir for all *.list files Michal Domonkos
2025-10-29 15:11 ` [PATCH 1/3] find-debuginfo: Refactor output directory Michal Domonkos
2026-01-14 22:18   ` Mark Wielaard
2025-10-29 15:11 ` [PATCH 2/3] find-debuginfo: Add test for output files presence Michal Domonkos
2026-01-14 22:33   ` Mark Wielaard
2026-01-15  9:30     ` Michal Domonkos
2025-10-29 15:11 ` [PATCH 3/3] find-debuginfo: Add -O/--output-dir switch Michal Domonkos
2026-01-14 22:50   ` Mark Wielaard
2026-01-15  9:34     ` Michal Domonkos
2025-10-30  8:00 ` find-debuginfo: Allow for specifying output dir for all *.list files Panu Matilainen
2026-01-08  8:11 ` Panu Matilainen
2026-01-08 10:16   ` Mark Wielaard
2026-01-14  9:20     ` Panu Matilainen
2026-01-14 22:54 ` Mark Wielaard
2026-01-15  9:57   ` Michal Domonkos

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).