Re: [PATCH v3 14/15] fdtfactor: Support compressed DTB inputs and outputs

[Doug Anderson]

Hi David,

On Wed, Nov 11, 2026 at 6:10 PM David Gibson
<david@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Sun, Nov 01, 2026 at 02:01:21PM -0800, Douglas Anderson wrote:
> > Extend `fdtfactor` to detect compressed input blobs by magic bytes or
> > file extension, transparently decompress them before factoring, and
> > compress the resulting `.dtbo` and `base.dtb` outputs when appropriate:
> ...
> Hmm.  Whenever I see a pretty complex chunk of code dedicated to
> auto-detecting and guessing the right thing to do, I worry about it
> doing something weird with an unexpected group of inputs.
>
> How much usability would you really lose, if you required that the
> compression method be explicitly supplied (I'd envisage a single
> option which controls both input and output sides)?

Very little in practice -- build pipelines factoring standalone `.dtb`
files already know which compression format they're using, and requiring
an explicit option catches mixed or unexpected inputs instead of
guessing.

In my draft v4 I've replaced the magic-byte/extension auto-detection and
`--base-compression` with a single `-C` / `--compression` option
(`none`, `lz4`, `lz4_legacy`, `lzma`, `xz`, `gzip`; default `none`)
controlling both input decompression and output compression for the base
DTB and `.dtbo` files (with `--output-base-dtb` defaulting to
`base.dtb.<ext>`). (In `-F` / `--fit` mode, each `/images/fdt-*` node
already explicitly declares its compression via its FIT `compression`
property.) That also lets us drop `detect_compression()`,
`split_dtb_filename()`, and the mixed-compression stats logic.


> > -* `-s`, `--stats`: Print size savings summary.
> > +* `-s`, `--stats`: Print uncompressed and compressed size savings.
>
> I don't think this hunk really adds anything useful.

Dropped in my draft v4.


> >      parser.add_argument(
> >          '-s', '--stats',
> >          action='store_true',
> > -        help='Print size statistics summary',
> > +        help='Print size statistics and compression summary',
>
> To, compression fits already in "size statistics".

Dropped in my draft v4.


> > +    # Compressed (.dtb.gz / .dtb.xz / .dtb.lzma / .dtb.lz4) modes
> > +    for comp_spec in "gz:gzip -9 -k -f:gzip -d -f" \
> > +     "xz:xz -k -f:xz -d -f" \
> > +     "lzma:xz --format=lzma -k -f:xz -d -f" \
> > +     "lz4:lz4 -12 -q -f -m:lz4 -d -q -f --rm" \
> > +     "lz4:lz4 -l -12 -q -f -m:lz4 -d -q -f --rm"; do
> > + ext="${comp_spec%%:*}"
>
> Because we use 4-character indent depth, we should consistently use
> spaces to indent in run_tests.sh, not mix tabs and spaces.  Mixing
> them here has the side effect of making these lines not look indented
> from the for statement in patch format.

Fixed across all patches touching `tests/run_tests.sh` in my draft v4.

-Doug