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

Upgrade to Gulp 4 #10266

Merged
merged 1 commit into from
Dec 17, 2018
Merged

Upgrade to Gulp 4 #10266

merged 1 commit into from
Dec 17, 2018

Conversation

timvandermeij
Copy link
Contributor

@timvandermeij timvandermeij commented Nov 18, 2018

This required the following changes in the Gulpfile:

  • Defining a series of tasks is no longer done with arrays, but with the gulp.series function. The web target is refactored to use a smaller number of tasks to prevent tasks from running multiple times.
  • Getting all tasks must now be done through the task registry.
  • Tasks that don't return anything must call done upon completion.

Moreover, this upgrade allows us to use the latest Node.js on Travis CI again.

Fixes most parts of #10177.

@timvandermeij
Copy link
Contributor Author

/botio-linux preview

@timvandermeij
Copy link
Contributor Author

timvandermeij commented Nov 18, 2018

It looks like the global gulp-cli on the bots needs to be updated; refer to gulpjs/gulp-cli#84 (comment). We should already have installed it globally on both bots; refer to https://github.com/mozilla/pdf.js/search?q=%22npm+install+-g%22&unscoped_q=%22npm+install+-g%22.

@brendandahl Could you perhaps update gulp-cli to the latest version on both bots? The CLI is compatible with both Gulp 3 and Gulp 4, so this should be safe to do.

@brendandahl
Copy link
Contributor

What version of gulp-cli do we need on the bots?
On linux it looks like we have the latest version:
CLI version 2.0.1
Local version 3.9.1

@timvandermeij
Copy link
Contributor Author

Hm, it looks like gulp-cliis correct then, but it somehow still uses Gulp 3.9.1 instead of Gulp 4. Looking at https://github.com/pattern-lab/edition-node-gulp/wiki/Updating-to-Gulp-4, perhaps it's a difference between the globally installed Gulp and a locally installed Gulp (CLI)?

@timvandermeij
Copy link
Contributor Author

timvandermeij commented Dec 11, 2018

It looks like Gulp 4 got the latest tag just now, so until this is rebased the master builds will fail unfortunately. I'll try to do this soon.

@timvandermeij
Copy link
Contributor Author

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/bd8161ddbad6eaf/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/bd8161ddbad6eaf/output.txt

Total script time: 4.87 mins

Published

@timvandermeij
Copy link
Contributor Author

/botio lint

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_lint from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/f2bf6e0cd97ea82/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_lint from @timvandermeij received. Current queue size: 0

Live output at: http://54.215.176.217:8877/a845b3e3ac01316/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/f2bf6e0cd97ea82/output.txt

Total script time: 0.76 mins

  • Lint: Passed

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/a845b3e3ac01316/output.txt

Total script time: 2.28 mins

  • Lint: Passed

@timvandermeij
Copy link
Contributor Author

/botio unittest

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_unittest from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/a8203b5f0d8a8d3/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_unittest from @timvandermeij received. Current queue size: 0

Live output at: http://54.215.176.217:8877/30e2d4e4534fa1c/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Failed

Full output at http://54.67.70.0:8877/a8203b5f0d8a8d3/output.txt

Total script time: 2.38 mins

  • Unit Tests: FAILED

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/30e2d4e4534fa1c/output.txt

Total script time: 4.60 mins

  • Unit Tests: Passed

@timvandermeij
Copy link
Contributor Author

/botio-linux unittest

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_unittest from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/1b45bb6179e5dab/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/1b45bb6179e5dab/output.txt

Total script time: 2.37 mins

  • Unit Tests: Passed

@timvandermeij
Copy link
Contributor Author

@Snuffleupagus Could you perhaps review this so we can resolve the build failures?

@Snuffleupagus
Copy link
Collaborator

Looks generally OK, based on a quick look :-)

However, there's two things which seem odd/wrong, but in the interest of un-breaking the builds I suppose that you could fix them in a follow-up PR (assuming it happens relatively soon) rather than here:

@wojtekmaj
Copy link
Contributor

FYI: This should fix tests on master

@timvandermeij
Copy link
Contributor Author

I'm not entirely sure what happened for the web target that it repeats the tasks. I'll have to look into that; most likely the targets just have superfluous dependencies or there are too many tasks for one purpose. The <anonymous> lines should be fixable by using named functions instead.

@wojtekmaj
Copy link
Contributor

Well, that's because you Gulp 4 don't have dependencies!

Here's more info and how to fix:

https://fettblog.eu/gulp-4-parallel-and-series/ CtrlF Gulp 4 does not have any dependency check

TL;DR

gulp.task("after-this", afterThis));
gulp.task("do-this-1", ["after-this"], doThis1));
gulp.task("do-this-2", ["after-this"], doThis2));

-->

gulp.task("after-this", afterThis));
gulp.task("do-this-1", doThis1));
gulp.task("do-this-2", doThis2));
gulp.series("after-this", gulp.parallel("do-this-1", "do-this-2");

This required the following changes in the Gulpfile:

- Defining a series of tasks is no longer done with arrays, but with the
  `gulp.series` function. The `web` target is refactored to use a
  smaller number of tasks to prevent tasks from running multiple times.
- Getting all tasks must now be done through the task registry.
- Tasks that don't return anything must call `done` upon completion.

Moreover, this upgrade allows us to use the latest Node.js on Travis CI
again.
@timvandermeij
Copy link
Contributor Author

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/1ec3a87e513f2b5/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/1ec3a87e513f2b5/output.txt

Total script time: 1.67 mins

Published

@timvandermeij
Copy link
Contributor Author

/botio test

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_test from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/d7c45c1c832a660/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_test from @timvandermeij received. Current queue size: 0

Live output at: http://54.215.176.217:8877/d4e7ff4823b07db/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/d7c45c1c832a660/output.txt

Total script time: 17.80 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: Passed

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/d4e7ff4823b07db/output.txt

Total script time: 23.16 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: Passed

@timvandermeij
Copy link
Contributor Author

/botio lint

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_lint from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/dcff95b202cdfec/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_lint from @timvandermeij received. Current queue size: 0

Live output at: http://54.215.176.217:8877/4ef198432368768/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/dcff95b202cdfec/output.txt

Total script time: 0.77 mins

  • Lint: Passed

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/4ef198432368768/output.txt

Total script time: 2.36 mins

  • Lint: Passed

@timvandermeij timvandermeij merged commit 417c234 into mozilla:master Dec 17, 2018
@timvandermeij timvandermeij deleted the gulp-4 branch December 17, 2018 16:00
@timvandermeij
Copy link
Contributor Author

Thank you for the feedback, @Snuffleupagus and @wojtekmaj! The builds should succeed again now, we got rid of quite a few old dependencies and Node.js doesn't report vulnerabilities anymore. I'll create a follow-up issue for the remaining improvements.

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

Successfully merging this pull request may close these issues.

5 participants