Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

python3Packages.watchdog: fix darwin-x86_64 build #171388

Merged
merged 4 commits into from
Jun 19, 2022

Conversation

cpcloud
Copy link
Contributor

@cpcloud cpcloud commented May 3, 2022

Description of changes

This change allows the watchdog package to build on x86_64-darwin.

It turns out that the use of the fsevents API isn't a hard requirement and
watchdog will fallback to a kqueue-based implementation of the equivalent
functionality on platforms that don't support fsevents or always, with the
included patch.

I don't have access to a macOS machine to test the functionality, so any help
there would be much appreciated.

Fixes: #113777
Closes: #156597

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@cpcloud cpcloud requested review from FRidh and jonringer as code owners May 3, 2022 11:30
@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label May 3, 2022
@ofborg ofborg bot requested a review from cillianderoiste May 3, 2022 11:45
@ofborg ofborg bot added 10.rebuild-darwin: 11-100 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels May 3, 2022
@cpcloud
Copy link
Contributor Author

cpcloud commented May 3, 2022

@ofborg build python3Packages.watchdog

@cpcloud cpcloud force-pushed the watchdog-darwin-x86_64 branch from 5e4f5fa to c9851a8 Compare May 3, 2022 15:18
@cpcloud
Copy link
Contributor Author

cpcloud commented May 3, 2022

@ofborg build python3Packages.watchdog

1 similar comment
@cpcloud
Copy link
Contributor Author

cpcloud commented May 3, 2022

@ofborg build python3Packages.watchdog

@cpcloud cpcloud force-pushed the watchdog-darwin-x86_64 branch from 9032a5a to 2e37657 Compare May 4, 2022 09:42
@cpcloud
Copy link
Contributor Author

cpcloud commented May 4, 2022

@ofborg build python3Packages.watchdog

@jonringer
Copy link
Contributor

is there an upstream PR? not much of this looks to be related to nix specifically

@Artturin Artturin added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label May 9, 2022
@Mindavi
Copy link
Contributor

Mindavi commented May 15, 2022

ZHF: #172160.

Also, as Jon said, please do consider making this an upstream PR and just fetchpatch'ing it. Then hopefully there's no need to update this patch every time the package is updated. Upstream looks quite active so I bet they're willing to take a patch and maybe test it on a Darwin machine.

@risicle
Copy link
Contributor

risicle commented May 16, 2022

On macos 10.15:

_______________________ test_separate_consecutive_moves ________________________

    @pytest.mark.flaky(max_runs=5, min_passes=1, rerun_filter=rerun_filter)
    def test_separate_consecutive_moves():
        mkdir(p('dir1'))
        mkfile(p('dir1', 'a'))
        mkfile(p('b'))
        start_watching(p('dir1'))
        mv(p('dir1', 'a'), p('c'))
        mv(p('b'), p('dir1', 'd'))
    
        dir_modif = DirModifiedEvent(p('dir1'))
        a_deleted = FileDeletedEvent(p('dir1', 'a'))
        d_created = FileCreatedEvent(p('dir1', 'd'))
    
        expected_events = [a_deleted, dir_modif, d_created, dir_modif]
    
        if platform.is_windows():
            expected_events = [a_deleted, d_created]
    
        if platform.is_bsd() or platform.is_darwin():
            # Due to the way kqueue works, we can't really order
            # 'Created' and 'Deleted' events in time, so creation queues first
            expected_events = [d_created, a_deleted, dir_modif, dir_modif]
    
        for expected_event in expected_events:
>           expect_event(expected_event)

