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

Update nsfw library to v2.2.4. #11975

Merged
merged 1 commit into from
Jan 5, 2023

Conversation

tsmaeder
Copy link
Contributor

@tsmaeder tsmaeder commented Dec 9, 2022

What it does

Updates nsfw dependency to v. 2.2.4 Fixes #11431
The update contains a fix I did Axosoft/nsfw#172 that should fix the issue and (hopefully) also get rid of the frequent PR check failures on ubuntu.

Contributed on behalf of ST Microelectronics

How to test

Manipulate file and folders in the workspace and make sure that they are properly reflected in Theia. I was able to reproduce the original issue by simple doing this:

  1. Make a copy of the Theia node_modules folder
  2. Open the folder as the workspace in Theia
  3. In the file explorer, select all the subfolders and press "delete". This moves the files to a trash folder.

Review checklist

Reminder for reviewers

Contributed on behalf of ST Microelectronics

Signed-off-by: Thomas Mäder <[email protected]>
@paul-marechal
Copy link
Member

I don't understand what fails on CI, but note that https://github.com/eclipse-theia/theia/actions/runs/3656095808/jobs/6178414596#step:9:4720 means we have a double free/dispose on one of the watchers for some reason.

@vince-fugnitto vince-fugnitto added dependencies pull requests that update a dependency file file-watchers issues related to filesystem watchers - nsfw labels Dec 14, 2022
@marcdumais-work
Copy link
Contributor

I see the exact same error on other PRs, so I do not think this is related to the nsfw update:
https://github.com/eclipse-theia/theia/actions/runs/3676473534/jobs/6217643338#step:9:4156

@tsmaeder
Copy link
Contributor Author

I don't have the time right now to look at the failures.

@marcdumais-work
Copy link
Contributor

I restarted the two failing test suites. Something is flaky, and if we retry it eventually passes, from what I have seen.

@paul-marechal Running the tests locally for something else and without the nsfw update, I just got a double "watcher dealocation" issue.

2022-12-15T14:21:51.821Z root INFO     ✅ should toggle Explorer (83ms)
2022-12-15T14:21:51.894Z root INFO     ✅ should toggle Source Control (67ms)
2022-12-15T14:21:51.954Z root INFO     ✅ should toggle Outline (55ms)
2022-12-15T14:21:52.008Z root INFO     ✅ should toggle Problems (48ms)
2022-12-15T14:21:52.076Z root INFO     ✅ should toggle Properties (66ms)
2022-12-15T14:21:52.101Z root ERROR [nsfw-watcher: 1803813] NSFW service error on "/tmp/theia-test-config-dir20221115-1803724-xolbsv.tqk6h": [Error: Service shutdown: root path changed (renamed or deleted)]
2022-12-15T14:21:52.131Z root ERROR [nsfw-watcher: 1803813] tried to de-allocate a disposed watcher: watcherId=5
2022-12-15T14:21:52.131Z root ERROR [nsfw-watcher: 1803813] tried to de-allocate a disposed watcher: watcherId=0
2022-12-15T14:21:52.132Z root ERROR Error: Unexpected SIGPIPE

@paul-marechal
Copy link
Member

Thanks for checking @marcdumais-work! I was mostly curious about the following claim:

The update contains a fix I did Axosoft/nsfw#172 that should fix the issue and (hopefully) also get rid of the frequent PR check failures on ubuntu.

But it seems like Ubuntu CI still fails for some reason then.

Copy link
Member

@paul-marechal paul-marechal left a comment

Choose a reason for hiding this comment

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

I did not notice any regressions, and our previous range would pull the new version on fresh install or upgrade anyway.

@paul-marechal paul-marechal merged commit a733057 into eclipse-theia:master Jan 5, 2023
@tsmaeder
Copy link
Contributor Author

tsmaeder commented Jan 6, 2023

Thanks for taking care of this one, folks, as I'm busy with some VS Code API tasks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies pull requests that update a dependency file file-watchers issues related to filesystem watchers - nsfw
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deleting a large quantity of files makes the UI unresponsive
4 participants