Skip to content

Commit

Permalink
enable travis
Browse files Browse the repository at this point in the history
  • Loading branch information
@silverwind
silverwind committed May 22, 2019
1 parent 86db4da commit a5aee4c
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 2 deletions.
3 changes: 3 additions & 0 deletions .travis.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
language: node_js
node_js:
- node
3 changes: 1 addition & 2 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,12 @@
"authors": "bash tools/authors",
"clean": "npm run perfectionist && node tools/fix-perfectionist.js",
"eslint": "eslint --quiet --color tools/*.js",
"lint": "npm run eslint && npm run stylelint",
"patch": "npx ver -p patch wikipedia-dark.user.css",
"minor": "npx ver -p minor wikipedia-dark.user.css",
"major": "npx ver -p major wikipedia-dark.user.css",
"perfectionist": "npx perfectionist wikipedia-dark.user.css wikipedia-dark.user.css --indentSize 2 --maxAtRuleLength 250",
"stylelint": "npx stylelint --silent --color -- wikipedia-dark.user.css",
"test": "npm run eslint && npm run stylelint",
"update": "npx updates -cu && npm install"
"update": "npx updates -cu && rm -rf node_modules && npm install"
}
}

23 comments on commit a5aee4c

@the-j0k3r
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@silverwind this rm -rf node_modules is Linux only, This errors out Windows side. Is there another way to do this that works cross OS?

@silverwind
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was under the impression that npm shims certain unix commands in the package scripts. Is this not the case?

Generally, I'd advice you to look into using a Unix-like shell, it's just less pain overall than trying to make things work in cmd or powershell. WSL or Cygwin are good options on Windows.

@the-j0k3r
Copy link
Member

@the-j0k3r the-j0k3r commented on a5aee4c Jul 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont use WSL, probably never will, I could use bash that comes with Git for windows but I lose all my gitfified prompt which is not nearly as nice and complete in bash.

I dont think its a good tradeoff just to make this not error. Also I dont understand why this is needed, thers has to be a better way to do this.

I was under the impression that npm shims certain unix commands in the package scripts. Is this not the case?

No idea, clearly not.

@silverwind
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently there is https://github.com/shelljs/shx for this purpose.

@Mottie
Copy link
Member

@Mottie Mottie commented on a5aee4c Jul 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Odd, the rm -rf node_modules works for me. I usually use git bash, but another very nice alternative is https://conemu.github.io/.

@the-j0k3r
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Odd, the rm -rf node_modules works for me

Its not odd at all with git bash, I dont use it though.

@xt0rted
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my scripts I use del-cli which is cross platform. I'm not sure how this would work for deleting the node_modules folder though, I only use it for cleaning up build assets.

@the-j0k3r
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK Ive tried shx it works and it doesnt.

This is the setup

Capture

When I do npm run update it deletes the node_modules folder but it also says it cant find it.

Console output

> npm run update

> [email protected] update C:\Users\Joker\github\Wikipedia-Dark-Upstream
> npx updates -cu && npm run remove && npm install

All packages are up to date.


> [email protected] remove C:\Users\Joker\github\Wikipedia-Dark-Upstream
> shx rm -rf node_modules

The system cannot find the path specified.
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] remove: `shx rm -rf node_modules`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] remove script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm WARN Local package.json exists, but node_modules missing, did you mean to install?

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\Joker\AppData\Roaming\npm-cache\_logs\2019-07-12T07_31_29_742Z-debug.log
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] update: `npx updates -cu && npm run remove && npm install`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] update script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm WARN Local package.json exists, but node_modules missing, did you mean to install?

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\Joker\AppData\Roaming\npm-cache\_logs\2019-07-12T07_31_29_859Z-debug.log

Debug log

0 info it worked if it ends with ok
1 verbose cli [
1 verbose cli   'C:\\Program Files\\nodejs\\node.exe',
1 verbose cli   'C:\\Users\\Joker\\AppData\\Roaming\\npm\\node_modules\\npm\\bin\\npm-cli.js',
1 verbose cli   'run',
1 verbose cli   'update'
1 verbose cli ]
2 info using [email protected]
3 info using [email protected]
4 verbose run-script [ 'preupdate', 'update', 'postupdate' ]
5 info lifecycle [email protected]~preupdate: [email protected]
6 info lifecycle [email protected]~update: [email protected]
7 verbose lifecycle [email protected]~update: unsafe-perm in lifecycle true
8 verbose lifecycle [email protected]~update: PATH: C:\Users\Joker\AppData\Roaming\npm\node_modules\npm\node_modules\npm-lifecycle\node-gyp-bin;C:\Users\Joker\github\Wikipedia-Dark-Upstream\node_modules\.bin;C:\Windows\System32;C:\Windows;C:\Windows\System32\wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\ProgramData\chocolatey\bin;C:\Program Files\Java\jdk1.8.0_162\bin;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem;C:\WINDOWS\System32\WindowsPowerShell\v1.0\;C:\WINDOWS\System32\OpenSSH\;C:\Program Files\Java\jdk1.8.0_172\bin;C:\Program Files\Git\cmd;C:\Program Files (x86)\Gpg4win\..\GnuPG\bin;C:\Program Files\nodejs\;C:\Users\Joker\AppData\Local\Microsoft\WindowsApps;C:\Users\Joker\AppData\Local\atom\bin;C:\Users\Joker\AppData\Local\Microsoft\WindowsApps;C:\Users\Joker\AppData\Roaming\npm
9 verbose lifecycle [email protected]~update: CWD: C:\Users\Joker\github\Wikipedia-Dark-Upstream
10 silly lifecycle [email protected]~update: Args: [ '/d /s /c', 'npx updates -cu && npm run remove && npm install' ]
11 silly lifecycle [email protected]~update: Returned: code: 1  signal: null
12 info lifecycle [email protected]~update: Failed to exec update script
13 verbose stack Error: [email protected] update: `npx updates -cu && npm run remove && npm install`
13 verbose stack Exit status 1
13 verbose stack     at EventEmitter.<anonymous> (C:\Users\Joker\AppData\Roaming\npm\node_modules\npm\node_modules\npm-lifecycle\index.js:301:16)
13 verbose stack     at EventEmitter.emit (events.js:200:13)
13 verbose stack     at ChildProcess.<anonymous> (C:\Users\Joker\AppData\Roaming\npm\node_modules\npm\node_modules\npm-lifecycle\lib\spawn.js:55:14)
13 verbose stack     at ChildProcess.emit (events.js:200:13)
13 verbose stack     at maybeClose (internal/child_process.js:1021:16)
13 verbose stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:283:5)
14 verbose pkgid [email protected]
15 verbose cwd C:\Users\Joker\github\Wikipedia-Dark-Upstream
16 verbose Windows_NT 10.0.17134
17 verbose argv "C:\\Program Files\\nodejs\\node.exe" "C:\\Users\\Joker\\AppData\\Roaming\\npm\\node_modules\\npm\\bin\\npm-cli.js" "run" "update"
18 verbose node v12.4.0
19 verbose npm  v6.10.0
20 error code ELIFECYCLE
21 error errno 1
22 error [email protected] update: `npx updates -cu && npm run remove && npm install`
22 error Exit status 1
23 error Failed at the [email protected] update script.
23 error This is probably not a problem with npm. There is likely additional logging output above.
24 verbose exit [ 1, true ]

Any clews?

@silverwind
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try putting it all in one command npx updates -cu && shx rm -rf node_modules && npm install. I think it's failing because it's deleting itself.

@silverwind
Copy link
Member Author

@silverwind silverwind commented on a5aee4c Jul 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you want to save yourself the headache and install coreutils for windows:

http://gnuwin32.sourceforge.net/packages/coreutils.htm

It looks pretty unmaintained, but I guess those tools never really change much anyways.

@Mottie
Copy link
Member

@Mottie Mottie commented on a5aee4c Jul 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's failing because it's deleting itself.

Ah, yes... I've encountered that issue before. I ended up doing this:

#!/usr/bin/env node
'use strict';

const fs = require('fs');
const rimraf = require('rimraf');

// See https://github.com/isaacs/rimraf/issues/102#issuecomment-412310309
rimraf('node_modules/!(rimraf|.bin)', fs, () => {});

@silverwind
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That wouldn't be a unclean module reinstallation. I have put this rm there because of past issues with npm not properly updating node_modules, maybe it can actually be removed if npm can guarantee a consistent module state without deleting everything before npm install.

@xt0rted
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried using this on Win10 (in the GitHub Dark repo) and it seemed to work as expected:

{
  "scripts": {
    "update": "npx updates -cu && npx rimraf node_modules && npm install"
  }
}

Since that's using npx it should be using a temp copy instead of the local copy correct? If I leave off the npm install portion the entire node_modules folder is deleted so it looks like a clean install happens at the end.

@the-j0k3r
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xt0rted that doesnt error out, but even if there are no updates it proceeds to install everything again.
SO wre adding quite a bit of tme for nothing when there are no updates

@Mottie
Copy link
Member

@Mottie Mottie commented on a5aee4c Jul 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, @silverwind is there a way to get npx updates -cu to return false if there are no updates?

@silverwind
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, but that sounds like a good addition, will add.

@silverwind
Copy link
Member Author

@silverwind silverwind commented on a5aee4c Jul 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, there is a suitable argument already:

    -E, --error-on-outdated       Exit with error code 2 on outdated packages

So this should work:

npx updates -E -c -u || (npx rimraf node_modules && npm install)

It seems like a inverse exit code might actually be more easy to use, hmm.

@silverwind
Copy link
Member Author

@silverwind silverwind commented on a5aee4c Jul 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added that so starting with updates 8.3.0, you can conditionally reinstall modules like this:

npx updates -S -c -u && npx rimraf node_modules && npm install

@silverwind
Copy link
Member Author

@silverwind silverwind commented on a5aee4c Jul 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the command was sillily named. Fixed that by publishing 8.4.0, so it's now -U over -S:

npx updates -cuU && npx rimraf node_modules && npm install

@the-j0k3r if that works for you, feel free to add it to our repositories. Might want to run npm i --save-dev rimraf to have it in the dependencies.

@Mottie
Copy link
Member

@Mottie Mottie commented on a5aee4c Jul 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need to use uU? Wouldn't it be easier to just have it named U?

@silverwind
Copy link
Member Author

@silverwind silverwind commented on a5aee4c Jul 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-u is the actual update action, without it will not write package.json. I prefer to have that separate so arguments stay composable. Thought it probably doesn't make much sense to run -U without -u.

@Mottie
Copy link
Member

@Mottie Mottie commented on a5aee4c Jul 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, what I meant was to have u stay the same, but U act like u but then error if no updates are available.

@Mottie
Copy link
Member

@Mottie Mottie commented on a5aee4c Jul 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But yeah, keeping them separate would probably be a better idea.

Please sign in to comment.