Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Update debian scripts for debhelper-compat 12 #9792

Closed
wants to merge 5 commits into from

Conversation

callahad
Copy link
Contributor

@callahad callahad commented Apr 12, 2021

This is part removing support for Debian Stretch and Ubuntu Xenial
following the end of life for Python 3.5.

Other notable changes:

  • Architecture is set to any to allow (but not mandate) ARM builds

  • The dh-virtualenv Dockerfile was overhauled to more closely follow the
    example provided upstream, including a necessary tweak to ensure
    builds work on Ubuntu bionic.

Signed-off-by: Dan Callahan danc@element.io

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
  • Pull request includes a sign off
  • Code style is correct (run the linters)

@callahad callahad requested a review from a team April 12, 2021 10:41
This is part removing support for Debian Stretch and Ubuntu Xenial
following the end of life for Python 3.5.

Other notable changes:

- Architecture is set to `any` to allow (but not mandate) ARM builds

- The dh-virtualenv Dockerfile was overhauled to more closely follow the
  example provided upstream, including a necessary tweak to ensure
  builds work on Ubuntu bionic.

Signed-off-by: Dan Callahan <danc@element.io>
@callahad
Copy link
Contributor Author

Looking at the artifacts produced for Buster (and excluding expected changes like timestamps and checksums, eggs vs wheels), here's what's different:

  • control/prerm, control/postrm, and control/postinst only call deb-systemd-invoke or systemctl, they do not also call invoke-rc.d or update-rc.d

  • control/conffiles no longer lists /etc/default/matrix-synapse and it is not present in the data files. Its content was:

    # Specify environment variables used when running Synapse
    # SYNAPSE_CACHE_FACTOR=0.5 (default)

Otherwise, everything else is effectively identical. All other modifications are as seen in this pull request.

Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll admit that I'm not very confident reviewing deb build scripts, but nothing added here looks inherently wrong.

Attempting to build the image and then a .deb with that image using distro=bionic worked flawlessly on my machine.

$ docker build --build-arg distro=ubuntu:bionic -t callahad/synapse-deb -f docker/Dockerfile-dhvirtualenv .
$ docker run -v /path/to/synapse/checkout:/synapse/source -v /output/deb/dir:/debs callahad/synapse-deb

docker/Dockerfile-dhvirtualenv Show resolved Hide resolved
docker/Dockerfile-dhvirtualenv Outdated Show resolved Hide resolved
debian/rules Outdated Show resolved Hide resolved
@anoadragon453 anoadragon453 requested a review from a team April 12, 2021 13:23
callahad and others added 2 commits April 12, 2021 15:06
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Signed-off-by: Dan Callahan <danc@element.io>
@callahad
Copy link
Contributor Author

callahad commented Apr 12, 2021

SyTest flaked, re-running. Otherwise everything is green.

 not ok 463 User can invite remote user to room with version 5
# Started: 2021-04-12 14:34:34.416
# Ended: 2021-04-12 14:34:44.563
# Timed out waiting for test at /sytest/run-tests.pl line 771.
# 0.113365: Registered new user @anon-20210412_142620-550:localhost:8875
# 0.143863: Registered new user @anon-20210412_142620-549:localhost:8837
# 1.070622: Invited user @anon-20210412_142620-550:localhost:8875 to !aGuqpsxlGstWktTddm:localhost:8837
# {}

Signed-off-by: Dan Callahan <danc@element.io>
@callahad
Copy link
Contributor Author

control/conffiles no longer lists /etc/default/matrix-synapse and it is not present in the data files.

Looks like this is to be expected with the move to just using dh_installsystemd. I've modified the service files to ensure that they don't fail if the file is absent, but do use it if present. Which should make this painless for new and existing installs alike.

This file goes away with dh_installsystemd and the removal of sysv init.

Our systemd service files will still read and use it if present.

Signed-off-by: Dan Callahan <danc@element.io>
@callahad
Copy link
Contributor Author

Another flakey sytest, on the (buster, multi-postgres, workers) job

 not ok 361 A prev_batch token from incremental sync can be used in the v1 messages API
