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(compiler-cli): ngtsc shim files not being generated on case-insensitive platforms #27466

Conversation

devversion
Copy link
Member

@devversion devversion commented Dec 4, 2018

Common insensitive platforms are win32/win64 (see: here)

Currently when running bazel build packages/core --define=compile=aot, the compiler-cli will throw because it cannot find the index.ngfactory.ts file in the compiler host. This is because the shim host wrapper is not properly generating the requested ngfactory file.

 Error: ENOENT: no such file or directory, open 'C:/users/paul/_bazel_paul/lm3s4mgv/execroot/angular/packages/core/index.ngfactory.ts'
    at Object.openSync (fs.js:434:3)
    at Object.readFileSync (fs.js:339:35)
    at CachedFileLoader.loadFile (C:\users\paul\_bazel_paul\lm3s4mgv\external\internal\tsc_wrapped\cache.ts:366:29)
    at C:\users\paul\_bazel_paul\lm3s4mgv\external\internal\tsc_wrapped\compiler_host.ts:413:34
    at Object.wrap (C:\users\paul\_bazel_paul\lm3s4mgv\external\internal\tsc_wrapped\perf_trace.ts:54:12)
    at CompilerHost.getSourceFile (C:\users\paul\_bazel_paul\lm3s4mgv\external\internal\tsc_wrapped\compiler_host.ts:412:22)
    at GeneratedShimsHostWrapper.getSourceFile (C:\users\paul\_bazel_paul\lm3s4mgv\execroot\angular\packages\compiler-cli\src\ngtsc\shims\src\host.ts:66:26)

This happens because we call getCanonicalFileName that returns a path that is different to the actual rootNames (passed to NgtscProgram) which are used to construct a map of generated files.

Since the generators always use the paths which are not normalized and TypeScript seems to pass "non"-canonical paths to the host getSourceFile, we should be able to stop manually calling getCanonicalFileName (this would then follow typescript internal semantics apparently)


Example:

// Generator Map:

'C:/users/paul/_bazel_paul/lm3s4mgv/execroot/angular/packages/core/index.ngfactory.ts' =>
'C:/users/paul/_bazel_paul/lm3s4mgv/execroot/angular/packages/core/index.ts',

// getSourceFile fileName
C:/users/paul/_bazel_paul/lm3s4mgv/execroot/angular/packages/core/index.ngfactory.ts

// after getCanonicalFileName (notice the lower-case drive name)
c:/users/paul/_bazel_paul/lm3s4mgv/execroot/angular/packages/core/index.ngfactory.ts

See source code of getCanonicalFileName.

@devversion devversion added the target: patch This PR is targeted for the next patch release label Dec 4, 2018
@mary-poppins
Copy link

You can preview f368ade at https://pr27466-f368ade.ngbuilds.io/.

@JoostK
Copy link
Member

JoostK commented Dec 4, 2018

I experience the same issue in macOS (APFS filesystem), so this is not Windows specific. Your fix works for me too 👍

…sitive platforms

