Обсуждение: Packaging request MobilityDB

Hi Esteban,

> How can we work together with the Debian packaging community to make available in Debian-based systems the
forthcomingMobilityDB 1.1 release ?
 

I recently went through the process of getting pgvector[0] packaged
for both upstream Debian and inclusion in PGDG APT repository; you can
read my summary of the process I followed to add a new package in [1]
and [2]. I can assist with getting the packaging work started for
MobilityDB if you'd like. If I run into any challenges, I can open an
issue on the GitHub repo. Let me know if you have any follow-up
questions.

King regards,

--Bradford

[0]: https://github.com/pgvector/pgvector
[1]: https://www.postgresql.org/message-id/CAOMoQbS%3D2feHy-Q6XVg8D_6_MBTCNnzR_cWy9OC_swN3F3o7nQ%40mail.gmail.com
[2]: https://www.postgresql.org/message-id/CAOMoQbQ%2BkXEOgaT1WUya9vf78a4Hmi13sNuHz2sEXVvjcMasRQ%40mail.gmail.com

I've spent some time looking at packaging MobilityDB for inclusion in
the Debian archive and PGDG. Here is a high-level summary of what I've
done so far:

* Opened an ITP bug against wnpp for MobilityDB [1]
* Created an initial source package with dh make_pgxs
  - Replaced default autopkgtest with customized one (see below for
    discussion)
* Created a new project on salsa under the PostgreSQL team [2]
  - I have not updated the CI/CD settings to point at d/gitlab-ci.yml
    (see below for discussion)
* Added a job for MobilityDB to the pgapt Jenkins jobs (patch attached)

Upstream isn't using pgxs for its build system and does not have an
installcheck target. The build system does have tests but these tests
are run with cmake and running that in autopkgtest would require
re-building the source package (or using the 'build-needed'
restriction). Additionally, the upstream tests include creating,
starting, and stoping a test cluster. I decided to replace the generated
'installcheck' script with a custom one that uses pg_virtualenv and
pg_regress to run upstream's test cases. I was able to (automatically)
reuse the test SQL queries from upstream but I was not able to reuse the
expected responses. This is because upstream's test uses UTC timezone
while pg_regress sets PGTZ to PST8PDT. Additionally, upstream's test
includes pre- and post-processing of the expected results to standardize
ERROR messages along with removing WARNING and CONTEXT lines. I have had
to skip one test case (052_tpoint.test) because it is currently failing
with the following error message in the Postgres log:

> *** stack smashing detected ***: terminated
> 2023-06-04 02:59:18.383 UTC [25831] LOG:  server process (PID 25852) was terminated by signal 6: Aborted
> 2023-06-04 02:59:18.383 UTC [25831] DETAIL:  Failed process was running: SELECT tgeompoint(Instant,PointZ,5676,1234)
'SRID=5676;Point(00 0)@2000-01-01';
 

I think this indicates a potential buffer overflow bug [3] but I haven't
had a chance to look into it.

I have skipped setting up the CI/CD settings for the packaging
repository on Salsa for now. The intent is to package the upcoming
release of MobilityDB 1.1 and since the release hasn't happened yet,
there is no upstream source archive to fetch with uscan. For my local
development and testing of the package, I've just used a "fake" source
archive that I created with git archive. Until the upstream release of
1.1, I'd expect the default GitLab pipeline to fail. This is why I have
added the package to pgapt's Jenkins jobs with the job group of
"git+upstream-snapshot".



[1]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1037101
[2]: https://salsa.debian.org/postgresql/mobilitydb
[3]: https://stackoverflow.com/a/1347464

Dear Bradford

Many thanks for your work.

Seconded! and tks for MobilityDB too !

* We solved the stack smashing problem you found.

* We set the log_error_verbosity to terse for the tests and removed the special treatment of ERROR, CONTEXT, and WARNING that may vary across PostgreSQL and PostGIS versions.

* For the tests, we can easily switch to the settings 'PST8PDT' for the timezone and 'Postgres, MDY' for the date style if this is the general way to do things. We simply need to regenerate all tests and push them in a new commit. We just wonder if the same settings are equally valid for all other distributions. Please advise.

Yeah, 'PST8PDT' is the way to go for PostgreSQL test [1]

I believe it's valid for other distributions.