# Started: 2021-04-12 20:35:24.566
# Ended: 2021-04-12 20:35:25.432
# Expected only one timeline event at tests/31sync/04timeline.pl line 409.
# 0.093688: Registered new user @anon-20210412_203031-355:localhost:8837
# 0.585465: Sent 'm.room.message' event in !rpgWuoQAgpoHSdirvC:localhost:8837: $6nFdKD6RhcRrpdvDo_bNI-vc3UOEa16Vcma0357-50Q
# 0.811684: Sent 'm.room.message' event in !rpgWuoQAgpoHSdirvC:localhost:8837: $n_k0FUKxwNOA17JoTIS8GJNaxF11CwPem4K10UMY9H0

@callahad
Copy link
Contributor Author

callahad commented Apr 13, 2021

This time the flakey SyTest was on buster, postgres, workers, redis

Details
not ok 411 Old leaves are present in gapped incremental syncs
# Started: 2021-04-12 22:01:27.515
# Ended: 2021-04-12 22:01:30.054
# Expected only 2 membership events at tests/10apidoc/09synced.pl line 396.
# 0.193599: Registered new user @anon-20210412_215424-436:localhost:8837
# 0.194552: Registered new user @anon-20210412_215424-438:localhost:8837
# 0.197405: Registered new user @anon-20210412_215424-437:localhost:8837
# 1.131526: User @anon-20210412_215424-437:localhost:8837 joined room
# { room_id => "!slNjFXEVPPvIjYdNGZ:localhost:8837" }
# 1.425742: User @anon-20210412_215424-438:localhost:8837 joined room
# { room_id => "!slNjFXEVPPvIjYdNGZ:localhost:8837" }
# 1.582264: Sent 'm.room.message' event in !slNjFXEVPPvIjYdNGZ:localhost:8837: $_nxmSX5Wf1KXxQWjoeA8sOusPgXLuWGoRQu59qB8pdY
# 1.707723: Sent 'm.room.message' event in !slNjFXEVPPvIjYdNGZ:localhost:8837: $IqLg65UIhtuviyXeWr_9bbw9rVNpr4xmRFpNZ4NP2Y0
# 1.816230: Sent 'm.room.message' event in !slNjFXEVPPvIjYdNGZ:localhost:8837: $0j3azy3SoIrw1s5ZsdAAaSMWKWG3bhS30nmctpfmGHM
# 1.894438: Sent 'm.room.message' event in !slNjFXEVPPvIjYdNGZ:localhost:8837: $Q-Cc_FeRnOHB5c-W20nieDoOaBPsbenqItRdU_NNUv4
# 1.971960: Sent 'm.room.message' event in !slNjFXEVPPvIjYdNGZ:localhost:8837: $jMCvUkDufR4AzsHKc7feS1gOteY-DKYycolivRNgaaA
# 2.047663: Sent 'm.room.message' event in !slNjFXEVPPvIjYdNGZ:localhost:8837: $g-BOm2Bxc3TXvO6rKP8cNfuC-FvmJbj_cYrZWC5OOZk
# 2.145319: Sent 'm.room.message' event in !slNjFXEVPPvIjYdNGZ:localhost:8837: $GPfV5UHithCgKY87Mrn3PPcNmPhj_lhZl5reSOTzbSE
# 2.198933: Sent 'm.room.message' event in !slNjFXEVPPvIjYdNGZ:localhost:8837: $eD4GzS7z8s8ORkd8-79QWZwWXJNb6Tl00JgkiSYEJ1o
# 2.271701: Sent 'm.room.message' event in !slNjFXEVPPvIjYdNGZ:localhost:8837: $iTuqHBQTMLNsmnXjaqYHdq8ZdNJpEMBBI3iCiflof-g
# 2.348262: Sent 'm.room.message' event in !slNjFXEVPPvIjYdNGZ:localhost:8837: $buhyvWzM4nI3FVETEvMv_qh4kRwgVYVKHNRO-BYElAI
# 2.415376: Sent 'm.room.message' event in !slNjFXEVPPvIjYdNGZ:localhost:8837: $-8j6SFXCkQnUAYH8j0i58d3Oxd_1eN7UDbMxYtOLd04
# 2.536207: assert_state_room_members_match: expected members:
# [
#   "\@anon-20210412_215424-436:localhost:8837",
#   "\@anon-20210412_215424-437:localhost:8837",
# ]
# 2.536374: assert_state_room_members_match: actual state:
# [
#   {
#     content          => { join_rule => "public" },
#     event_id         => "\$jfulufyojdqrXRHh-KDn2E4G-ijXcq3pal1dJUGKp-Q",
#     origin_server_ts => JSON::number(1618264888049),
#     sender           => "\@anon-20210412_215424-436:localhost:8837",
#     state_key        => "",
#     type             => "m.room.join_rules",
#     unsigned         => { age => JSON::number(1992) },
#   },
#   {
#     content          => {
#                           creator => "\@anon-20210412_215424-436:localhost:8837",
#                           room_version => 6,
#                         },
#     event_id         => "\$Bq9z_YwWxxZ1yEPgwLBzoDD7Lf_tXd7an71OuzgeMnw",
#     origin_server_ts => JSON::number(1618264887766),
#     sender           => "\@anon-20210412_215424-436:localhost:8837",
#     state_key        => "",
#     type             => "m.room.create",
#     unsigned         => { age => JSON::number(2275) },
#   },
#   {
#     content          => { name => "A room name" },
#     event_id         => "\$MV687TG7hNPp9DvUd35XqaRdvvUVbF-fyqkyupW6hT4",
#     origin_server_ts => JSON::number(1618264888435),
#     sender           => "\@anon-20210412_215424-436:localhost:8837",
#     state_key        => "",
#     type             => "m.room.name",
#     unsigned         => { age => JSON::number(1606) },
#   },
#   {
#     content          => { displayname => "anon-20210412_215424-436", membership => "join" },
#     event_id         => "\$T5x_3jppujKLFpm0-u4uZwbiH3VBUavsYg8S03pgNlM",
#     origin_server_ts => JSON::number(1618264887852),
#     sender           => "\@anon-20210412_215424-436:localhost:8837",
#     state_key        => "\@anon-20210412_215424-436:localhost:8837",
#     type             => "m.room.member",
#     unsigned         => { age => JSON::number(2189) },
#   },
#   {
#     content          => { displayname => "anon-20210412_215424-437", membership => "join" },
#     event_id         => "\$c78lgnGsL_DTTWE_BveuZf2S93TBT2_iDt2IDcv0kEY",
#     origin_server_ts => JSON::number(1618264888595),
#     sender           => "\@anon-20210412_215424-437:localhost:8837",
#     state_key        => "\@anon-20210412_215424-437:localhost:8837",
#     type             => "m.room.member",
#     unsigned         => { age => JSON::number(1446) },
#   },
#   {
#     content          => {
#                           ban => JSON::number(50),
#                           events => {
#                             "m.room.avatar"             => JSON::number(50),
#                             "m.room.canonical_alias"    => JSON::number(50),
#                             "m.room.encryption"         => JSON::number(100),
#                             "m.room.history_visibility" => JSON::number(100),
#                             "m.room.name"               => JSON::number(50),
#                             "m.room.power_levels"       => JSON::number(100),
#                             "m.room.server_acl"         => JSON::number(100),
#                             "m.room.tombstone"          => JSON::number(100),
#                           },
#                           events_default => JSON::number(0),
#                           invite => JSON::number(50),
#                           kick => JSON::number(50),
#                           redact => JSON::number(50),
#                           state_default => JSON::number(50),
#                           users => {
#                             "\@anon-20210412_215424-436:localhost:8837" => JSON::number(100),
#                           },
#                           users_default => JSON::number(0),
#                         },
#     event_id         => "\$2fDkgyJFTOTWQkhD0-2h36ltD3XE_DchGSdAm6_YHLo",
#     origin_server_ts => JSON::number(1618264887925),
#     sender           => "\@anon-20210412_215424-436:localhost:8837",
#     state_key        => "",
#     type             => "m.room.power_levels",
#     unsigned         => { age => JSON::number(2116) },
#   },
#   {
#     content          => { displayname => "anon-20210412_215424-438", membership => "join" },
#     event_id         => "\$ZATUuFA2VpUTNJHlTXqWf533EAqiLDdTfhEAd8LJQ50",
#     origin_server_ts => JSON::number(1618264888880),
#     sender           => "\@anon-20210412_215424-438:localhost:8837",
#     state_key        => "\@anon-20210412_215424-438:localhost:8837",
#     type             => "m.room.member",
#     unsigned         => { age => JSON::number(1161) },
#   },
#   {
#     content          => { history_visibility => "shared" },
#     event_id         => "\$XlnVNnFd6nB_AX4aGXcxBir0zGff2W7sFXcfFW6eIhs",
#     origin_server_ts => JSON::number(1618264888156),
#     sender           => "\@anon-20210412_215424-436:localhost:8837",
#     state_key        => "",
#     type             => "m.room.history_visibility",
#     unsigned         => { age => JSON::number(1885) },
#   },
# ]

