Merge ~fnordahl/ubuntu/+source/ceph:bug/1940697-hirsute into ~ubuntu-server-dev/ubuntu/+source/ceph:ubuntu/hirsute

[Frode Nordahl]

[Christian Ehrhardt  (paelzer)]

Hi,
'm unsure who you expect to review this but I have seen this flying by and will give you a few quick hints.

1. you still target "UNRELEASED"
2. I haven't read into the case, but this sounds a bit like adding Features on SRUs?
   Reading the SRU template it seems more like re-adding a feature dropped by accident, so it
   should be fine I guess.
3. nasm is in universe
   nasm | 2.15.05-1 | impish/universe | source, amd64, arm64, armhf, i386, ppc64el, riscv64, s390x

I have not seen a build log of yours (I generally recommend to add a PPA link to such MRs), but if ceph (main) adds a runtime dependency by this it would be a problem. Have you checked if this really is only having an impact at build time? If so I'd recommend to elaborate about it in the SRU template of your bug anyway.

[Corey Bryant (corey.bryant)]

Thanks Frode. I see libec_isa.so in the focal build but didn't get any hits for nasm: https://launchpadlibrarian.net/550559568/buildlog_ubuntu-focal-amd64.ceph_15.2.13-0ubuntu0.20.04.2_BUILDING.txt.gz

So it seems somehow the library is generated by the build in focal without nasm, but I'm probably missing something. Any ideas?

Fyi, I'm good with UNRELEASED. When I sponsor uploads that live in git repos I'll release/tag them right before upload. It also gives us the option to let the code live in the repo and get picked up on a future upload with more batched changes.

Attaching build log that I got from Frode: https://launchpad.net/~fnordahl/+archive/ubuntu/lp1940697/+packages

[Corey Bryant (corey.bryant)]

I noticed that the impish arm64 build installs libec_isa.so: https://launchpadlibrarian.net/548509636/buildlog_ubuntu-impish-arm64.ceph_16.2.5-0ubuntu2_BUILDING.txt.gz

[Frode Nordahl (fnordahl)]

That's weird, as noted on https://bugs.launchpad.net/charm-ceph-mon/+bug/1940697/comments/1 it is not in the currently published package. I'll take another look.

[Corey Bryant (corey.bryant)]

For impish it's only included in the arm64 build and all other architectures are missing it. For focal it was only included in the arm64 and amd64 builds and all other architectures were missing it.

[Corey Bryant (corey.bryant)]

~/pkg/ussuri/upstream/ceph$ git-pretty v15.2.13..v16.2.0|grep -i nasm
 - [d72c9205dc0] cmake: only display the result of checking nasm once
 - [f22cb92d51f] nasm-wrapper: Make nasm-wrapper OS independent
 - [497b4d6f72a] cmake: remove yasm since it's been replaced by nasm
 - [58a946ca06a] cmake: use nasm to replace yasm to build isa-l
 - [a944232ad40] install-deps.sh: install nasm for isa-l library
 - [0042d244d48] cmake: wrapper nasm as assembler to deal with some options
 - [6d62cc05d94] cmake: check nasm and define related MACRO
 - [bf5794b2f08] cmake: define cmake macro to check nasm assembler

commit 497b4d6f72add5420b13df0d26ec48be37472adc
Author: Liu Changcheng <email address hidden>
Date: Wed Oct 28 15:26:41 2020 +0800

    cmake: remove yasm since it's been replaced by nasm

    Signed-off-by: Changcheng Liu <email address hidden>

It seems we should drop yasm in addition to what you have here in this change. Could we update the commit message with more details point to the move from yasm to nasm? Worth noting, yasm was/is also in universe.

[Corey Bryant (corey.bryant)]

15.2.13 == focal
16.2.0 == hirsute

[Frode Nordahl (fnordahl)]

Yes, that makes sense, will do.

This spurs a question: I added the nasm as build-dep only on amd64 in d/control on the basis of the autoconf scripts asserting x86 before looking for it, but should we perhaps add it for all architectures and leave it up to the projects autoconf to figure out when to use it?

[Corey Bryant (corey.bryant)]

Maybe..

This commit is pretty old from the package.