Apparently datestyle is not documented here, but pg_regress does:

     /*
      * Set timezone and datestyle for datetime-related tests
      */
     setenv("PGTZ", "PST8PDT", 1);
     setenv("PGDATESTYLE", "Postgres, MDY", 1);

see also [2] which might be useful as it mentions some corner cases for other systems.

[1]: https://www.postgresql.org/docs/current/regress-evaluation.html#id-1.6.20.6.8

[2]: https://doxygen.postgresql.org/pg__regress_8c_source.html#l00702

---
Cédric Villemain +33 (0)6 20 30 22 52
https://Data-Bene.io
PostgreSQL: Expertise - Formation - Développement - Assistance Technique

I suspect this may be an actual bug. I tested develop (07fafdf1) on both
macOS and Debian. On macOS, changing the timezone is reflected in the
output but not the datestyle. On Debian, changes to timezone and
datestyle are reflected in the output.

On macOS

```sql
SET timezone TO 'Europe/Berlin';
SET datestyle TO 'German';
SELECT now();
--                now
-- ---------------------------------
--  08.06.2023 02:45:35.214256 CEST
-- (1 row)

-- SELECT set(ARRAY [timestamptz '2000-01-01', '2000-01-02', '2000-01-03']);
--                                       set
-- --------------------------------------------------------------------------------
--  {"2000-01-01 00:00:00+01", "2000-01-02 00:00:00+01", "2000-01-03
00:00:00+01"}
-- (1 row)
```

On Debian Bookworm

```sql
SET timezone TO 'Europe/Berlin';
SET datestyle TO 'German';
SELECT now();
--                now
-- ---------------------------------
--  08.06.2023 02:53:11.748104 CEST
-- (1 row)

SELECT set(ARRAY [timestamptz '2000-01-01', '2000-01-02', '2000-01-03']);
--                                         set
-- -----------------------------------------------------------------------------------
--  {"01.01.2000 00:00:00 CET", "02.01.2000 00:00:00 CET", "03.01.2000
00:00:00 CET"}
-- (1 row)
```

--Bradford

Esteban,

I've just pulled the latest commit on develop and built snapshot
packages locally with sbuild for both amd64 and arm64; packages build on
both platforms but there are two test failing on amd64 and four tests
are failing on arm64

* 055_geography_funcs (arm64)
* 064_tpoint_distance (amd64, arm64)
* 076_tpoint_analytics (amd64, arm64)
* 082_npointset_tbl (arm64)

I've attached the first 100 lines of the diffs from the failing tests
but I haven't had a chance to investigate theses test failure further.

-- Bradford


On Wed, Jun 14, 2023 at 10:36 AM Esteban Zimanyi
<estebanzimanyi@gmail.com> wrote:
>
> Dear all
>
> Is there anything we can do to advance the packaging on MobilityDB ?
>
> Please let us know.
>
> Regards
>
> Esteban
>

Quick update on the status of packaging MobilityDB:

* Simplified the package's autopkgtest to use the upstream's test
queries and expected output [1]
- pg_regress uses a different set of psql options and directory
structure (sql & expected vs queries & expected)
* Debian 12 (bookworm/stable) amd64
- builds successfully
- piuparts passes
- autopkgtest passes
* Debian 12 (bookworm/stable) arm64
- builds successfully
- piuparts passes
- autopkgtest fails (failing test cases below)
* Debian 13 (trixie/testing)
- builds successfully
- piuparts passes
- autopkgtests fail for trixie (testing)
* test failure looks to be an issue with the newer libproj (9.2.1)
included in trixie

Once the test failure for bookworm arm64 is fixed, I think MobilityDB
could go ahead and tag the 1.1 release. This will allow for a typical
packaging pattern of pulling upstream source archives from a tag/release
page on GitHub. I don't know if the test failure on trixie needs to be a
blocker at the moment since bookworm was just released; as a data point,
the latest Ubuntu distro (lunar/23.04) is using libproj 9.1.1 (same as
bookworm). I just checked and it looks like the next Ubuntu release
(mantic/23.10) will include libproj 9.2.1.

[1]: https://salsa.debian.org/postgresql/mobilitydb/-/commit/1402c0a837f733d43fe4abdd03b815b41bf80ce0

Regards,

--Bradford

Here is the regression diff for test failure on bookworm arm64

2 tests failed ...
--- /tmp/autopkgtest.KVtGQg/autopkgtest_tmp/055_geography_funcs.out
2023-06-30 04:40:00.150375986 +0000
+++ /tmp/autopkgtest.KVtGQg/build.nb7/src/mobilitydb/test/point/expected/055_geography_funcs.test.out
2023-06-14 12:48:54.000000000 +0000
@@ -35,9 +35,9 @@
(1 row)

SELECT ST_LineLocatePoint(geography 'Linestring(1 1,2 2)', 'Point(1 1)', false);
- st_linelocatepoint
------------------------
- 2.398573818138993e-17
+ st_linelocatepoint
+--------------------
+ 0
(1 row)

SELECT ST_LineInterpolatePoint(geography 'Linestring empty', 0.1);
--- /tmp/autopkgtest.KVtGQg/autopkgtest_tmp/082_npointset_tbl.out
2023-06-30 04:41:48.855089230 +0000
+++ /tmp/autopkgtest.KVtGQg/build.nb7/src/mobilitydb/test/npoint/expected/082_npointset_tbl.test.out
2023-06-14 12:48:54.000000000 +0000
@@ -28,9 +28,9 @@
(1 row)

SELECT npointset '{"Npoint(1,0.5)", "Npoint(2,0.5)", "Npoint(3,0.5)"}'::stbox;
- stbox
--------------------------------------------------------------------------------------------------
- SRID=5676;STBOX
X((24.98171384036678,25.103669244214274),(71.25850933871267,80.96965130918636))
+ stbox
+------------------------------------------------------------------------------------------------
+ SRID=5676;STBOX
X((24.98171384036678,25.10366924421426),(71.25850933871268,80.96965130918636))
(1 row)

SELECT COUNT(*) FROM tbl_npoint WHERE np::npointset IS NOT NULL;

Here is the regression diff for the test failure on trixie amd64

1 tests failed ...
--- /tmp/autopkgtest.eBycLQ/autopkgtest_tmp/056_tpoint_spatialfuncs.out
2023-06-30 04:55:01.810972446 +0000
+++ /tmp/autopkgtest.eBycLQ/build.OTW/src/mobilitydb/test/point/expected/056_tpoint_spatialfuncs.test.out
2023-06-18 14:44:23.000000000 +0000
@@ -113,9 +113,17 @@
(1 row)

SELECT ROUND(MIN(xmin(transform(transform(setSRID(b,3812), 5676),
3812)))::numeric, 1) FROM tbl_stbox;
-ERROR: transform: File not found or invalid (1029)
+ round
+-------
+ 0.5
+(1 row)
+
SELECT MIN(xmin(round(transform(transform(setSRID(b, 3812), 5676),
3812), 1))) FROM tbl_stbox;
-ERROR: transform: File not found or invalid (1029)
+ min
+-----
+ 0.5
+(1 row)
+
SELECT SRID(tgeompoint 'Point(1 1)@2000-01-01');
srid
------

Hi,

I've finally found the time to look at MobilityDB.

Re: Bradford Boyle
> * Debian 13 (trixie/testing)
> - builds successfully
> - piuparts passes
> - autopkgtests fail for trixie (testing)
> * test failure looks to be an issue with the newer libproj (9.2.1)
> included in trixie

When I run the tests on unstable I get this:

$ debian/tests/installcheck
Creating new PostgreSQL cluster 15/regress ...
test 001_set                      ... *** /tmp/pg_virtualenv.ye3HhD/log/postgresql-15-regress.log (last 100 lines) ***
2023-07-31 14:44:57.593 UTC [321171] LOG:  starting PostgreSQL 15.3 (Debian 15.3-0+deb12u1) on x86_64-pc-linux-gnu,
compiledby gcc (Debian 12.2.0-14) 12.2.0, 64-bit
 
2023-07-31 14:44:57.593 UTC [321171] LOG:  listening on IPv4 address "127.0.0.1", port 5433
2023-07-31 14:44:57.594 UTC [321171] LOG:  could not bind IPv4 address "127.0.0.1": Address already in use
2023-07-31 14:44:57.594 UTC [321171] HINT:  Is another postmaster already running on port 5433? If not, wait a few
secondsand retry.
 
