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

gulp.dest() returns before finishing (cb does work) #1254

Closed
jhannwong opened this issue Sep 14, 2015 · 19 comments
Closed

gulp.dest() returns before finishing (cb does work) #1254

jhannwong opened this issue Sep 14, 2015 · 19 comments

Comments

@jhannwong
Copy link

`gulp.dest() returns before files are actually copied to destination

Turns out it is not cb that doesn't work, but gulp.dest() that doesn't complete at the time that its stream is returned to another task.

In my exploration below, I found that listening on gulp.dest().on('end') solves this problem.

Incidentally, del also returns before finishing. However, it does end properly on a Promise.then(), unlike gulp.dest() which does not.

This is outdated

I have 3 tasks: 'clean', 'gather-assets', 'deploy-assets'. And they must be run in that order. Their dependencies are specified accordingly.

The 'clean' task takes in cb, and ends with calling cb().

Same for the 'gather-assets' task. Before calling cb(), it calls gulp.src().pipe(gulp.dest()) 4 times, copying files from 4 different locations.

Task 'deploy-assets' is a single gulp.src().pipe(gulp.dest()).

Even task gather-assets' fails, with errorENOENT, chmod. I'm guessing that 'clean' tasks wiped out the files before 'gather-assets' task can do achmod`.

Now, if I call 'clean' task manually first, then 'gather-assets' task runs fine. I run 'gather-assets' task right away after that, and I see the above chmod error.

Note: The above error happens half the time. I counted an average of 5 times out of 10 tries.

Using Gulp 3.9.

I tried run-sequence. Different errors, but same non-deterministic (race-condition) outcome.

Note to self: Read more about promises.

@TrySound
Copy link
Contributor

@hannwong Show code.

@TrySound
Copy link
Contributor

I think you use the latest version of del module, which has different api. For now it returns promise and you can just

gulp.task('clean', function () {
  return del('path');
});

@jhannwong
Copy link
Author

The codes:

gulp.task('clean', function(cb) {
    del('app');
    del('src/scss');
    del('src/js');
    del(['src/assets/**', '!src/assets', '!src/assets/custom', '!src/assets/custom/**']);
    cb();
});

gulp.task('gather-assets', ['clean'], function(cb) {
    // Copy jQuery.
    gulp.src(config.vendor_path + '/app/assets/jquery/**/*',
            {base: config.Con_path + '/app/assets'})
        .pipe(gulp.dest('./src/assets'));

    // Copy Font Awesome.
    gulp.src(config.vendor_path + '/app/assets/font-awesome/**/*',
             {base: config.Con_path + '/app/assets'})
        .pipe(gulp.dest('src/assets'));

    // Copy js files.
    gulp.src(config.Con_path + '/src/js/**/*', {base: config.Con_path + '/src'})
        .pipe(gulp.dest('src'));

    // Copy scss files.
    gulp.src(config.Con_path + '/src/scss/**/*', {base: config.Con_path + '/src'})
        .pipe(gulp.dest('src'));

    cb();
});

I have omitted the 3rd step. Even the 2nd step, involving 2 tasks, breaks.

Note that when I remove the last del() in task 'clean', things work 10 out of 10 times.
This issue is very likely a race condition. The last del() should take a longer time to complete.

@TrySound
Copy link
Contributor

@hannwong del is async function. streams are async too. You end task before del and stream ends.

@jhannwong
Copy link
Author

@TrySound If del() is an async function, how do I do a 'clean' task that must be completed before other tasks begin?

You saved a lot of my time by confirming that del() is an async function. Thanks!

@TrySound
Copy link
Contributor

@hannwong Keep calm and read documentation!

gulp.task('clean', function() {
    return Promise.all([
        del('app'),
        del('src/scss'),
        del('src/js'),
        del(['src/assets/**', '!src/assets', '!src/assets/custom', '!src/assets/custom/**'])
    ]);
});

gulp.task('js', function () {
    // returning a stream informs gulp when stream ends.
    return gulp.src(config.Con_path + '/src/js/**/*', {base: config.Con_path + '/src'})
        .pipe(gulp.dest('src'));
});

gulp.task('css', function () {
    return gulp.src(config.Con_path + '/src/scss/**/*', {base: config.Con_path + '/src'})
        .pipe(gulp.dest('src'));
});

gulp.task('build', ['clean'], function () {
    gulp.start(['js', 'css']);
});