debian/matrix-synapse-py3.preinst Show resolved Hide resolved
Comment on lines +6 to +7
interest-noawait /usr/bin/python3.8
interest-noawait /usr/bin/python3.9
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hum; the fact we forgot to do this sooner isn't good. I wonder if we should add a comment somewhere?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(or build this dynamically?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say we could generate this based on our Trove classifiers, but... 😉

@@ -5,7 +5,7 @@ Description=Synapse Matrix homeserver
Type=notify
User=matrix-synapse
WorkingDirectory=/var/lib/matrix-synapse
EnvironmentFile=/etc/default/matrix-synapse
EnvironmentFile=-/etc/default/matrix-synapse
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the logic behind removing /etc/default/matrix-synapse?

it feels odd to remove the file but leave this in place.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still suggest people use it in our docs, so we might as well still honor it if present. Plus, existing installations may depend on its behavior.

Systemd upstream considers EnvironmentFile as hack that would ideally go away, but there's no clear replacement other than directly editing the Synapse config or overriding the systemd unit file locally.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mostly I feel like if we're going to leave the EnvironmentFile directive in place, we should probably also leave an example file there too. In my experience people get scared when they discover a config file doesn't exist at all.

I don't feel too strongly about it though.

%:
dh $@ --with python-virtualenv --with systemd
dh $@ --with python-virtualenv --buildsystem=pybuild
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note to my future self: previously we used the python_distutils build system; switching to pybuild matches the recommendation at https://wiki.debian.org/Python/LibraryStyleGuide. I don't think there was a good reason why we used python_distutils, other than that it was the default.

docker/Dockerfile-dhvirtualenv Show resolved Hide resolved
docker/Dockerfile-dhvirtualenv Show resolved Hide resolved
docker/Dockerfile-dhvirtualenv Show resolved Hide resolved
@richvdh
Copy link
Member

richvdh commented Apr 13, 2021

Looking at the artifacts produced for Buster (and excluding expected changes like timestamps and checksums, eggs vs wheels), here's what's different:

  • control/prerm, control/postrm, and control/postinst only call deb-systemd-invoke or systemctl, they do not also call invoke-rc.d or update-rc.d

Also: preinst is removed. per my review: I'm not sure that is safe.

@callahad
Copy link
Contributor Author

Also: preinst is removed. per my review: I'm not sure that is safe.

Yep, but that's part of the pull request itself -- that comment was trying to call attention to the additional changes in the generated artifacts above and beyond what's evident in the PR :)

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we might as well merge this rather than get hung up on the fine details. OTOH the changelog is now out of date.

lgtm once the changelog is updated.

@@ -1,6 +1,7 @@
matrix-synapse-py3 (1.31.0+nmu1) UNRELEASED; urgency=medium

* Skip tests when DEB_BUILD_OPTIONS contains "nocheck".
* Update Debian build scripts for debhelper-compat level 12.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs moving to the right version, since the world has moved on in the last couple of months

@callahad
Copy link
Contributor Author

callahad commented Jun 8, 2021

(Holding this back until the 1.36 RC is branched; I'd like to couple this PR with one to actually build debs in CI)

@erikjohnston
Copy link
Member

1.36 has been both branched and released

@erikjohnston
Copy link
Member

@callahad is this still on your radar?

@richvdh
Copy link
Member

richvdh commented Jul 26, 2021

I think much of this got re-done for #10429.

@richvdh
Copy link
Member

richvdh commented Aug 5, 2021

I'm going to close this, because I think most of it is redundant now.

@richvdh richvdh closed this Aug 5, 2021
@callahad callahad mentioned this pull request Nov 7, 2021
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants