feat: support win32 by waitingsong · Pull Request #11 · eggjs/scripts

waitingsong · 2017-11-07T09:09:41Z

it should stop with --port if start cluster with custom port number

Checklist

npm test passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

Description of change

it should stop with --port if start cluster with custom port number

codecov-io · 2017-11-07T09:20:41Z

Codecov Report

Merging #11 into master will decrease coverage by 15.22%.
The diff coverage is 28.2%.

@@             Coverage Diff             @@
##           master      #11       +/-   ##
===========================================
- Coverage     100%   84.77%   -15.23%     
===========================================
  Files           6        6               
  Lines         169      197       +28     
===========================================
- Hits          169      167        -2     
- Misses          0       30       +30

Impacted Files	Coverage Δ
lib/cmd/stop.js	`93.54% <100%> (-6.46%)`	⬇️
lib/helper.js	`48.78% <16%> (-51.22%)`	⬇️
lib/cmd/start.js	`93.39% <46.15%> (-6.61%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0016e28...9199d58. Read the comment docs.

fengmk2 · 2017-11-07T10:33:03Z

@atian25 add appveyor tests

atian25 · 2017-11-07T10:40:29Z

but egg-scripts start is not support windows yet, so appveyor will break, should we still open it?

waitingsong · 2017-11-07T10:42:24Z

I run test local all passed under win7. not support ?

atian25 · 2017-11-07T10:44:32Z

https://github.com/eggjs/egg-scripts/blob/master/lib/cmd/start.js#L180-L196

tails is support at win? maybe you had install some tool such as git bin (with an options to install bash tools)

waitingsong · 2017-11-07T10:46:36Z

oh, git installed .maybe can be fixed for win32

waitingsong · 2017-11-07T11:09:54Z

 stdout = stdout.split(/\n+/g).splice(0, 100);
 +            this.logger.error(stdout);
 +            this.logger.error('Start failed, see %s', stderr);

Whether need convert stdout to string by stdout.join('\n') ?

popomore · 2017-11-07T12:10:06Z

lib/cmd/start.js

+            break;
+          } else {
+            const str = fs.readFileSync(stderr, 'utf-8');
+            let stdout = str ? str.slice(0, 50000).replace(/\r/g, '\n') : '';


you can simply print all on windows

我是参考源代码 tail 前100行，故以一行500字节*100行计算来截取 50000字节。

popomore · 2017-11-07T12:11:44Z

lib/cmd/stop.js

-    }
-
    const { argv } = context;
+    const port = argv.port || (process.env && process.env.PORT);


Is there other way to stop without port

有其他方法。但相当复杂
1、修改egg-cluster, 在启动 worker 进程时，更新窗口的 title
2、在关闭服务时借助node-ffi 找到任何一个匹配 title 的 worker 窗口获取其pid
3、借助node-ffi 调用 ntdll.NtQueryInformationProcess() 方法，根据pid找到父级pid（即master进程），然后kill。

我计划实现（1, 2），并且可根据参数隐藏worker的窗口。
因为master进程没有窗口，也就无法设置其窗口 title，所以只有在 worker 上面设置了。否则就可以跳过（3）直接找到 master 进程窗口然后 kill 之。

（3）这个 NtQueryInformationProcess 需要的参数结构Struct极其复杂，目前搞不定。

Or we can store port when app starts, so it’s unnecessary to give the port argument.

能记录所有进程 pid 那当然就方便了

atian25 · 2017-11-07T13:31:00Z

so just like Nginx `run/pid`? @popomore 发自我的 iPhone

…

在 2017年11月7日，21:19，waiting ***@***.***> 写道： @waitingsong commented on this pull request. In lib/cmd/stop.js: > const { argv } = context; + const port = argv.port || (process.env && process.env.PORT); 能记录所有进程 pid 那当然就方便 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

atian25 · 2017-11-08T00:24:54Z

https://ci.appveyor.com/project/eggjs/egg-scripts

had enabled, need an empty commit to trigger it.

btw, the cov is low.

waitingsong · 2017-11-08T02:09:35Z

commit one, but seems not triggered. only master branch?

atian25 · 2017-11-08T02:23:35Z

try again and see, just delete and recreate it.

atian25 · 2017-11-13T03:09:20Z

lib/cmd/start.js

+            isSuccess = false;
+            break;
+          } else {
+            const str = fs.readFileSync(stderr, 'utf-8');


use yield fs.readFile

feat: support win32

5c5d45b

it should stop with --port if start cluster with custom port number

waitingsong added 2 commits November 7, 2017 18:21

feat: export helper.findNodeProcessWin()

eaf7ac4

test: helper.findNodeProcessWin()

6faa1e5

fengmk2 assigned atian25 Nov 7, 2017

waitingsong added 2 commits November 7, 2017 19:02

feat: support win32 within checkStatus()

28cf6f3

fix: win32 typo

d4982e2

fix: convert stdout to string within checkStatus()

4fe668f

popomore reviewed Nov 7, 2017

View reviewed changes

dummy

c3eda38

waitingsong added 3 commits November 8, 2017 10:34

dummy

7e63114

fix: drop duplicate pid value from ipv4/v6 within findNodeProcessWin()

6b191d2

Format style

5fa56fb

atian25 reviewed Nov 13, 2017

View reviewed changes

lib/cmd/start.js Outdated

isSuccess = false;

break;

} else {

const str = fs.readFileSync(stderr, 'utf-8');

Copy link

Member

atian25 Nov 13, 2017

Choose a reason for hiding this comment

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

use yield fs.readFile

Use yield instead of sync read

03bd2f9

popomore mentioned this pull request Nov 14, 2017

fix: should stop app when baseDir is symlink #12

Merged

4 tasks

Merge branch 'master' into win_support

9199d58

waitingsong closed this Apr 4, 2018

Conversation

waitingsong commented Nov 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Affected core subsystem(s)

Description of change

Uh oh!

codecov-io commented Nov 7, 2017 • edited by codecov bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

fengmk2 commented Nov 7, 2017

Uh oh!

atian25 commented Nov 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

waitingsong commented Nov 7, 2017

Uh oh!

atian25 commented Nov 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

waitingsong commented Nov 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

waitingsong commented Nov 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

popomore Nov 7, 2017

Choose a reason for hiding this comment

Uh oh!

waitingsong Nov 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

popomore Nov 7, 2017

Choose a reason for hiding this comment

Uh oh!

waitingsong Nov 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

popomore Nov 7, 2017

Choose a reason for hiding this comment

Uh oh!

waitingsong Nov 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atian25 commented Nov 7, 2017 via email

Uh oh!

atian25 commented Nov 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

waitingsong commented Nov 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

atian25 commented Nov 8, 2017

Uh oh!

atian25 Nov 13, 2017

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

waitingsong commented Nov 7, 2017 •

edited

Loading

codecov-io commented Nov 7, 2017 •

edited by codecov bot

Loading

atian25 commented Nov 7, 2017 •

edited

Loading

atian25 commented Nov 7, 2017 •

edited

Loading

waitingsong commented Nov 7, 2017 •

edited

Loading

waitingsong commented Nov 7, 2017 •

edited

Loading

waitingsong Nov 7, 2017 •

edited

Loading

waitingsong Nov 7, 2017 •

edited

Loading

waitingsong Nov 7, 2017 •

edited

Loading

atian25 commented Nov 8, 2017 •

edited

Loading

waitingsong commented Nov 8, 2017 •

edited

Loading