Bug#1032120: tiledb: uses atomic operations, but is not linked to libatomic

[Graham Inggs]

Source: tiledb
Version: 2.14.1-1
Severity: serious
Tags: ftbfs patch
Control: affects -1 src:tiledb-py

Hi Dirk

TileDB uses atomic operations but it is not linked to libatomic on the
architectures that need it. This can be seen in recent build logs for
armel[1], mipsel[2], powerpc[3], riscv64[4] and sh4[5], where warnings
like the following are emitted:

dpkg-shlibdeps: warning: symbol __atomic_fetch_add_8 used by
debian/libtiledb2.13/usr/lib/arm-linux-gnueabi/libtiledb.so.2.14 found
in none of the libraries

and

dpkg-shlibdeps: warning: symbol __atomic_compare_exchange_1 used by
debian/libtiledb2.13/usr/lib/riscv64-linux-gnu/libtiledb.so.2.14 found
in none of the libraries

This causes at least tiledb-py to FTBFS on riscv64[6]. I have tested
the patch below on riscv64 and was able to build tiledb-py
successfully. It should do the right thing on the other architectures
as well.

Regards
Graham


[1] https://buildd.debian.org/status/fetch.php?pkg=tiledb&arch=armel&ver=2.14.1-1&stamp=1676252451&raw=0
[2] https://buildd.debian.org/status/fetch.php?pkg=tiledb&arch=mipsel&ver=2.14.1-1&stamp=1676341362&raw=0
[3] https://buildd.debian.org/status/fetch.php?pkg=tiledb&arch=powerpc&ver=2.14.1-1&stamp=1676251162&raw=0
[4] https://buildd.debian.org/status/fetch.php?pkg=tiledb&arch=riscv64&ver=2.14.1-1&stamp=1676255427&raw=0
[5] https://buildd.debian.org/status/fetch.php?pkg=tiledb&arch=sh4&ver=2.14.1-1&stamp=1676262874&raw=0
[6] https://buildd.debian.org/status/logs.php?pkg=tiledb-py&arch=riscv64


--- a/debian/rules
+++ b/debian/rules
@@ -9,6 +9,11 @@
# Workaround until
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=953855 is fixed
#export DEB_CXXFLAGS_MAINT_APPEND = -DSPDLOG_FMT_EXTERNAL -DFMT_HEADER_ONLY

+# Link with -latomic on architectures that need it
+ifneq (,$(filter $(DEB_HOST_ARCH), armel mipsel powerpc riscv64 sh4))
+ export DEB_LDFLAGS_MAINT_APPEND += -Wl,--no-as-needed -latomic
-Wl,--as-needed
+endif
+
#PKG_SHORT_DESC := $(shell grep 'Description:' debian/control | tail
-n 1 | cut -d':' -f2 | sed 's/^\s//')

#ifeq ($(filter nocheck,$(DEB_BUILD_PROFILES)),)

[Graham Inggs]

Hi Dirk

Thanks for the quick upload!
Already got a successful build of tiledb-py on riscv64[1].

On Tue, 28 Feb 2023 at 17:14, Dirk Eddelbuettel <e...@debian.org> wrote:
> TileDB upstream does everything in CMake (which I do not fully speak) so
> "eventually" it would be nice to port this.

After testing the patch which I eventually submitted, I found a bug[2]
that in message #5, discusses a way to automatically link libatomic
when required by calling the linker with -pthread, instead of linking
with -lpthread.

Then, in message #25, the way to do this in CMake is shown:

set(THREADS_PREFER_PTHREAD_FLAG ON)
find_package(Threads REQUIRED)

I tried applying this patch to TileDB, but it did not work for me.
Perhaps upstream can make more sense of what is suggested in that bug
report and in the linked cmake issue[3].

Regards
Graham


