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

Fix ENOTEMPTY error when cleaning up #634

Merged
merged 4 commits into from
Oct 10, 2021
Merged

Fix ENOTEMPTY error when cleaning up #634

merged 4 commits into from
Oct 10, 2021

Conversation

razn-v
Copy link
Contributor

@razn-v razn-v commented Oct 9, 2021

Resolve #334

The issue seemed to appear when importing a local asset and using it in a src field.
Windows couldn't properly close the handle of the asset, making the bundled directory impossible to delete.

The workaround was to manually remove each file of the directory using execa, then using fs.

N.B. For now it seems like we got a EPERM error instead, thinking if it's specific to actions or not (since my tests don't report anything).


IssueHunt Summary

Referenced issues

This pull request has been submitted to:


@razn-v
Copy link
Contributor Author

razn-v commented Oct 9, 2021

My bad... made a mistake when testing, closing this PR for now.

@razn-v razn-v closed this Oct 9, 2021
@razn-v razn-v reopened this Oct 10, 2021
@razn-v
Copy link
Contributor Author

razn-v commented Oct 10, 2021

Well... we got a EPERM error from actions but I got none on my local machine. Is it specific to actions?

@JonnyBurger
Copy link
Member

Hmm, maybe you need to wrap it the previous code in an else statement? Good idea to use CLI!

@razn-v
Copy link
Contributor Author

razn-v commented Oct 10, 2021

Yeah I tried to not resort to CLI, because it's not a very clean way to do thing, but Windows didn't let me any other choice lol. I'll try wrapping the code in a else branch as you said.

@razn-v
Copy link
Contributor Author

razn-v commented Oct 10, 2021

Got an error with deleting the first directory this time (https://github.com/remotion-dev/remotion/runs/3852364585?check_suite_focus=true#step:9:96), I'll try executing del for it too.

This is also an error which I do not get on my local machine...

@JonnyBurger
Copy link
Member

Hmm, does it even use cmd for execa on Windows? 🤔

@razn-v
Copy link
Contributor Author

razn-v commented Oct 10, 2021

@razn-v
Copy link
Contributor Author

razn-v commented Oct 10, 2021

Hmm, does it even use cmd for execa on Windows? 🤔

It seemed to not accept options without cmd, this is why I used it.

@JonnyBurger JonnyBurger merged commit dd94bff into remotion-dev:main Oct 10, 2021
@JonnyBurger
Copy link
Member

@razn-v Nice, thank you! 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate windows / node v14 recursive bug
2 participants