@jhannwong jhannwong changed the title cb doesn't work cb doesn't work (node newbie alert!) Sep 14, 2015
@jhannwong
Copy link
Author

@TrySound Thanks a lot for your help!

Just to confirm, I don't need to explicitly wrap task 'gather-assets' into a single Promise, do I?

Codes that work perfectly...

gulp.task('clean', function() {
    return Promise.all([
        del('app'),
        del('src/scss'),
        del('src/js'),
        del(['src/assets/**', '!src/assets', '!src/assets/tpg', '!src/assets/tpg/**'])
    ]);
});

gulp.task('gather-assets', ['clean'], function(cb) {
    // Copy jQuery.
    gulp.src(config.vendor_path + '/app/assets/jquery/**/*',
            {base: config.vendor_path + '/app/assets'})
    .pipe(gulp.dest('./src/assets'));

    // Copy Font Awesome.
    gulp.src(config.vendor_path + '/app/assets/font-awesome/**/*',
             {base: config.vendor_path + '/app/assets'})
    .pipe(gulp.dest('src/assets'));

    // Copy js files.
    gulp.src(config.vendor_path + '/src/js/**/*', {base: config.vendor_path + '/src'})
    .pipe(gulp.dest('src'));

    // Copy scss files.
    gulp.src(config.vendor_path + '/src/scss/**/*', {base: config.vendor_path + '/src'})
        .pipe(gulp.dest('src'));

    cb();
});

gulp.task('deploy-assets', ['gather-assets'], function() {
    gulp.src('./src/assets/**/*', {base: './src/assets'})
    .pipe(gulp.dest('./app/assets'));
});

@TrySound
Copy link
Contributor

@hannwong Split gether-assets with small tasks. It's like small modules. Then you can have more control. For now deploy-assets will start before gather-assets ends.

@jhannwong
Copy link
Author

@TrySound I have split task 'gather-assets' into smaller tasks.

But I seem to have come upon another unrelated problem: gulp.src() is not finding all the files in src/assets.

gulp.task('gather-jquery', ['clean'], function(cb) {
    // Copy jQuery.
    gulp.src(config.Con_path + '/app/assets/jquery/**/*',
             {base: config.Con_path + '/app/assets'})
        .pipe(gulp.dest('./src/assets'));
    cb();
});

gulp.task('gather-fontawesome', ['clean'], function(cb) {
    // Copy Font Awesome.
    gulp.src(config.vendor_path + '/app/assets/font-awesome/**/*',
             {base: config.vendor_path + '/app/assets'})
        .pipe(gulp.dest('src/assets')),
    cb();
});

gulp.task('gather-js', ['clean'], function(cb) {
    // Copy js files.
    gulp.src(config.vendor_path + '/src/js/**/*', {base: config.vendor_path + '/src'})
        .pipe(gulp.dest('src'));
    cb();
});

gulp.task('gather-scss', ['clean'], function(cb) {
    // Copy scss files.
    gulp.src(config.vendor_path + '/src/scss/**/*', {base: config.vendor_path + '/src'})
        .pipe(gulp.dest('src'));
    cb();
});

gulp.task('deploy-assets', ['gather-jquery', 'gather-fontawesome', 'gather-js', 'gather-scss'], function(cb) {
    gulp.src('src/assets/**/*', {base: 'src'})
        .pipe(gulp.dest('app'));
    cb();
});

I have 3 folders inside folder src/assets, but only 1 of them is copied over to app/assets.

@TrySound
Copy link
Contributor

@hannwong Do not call cb, return streams.

@TrySound
Copy link
Contributor

@hannwong clean task calls everytime you use it in dependencies. And why copy a few times? Why not copy once where they should be?

@jhannwong
Copy link
Author

@TrySound How do I return stream when I have 4-5 separate gulp.src().pipe() calls?

I have combined the copying into a single step, no more intermediate steps.

gulp.task('clean', function() {
    return Promise.all([
        del('app'),
        del('src/scss'),
        del('src/js'),
        del(['src/assets/**', '!src/assets', '!src/assets/tpg', '!src/assets/tpg/**'])
    ]);
});

