vdirsyncer: deprecate vdirsyncerStable #103073

magnetophon · 2020-11-07T11:43:49Z

Motivation for this change

Things done

SuperSandro2000 · 2020-11-07T15:59:00Z

khal:

looking for now-outdated files... none found
pickling environment... done
checking consistency... /build/khal-0.10.2/doc/source/man.rst: WARNING: document isn't included in any toctree
done
writing... khal.1 { usage configure standards faq license } /build/khal-0.10.2/doc/source/usage.rst:53: WARNING: unknown option: --color
/build/khal-0.10.2/doc/source/usage.rst:53: WARNING: unknown option: --color
/build/khal-0.10.2/doc/source/usage.rst:395: WARNING: unknown option: --interactive
/build/khal-0.10.2/doc/source/usage.rst:395: WARNING: unknown option: -i
/build/khal-0.10.2/doc/source/configure.rst:17: WARNING: unknown option: -c
path/to/config
done
build succeeded, 7 warnings.

The manual pages are in build/man.

make: /bin/sh: No such file or directory
make: *** [Makefile:133: man] Error 127
make: Leaving directory '/build/khal-0.10.2/doc'

magnetophon · 2020-11-07T16:27:23Z

@SuperSandro2000 Thanks for the feedback, but I am not getting those errors.
When I run nix-env -f $NIXPKGS -iA khal, I do get build succeeded, 7 warnings., but I don't get those make errors.

Both vdirsyncer sync and khal list -o work as expected here.

How did you install khal?

pkgs/top-level/all-packages.nix

jonringer · 2020-11-07T18:58:21Z

@SuperSandro2000 is that on darwin?

loewenheim

Looks good!

SuperSandro2000 · 2020-11-08T00:06:40Z

@SuperSandro2000 is that on darwin?

yes but now it fails with

tests/khalendar_test.py .............................                    [ 58%]                                                                                                   [20/48634]tests/khalendar_utils_test.py .......................................    [ 71%]                                                                                                             tests/parse_datetime_test.py ........................................... [ 86%]                                                                                                             ....                                                                     [ 87%]                                                                                                             tests/settings_test.py ............                                      [ 91%]                                                                                                             tests/terminal_test.py ...                                               [ 92%]                                                                                                             tests/utils_test.py .......                                              [ 95%]                                                                                                             tests/vdir_test.py X..                                                   [ 96%]                                                                                                             tests/vtimezone_test.py ...                                              [ 97%]                                                                                                             tests/ui/test_calendarwidget.py ...                                      [ 98%]                                                                                                             tests/ui/test_editor.py s...                                             [ 99%]                                                                                                             tests/ui/test_widgets.py .                                               [100%]                                                                                                                                                                                                                                                                                                         =================================== FAILURES ===================================                                                                                                            ___________________ test_vertical_month_unicode_weekdeays_gr ___________________                                                                                                                                                                                                                                                                                                            def test_vertical_month_unicode_weekdeays_gr():                                                                                                                                                 try:                                                                                                                                                                                            locale.setlocale(locale.LC_ALL, 'el_GR.UTF-8')                                                                                                                                              vert_str = vertical_month(month=12, year=2011,                                                                                                                                                                        today=dt.date(2011, 12, 12))
            # on some OSes, Greek locale's abbreviated day of the week and
            # month names have accents, on some they haven't
>           assert strip_accents('\n'.join([line.lower() for line in vert_str])) == \
                '\n'.join(example_gr)
E               AssertionError: assert '\x1b[1m    δ...  1  2  3  4 ' == '\x1b[1m     ...  1  2  3  4 '
E                 -      δε τρ τε πε πα σα κυ
E                 ?         -
E                 +     δε τρ τε πε πα σα κυ
E                 - δεκ  28 29 30  1  2  3  4
E                 ?         -
E                 + δεκ 28 29 30  1  2  3  4
E                 -       5  6  7  8  9 10 11 ...
E
E                 ...Full output truncated (39 lines hidden), use '-vv' to show

tests/cal_display_test.py:368: AssertionError
_________________________ test_vertical_month_abbr_fr __________________________

    def test_vertical_month_abbr_fr():
        # see issue #653
        try:
            locale.setlocale(locale.LC_ALL, 'fr_FR.UTF-8')
            vert_str = vertical_month(month=12, year=2011,
                                      today=dt.date(2011, 12, 12))
>           assert '\n'.join(vert_str) == '\n'.join(example_fr)
E           AssertionError: assert '\x1b[1m    L...  1  2  3  4 ' == '\x1b[1m     ...  1  2  3  4 '
E             -       lu ma me je ve sa di
E             ?         ^^^  ^  ^  ^  ^  ^  ^
E             +     Lu Ma Me Je Ve Sa Di
E             ?         ^  ^  ^  ^  ^  ^  ^
E             - déc.  28 29 30  1  2  3  4
E             ?        --
E             + déc 28 29 30  1  2  3  4 ...
E
E             ...Full output truncated (40 lines hidden), use '-vv' to show

tests/cal_display_test.py:391: AssertionError
=========================== short test summary info ============================
FAILED tests/cal_display_test.py::test_vertical_month_unicode_weekdeays_gr - ...
FAILED tests/cal_display_test.py::test_vertical_month_abbr_fr - AssertionErro...
============= 2 failed, 285 passed, 2 skipped, 2 xpassed in 7.02s ==============

DamienCassou

Thank you for your work. When I read the changes made by the commit in this PR, I see:

stable.nix was deleted and vdirsyncerStable is deprecated;
default.nix is changed significantly.

I think the story your commits should tell is something like:

what was in default.nix is deleted
stable.nix becomes the new default

To tell this story, I think I would decompose the PR into 2 commits:

The first commit deletes default.nix and makes vdirsyncer points to stable.nix
The second commit renames stable.nix into default.nix

The deprecation of vdirsyncerStable can happen in any of these 2 commits or a third one. I would put it in a third commit because I like small and atomic commits but that's up to you.

magnetophon · 2020-11-08T10:38:12Z

@DamienCassou Like this?

pkgs/top-level/aliases.nix

DamienCassou · 2020-11-08T10:51:46Z

Like this?

Exactly, great job. This is so much easier to understand, thank you. I would only add a sentence to the first commit explaining why the file is deleted.

magnetophon · 2020-11-08T10:53:31Z

I would only add a sentence to the first commit explaining why the file is deleted.

How would you word it?

DamienCassou · 2020-11-08T13:15:35Z

I would only add a sentence to the first commit explaining why the file is deleted.
How would you word it?

vdirsyncer: delete default.nix, point vdirsyncer to stable.nix

TLDR: default.nix was pointing to an unmaintained code base whereas stable.nix is up-to-date and maintained.

History
1. At first their was one Python version of vdirsyncer that had been working fine for years. Then, maintenance decreased and the package was marked as broken in nixpkgs.
2. The original author (@untitaker on github.com) of vdirsyncer decided to re-implement (part of) vdirsyncer in Rust. Nixpkgs made `vdirsyncer` point to the Rust version and renamed the Python historical version to `vdirsyncerStable`.
3. Eventually, @untitaker gave up on the Rust version.
4. Someone else (@WhyNotHugo on github.com) decided to take over maintenance of the Python version.
5.  Mario Rodas (@marsam on github) and Damien Cassou updated the `vdirsyncerStable` to point to the work of @WhyNotHugo and mark the package as working again.

@untitaker

TLDR: default.nix was pointing to an unmaintained code base whereas stable.nix is up-to-date and maintained. History 1. At first their was one Python version of vdirsyncer that had been working fine for years. Then, maintenance decreased and the package was marked as broken in nixpkgs. 2. The original author (@untitaker on github.com) of vdirsyncer decided to re-implement (part of) vdirsyncer in Rust. Nixpkgs made `vdirsyncer` point to the Rust version and renamed the Python historical version to `vdirsyncerStable`. 3. Eventually, @untitaker gave up on the Rust version. 4. Someone else (@WhyNotHugo on github.com) decided to take over maintenance of the Python version. 5. Mario Rodas (@marsam on github) and Damien Cassou updated the `vdirsyncerStable` to point to the work of @WhyNotHugo and mark the package as working again.

magnetophon · 2020-11-08T13:22:29Z

@DamienCassou Thanks for the text!

pkgs/top-level/aliases.nix

SuperSandro2000 · 2020-11-09T09:16:02Z

Result of nixpkgs-review pr 103073 run on x86_64-darwin 1

1 package failed to build:

khal

2 packages built:

python37Packages.vdirsyncer
vdirsyncer (python38Packages.vdirsyncer)

khal

>           assert strip_accents('\n'.join([line.lower() for line in vert_str])) == \
                '\n'.join(example_gr)
E               AssertionError: assert '\x1b[1m    δ...  1  2  3  4 ' == '\x1b[1m     ...  1  2  3  4 '
E                 -      δε τρ τε πε πα σα κυ
E                 ?         -
E                 +     δε τρ τε πε πα σα κυ
E                 - δεκ  28 29 30  1  2  3  4
E                 ?         -
E                 + δεκ 28 29 30  1  2  3  4
E                 -       5  6  7  8  9 10 11 ...
E
E                 ...Full output truncated (39 lines hidden), use '-vv' to show

tests/cal_display_test.py:368: AssertionError
_________________________ test_vertical_month_abbr_fr __________________________

    def test_vertical_month_abbr_fr():
        # see issue #653
        try:
            locale.setlocale(locale.LC_ALL, 'fr_FR.UTF-8')
            vert_str = vertical_month(month=12, year=2011,
                                      today=dt.date(2011, 12, 12))
>           assert '\n'.join(vert_str) == '\n'.join(example_fr)
E           AssertionError: assert '\x1b[1m    L...  1  2  3  4 ' == '\x1b[1m     ...  1  2  3  4 '
E             -       lu ma me je ve sa di
E             ?         ^^^  ^  ^  ^  ^  ^  ^
E             +     Lu Ma Me Je Ve Sa Di
E             ?         ^  ^  ^  ^  ^  ^  ^
E             - déc.  28 29 30  1  2  3  4
E             ?        --
E             + déc 28 29 30  1  2  3  4 ...
E
E             ...Full output truncated (40 lines hidden), use '-vv' to show

tests/cal_display_test.py:391: AssertionError
=========================== short test summary info ============================
FAILED tests/cal_display_test.py::test_vertical_month_unicode_weekdeays_gr - ...
FAILED tests/cal_display_test.py::test_vertical_month_abbr_fr - AssertionErro...
============= 2 failed, 285 passed, 2 skipped, 2 xpassed in 6.39s ==============

peterhoeg · 2020-11-16T12:23:45Z

All good - tested here!

wirew0rm · 2020-11-28T13:56:29Z