2023-07-31 14:44:57.594 UTC [321171] LOG:  listening on Unix socket "/tmp/.s.PGSQL.5433"
2023-07-31 14:44:57.595 UTC [321174] LOG:  database system was shut down at 2023-07-31 14:44:57 UTC
2023-07-31 14:44:57.597 UTC [321171] LOG:  database system is ready to accept connections
2023-07-31 14:45:00.903 UTC [321209] myon@postgres ERROR:  Could not parse the set: Missing open brace at character 29
2023-07-31 14:45:00.903 UTC [321209] myon@postgres STATEMENT:  /* Errors */
    SELECT tstzset '2000-01-01, 2000-01-02';
Dropping cluster 15/regress ...

(After setting : ${AUTOPKGTEST_TMP:=/tmp} )

debian/copyright: LICENSE mentions the ULB - haven't checked the rest
of the source yet

debian/tests/installcheck: Why is that .so trickery necessary? If
mobilitydb needs postgis that way, it should link against it.

debian/control.in: This would be the first package to use PGVERSION in
the Build-Depends list. I'm slightly surprised it works, but
apparently I coded the logic in pg_buildext correctly. ;)

Is the postgis version used at build time relevant? There is a
postgresql-postgis package that could be used instead. (If the version
is relevant, we need a solution that works when more than one version
is targeted.)

Christoph

Re: Esteban Zimanyi
> Many thanks Christoph for your work !
> Regarding one of the request by Bradford, please notice that we have just
> created the following tag for facilitating the packaging
> https://github.com/MobilityDB/MobilityDB/releases/tag/v1.1.0-alpha

Yes I was building off that tag.

Christoph

Hi,

Thank you for the initial round of feedback.

> When I run the tests on unstable I get this:
>
> $ debian/tests/installcheck
> Creating new PostgreSQL cluster 15/regress ...
> test 001_set ... *** /tmp/pg_virtualenv.ye3HhD/log/postgresql-15-regress.log (last 100 lines) ***
> ...
> 2023-07-31 14:45:00.903 UTC [321209] myon@postgres ERROR: Could not parse the set: Missing open brace at character
29
> 2023-07-31 14:45:00.903 UTC [321209] myon@postgres STATEMENT: /* Errors */
> SELECT tstzset '2000-01-01, 2000-01-02';
> Dropping cluster 15/regress ...

I've made some changes to how the autopkg tests are run and the tests
are running and passing when building with sbuild on arm64 (I don't have
an amd64 environment available at the momment). Now that there is a
pre-release tag on GitHub, I've enabled the CI/CD in s.d.o to point at
d/gitlab-ci.yml

> debian/copyright: LICENSE mentions the ULB - haven't checked the rest
> of the source yet

I still need to update d/copyright -- there are several different
copyrights in the source (PGDG, ULB, PostGIS, etc) that I need to add to
d/copyright. This is the area of packaging that I am less familiar with,
so any high-level suggestions would be appreciated.

> debian/tests/installcheck: Why is that .so trickery necessary? If
> mobilitydb needs postgis that way, it should link against it.

Mobilitydb needs postgis -- without setting shared_preload_libraries,
tests fail with the errors similar to this one

ERROR: could not load library
"/usr/lib/postgresql/15/lib/libMobilityDB-1.1.so":
/usr/lib/postgresql/15/lib/libMobilityDB-1.1.so: undefined symbol:
ST_Intersects

I've spent some time trying to patch the CMake build to link mobilitydb
against postgis with no success. If I update CMakeLists.txt to link with
-L /usr/lib/postgresql/15/lib -lpostgis-3, the build fails with

/usr/bin/ld: cannot find -lpostgis-3.so: No such file or directory

I think this is because postgis-3.so doesn't have a SONAME field set. If
the build links w/ /usr/lib/postgresql/15/lib/postgis-3.so, then
dpkg-shlibdebs fails with

cannot find library /usr/lib/postgresql/15/lib/postgis-3.so needed by
build/libMobilityDB-1.1.so (ELF format: 'elf64-x86-64' abi:
'0201003e00000000'; RPATH: '')

> debian/control.in: This would be the first package to use PGVERSION in
> the Build-Depends list. I'm slightly surprised it works, but
> apparently I coded the logic in pg_buildext correctly. ;)

> Is the postgis version used at build time relevant? There is a
> postgresql-postgis package that could be used instead. (If the version
> is relevant, we need a solution that works when more than one version
> is targeted.)

I've updated d/control to use the postgresql-postgis package.

--Bradford