a_deleted  = <FileDeletedEvent: event_type=deleted, src_path='/private/tmp/nix-build-python3.9-watchdog-2.1.7.drv-0/tmpkj5_dju1/dir1/a', is_directory=False>
d_created  = <FileCreatedEvent: event_type=created, src_path='/private/tmp/nix-build-python3.9-watchdog-2.1.7.drv-0/tmpkj5_dju1/dir1/d', is_directory=False>
dir_modif  = <DirModifiedEvent: event_type=modified, src_path='/private/tmp/nix-build-python3.9-watchdog-2.1.7.drv-0/tmpkj5_dju1/dir1', is_directory=True>
expected_event = <FileCreatedEvent: event_type=created, src_path='/private/tmp/nix-build-python3.9-watchdog-2.1.7.drv-0/tmpkj5_dju1/dir1/d', is_directory=False>
expected_events = [<FileCreatedEvent: event_type=created, src_path='/private/tmp/nix-build-python3.9-watchdog-2.1.7.drv-0/tmpkj5_dju1/di...t_type=modified, src_path='/private/tmp/nix-build-python3.9-watchdog-2.1.7.drv-0/tmpkj5_dju1/dir1', is_directory=True>]

tests/test_emitter.py:343: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

expected_event = <FileCreatedEvent: event_type=created, src_path='/private/tmp/nix-build-python3.9-watchdog-2.1.7.drv-0/tmpkj5_dju1/dir1/d', is_directory=False>
timeout = 2

    def expect_event(expected_event, timeout=2):
        """ Utility function to wait up to `timeout` seconds for an `event_type` for `path` to show up in the queue.
    
        Provides some robustness for the otherwise flaky nature of asynchronous notifications.
        """
        try:
            event = event_queue.get(timeout=timeout)[0]
>           assert event == expected_event
E           AssertionError: assert <FileDeletedEvent: event_type=deleted, src_path='/private/tmp/nix-build-python3.9-watchdog-2.1.7.drv-0/tmpkj5_dju1/dir1/a', is_directory=False> == <FileCreatedEvent: event_type=created, src_path='/private/tmp/nix-build-python3.9-watchdog-2.1.7.drv-0/tmpkj5_dju1/dir1/d', is_directory=False>

event      = <FileDeletedEvent: event_type=deleted, src_path='/private/tmp/nix-build-python3.9-watchdog-2.1.7.drv-0/tmpkj5_dju1/dir1/a', is_directory=False>
expected_event = <FileCreatedEvent: event_type=created, src_path='/private/tmp/nix-build-python3.9-watchdog-2.1.7.drv-0/tmpkj5_dju1/dir1/d', is_directory=False>
timeout    = 2

tests/test_emitter.py:98: AssertionError

@siraben
Copy link
Member

siraben commented May 18, 2022

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

6 packages marked as broken and skipped:
  • hydrus
  • maestral
  • python310Packages.Nikola
  • python310Packages.maestral
  • python39Packages.Nikola
  • python39Packages.maestral
38 packages failed to build:
  • ablog
  • aws-sam-cli
  • chia
  • coconut (python39Packages.coconut)
  • hovercraft
  • markdown-pp
  • mkdocs (python39Packages.mkdocs)
  • netbox
  • octoprint
  • paperless-ngx
  • pympress
  • python310Packages.bpython
  • python310Packages.chalice
  • python310Packages.coconut
  • python310Packages.easywatch
  • python310Packages.lektor
  • python310Packages.mkdocs
  • python310Packages.mkdocs-material
  • python310Packages.mkdocs-minify
  • python310Packages.mkdocs-redirects
  • python310Packages.ndjson
  • python310Packages.pytest-watch
  • python310Packages.staticjinja
  • python310Packages.watchdog
  • python39Packages.bpython
  • python39Packages.chalice
  • python39Packages.easywatch
  • python39Packages.lektor
  • python39Packages.mkdocs-material
  • python39Packages.mkdocs-minify
  • python39Packages.mkdocs-redirects
  • python39Packages.ndjson
  • python39Packages.pytest-watch
  • staticjinja (python39Packages.staticjinja)
  • python39Packages.watchdog
  • skjold
  • streamlit
  • topydo

@SuperSandro2000
Copy link
Member

@ofborg build python3Packages.watchdog

Please edit your comments in the future and append a space to trigger ofborg again.

@@ -20,6 +20,10 @@ buildPythonPackage rec {
sha256 = "sha256-P9R4FTU76cRO68lMwo/iaysMW9iJ2vxKWny9+SQUNIA=";
};

patches = lib.optionals (stdenv.isDarwin && !stdenv.isAarch64) [
Copy link
Member

Choose a reason for hiding this comment

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

Can we make this unconditional?

@risicle
Copy link
Contributor

risicle commented May 28, 2022

Do we want to get this in for 22.05? It's possible my result is spurious and I don't think this breaks anything AFAIK.

@cpcloud
Copy link
Contributor Author

cpcloud commented May 28, 2022

I won't have time to work on this for a week or two, and it sounds like there's some additional legwork to do, which in principle I agree should be done.

@jonringer I believe this patch is related to nix. Nix doesn't support the late(r|st) macos sdk which this package requires to compile on macos machines with the later sdk. The package works fine on older sdks without the patch.

@jmgilman
Copy link
Contributor

May be unrelated, but I was unable to get the tests to pass on this package with darwin-aarch64 unless I explicitly disabled the fsevents test (among other things).

@vcunat vcunat merged commit 4f5a439 into NixOS:master Jun 19, 2022
@cpcloud cpcloud deleted the watchdog-darwin-x86_64 branch June 19, 2022 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: darwin Running or building packages on Darwin 6.topic: python 10.rebuild-darwin: 11-100 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pythonPackages.watchdog 2.0.0 is broken on darwin
10 participants