This seems to have broken the hydra build (and it won't build for me locally either)?

First evaluation after the merge broke, subsequent evaluations use the cached failure:
https://hydra.nixos.org/build/130683519#tabs-summary

Looking at the log, the error seems to be the same as the one I'm seeing locally and which was also discussed previously in this nixpkgs PR and this upstream Issue.

If I locally disable the flaky test, it works for me locally:

--- a/pkgs/development/python-modules/vdirsyncer/default.nix
+++ b/pkgs/development/python-modules/vdirsyncer/default.nix
@@ -53,7 +53,10 @@ buildPythonPackage rec {
-  disabledTests = [ "test_verbosity" ];
+  disabledTests = [
+    "test_verbosity" # was always disabled
+    "test_create_collections" # flaky test sometimes exceeds its deadline
+  ];

jonringer · 2020-11-28T16:41:25Z

@wirew0rm if would like to make a PR, I'll be happy to review it

magnetophon requested review from FRidh and jonringer as code owners November 7, 2020 11:43

magnetophon mentioned this pull request Nov 7, 2020

vdirsyncer: why is there a default plus a stable? #103026

Closed

ofborg bot added 6.topic: python 8.has: clean-up labels Nov 7, 2020

ofborg bot requested a review from loewenheim November 7, 2020 11:54

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Nov 7, 2020

jonringer reviewed Nov 7, 2020

View reviewed changes

pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved

loewenheim approved these changes Nov 7, 2020

View reviewed changes

magnetophon force-pushed the vdirsyncer branch from e2feb75 to 425f1f9 Compare November 7, 2020 19:09

ofborg bot requested a review from loewenheim November 7, 2020 19:21

loewenheim approved these changes Nov 7, 2020

View reviewed changes

DamienCassou requested changes Nov 8, 2020

View reviewed changes

magnetophon force-pushed the vdirsyncer branch from 425f1f9 to 160e6d9 Compare November 8, 2020 10:37

gebner reviewed Nov 8, 2020

View reviewed changes

pkgs/top-level/aliases.nix Outdated Show resolved Hide resolved

magnetophon force-pushed the vdirsyncer branch from 160e6d9 to 29740fa Compare November 8, 2020 10:49

ofborg bot requested a review from loewenheim November 8, 2020 11:01

magnetophon added 2 commits November 8, 2020 14:19

vdirsyncer: rename stable.nix to default.nix

9f3465e

magnetophon force-pushed the vdirsyncer branch from 29740fa to 8668e88 Compare November 8, 2020 13:21

loewenheim approved these changes Nov 8, 2020

View reviewed changes

ofborg bot requested a review from loewenheim November 8, 2020 13:32

jonringer reviewed Nov 8, 2020

View reviewed changes

pkgs/top-level/aliases.nix Show resolved Hide resolved

magnetophon force-pushed the vdirsyncer branch 2 times, most recently from 1bf3f89 to 6069aaa Compare November 9, 2020 08:35

vdirsyncerStable: deprecate

879bd1a

magnetophon force-pushed the vdirsyncer branch from 6069aaa to 879bd1a Compare November 10, 2020 00:28

loewenheim approved these changes Nov 10, 2020

View reviewed changes

peterhoeg merged commit 89fae7b into NixOS:master Nov 16, 2020

magnetophon deleted the vdirsyncer branch November 16, 2020 13:49

wirew0rm mentioned this pull request Nov 28, 2020

vdirsyncer: disable flaky test #105277

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

vdirsyncer: deprecate vdirsyncerStable #103073

vdirsyncer: deprecate vdirsyncerStable #103073

magnetophon commented Nov 7, 2020 •

edited

SuperSandro2000 commented Nov 7, 2020 •

edited

magnetophon commented Nov 7, 2020

jonringer commented Nov 7, 2020

loewenheim left a comment

SuperSandro2000 commented Nov 8, 2020

DamienCassou left a comment •

edited

magnetophon commented Nov 8, 2020

DamienCassou commented Nov 8, 2020

magnetophon commented Nov 8, 2020

DamienCassou commented Nov 8, 2020

magnetophon commented Nov 8, 2020

SuperSandro2000 commented Nov 9, 2020 •

edited

peterhoeg commented Nov 16, 2020

wirew0rm commented Nov 28, 2020

jonringer commented Nov 28, 2020

vdirsyncer: deprecate vdirsyncerStable #103073

vdirsyncer: deprecate vdirsyncerStable #103073

Conversation

magnetophon commented Nov 7, 2020 • edited

Motivation for this change

Things done

SuperSandro2000 commented Nov 7, 2020 • edited

magnetophon commented Nov 7, 2020

jonringer commented Nov 7, 2020

loewenheim left a comment

Choose a reason for hiding this comment

SuperSandro2000 commented Nov 8, 2020

DamienCassou left a comment • edited

Choose a reason for hiding this comment

magnetophon commented Nov 8, 2020

DamienCassou commented Nov 8, 2020

magnetophon commented Nov 8, 2020

DamienCassou commented Nov 8, 2020

magnetophon commented Nov 8, 2020

SuperSandro2000 commented Nov 9, 2020 • edited

peterhoeg commented Nov 16, 2020

wirew0rm commented Nov 28, 2020

jonringer commented Nov 28, 2020

magnetophon commented Nov 7, 2020 •

edited

SuperSandro2000 commented Nov 7, 2020 •

edited

DamienCassou left a comment •

edited

SuperSandro2000 commented Nov 9, 2020 •

edited