[1] https://buildd.debian.org/status/logs.php?pkg=tiledb-py&arch=riscv64
[2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=895969
[3] https://cmake.org/Bug/view.php?id=14767

[Dirk Eddelbuettel]

Hi Graham,

On 28 February 2023 at 19:36, Graham Inggs wrote:
| Hi Dirk
|
| Thanks for the quick upload!
| Already got a successful build of tiledb-py on riscv64[1].
|
| On Tue, 28 Feb 2023 at 17:14, Dirk Eddelbuettel <e...@debian.org> wrote:
| > TileDB upstream does everything in CMake (which I do not fully speak) so
| > "eventually" it would be nice to port this.
|
| After testing the patch which I eventually submitted, I found a bug[2]
| that in message #5, discusses a way to automatically link libatomic
| when required by calling the linker with -pthread, instead of linking
| with -lpthread.
|
| Then, in message #25, the way to do this in CMake is shown:
|
| set(THREADS_PREFER_PTHREAD_FLAG ON)
| find_package(Threads REQUIRED)
|
| I tried applying this patch to TileDB, but it did not work for me.
| Perhaps upstream can make more sense of what is suggested in that bug
| report and in the linked cmake issue[3].

That's quite helpful. I will pass that on.

Cheers, Dirk

| Regards
| Graham
|
|
| [1] https://buildd.debian.org/status/logs.php?pkg=tiledb-py&arch=riscv64
| [2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=895969
| [3] https://cmake.org/Bug/view.php?id=14767

--
dirk.eddelbuettel.com | @eddelbuettel | e...@debian.org

[Nilesh Patra]

Quoting Andreas Tille:
> your recent upload of tiledb 2.15.0-1 into unstable is quite unfortunate
> in freeze policy. You should have waited at least until 2.14.1-2 fixing
> this bug to migrate to testing. You would need to ask release team for
> migration of 2.15.0 which will probably be refused

The changes between 2.14 and 2.15 are non-trivial and are likely to be rejected by
release team at the moment, considering there are no autopkgtests either.

> even if you would be
> able to fix the regression in autopkgtest[1].

This is because the version of tiledb-py is not compatible with tiledb 2.15.0. Uploading
a new version of tiledb-py would fix this, but:

- It'd be useless unless tiledb 2.15.0 migrates.
- It seems to need pyarrow for tests, and other functionalities, which is un-packaged.

So current situation is:

* tiledb 2.14.1-1 in testing (and next stable) is marked for auto-removal.
* consequently, tiledb-py is also marked for autoremoval.
* tiledb 2.15.0-1 can't migrate because of freeze policy.

What a freaking mess!

Dirk, we could have avoided all of it if you had quite literally ** waited ** for a few hours
to let tiledb migrate on the 8th of March. I'm sad to see such mess-ups happening at this time as I
had put quite some efforts on tiledb-py myself too.

The solution right now is what Andreas said:

> My recommendation would be to upload a version
>
> 2.15.0-2+really_2.14.1

Dirk, please fix this so tiledb can make a (valid) stable release.

> [1]: https://tracker.debian.org/pkg/tiledb

Thanks,
Nilesh

[Nilesh Patra]

On Sun, Mar 12, 2023 at 07:27:46PM +0530, Nilesh Patra wrote:
> On 3/12/23 19:00, Dirk Eddelbuettel wrote:
> >
> > Why not wait a week on 2.15.0-1 which now 'Too young, only 4 of 10 days old'?
>
> It won't migrate as told earlier too. By tomorrow it would have 'blocked by freeze' written over there.

And it has that written over there now. Another better option can be to
add autopkgtests to tiledb, as this is not a key package and hence can
migrate w/o freeze block.

Let me know what you think.

--
Best,
Nilesh

[Nilesh Patra]

Hi,

I am removing myself from the uploaders field/maintenance of tiledb-py. Feel
free to update it once tiledb is ready for migration. The repository
lives at python team namespace.

--
Best,
Nilesh

[Andreas Tille]

[Release team in CC]

Am Wed, Mar 15, 2023 at 08:45:21AM +0530 schrieb Nilesh Patra:
> I am removing myself from the uploaders field/maintenance of tiledb-py. Feel
> free to update it once tiledb is ready for migration. The repository
> lives at python team namespace.

I admit I'm strongly in favour of following release policy properly
instead of putting work on other DDs who need to check rdepends. I
became aware of this bug since genomicsdb received a testing removal
warning.

Thus I insist that the correct fix for this violation of the freeze
policy is to upload

2.15.0-2+really_2.14.1

(or at your preference using an epoch) as I wrote before in this bug
log[1]) @Dirk, I would be happy if you could simply say sorry guys
for creating this noise and follow freeze policy.

Kind regards
Andreas.

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1032120#37

--
http://fam-tille.de

[Adam Cecile]

Hello,

It seems unittests are running against very new pandas version that wants pyarrow (but it does not seems to be available in the archive).
Can you check while self.use_arrow evaluate to True ? That's mostly the root cause of the failure.
I'll try to have a look when I'll be in front of my computer.

Regards, Adam.

On March 25, 2023 2:55:39 PM GMT+01:00, Andreas Tille <and...@an3as.eu> wrote:

Hi,

as you can read in the bug log, there was an upload of a new version of
tiledb a couple of hours before it has migrated to testing.  Thus the
package remains affected by a testing removal (together with its two
reverse dependencies tiledb and genomicsdb).  To follow the freeze
policy I reverted the version bump and NMUed tiledb
2.15.0really2.14.1-0.1 to experimental (since the maintainer did not
responded).

As we can see tiledb-py does not build against tiledb 2.15.0[1]

I've now forced (Build-)Depends to
   tibtiledb-dev (>= 2.15.0really2.14.1~)
but it seems Salsa CI autopkgtest does not respect the setting

variables:
  # Build against tiledb in experimental
  RELEASE: 'experimental'

and thus the autopkgtest log does not reproduce the error I've got
in my local build:

...

=================================== FAILURES ===================================
_______________ TestNumpyToArray.test_from_numpy_empty_str[1-0] ________________

self = <tests.test_libtiledb.TestNumpyToArray object at 0x7fe177722710>
empty_str = '', num_strs = 1

    @pytest.mark.parametrize("empty_str", ["", b""])
    @pytest.mark.parametrize("num_strs", [1, 1000])
    def test_from_numpy_empty_str(self, empty_str, num_strs):
        uri = self.path("test_from_numpy_empty_str")
        np_array = np.asarray([empty_str] * num_strs, dtype="O")
        tiledb.from_numpy(uri, np_array)
....
        with tiledb.open(uri, "r") as A:
            assert_array_equal(A[:], np_array)
            if has_pandas():
assert_array_equal(A.query(use_arrow=True).df[:][""], np_array)

tests/test_libtiledb.py:3356:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _.
/usr/lib/python3/dist-packages/tiledb/multirange_indexing.py:192: in __getitem__
    return self if self.return_incomplete else self._run_query()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _.

self = <tiledb.multirange_indexing.DataFrameIndexer object at 0x7fe1779ef150>

    def _run_query(self) -> Union[DataFrame, Table]:
        if self.pyquery is not None:
            self.pyquery.submit()
....
        if self.pyquery is None:
            df = DataFrame(self._empty_results)
        elif self.use_arrow:
            with timing("buffer_conversion_time"):
table = self.pyquery._buffers_to_pa_table()
E               ModuleNotFoundError: No module named 'pyarrow'

/usr/lib/python3/dist-packages/tiledb/multirange_indexing.py:329: ModuleNotFoundError
_______________ TestNumpyToArray.test_from_numpy_empty_str[1-1] ________________

self = <tests.test_libtiledb.TestNumpyToArray object at 0x7fe177722490>
empty_str = b'', num_strs = 1

    @pytest.mark.parametrize("empty_str", ["", b""])
    @pytest.mark.parametrize("num_strs", [1, 1000])
    def test_from_numpy_empty_str(self, empty_str, num_strs):
        uri = self.path("test_from_numpy_empty_str")
        np_array = np.asarray([empty_str] * num_strs, dtype="O")
        tiledb.from_numpy(uri, np_array)
....
        with tiledb.open(uri, "r") as A:
            assert_array_equal(A[:], np_array)
            if has_pandas():
assert_array_equal(A.query(use_arrow=True).df[:][""], np_array)

tests/test_libtiledb.py:3356:
/usr/lib/python3/dist-packages/tiledb/multirange_indexing.py:192: in __getitem__
    return self if self.return_incomplete else self._run_query()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _.

self = <tiledb.multirange_indexing.DataFrameIndexer object at 0x7fe177a16ed0>

variables:
  # Build against tiledb in experimental
  RELEASE: 'experimental'
    def _run_query(self) -> Union[DataFrame, Table]:
        if self.pyquery is not None:
            self.pyquery.submit()
....
        if self.pyquery is None:
            df = DataFrame(self._empty_results)
        elif self.use_arrow:
            with timing("buffer_conversion_time"):
table = self.pyquery._buffers_to_pa_table()
E               ModuleNotFoundError: No module named 'pyarrow'

/usr/lib/python3/dist-packages/tiledb/multirange_indexing.py:329: ModuleNotFoundError
______________ TestNumpyToArray.test_from_numpy_empty_str[1000-0] ______________

self = <tests.test_libtiledb.TestNumpyToArray object at 0x7fe177722510>
empty_str = '', num_strs = 1000

    @pytest.mark.parametrize("empty_str", ["", b""])
    @pytest.mark.parametrize("num_strs", [1, 1000])
    def test_from_numpy_empty_str(self, empty_str, num_strs):
        uri = self.path("test_from_numpy_empty_str")
        np_array = np.asarray([empty_str] * num

=================================== FAILURES ===================================
_______________ TestNumpyToArray.test_from_numpy_empty_str[1-0] ________________

self = <tests.test_libtiledb.TestNumpyToArray object at 0x7fe177722710>
empty_str = '', num_strs = 1

    @pytest.mark.parametrize("empty_str", ["", b""])
    @pytest.mark.parametrize("num_strs", [1, 1000])
    def test_from_numpy_empty_str(self, empty_str, num_strs):
        uri = self.path("test_from_numpy_empty_str")
        np_array = np.asarray([empty_str] * num_strs, dtype="O")
        tiledb.from_numpy(uri, np_array)
....
        with tiledb.open(uri, "r") as A:
            assert_array_equal(A[:], np_array)
            if has_pandas():
assert_array_equal(A.query(use_arrow=True).df[:][""], np_array)

tests/test_libtiledb.py:3356:
...


I admit I'm a bit astonished since when tiledb 2.14.1 was available
in unstable the autopkgtest used to work.  Any idea how to fix this
issue would be welcome.

Kind regards,
   Andreas

[1] https://salsa.debian.org/python-team/packages/tiledb-py/-/pipelines/514328