Common insensitive platforms are `win32/win64` (see:
[here](https://github.com/Microsoft/TypeScript/blob/3e4c5c95abd515eb9713b881d27ab3a93cc00461/src/compiler/sys.ts#L681-L682))

Currently when running `bazel build packages/core --define=compile=aot`, the `compiler-cli` will throw because it cannot find the `index.ngfactory.ts` file in the compiler host. This is because the shim host wrapper is not properly generating the requested `ngfactory` file.

This happens because we call `getCanonicalFileName` that returns a path that is different to the actual program filenames that are used to construct a map of generated files. Since the generators always use the paths which are not "canonical" and pases them internally like that, we can just stop manually calling `getCanonicalFileName`.
@devversion devversion force-pushed the fix/compiler-cli-ngtsc-shim-files-windows branch from f368ade to 95e6b07 Compare December 4, 2018 23:13
@devversion devversion changed the title fix(compiler-cli): ngtsc shim files not being generated on windows fix(compiler-cli): ngtsc shim files not being generated on case-insensitive platforms Dec 4, 2018
@devversion
Copy link
Member Author

@JoostK Good catch. Didn't know that MacOS (APFS) is case-insensitive. Updated the title/description. Thanks

@mary-poppins
Copy link

You can preview 95e6b07 at https://pr27466-95e6b07.ngbuilds.io/.

@devversion devversion added the action: merge The PR is ready for merge by the caretaker label Dec 6, 2018
@IgorMinar IgorMinar closed this in ca1c430 Dec 6, 2018
IgorMinar pushed a commit that referenced this pull request Dec 6, 2018
…sitive platforms (#27466)

Common insensitive platforms are `win32/win64` (see:
[here](https://github.com/Microsoft/TypeScript/blob/3e4c5c95abd515eb9713b881d27ab3a93cc00461/src/compiler/sys.ts#L681-L682))

Currently when running `bazel build packages/core --define=compile=aot`, the `compiler-cli` will throw because it cannot find the `index.ngfactory.ts` file in the compiler host. This is because the shim host wrapper is not properly generating the requested `ngfactory` file.

This happens because we call `getCanonicalFileName` that returns a path that is different to the actual program filenames that are used to construct a map of generated files. Since the generators always use the paths which are not "canonical" and pases them internally like that, we can just stop manually calling `getCanonicalFileName`.

PR Close #27466
ngfelixl pushed a commit to ngfelixl/angular that referenced this pull request Jan 28, 2019
…sitive platforms (angular#27466)

Common insensitive platforms are `win32/win64` (see:
[here](https://github.com/Microsoft/TypeScript/blob/3e4c5c95abd515eb9713b881d27ab3a93cc00461/src/compiler/sys.ts#L681-L682))

Currently when running `bazel build packages/core --define=compile=aot`, the `compiler-cli` will throw because it cannot find the `index.ngfactory.ts` file in the compiler host. This is because the shim host wrapper is not properly generating the requested `ngfactory` file.

This happens because we call `getCanonicalFileName` that returns a path that is different to the actual program filenames that are used to construct a map of generated files. Since the generators always use the paths which are not "canonical" and pases them internally like that, we can just stop manually calling `getCanonicalFileName`.

PR Close angular#27466
devversion added a commit to devversion/angular that referenced this pull request Feb 19, 2019
…tforms

This change is kind of similar to angular#27466, but instead of ensuring that
these shims can be generated, we also need to make sure that developers
are able to also use the factory shims like with `ngc`.

This issue is now surfacing because we have various old examples which
are now also built with `ngtsc`  (due to the bazel migration). On case insensitive
platforms (e.g. windows) these examples cannot be built because ngtsc fails
the app imports a generated shim file (such as the factory shim files).

This is because the `GeneratedShimsHostWrapper` TypeScript host uses
the `getCanonicalFileName` method in order to check whether a given
file/module exists in the generator file maps. e.g.

```
// Generator Map:
'C:/users/paul/_bazel_paul/lm3s4mgv/execroot/angular/packages/core/index.ngfactory.ts' =>
'C:/users/paul/_bazel_paul/lm3s4mgv/execroot/angular/packages/core/index.ts',

// Path passed into `fileExists`
C:/users/paul/_bazel_paul/lm3s4mgv/execroot/angular/packages/core/index.ngfactory.ts

// After getCanonicalFileName (notice the **lower-case drive name**)
c:/users/paul/_bazel_paul/lm3s4mgv/execroot/angular/packages/core/index.ngfactory.ts
```

As seen above, the generator map does not use the canonical file names, as well as
TypeScript internally does not pass around canonical file names. We can fix this by removing
the manual call to `getCanonicalFileName` and just following TypeScript internal-semantics.
benlesh pushed a commit that referenced this pull request Feb 21, 2019
…tforms (#28831)

This change is kind of similar to #27466, but instead of ensuring that
these shims can be generated, we also need to make sure that developers
are able to also use the factory shims like with `ngc`.

This issue is now surfacing because we have various old examples which
are now also built with `ngtsc`  (due to the bazel migration). On case insensitive
platforms (e.g. windows) these examples cannot be built because ngtsc fails
the app imports a generated shim file (such as the factory shim files).

This is because the `GeneratedShimsHostWrapper` TypeScript host uses
the `getCanonicalFileName` method in order to check whether a given
file/module exists in the generator file maps. e.g.

```
// Generator Map:
'C:/users/paul/_bazel_paul/lm3s4mgv/execroot/angular/packages/core/index.ngfactory.ts' =>
'C:/users/paul/_bazel_paul/lm3s4mgv/execroot/angular/packages/core/index.ts',

// Path passed into `fileExists`
C:/users/paul/_bazel_paul/lm3s4mgv/execroot/angular/packages/core/index.ngfactory.ts

// After getCanonicalFileName (notice the **lower-case drive name**)
c:/users/paul/_bazel_paul/lm3s4mgv/execroot/angular/packages/core/index.ngfactory.ts
```

As seen above, the generator map does not use the canonical file names, as well as
TypeScript internally does not pass around canonical file names. We can fix this by removing
the manual call to `getCanonicalFileName` and just following TypeScript internal-semantics.

PR Close #28831
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants