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

Added stream end to the transform stream to determine when execution is done #17

Closed
wants to merge 1 commit into from

Conversation

bleadof
Copy link

@bleadof bleadof commented Oct 8, 2014

I had a problem that the gulp never knew when this particular stream ended, so here's a solution.

I wanted to fix the tests to instead of call(done) to listen end event and then call done, but for some reason the compare stream also never emits end. I added push(null) to bunch of places but gave up after an hour of fiddling with them. The code is semi hard to read with multiple transform streams.

I'd write the tests so that I would use highland instead of the transform streams. With highland you can use doto to do the actual expects so you won't have to deal with transform streams. I would look into this at some point, but currently I'm too busy. So here's my drive-by PR. 😁

tummychow added a commit to tummychow/goose that referenced this pull request Oct 16, 2014
I'm finally hitting my stride on this project, and pressing F5 is
finally starting to get old. This commit brings in browsersync and a
bunch of gulp.watches to make workflow smoother.

Due to m19c/gulp-run#17, gulp-run does not notify gulp of when the
task completes. To fix this, I reimplemented those tasks in terms of the
standard child_process module. I'm pretty pleased with how the tasks
look now.
@cbarrick
Copy link
Collaborator

Thanks for the PR!

Before I merge, I have a few questions. First, is it our responsibility to end the stream? Other plugins, like gulp-rename, don't appear to close the stream. gulp.dest appears to close the stream, but it's not very clear to me if we also share this responsibility.

Second, what does this mean when we have multiple input files? If we end the stream in the first call to _transform (even asynchronously), couldn't we conceivably be closing before all files have been passed through?

I know this is an old thread, and I apologize for that. If this is still an issue, could you post your usage so I can check this out with more detail?

@taylor1791
Copy link

I am running into the same problem. If gulp-run does not close the steam how would you do it in a simple example. Take this small bit for example.

gulp.task( 'create-hash',  function() {

  var run = require( 'gulp-run' );

  return run( 'git rev-parse HEAD' )
    .exec()
    .pipe( gulp.dest( '.' ) ) );
} );

@cbarrick
Copy link
Collaborator

Hmmm... Other than the syntax error, the example you gave above works for me.

Specifically, the following gulp task calls the command git rev-parse HEAD and put the resulting hash in a file called ./git.

var gulp = require('gulp');
var run = require( './' ); // called from within the gulp-run repo

gulp.task( 'create-hash',  function() {
  return run( 'git rev-parse HEAD' )
    .exec()
    .pipe( gulp.dest( '.' ) );
} );

That doesn't mean that nothing's broken, but the issue must be dependent on a more complex task. Or am I missing something?

@taylor1791
Copy link

Sorry, I didn't see your response or the syntax error. When I run gulp create-hash I get

[18:27:50] Using gulpfile ~/src/scrap/Gulpfile.js
[18:27:50] Starting 'create-hash'...
[18:27:50] $ git rev-parse HEAD 
9433xxxb20

I do not see [18:34:35] Finished 'create-hash' after 39 ms, since there wasn't a stream end. This can prevent a subsequent task from running if it has create-hashas a dependency.

Here is an illustration of the example.

$ gulp test
[18:41:45] Using gulpfile ~/src/scrap/Gulpfile.js
[18:41:45] Starting 'create-hash'...
[18:41:45] $ git rev-parse HEAD 
94336bc45e38d203e04fbb240f6e93618c4b4b20

Where

Gulpfile.js

var gulp = require('gulp');
var run = require( 'gulp-run' ); // called from within the gulp-run repo

gulp.task( 'create-hash',  function() {
  return run( 'git rev-parse HEAD' )
    .exec()
    .pipe( gulp.dest( '.' ) );
} );

gulp.task( 'test', [ 'create-hash' ], function() {
  return gulp.src( 'git' )
    .pipe( gulp.dest( 'git.hash' ) );
} );

@taylor1791
Copy link

@cbarrick Did you get a chance to try out my new example?

@cbarrick
Copy link
Collaborator

I just pushed a possible fix to the branch called 17.

gulp-run now properly closes the stream when used with .exec, like @bleadof's fix, but it also properly handles being in the middle of a pipeline, i.e. gulp-run will not close the stream if it didn't start it.

I'll get some proper tests and merge this into master asap

@taylor1791
Copy link

This is great news! Thanks for continuing to investigate this issue.

@cbarrick cbarrick self-assigned this Mar 29, 2015
@cbarrick
Copy link
Collaborator

I've merged my branch and published as v1.6.7

@cbarrick cbarrick closed this Mar 29, 2015
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.

3 participants