broccolijs / broccoli-plugin

Base class for Broccoli plugins
MIT License
25 stars 25 forks source link

better "cleanup" idea #16

Closed stefanpenner closed 6 years ago

stefanpenner commented 8 years ago

instead of sync cleanup here: https://github.com/broccolijs/broccoli-plugin/blob/master/read_compat.js#L69-L71

We should consider:

  1. check if $TMPDIR is writable
  2. if isn't, continue as today
  3. if it is, move file to be deleted to $TMPDIR
  4. then delete

This will prevent the, early exit leaving tmp in a messy state, instead deferring the messy state to OS TMPDIR periodic cleanup.

Thoughts? Something like: https://github.com/stefanpenner/move-to-tmp-and-remove/blob/master/index.js#L1-L21

joliss commented 8 years ago

Just so I understand the problem correctly... This is about double SIGINT interrupting the rimraf? I just looked at https://github.com/ember-cli/ember-cli/pull/6198 - how long does cleanup usually take?

It seems to me that the approach you outlined probably wouldn't help all that much, because it only leaves the files of the currently-cleaning-up node in /tmp, but all later nodes that haven't been cleaned up yet will still have their files around.

With Broccoli 1.0, this problem should go away anyway, because it puts tmp files in /tmp. I'm in the middle of updating the watcher code and I'll ping you when I have it in a usable state.

joliss commented 8 years ago

I'd like to get Ember CLI on Broccoli 1.0 pretty soon!

stefanpenner commented 8 years ago

how long does cleanup usually take?

Depends on the platform, and the size of the project. But it is quite common to kill before it is complete

It seems to me that the approach you outlined probably wouldn't help all that much, because it only leaves the files of the currently-cleaning-up node in /tmp, but all later nodes that haven't been cleaned up yet will still have their files around.

Ya thats true, well sorta, for example we can make the deletion (once in tmp) async. And if the process is killing during the async cleanup phase, the system will eventually purge /tmp.

With Broccoli 1.0, this problem should go away anyway, because it puts tmp files in /tmp. I'm in the middle of updating the watcher code and I'll ping you when I have it in a usable state.

Should we expect this is weeks / months etc? If you open an issue with steps required, others can help. I am eager to help, so a TODO list from your perspective would be great.

joliss commented 8 years ago

I'd say weeks to get Ember CLI on Broccoli 1.0.

The main thing I'm working on is updating the watcher, but there is a bunch of code in Ember CLI that depends on the builder internals (the node tree) that will need to be updated. Let me finish up the watcher stuff, and then I'll take another look and open an issue on the ember-cli repo to list any remaining things to be done.

givanse commented 6 years ago

I'm assuming this can be closed. https://github.com/broccolijs/broccoli/blob/master/CHANGELOG.md#100-beta1