gulp.task('deploy-assets', ['clean'], function(cb) {
    // Copy jQuery.
    gulp.src(config.vendor_path + '/app/assets/jquery/**/*',
             {base: config.vendor_path + '/app/assets'})
        .pipe(gulp.dest('app/assets'));

    // Copy Font Awesome.
    gulp.src(config.vendor_path + '/app/assets/font-awesome/**/*',
             {base: config.vendor_path + '/app/assets'})
        .pipe(gulp.dest('app/assets'));

    // Copy our own assets.
    gulp.src('src/assets/ours/**/*',
             {base: 'src/assets'})
        .pipe(gulp.dest('app/assets'));

    // Copy js files.
    gulp.src(config.vendor_path + '/src/js/**/*',
             {base: config.vendor_path + '/src'})
        .pipe(gulp.dest('src'));

    // Copy scss files.
    gulp.src(config.vendor_path + '/src/scss/**/*',
                   {base: config.vendor_path + '/src'})
        .pipe(gulp.dest('src'));

    cb();
});

gulp.task('build', ['deploy-assets'], function() {
    runSequence(['css', 'js', 'js-materialize', 'html']);
});

Oh dear. Another gulp.src() difficulty. The following returns zero items.

    gulp.src('src/js/**/*')
        .pipe(debug());

In contrast, I am able to list all the files under src/assets/**/*.

@TrySound
Copy link
Contributor

@hannwong

gulp.task('gather-jquery', ['clean'], function() {
    // Copy jQuery.
    return gulp.src(config.Con_path + '/app/assets/jquery/**/*',
             {base: config.Con_path + '/app/assets'})
        .pipe(gulp.dest('./src/assets'));
});

gulp.task('gather-fontawesome', ['clean'], function() {
    // Copy Font Awesome.
    return gulp.src(config.vendor_path + '/app/assets/font-awesome/**/*',
             {base: config.vendor_path + '/app/assets'})
        .pipe(gulp.dest('src/assets')),
});

gulp.task('gather-js', ['clean'], function() {
    // Copy js files.
    return gulp.src(config.vendor_path + '/src/js/**/*', {base: config.vendor_path + '/src'})
        .pipe(gulp.dest('src'));
});

gulp.task('gather-scss', ['clean'], function() {
    // Copy scss files.
    return gulp.src(config.vendor_path + '/src/scss/**/*', {base: config.vendor_path + '/src'})
        .pipe(gulp.dest('src'));
});

gulp.task('deploy-assets', ['clean', 'gather-jquery', 'gather-fontawesome', 'gather-js', 'gather-scss'], function() {
    return gulp.src('src/assets/**/*', {base: 'src'})
        .pipe(gulp.dest('app'));
});

@callumacrae
Copy link
Member

@jhannwong
Copy link
Author

The following is in reverse-chrono order. The top attempt works.

It appears that gulp relies on tools that are not yet thread- or concurrency-safe? I must admit that gulp is fun in that it forces me to think concurrently. No more singular 'clean' task! Thanks @TrySound for hammering the "concurrent way of thinking" into me!

Eventual 'build' task that works

Each 'deploy-some-files' task does its own 'cleaning' (removing of existing files in destination). I'm beginning to love gulp, where concurrency is the default. But I'm gonna have immense difficulty looking for hires who can work with gulp, especially hires that aren't required to do any concurrency programming.

gulp.task('deploy-jquery', function() {
    var promise = Promise.resolve(del('app/assets/jquery'));
    promise.then(function() {
        gulp.src(config.Con_path + '/app/assets/jquery/**/*',
                 {base: config.Con_path + '/app/assets'})
            .pipe(gulp.dest('app/assets'));
    });
});

gulp.task('deploy-fontawesome', function() {
    var promise = Promise.resolve(del('app/assets/font-awesome'));
    promise.then(function() {
        gulp.src(config.Con_path + '/app/assets/font-awesome/**/*',
                 {base: config.Con_path + '/app/assets'})
            .pipe(gulp.dest('app/assets'));
    });
});

gulp.task('deploy-ours', function() {
    var promise = Promise.resolve(del('app/assets/ours'));
    promise.then(function() {
        gulp.src('src/assets/ours/**/*',
                 {base: 'src/assets'})
            .pipe(gulp.dest('app/assets'));
    });
});

// Async task required by task 'js'.
// Special case: Nested 'materialize' folder is not in this task's jurisdiction.
gulp.task('deploy-js', function(cb) {
    var promise = Promise.resolve(del(['src/js/*', '!src/js/materialize',
                                       'app/assets/_con/js']));
    promise.then(function() {
        gulp.src([config.Con_path + '/src/js/**/*',
                  '!' + config.Con_path + '/src/js/materialize',
                  '!' + config.Con_path + '/src/js/materialize/**/*'],
                 {base: config.Con_path + '/src'})
            .pipe(gulp.dest('src'))
            .on('end', function() { cb(); });
    });
});

