Skip to content

🏗 Add a check for the default gulp executable path#22459

Merged
rsimha merged 2 commits intoampproject:masterfrom
rsimha:2019-05-22-WarnGulp
May 23, 2019
Merged

🏗 Add a check for the default gulp executable path#22459
rsimha merged 2 commits intoampproject:masterfrom
rsimha:2019-05-22-WarnGulp

Conversation

@rsimha
Copy link
Copy Markdown
Contributor

@rsimha rsimha commented May 23, 2019 •
edited
Loading

This PR detects if there's a possibly incorrect version of gulp somewhere in the user's $PATH, and prints a warning before carrying on.

This was prompted by a report from @choumx who saw the error below, which turned out to be due to an older version of gulp present in /usr/local/bin/.

willchou$ gulp
[19:50:21] Using gulpfile ~/amphtml/gulpfile.js
TypeError: Cannot read property 'apply' of undefined
    at /usr/local/lib/node_modules/gulp/bin/gulp.js:129:20
    at process._tickCallback (internal/process/next_tick.js:61:11)
    at Function.Module.runMain (internal/modules/cjs/loader.js:757:11)
    at startup (internal/bootstrap/node.js:283:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:622:3)

willchou$ gulp --version
[19:50:26] CLI version 3.9.1
[19:50:26] Local version 4.0.2

Notes:

The gulp checks in check-package-manager.js:

  • ... are only triggered during yarn (which is run much less frequently), and don't slow down actual invocations of gulp
  • ... do not prevent the yarn command from running even if they find problems

@rsimha rsimha requested a review from dreamofabear May 23, 2019 04:42
@rsimha rsimha self-assigned this May 23, 2019
@rsimha rsimha requested a review from erwinmombay May 23, 2019 04:42
@rsimha
Copy link
Copy Markdown
Contributor Author

rsimha commented May 23, 2019 •
edited
Loading

@choumx I've tested this on macOS and Linux, and it seemed to do the trick in all the cases I tried. Let me know if it works for your case.

@rsimha rsimha changed the title 🏗 Add a check for incorrect gulp paths 🏗 Add a check for the default gulp executable path May 23, 2019
erwinmombay
erwinmombay reviewed May 23, 2019
View reviewed changes
@rsimha rsimha merged commit e15be66 into ampproject:master May 23, 2019
@rsimha rsimha deleted the 2019-05-22-WarnGulp branch May 23, 2019 18:10
@rsimha rsimha mentioned this pull request May 23, 2019
Merged
@rsimha rsimha mentioned this pull request Aug 20, 2019
Merged
@rsimha rsimha mentioned this pull request Feb 8, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erwinmombay erwinmombay erwinmombay approved these changes

@dreamofabear dreamofabear Awaiting requested review from dreamofabear

Assignees

@rsimha rsimha

Labels

cla: yes WG: infra

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rsimha @erwinmombay @googlebot