commit c144f8e59619512bc3de02c680aa4a97b5874522
Author: Dmitry Smirnov <email address hidden>
Date: Thu Sep 4 04:13:50 2014 +1000

    Build-Depends: mark "yasm" as [amd64] (Closes: #760383)

      [ Thorsten Glaser <email address hidden> ]
      * Make yasm dependency amd64 only, it isn’t used elsewhere but
        breaks x32 (which is mis-detected as amd64), (Closes: #760383).

[Frode Nordahl (fnordahl)]

Both hirsute and impish branches updated with new wording for changelog and commit messages, including references to commits documenting the move upstream.

[Corey Bryant (corey.bryant)]

Seems ok to me. I took a look at your latest build and compared vs the latest impish amd64 build to look at the files provided by binary packages, toward the end of the builds:

[1] https://launchpad.net/ubuntu/+source/ceph/16.2.5-0ubuntu2/+build/21782588

ceph-base_16.2.5-0ubuntu2.1_amd64.deb
-------------------------------------

Your build adds new files:

-rw-r--r-- root/root 191336 2021-08-24 06:39 ./usr/lib/x86_64-linux-gnu/ceph/erasure-code/libec_isa.so

librbd1_16.2.5-0ubuntu2.1_amd64.deb
-----------------------------------

Your build adds new files:

drwxr-xr-x root/root 0 2021-08-24 06:39 ./usr/lib/x86_64-linux-gnu/ceph/
drwxr-xr-x root/root 0 2021-08-24 06:39 ./usr/lib/x86_64-linux-gnu/ceph/librbd/
lrwxrwxrwx root/root 0 2021-08-24 06:39 ./usr/lib/x86_64-linux-gnu/ceph/librbd/libceph_librbd_parent_cache.so -> libceph_librbd_parent_cache.so.1
lrwxrwxrwx root/root 0 2021-08-24 06:39 ./usr/lib/x86_64-linux-gnu/ceph/librbd/libceph_librbd_parent_cache.so.1 -> libceph_librbd_parent_cache.so.1.0.0
-rw-r--r-- root/root 1460544 2021-08-24 06:39 ./usr/lib/x86_64-linux-gnu/ceph/librbd/libceph_librbd_parent_cache.so.1.0.0
lrwxrwxrwx root/root 0 2021-08-24 06:39 ./usr/lib/x86_64-linux-gnu/ceph/librbd/libceph_librbd_pwl_cache.so -> libceph_librbd_pwl_cache.so.1
lrwxrwxrwx root/root 0 2021-08-24 06:39 ./usr/lib/x86_64-linux-gnu/ceph/librbd/libceph_librbd_pwl_cache.so.1 -> libceph_librbd_pwl_cache.so.1.0.0
-rw-r--r-- root/root 3411824 2021-08-24 06:39 ./usr/lib/x86_64-linux-gnu/ceph/librbd/libceph_librbd_pwl_cache.so.1.0.0

ceph-common_16.2.5-0ubuntu2.1_amd64.deb
---------------------------------------

Your build adds new files:

lrwxrwxrwx root/root 0 2021-08-24 06:39 ./usr/lib/x86_64-linux-gnu/ceph/crypto/libceph_crypto_isal.so -> libceph_crypto_isal.so.1
lrwxrwxrwx root/root 0 2021-08-24 06:39 ./usr/lib/x86_64-linux-gnu/ceph/crypto/libceph_crypto_isal.so.1 -> libceph_crypto_isal.so.1.0.0
-rw-r--r-- root/root 84128 2021-08-24 06:39 ./usr/lib/x86_64-linux-gnu/ceph/crypto/libceph_crypto_isal.so.1.0.0

[Christian Ehrhardt  (paelzer)]

Due to that change also dependencies of librbd1 expand

from
Depends: librados2 (= 16.2.5-0ubuntu2), libc6 (>= 2.33), libcryptsetup12 (>= 2:2.0), libgcc-s1 (>= 3.0), libssl1.1 (>= 1.1.0), libstdc++6 (>= 9)

to
 Depends: librados2 (= 16.2.5-0ubuntu2.1), libaio1 (>= 0.3.93), libc6 (>= 2.33), libcryptsetup12 (>= 2:2.0), libgcc-s1 (>= 3.4), libnuma1 (>= 2.0.11), libpmem1 (>= 1.4), libpmemobj1 (>= 1.4), libssl1.1 (>= 1.1.0), libstdc++6 (>= 11), libuuid1 (>= 2.16)

That adds:
+ libaio1 (>= 0.3.93)
+ libnuma1 (>= 2.0.11)
+ libpmem1 (>= 1.4)
+ libpmemobj1 (>= 1.4)
+ libuuid1 (>= 2.16)

Of which libpmemobj1 is only in universe.
I know that pmem is in main, but I don't remember right away if there was
a reason to hold this one back or if it could be promoted. At least that
would be an extra hurdle to cross for this change.

[Frode Nordahl (fnordahl)]

The librbd changes was indeed unreleased in the ubuntu/impish branch already, I wonder if this is the reason why they have not been progressed previously.

diff --git a/debian/changelog b/debian/changelog
index 9d13e93..493cee7 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+ceph (16.2.5-0ubuntu0.21.04.2) UNRELEASED; urgency=medium
+
+  * d/control: The ISA-L EC plugin changed its build dependency from yasm to
+    nasm 2.13.03 or newer (LP: 1940697)
+
+ -- Frode Nordahl <[e-mail]>  Tue, 24 Aug 2021 08:39:48 +0200
+
 ceph (16.2.5-0ubuntu0.21.04.1) hirsute; urgency=medium
 
   * New upstream point release (LP: #1935817).
diff --git a/debian/control b/debian/control
index 464c9bf..00f76f8 100644
--- a/debian/control
+++ b/debian/control
@@ -72,6 +72,7 @@ Build-Depends:
  libxml2-dev,
  lsb-release,
  luarocks,
+ nasm (>= 2.13.03) [amd64],
  pkg-config,
  python3-cherrypy3,
  python3-dev,
@@ -83,7 +84,6 @@ Build-Depends:
  valgrind [amd64 armhf i386 powerpc],
  virtualenv,
  xfslibs-dev,
- yasm [amd64],
  zlib1g-dev,
 Build-Conflicts:
  libcrypto++-dev,