Summary: there are too many apparently SRU-inappropriate changes bundled into this upload; please remove them and supply the minimal necessary changes only.
Full review: This bug has had some unavoidable churn in -proposed due to external factors resulting in multiple releases of tzdata upstream in quick succession. This type of thing usually increases the risk of error in my experience, so I gave this a more thorough SRU review than usual from the perspective of the changes that would be experienced by users. Specifically this is the diff to this upload all the way from what is currently in the -updates pockets. The pulling in of the actual upstreams (tzdata and icu-data) look fine, assuming that it's OK for the icu-data version to mismatch tzdata as the newest icu-data wasn't available at the time of upload. I haven't checked the updates to *.pot and *.po but I assume that they are also fine. The addition and improvement of automated tests are always OK and very welcome. However, some remaining changes I'm not sure about. These weren't documented in the SRU paperwork, though we (bdrung and I) did discuss them extensively in #ubuntu-release - thank you for explaining them. However I'm still not sure if these changes are appropriate for an SRU. Generating debconf templates with Python instead of bash seems like a reasonable improvement, but I'd expect this to be done in the development release only. SRU policy says: > ...the requirements for stable updates are not necessarily the same as those in the development release. When preparing future releases, one of our goals is to construct the most elegant and maintainable system possible, and this often involves fundamental improvements to the system's architecture, rearranging packages to avoid bundled copies of other software so that we only have to maintain it in one place, and so on. However, once we have completed a release, the priority is normally to minimise risk caused by changes not explicitly required to fix qualifying bugs, and this tends to be well-correlated with minimising the size of those changes. As such, the same bug may need to be fixed in different ways in stable and development releases. I'd therefore not expect this change to be present in the SRU, but instead for the existing mechanism to be used. If the existing mechanism is broken or not practical in some way, then I think this needs a separate SRU bug with a full justification of the reasons so that regression risk can be considered, and a QA plan can be agreed. I appreciate that you added a dep8 test, but it isn't clear to me that the dep8 test covers *all* the previous behaviour. That discussion should happen in that bug. There are also multiple and extensive changes and refactorings to debian/tzdata.config. Some of these are unnecessary (reordering and normalising of shell syntax) and these mask the remaining changes, making the rest harder to review. Looking past those, there are some additional indirections, but also the removal of at least one indirection (upstream source at [1]) that we discussed. There's also the new use of convert_timezone. If these changes are necessary then they should be covered by separate bugs with their own individual SRU justifications. I've tried to cover what would be needed if you feel you *must* push these additional changes in an SRU. But please also consider that this consumes a great deal of SRU reviewers' time (which is currently in short supply), and the answer may still be no. I strongly suggest that you simply stop trying to push extensive changes in SRUs except where there's actually a significant and direct benefit to users to do so. I think it'll probably be fine to just update upstream, icu-data and update the templates with the existing bash generator only? This would then be much easier to review and land safely. Note that I've not actually found anything wrong with your changes here. But it's not enough for the upload to merely be correct. It's also necessary to demonstrate to others why and how it is correct, as well as why and how choices have been made such that the risk of an inadvertent oversight has been minimised. This aspect is currently mostly missing, resulting in a significant review burden on others. This apsect should be taken into consideration when deciding whether to seek an SRU backport of something or not. [1] https://git.launchpad.net/~ubuntu-core- dev/ubuntu/+source/tzdata/commit/?id=4c2b898ef99c134a9d202405643136d4b0d38048 ** Changed in: tzdata (Ubuntu Bionic) Status: Fix Committed => Triaged ** Changed in: tzdata (Ubuntu Focal) Status: Fix Committed => Triaged ** Changed in: tzdata (Ubuntu Jammy) Status: Fix Committed => Triaged ** Changed in: tzdata (Ubuntu Kinetic) Status: Fix Committed => Triaged -- You received this bug notification because you are a member of Ubuntu Touch seeded packages, which is subscribed to tzdata in Ubuntu. https://bugs.launchpad.net/bugs/2012599 Title: tzdata 2023a/2023b/2023c release - Egypt restoring DST Status in tzdata package in Ubuntu: Fix Committed Status in tzdata source package in Bionic: Triaged Status in tzdata source package in Focal: Triaged Status in tzdata source package in Jammy: Triaged Status in tzdata source package in Kinetic: Triaged Status in tzdata source package in Lunar: Fix Committed Bug description: [ Impact ] Recently the Egyptian authorities decided to restore DST. The first switch after the change is expected to take place on last Friday of April. The upstream tzdata 2023a release already reflects this change. The 2023a release contains the following changes: * Egypt now uses DST again, from April through October. * This year Morocco springs forward April 23, not April 30. * Palestine delays the start of DST this year. * Much of Greenland still uses DST from 2024 on. The 2023c release reverts the changes done in 2023b. [ Test Plan ] Test cases were added to the autopkgtest to cover the testing: * python: test_2023a * python: test_2023c * python: test_systemv_timezones (for releases <= 20.04 LTS) * python-icu: test_2023a (only for kinetic, jammy, focal) So the test plan is to check that the autopkgtest succeeds. Alternatively the python test_2023a can be run manually: * Set system timezone to Egypt (e.g. Africa/Cairo). * Set system time to 2023-04-27 23:59. * Wait and observe what happens at 2023-04-28 0:00 [Test Case for releases <= 20.04 LTS] Additionally, an upstream update of tzdata removed the 'old' SystemV timezones, so we should ensure that they are kept in Ubuntu 20.04 LTS and earlier releases. This is done by the test_systemv_timezones test case or can be checked manually with the following: diff <(zdump -v America/Phoenix | cut -d' ' -f2-) <(zdump -v SystemV/MST7 | cut -d' ' -f2-) [ Where problems could occur ] * Systems with incorrect timezone set (e.g. located outside of Egypt but still using Egyptian time) may observe unexpected time shift. [ Other Info ] The autopkgtest for chrony is flaky on jammy and kinetic (see bug #2002910). * More information with sources: https://en.wikipedia.org/wiki/Daylight_saving_time_in_Egypt#:~:text=Daylight%20saving%20time%20(DST)%20has,(DST)%20as%20of%202023. The SRUs to the stable releases include the recent changes for generating the debconf template (switching from shell code to Python) and the timezone mappings in convert_timezone(). This is done to ease future tzdata updates since those parts of the packaging need to be updated when timezone are added, renamed, or removed. Having the same code in that part makes backporting the changes easier. I added a consistency check to generate_debconf_templates in 2023c-2 which I want to backport in a future SRU. Previous SRUs did sometimes forget to update convert_timezone or the exclusion list for the debconf template for the changes from upstream. All added tests (for testing the debconf template and convert_timezone) were also backported to catch missing those changes. The sorting change of the debconf template was included in the SRU to allow taking the debconf translations from later releases. debian/test_timezone_conversions finds following issues in the kinetic packaging 2022g-0ubuntu0.22.10.1: ``` ERROR: Following 14 timezones can be selected, but will be converted: Asia/Rangoon Europe/Uzhgorod Europe/Zaporozhye US/Alaska US/Aleutian US/Arizona US/Central US/Eastern US/Hawaii US/Indiana-Starke US/Michigan US/Mountain US/Pacific US/Samoa ERROR: Following 5 timezones cannot be selected, but are not converted: America/Fort_Wayne America/Indianapolis America/Knox_IN America/Louisville Pacific/Enderbury ERROR: Following 3 timezones are conversion targets, but are not available: Asia/Riyadh87 Asia/Riyadh88 Asia/Riyadh89 ERROR: Following 4 timezones are conversion targets, but are not selectable: America/Indianapolis Asia/Riyadh87 Asia/Riyadh88 Asia/Riyadh89 ``` To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/tzdata/+bug/2012599/+subscriptions -- Mailing list: https://launchpad.net/~touch-packages Post to : touch-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~touch-packages More help : https://help.launchpad.net/ListHelp