// Async task required by task 'js-materialize'.
gulp.task('deploy-js-materialize', function(cb) {
    var promise = Promise.resolve(del(['src/js/materialize',
                                       'app/assets/materialize/js']));
    promise.then(function() {
        return gulp.src(config.Con_path + '/src/js/materialize/**/*',
                        {base: config.Con_path + '/src/js'})
            .pipe(gulp.dest('src/js'))
            .on('end', function() { cb(); });
    });
});

// Async task required by task 'css'.
gulp.task('deploy-scss', function(cb) {
    var promise = Promise.resolve(del(['src/scss',
                                       'app/assets/_con/css']));
    promise.then(function() {
        return gulp.src(config.Con_path + '/src/scss/**/*',
                        {base: config.Con_path + '/src'})
            .pipe(gulp.dest('src'))
            .on('end', function() { cb(); });
    });
});

// Notice we don't care when gulp.dest() finishes for these tasks. No post-processing.
gulp.task('deploy-assets', ['deploy-jquery', 'deploy-fontawesome', 'deploy-ours']);

gulp.task('build', ['css', 'js', 'js-materialize', 'html', 'deploy-assets']);

// This is called prior to doing git-related operations.
gulp.task('clean-all', function() {
    del('app');
    del('src/scss');
    del('src/js');
    del(['src/assets/**', '!src/assets', '!src/assets/ours', '!src/assets/ours/**']);
});

Catching event end works

gulp.task('deploy-js', function(cb) {
    var promise = Promise.resolve(del(['src/js/*', '!src/js/materialize']));
    promise.then(function() {
        gulp.src([config.Con_path + '/src/js/**/*',
                  '!' + config.Con_path + '/src/js/materialize',
                  '!' + config.Con_path + '/src/js/materialize/**/*'],
                 {base: config.Con_path + '/src'})
            .pipe(gulp.dest('src'))
            .on('end', function() {
                cb();
            });
    }, function() {
        console.log('Error cleaning src/js!');
    });
});

gulp.task('js', ['deploy-js'], function() {
    return gulp.src('src/js/**/*').pipe(debug());
});

This attempt puts cleaning responsibility on deploy tasks

gulp.task('deploy-js', function(cb) {
    var promise = Promise.resolve(del(['src/js/*', '!src/js/materialize']));
    promise.then(function() {
        gulp.src([config.Con_path + '/src/js/**/*',
                         '!' + config.Con_path + '/src/js/materialize',
                         '!' + config.Con_path + '/src/js/materialize/**/*'],
                        {base: config.Con_path + '/src'})
            .pipe(gulp.dest('src'));
        cb();
    }, function() {
        console.log('Error cleaning src/js!');
    });
});

gulp.task('js', ['deploy-js'], function() {
    return gulp.src('src/js/**/*').pipe(debug());
});

The above debug output shows that cb doesn't work. The file copy did not complete (zero files in destination) when task 'js' was started.

Returning the stream instead of calling cb also doesn't work.

Older attempts...

@TrySound It appears that gulp.src().pipe(gulp.dest()) returns success on a Promise even though the file copying has not yet completed. I wrapped all 5 of them in my task 'deploy-assets' into a Promise.all().

The only way I can fix this is to block right after Promise.all() and poll the file copy destinations until the files are found to be copied over.

Is there something I can do to fix the lack of proper "success" feedback from gulp.src().pipe(gulp.dest())? Or am I still missing something?

@jhannwong
Copy link
Author

@callumacrae The destinations are different. But that is a very good recipe! Thanks for the heads up.

@jhannwong jhannwong changed the title cb doesn't work (node newbie alert!) gulp.dest() returns before finishing (cb does work) Sep 15, 2015
@TrySound
Copy link
Contributor

@hannwong Stop to invent new. I already said what do you need. And read more documentation.

@jhannwong
Copy link
Author

@TrySound Thanks for your advice! :-)

@jhannwong
Copy link
Author

@contra TrySound has helped me with this issue.

I'm unsubscribing from it now. I can't close it. Just letting you know I don't need this issue anymore.

Thanks!

@phated phated closed this as completed Sep 15, 2015
@gulpjs gulpjs locked and limited conversation to collaborators Sep 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants