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

fix: angular example ts_scripts path in Windows #1605

Merged
merged 1 commit into from
Feb 5, 2020

Conversation

alan-agius4
Copy link
Contributor

@alan-agius4 alan-agius4 commented Feb 5, 2020

In Windows _/ts_scripts.js will be expanded to C:/Program Files/Git/ts_scripts.js.

[
  'C:\\users\\alag\\_bazel_alag\\rj2iva3m\\external\\build_bazel_rules_nodejs\\internal\\node\\_node_bin\\node',
  'C:\\users\\alag\\_bazel_alag\\rj2iva3m\\execroot\\examples_angular\\node_modules\\html-insert-assets\\src\\main.js',
  '--html=src/example/index.html',
  '--out=bazel-out/x64_windows-fastbuild/bin/src/index.html',
  '--roots=.',
  'bazel-out/x64_windows-fastbuild/bin/src',
  '--assets',
  'bazel-out/x64_windows-fastbuild/bin/src/styles.css',
  'external/npm/node_modules/@angular/material/prebuilt-themes/deeppurple-amber.css',
  'external/npm/node_modules/zone.js/dist/zone.min.js',
  './_C:/Program Files/Git/ts_scripts.js'
]

This seems like some character escaping issue but in general escaping is hell and this is a more straightforward fix.

Partially addresses: #1604

@alan-agius4
Copy link
Contributor Author

alan-agius4 commented Feb 5, 2020

CI failure seems to be unrelated

  | //examples:examples_angular                                              PASSED in 211.7s
  | //examples:examples_angular_view_engine                                  PASSED in 153.8s
  | //examples:examples_app                                                  PASSED in 29.4s
  | //examples:examples_closure                                              PASSED in 19.8s
  | //examples:examples_jest                                                 PASSED in 23.5s
  | //examples:examples_kotlin                                               PASSED in 65.5s
  | //examples:examples_nestjs                                               PASSED in 46.9s
  | //examples:examples_parcel                                               PASSED in 41.3s
  | //examples:examples_protocol_buffers                                     PASSED in 68.8s
  | //examples:examples_react_webpack                                        PASSED in 21.6s
  | //examples:examples_vendored_node                                        PASSED in 23.2s
  | //examples:examples_vendored_node_and_yarn                               PASSED in 18.6s
  | //examples:examples_web_testing                                          PASSED in 38.1s
  | //examples:examples_webapp                                               PASSED in 31.6s
  | //examples:examples_worker                                               PASSED in 9.1s
  | //examples:examples_angular_bazel_architect                              FAILED in 3 out of 3 in 52.0s

@gregmagolan
Copy link
Collaborator

Yes. CI failure was due to inflight conflict with two PRs. Fix is here: #1607. Not related to this.

@gregmagolan
Copy link
Collaborator

Thanks @alan-agius4 . We need some Windows coverage on this example. The /integration/angular is also not tested on Windows on angular repo AFAIK.

Copy link
Collaborator

@gregmagolan gregmagolan left a comment

Choose a reason for hiding this comment

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

Can you also make the same change in /examples/angular_view_engine?
Also rebase to master and it should be green now.

In Windows `_/ts_scripts.js` will be expanded to `C:/Program Files/Git/ts_scripts.js`.

```
[
  'C:\\users\\alag\\_bazel_alag\\rj2iva3m\\external\\build_bazel_rules_nodejs\\internal\\node\\_node_bin\\node',
  'C:\\users\\alag\\_bazel_alag\\rj2iva3m\\execroot\\examples_angular\\node_modules\\html-insert-assets\\src\\main.js',
  '--html=src/example/index.html',
  '--out=bazel-out/x64_windows-fastbuild/bin/src/index.html',
  '--roots=.',
  'bazel-out/x64_windows-fastbuild/bin/src',
  '--assets',
  'bazel-out/x64_windows-fastbuild/bin/src/styles.css',
  'external/npm/node_modules/@angular/material/prebuilt-themes/deeppurple-amber.css',
  'external/npm/node_modules/zone.js/dist/zone.min.js',
  './_C:/Program Files/Git/ts_scripts.js'
]
```

This seems like some character escaping issue but in general escaping is hell and this is a more straightforward fix.

Partially addresses: #1604
@alan-agius4
Copy link
Contributor Author

@gregmagolan done!

Copy link
Collaborator

@gregmagolan gregmagolan left a comment

Choose a reason for hiding this comment

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

Thank you

@alexeagle alexeagle merged commit 30d0f37 into master Feb 5, 2020
@alan-agius4 alan-agius4 deleted the alan-agius4-windows-example branch February 5, 2020 20:47
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.

4 participants