Fix devserver on windows #327

devversion · 2018-11-24T10:47:55Z

No description provided.

googlebot · 2018-11-24T10:47:57Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

examples/devserver/index.html

devserver/devserver/devserver.go

filipesilva

Overall LGTM, just failing some tests.

devserver/main.go

devserver/devserver/BUILD.bazel

devversion · 2018-11-26T21:56:48Z

@alexeagle @gregmagolan @filipesilva This one is ready now. Can you please have a look? thanks!

filipesilva · 2018-11-27T09:36:08Z

@googlebot I am ok with my commits being contributed to this project.

filipesilva

LGTM, great work @devversion!

alexeagle

Could you minimize the Delta in Go code? This runs in Google3 also. We'll need another reviewer for that part

devversion · 2018-11-27T12:49:36Z

@alexeagle Unfortunately I don't think that I can reduce the diff here. Especially because the Go code is the breaking part for the runfiles on Windows.

The go code currently just assumes that all runfiles are relative to the resolved root directory. This is not the case on Windows, and therefore we now resolve through the Bazel runfile utilities from Go.

Can you elaborate what specific part runs inside of Google3? the whole devserver or just the individual parts like concatjs? I've made concatjs on its own backwards compatible, so it still runs the realFileSystem respecting the passed root directory if no custom file system has been specified.

devversion · 2018-12-22T17:03:44Z

The failure of ci/circleci: test is unrelated. Other PRs seem to run into similar flakiness. Probably because of the resources Bazel acquires in the CircleCI containers.

devversion · 2018-12-22T17:04:38Z

@alexeagle If you find some time, could you start the tests again? 😄 (Waiting for a project member to verify this pull request.)

evmar

I had two minor comments, but I don't think I'm a good reviewer for this anyway so don't let me stop you.

devserver/concatjs/concatjs.go

googlebot · 2019-01-15T01:26:25Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

googlebot · 2019-01-15T01:29:29Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

googlebot · 2019-01-15T01:30:01Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

googlebot · 2019-01-16T18:21:32Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

googlebot · 2019-01-16T18:22:01Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

…e helpers

googlebot · 2019-01-17T19:01:57Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

googlebot · 2019-01-17T19:14:25Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

alexeagle · 2019-01-18T15:35:51Z

Note, we didn't merge the whole change, still needs another PR to enable the test on windows, by removing:
https://github.com/bazelbuild/rules_typescript/blob/master/.bazelci/presubmit.yml#L54

Both bazelbuild/rules_typescript#327 and bazelbuild/rules_typescript#378 have circleci `test` jobs being killed: ``` Server terminated abruptly (error code: 14, error message: '', log file: '/home/circleci/.cache/bazel/_bazel_circleci/cced162ff126e466524550c57f0d7df5/server/jvm.out') ``` According to the note in https://github.com/bazelbuild/rules_typescript/blob/3671b8d5b6f5731e191098b6d5243c44f474c0da/.circleci/bazel.rc#L32-L38 this means the memory limit should be reduced. Closes bazel-contrib#380 PiperOrigin-RevId: 228881108

Both bazelbuild/rules_typescript#327 and bazelbuild/rules_typescript#378 have circleci `test` jobs being killed: ``` Server terminated abruptly (error code: 14, error message: '', log file: '/home/circleci/.cache/bazel/_bazel_circleci/cced162ff126e466524550c57f0d7df5/server/jvm.out') ``` According to the note in https://github.com/bazelbuild/rules_typescript/blob/845b16ca5f8202df3c3f845df2e9629e85217397/.circleci/bazel.rc#L32-L38 this means the memory limit should be reduced. Closes bazel-contrib#380 PiperOrigin-RevId: 228881108

googlebot added the cla: no label Nov 24, 2018

devversion commented Nov 26, 2018

View reviewed changes

examples/devserver/index.html Outdated Show resolved Hide resolved

devserver/devserver/devserver.go Outdated Show resolved Hide resolved

filipesilva reviewed Nov 26, 2018

View reviewed changes

devserver/main.go Outdated Show resolved Hide resolved

filipesilva reviewed Nov 26, 2018

View reviewed changes

devserver/devserver/BUILD.bazel Outdated Show resolved Hide resolved

filipesilva mentioned this pull request Nov 26, 2018

native.sh_binary converts binary arguments starting with / bazelbuild/bazel#6764

Closed

devversion force-pushed the wip/devserver-windows branch 2 times, most recently from 41d0812 to 88f358b Compare November 26, 2018 19:26

devversion changed the title ~~Testing with devserver on windows~~ Fix devserver on windows Nov 27, 2018

devversion force-pushed the wip/devserver-windows branch from 3e99790 to f62e6f1 Compare November 27, 2018 09:30

filipesilva mentioned this pull request Nov 27, 2018

[WIP] ts_devserver on Windows #283

Closed

filipesilva approved these changes Nov 27, 2018

View reviewed changes

alexeagle reviewed Nov 27, 2018

View reviewed changes

devversion mentioned this pull request Dec 9, 2018

feat: depend on devserver binaries #347

Closed

alexeagle requested a review from evmar December 12, 2018 23:16

devversion force-pushed the wip/devserver-windows branch 7 times, most recently from 4ea44e3 to 3d38e11 Compare December 22, 2018 16:50

devversion force-pushed the wip/devserver-windows branch from c242b94 to 1b52aa7 Compare December 26, 2018 15:46

evmar reviewed Jan 8, 2019

View reviewed changes

devserver/concatjs/concatjs.go Outdated Show resolved Hide resolved

devserver/concatjs/concatjs.go Outdated Show resolved Hide resolved

evmar self-requested a review January 8, 2019 20:50

googlebot added the cla: no label Jan 15, 2019

mgechev added cla: yes and removed cla: no labels Jan 15, 2019

Update method name

cc1a219

mgechev force-pushed the wip/devserver-windows branch from b14160d to cc1a219 Compare January 15, 2019 01:29

googlebot added cla: no and removed cla: yes labels Jan 15, 2019

mgechev added cla: yes and removed cla: no labels Jan 15, 2019

Do not depend on external rules_go package

2e761b5

googlebot added cla: no and removed cla: yes labels Jan 16, 2019

mgechev added cla: yes and removed cla: no labels Jan 16, 2019

Add logic to resolve runfiles within G3 without using external runfil…

1bf1739

…e helpers

googlebot added cla: no and removed cla: yes labels Jan 17, 2019

mgechev added cla: yes and removed cla: no labels Jan 17, 2019

alexeagle closed this in 4155687 Jan 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix devserver on windows #327

Fix devserver on windows #327

devversion commented Nov 24, 2018 •

edited

Loading

googlebot commented Nov 24, 2018

filipesilva left a comment

devversion commented Nov 26, 2018 •

edited

Loading

filipesilva commented Nov 27, 2018

filipesilva left a comment

alexeagle left a comment

devversion commented Nov 27, 2018 •

edited

Loading

devversion commented Dec 22, 2018

devversion commented Dec 22, 2018

evmar left a comment

googlebot commented Jan 15, 2019

googlebot commented Jan 15, 2019

googlebot commented Jan 15, 2019

googlebot commented Jan 16, 2019

googlebot commented Jan 16, 2019

googlebot commented Jan 17, 2019

googlebot commented Jan 17, 2019

alexeagle commented Jan 18, 2019

Fix devserver on windows #327

Fix devserver on windows #327

Conversation

devversion commented Nov 24, 2018 • edited Loading

googlebot commented Nov 24, 2018

filipesilva left a comment

Choose a reason for hiding this comment

devversion commented Nov 26, 2018 • edited Loading

filipesilva commented Nov 27, 2018

filipesilva left a comment

Choose a reason for hiding this comment

alexeagle left a comment

Choose a reason for hiding this comment

devversion commented Nov 27, 2018 • edited Loading

devversion commented Dec 22, 2018

devversion commented Dec 22, 2018

evmar left a comment

Choose a reason for hiding this comment

googlebot commented Jan 15, 2019

googlebot commented Jan 15, 2019

googlebot commented Jan 15, 2019

googlebot commented Jan 16, 2019

googlebot commented Jan 16, 2019

googlebot commented Jan 17, 2019

googlebot commented Jan 17, 2019

alexeagle commented Jan 18, 2019

devversion commented Nov 24, 2018 •

edited

Loading

devversion commented Nov 26, 2018 •

edited

Loading

devversion commented Nov 27, 2018 •

edited

Loading