lukeed / taskr

A fast, concurrency-focused task automation tool.
MIT License
2.53k stars 74 forks source link

`fly.clear` is executed early / not awaited #193

Closed frantic1048 closed 8 years ago

frantic1048 commented 8 years ago

versions:

Platform: Arch Linux 4.6.5-1-zen x86_64 latest Node: v6.4.0 Fly version: 1.3.1

It's a little hard to describe the issue. First of all, I will explain what the sample contains. And then I describe the operations and what's weird.

sample project

The repo is here: https://github.com/frantic1048/fly-issue-193

overall sturcture:

.
├── dist            // output path
├── flyfile.js
├── node_modules
├── package.json
└── src             // source path
    ├── a
    │   ├── a1.txt
    │   ├── a2.txt
    │   ├── a3.txt
    │   └── a4.txt
    └── b
        ├── b1.txt
        ├── b2.txt
        ├── b3.txt
        └── b4.txt

flyfile.js : just some copy/paste tasks and a clean task, and two top-level task to run tasks in serial/parallel.

const paths = {
  a: {
    source: 'src/a/*.txt',
    target: 'dist/a'
  },
  b: {
    source: 'src/b/*.txt',
    target: 'dist/b'
  },
  all: {target: 'dist'}
}

exports.clean = function * () {
  yield this.clear(paths.all.target)
}

exports.a = function * () {
  yield this
    .source(paths.a.source)
    .target(paths.a.target)
}

exports.b = function * () {
  yield this
    .source(paths.b.source)
    .target(paths.b.target)
}

exports.serial = function * () {
  yield this.start('clean')
  yield this.start('a')
  yield this.start('b')
}

exports.parallel = function * () {
  yield this.start('clean')
  yield this.start(['a', 'b'], {parallel: true})
}

package.json : the only dependency is fly, just npm i to install it.

{
  "name": "fly-missing-file",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1",
    "cc": "fly clean",
    "s": "DEBUG=\"fly*\" fly serial",
    "p": "DEBUG=\"fly*\" fly parallel"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "devDependencies": {
    "fly": "^1.3.1"
  }
}

.txt : every .txt file is just Lorem ipsum . Other content also is ok.

Lorem ipsum dolor sit amet, consectetur adipisicing elit. Aperiam, iste, iure, quod voluptates soluta quidem commodi animi recusandae unde obcaecati amet eaque facere similique. Necessitatibus, iusto possimus minima numquam modi.

reproducing operation

Ensure you have runned npm install to get the dependencies ready.

Now, the dist folder is empty, we run paralleled task.

npm run p

Then dist becomes:

dist/
├── a
│   ├── a1.txt
│   ├── a2.txt
│   ├── a3.txt
│   └── a4.txt
└── b
    ├── b1.txt
    ├── b2.txt
    ├── b3.txt
    └── b4.txt

Every thing works great. Then we run the paralled task the second time.

npm run p

Then whole dist disappeared, even the dist folder itself. While debug log shows all the *.txt were written.

We can type npm run p again, then all the files are back. This time we delete some of the output file:

dist/
├── a
│   ├── a1.txt
│   └── a2.txt
└── b
    └── b4.txt

Then type npm run p, we get:

dist/
├── a
│   ├── a3.txt
│   └── a4.txt
└── b
    ├── b1.txt
    ├── b2.txt
    └── b3.txt

It just like all the output are "inverted", this situation also happens replacing all the npm run p by npm run s(serial version).

And every time running npm run p, the debug log shows every output file was written, log like:

[frantic@RabbitHouseCafe fly-missing-file]$ npm run p

