Comment 5 for bug 1988418

Revision history for this message
Dave Jones (waveform) wrote :

> My standard way of reviewing a merge is to do the merge myself and
> see what comes up different. Here are my observations:
>
> conf/initramfs.conf: the commented COMPRESSLEVEL setting should
> match the default, which for us is 1, not 3.

Good point! Slightly weird that it's set to 3 given that upstream's
default is 9. Still, ours should indeed be consistent with our
defaults -- I'll amend it.

> debian/control: upstream has Recommends: ${busybox:Recommends},
> zstd, but you only have Recommends: zstd. Why?

This is a part where the range-diff comes in handy (sorry this is
probably going to wrap horribly):

 1: 896377e ! 1: 1d9229b - Make busybox-initramfs a real runtime dependency, fixing kernel install failures with cryptsetup
    @@ debian/control: Description: generic modular initramfs generator (automation)
      Package: initramfs-tools-core
      Architecture: all
      Multi-Arch: foreign
    --Recommends: ${busybox:Recommends}, pigz
    +-Recommends: ${busybox:Recommends}, zstd
     -Depends: klibc-utils (>= 2.0.4-8~), cpio (>= 2.12), kmod, udev, coreutils (>= 8.24), logsave | e2fsprogs (<< 1.45.3-1~), ${misc:Depends}
    -+Recommends: pigz
    ++Recommends: zstd
     +Depends: ${busybox:Depends}, klibc-utils (>= 2.0.4-8~), cpio (>= 2.12), kmod, udev, coreutils (>= 8.24), logsave | e2fsprogs (<< 1.45.3-1~), ${misc:Depends}
      Suggests: bash-completion
      Breaks: initramfs-tools (<< 0.121~), ${busybox:Breaks}

This is showing that one part of our delta moves ${busybox:Recommends}
to ${busybox:Depends} ("Make busybox-initramfs a real runtime
dependency..."). That part of our delta hasn't changed, but the
context of it has. Re-ordering the lines a bit, the previous diff was:

    --Recommends: ${busybox:Recommends}, pigz
    -+Recommends: pigz
     -Depends: klibc-utils (>= 2.0.4-8~), cpio (>= 2.12), kmod, udev, coreutils (>= 8.24), logsave | e2fsprogs (<< 1.45.3-1~), ${misc:Depends}
     +Depends: ${busybox:Depends}, klibc-utils (>= 2.0.4-8~), cpio (>= 2.12), kmod, udev, coreutils (>= 8.24), logsave | e2fsprogs (<< 1.45.3-1~), ${misc:Depends}

But upstream changed pigz to zstd in Recommends so now the delta is:

    +-Recommends: ${busybox:Recommends}, zstd
    ++Recommends: zstd
     -Depends: klibc-utils (>= 2.0.4-8~), cpio (>= 2.12), kmod, udev, coreutils (>= 8.24), logsave | e2fsprogs (<< 1.45.3-1~), ${misc:Depends}
     +Depends: ${busybox:Depends}, klibc-utils (>= 2.0.4-8~), cpio (>= 2.12), kmod, udev, coreutils (>= 8.24), logsave | e2fsprogs (<< 1.45.3-1~), ${misc:Depends}

In other words, it's a context change but no real change in the delta.

> debian/rules: you caught that there were build-time tests added by
> us that should be retained even though upstream dropped their
> override; good (I missed this).
>
> hook-functions: upstream landed the change to exclude modems from
> the net usb drivers, but we had cdc-phonet.ko in our list but Debian
> does not. I do not know why Debian did not include it; I think we
> should include it in the merge and forward to Debian for
> consideration.

I think Debian does include it, just in a slightly different position?
At least, I can see cdc-phonet.ko added after "can" in the following
diff:

    $ git diff old/ubuntu..merge/0.142ubuntu1 hook-functions
    ...
    - appletalk arcnet bonding can dummy.ko \
    - hamradio hippi ifb.ko irda macvlan.ko \
    - macvtap.ko pcmcia sb1000.ko team tokenring \
    - tun.ko veth.ko wan wimax wireless \
    - cdc_mbim.ko cdc-phonet.ko hso.ko huawei_cdc_ncm.ko ipheth.ko kalmia.ko \
    - lg-vl600.ko qmi_wwan.ko sierra_net.ko cdc_subset.ko gl620a.ko \
    - net1080.ko plusb.ko cx82310_eth.ko zaurus.ko \
    - xen-netback.ko
    + appletalk arcnet bonding can cdc-phonet.ko \
    + cdc_mbim.ko cdc_subset.ko cx82310_eth.ko \
    + dummy.ko gl620a.ko hamradio hippi hso.ko \
    + huawei_cdc_ncm.ko ifb.ko ipheth.ko irda \
    + kalmia.ko lg-vl600.ko macvlan.ko macvtap.ko \
    + net1080.ko pcmcia plusb.ko qmi_wwan.ko \
    + sb1000.ko sierra_net.ko team tokenring tun.ko \
    + veth.ko wan wimax wireless xen-netback.ko \
    + zaurus.ko
    ...

I was fairly sure I'd verified that all the same drivers were in that
section given it was one of the "merged upstream" bits in the
range-diff, but let's double-check:

    >>> a = """- appletalk arcnet bonding can dummy.ko \
    ... - hamradio hippi ifb.ko irda macvlan.ko \
    ... - macvtap.ko pcmcia sb1000.ko team tokenring \
    ... - tun.ko veth.ko wan wimax wireless \
    ... - cdc_mbim.ko cdc-phonet.ko hso.ko huawei_cdc_ncm.ko ipheth.ko kalmia.ko \
    ... - lg-vl600.ko qmi_wwan.ko sierra_net.ko cdc_subset.ko gl620a.ko \
    ... - net1080.ko plusb.ko cx82310_eth.ko zaurus.ko \
    ... - xen-netback.ko
    ... """
    >>> b = """+ appletalk arcnet bonding can cdc-phonet.ko \
    ... + cdc_mbim.ko cdc_subset.ko cx82310_eth.ko \
    ... + dummy.ko gl620a.ko hamradio hippi hso.ko \
    ... + huawei_cdc_ncm.ko ifb.ko ipheth.ko irda \
    ... + kalmia.ko lg-vl600.ko macvlan.ko macvtap.ko \
    ... + net1080.ko pcmcia plusb.ko qmi_wwan.ko \
    ... + sb1000.ko sierra_net.ko team tokenring tun.ko \
    ... + veth.ko wan wimax wireless xen-netback.ko \
    ... + zaurus.ko
    ... """
    >>> a = {word for line in a.splitlines() for word in line.split() if word not in ('-', '+', '\\')}
    >>> b = {word for line in b.splitlines() for word in line.split() if word not in ('-', '+', '\\')}
    >>> a == b
    True

> + elif [ -n "${DEVICE}" ] || [ -n "${DEVICE6}" ]; then
>
> FWIW I would write this as elif [ -n "$DEVICE$DEVICE6" ]; then

Heh, the pedant in me balks slightly at that, but it is indeed more
efficient in shell and god knows initramfs-tools is slow enough as it
is! I'll amend it :)