> fly-missing-file@1.0.0 p /home/frantic/work/demoo/fly-missing-file
> DEBUG="fly*" fly parallel

  fly:find find this file: flyfile.js +0ms
  fly:plugins beginning to look for plugins +5ms
  fly:find find this file: package.json +1ms
  fly:read read this file: /home/frantic/work/demoo/fly-missing-file/package.json +0ms
  fly:plugins parse fly-* plugins from `package.json` +2ms
  fly chdir '/home/frantic/work/demoo/fly-missing-file' +1ms
  fly:log Flying with /home/frantic/work/demoo/fly-missing-file/flyfile.js... +2ms
  fly start [ 'parallel' ] in sequence +0ms
  fly run 'parallel' +2ms
  fly:log Starting parallel +0ms
  fly start [ 'clean' ] in sequence +0ms
  fly run 'clean' +0ms
  fly:log Starting clean +0ms
  fly clear [ 'dist' ] +0ms
  fly:log Finished clean in 4 ms +4ms
  fly start [ 'a', 'b' ] in parallel +0ms
  fly run 'a' +0ms
  fly:log Starting a +0ms
  fly source [ 'src/a/*.txt' ] +0ms
  fly run 'b' +3ms
  fly:log Starting b +0ms
  fly source [ 'src/b/*.txt' ] +0ms
  fly:read read this file: src/a/a1.txt +6ms
  fly:read read this file: src/a/a2.txt +0ms
  fly:read read this file: src/a/a3.txt +0ms
  fly:read read this file: src/a/a4.txt +0ms
  fly:read read this file: src/b/b1.txt +1ms
  fly:read read this file: src/b/b2.txt +0ms
  fly:read read this file: src/b/b3.txt +0ms
  fly:read read this file: src/b/b4.txt +0ms
  fly:write write this file: dist/a/a1.txt +2ms
  fly:write write this file: dist/a/a2.txt +1ms
  fly:write write this file: dist/a/a3.txt +0ms
  fly:write write this file: dist/a/a4.txt +0ms
  fly:log Finished a in 13 ms +0ms
  fly:write write this file: dist/b/b1.txt +1ms
  fly:write write this file: dist/b/b2.txt +0ms
  fly:write write this file: dist/b/b3.txt +0ms
  fly:write write this file: dist/b/b4.txt +0ms
  fly:log Finished b in 11 ms +0ms
  fly:log Finished parallel in 18 ms +1ms

i.e. I haven't seen any file to output is missing in the log.

Sorry for such long text ~(>_<~)

lukeed commented 8 years ago

Hey @frantic1048, thanks for the detail!

This seems to be a serial ordering issue 🤔

What happens if you run npm run s? And can you package this test repo so that I can clone into quickly?

Thanks!

frantic1048 commented 8 years ago

npm run s behaves likely to npm run p .

The repo is here: https://github.com/frantic1048/fly-issue-193

lukeed commented 8 years ago

Thanks!

lukeed commented 8 years ago

@frantic1048 It's still definitely a bug (I'm working on it) but you can get 100% reliable performance by removing the call to your clean task at the start of serial and parallel. To be clear, here and here.

This is because, by default, your destination files are removed/overwritten.

frantic1048 commented 8 years ago

Normally, I put a clean task at the start of a top-level task, for a clean building on first start-up. e.g. I want to imediately build and pack a release. But I don't use clean in .watch sequence.

I could understand the overwritten. And the removed means when writting a.txt, fly will try to remove a.txt first or ... ?

lukeed commented 8 years ago

Well, the problem is that fly.clear isn't awaited. The command itself "finishes" but not its method. Aka, fly.clear was run but not completed before your task starts. It's a simple yield missing somewhere, I bet.

We use fs.writeFile (docs), so the destination file is entirely replaced.

The only time this doesn't work is if you delete a source file -- the destination copy will still be there.... However, I do the same practice you do in my own projects. I don't have this problem. I have to look closely to see what's different.

frantic1048 commented 8 years ago

Yes, I was gessing there's some reason like that, but I couldn't explain why the output is just like being (not always) inverted.

First time I met this issue, was two weeks ago, I'm packing project for my teammate. Then he tells me some files are missing, and I checked the build folder, some files are not there ( ̄□ ̄)

I first tried splitting the parallel tasks in serial(I don't know why I did this...), it works, but sometimes still fail... I type rm -rf dist && npm run build like folks meeting trouble type rm -rf node_modules && npm i, it works (/ω\)

After that, I tried to construct a minimal project to reproduce this issue, used many silly operations but it works well. Finally, I did delete some output file I found the issue happen. Further, I find the condition as described above.

lukeed commented 8 years ago

Yeah 😠 annoying issue; thanks for your patience!

I'll have some time to look at it tonight.

lukeed commented 8 years ago

@frantic1048 Solved :) Will ship with version 1.4, coming shortly