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(bazel): ng_package writes unrelevant definitions to bazel out #27519

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@devversion
Copy link
Member

devversion commented Dec 6, 2018

In order to keep the bazel bin directory as clean as possible, we should not write definition files that are not relevant to a ng_package to an unexpected location in the bazel-bin directory. This currently just happens because we only filter out external definition files while we also should filter out definitions that just aren't in the current package.

The packager.ts file currently tries to write these files to the package output, but fails because those are not inside of the current label's package, so the logic to create a relative path for the file fails and the definition will be copied to a location like:

// Notice the double "bazel-out" here.
C:\Users\Paul\_bazel_Paul\kn4tsvyh\execroot\angular_material\bazel-out\x64_windows-fastbuild\bin\src\bazel-out\x64_windows-fastbuild\bin\src\cdk

function writeFileFromInputPath(inputPath: string, fileContent: string) {
// We want the relative path from the given file to its ancestor "root" directory.
// This root depends on whether the file lives in the source tree (srcDir) as a basic file
// input to ng_package, the bin output tree (binDir) as the output of another rule, or
// the genfiles output tree (genfilesDir) as the output of a genrule.
let rootDir: string;
if (inputPath.includes(binDir)) {
rootDir = binDir;
} else if (inputPath.includes(genfilesDir)) {
rootDir = genfilesDir;
} else {
rootDir = srcDir;
}
const outputPath = path.join(out, path.relative(rootDir, inputPath));
// Always ensure that the target directory exists.
shx.mkdir('-p', path.dirname(outputPath));
fs.writeFileSync(outputPath, fileContent, 'utf-8');
}

This logic writes the definition to an unexpected location (but there is nothing wrong with that logic because we should never pass paths that aren't in the current label's package)

fix(bazel): ng_package writes unrelevant definitions to bazel out
In order to keep the bazel bin directory as clean as possible, we should not write definition files that are not relevant to a `ng_package` to an undesired location in the bazel bin directory. This currently just happens because we only filter out external definition files while we also should filter out definitions that aren't just in the current package.

The `packager.ts` file currently tries to write these files to the package, but fails because those are not inside of the current package. So the logic to create a relative path for the file fails, and the definition will be copied to a location like:

```js
// Notice the double "bazel-out" here.
C:\Users\Paul\_bazel_Paul\kn4tsvyh\execroot\angular_material\bazel-out\x64_windows-fastbuild\bin\src\bazel-out\x64_windows-fastbuild\bin\src\cdk
```

[See logic that causes this](https://github.com/angular/angular/blob/4f9374951d67c75f67a31c110bd61ab72563db7d/packages/bazel/src/ng_package/packager.ts#L105-L124) (nothing wrong with that logic because it assumes that only paths from within the package are passed to it)

@devversion devversion force-pushed the devversion:fix/bazel-unrelevant-bazel-out-definitions-ng-package branch from 0b6ef91 to b3c60ac Dec 6, 2018

@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Dec 6, 2018

@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Dec 6, 2018

@devversion

This comment has been minimized.

Copy link
Member Author

devversion commented Dec 7, 2018

I've been looking into a good way to add a test for this, but I don't think we could ensure that this doesn't happen again in a reliable and generic way.

mhevery added a commit that referenced this pull request Dec 13, 2018

fix(bazel): ng_package writes unrelevant definitions to bazel out (#2…
…7519)

In order to keep the bazel bin directory as clean as possible, we should not write definition files that are not relevant to a `ng_package` to an undesired location in the bazel bin directory. This currently just happens because we only filter out external definition files while we also should filter out definitions that aren't just in the current package.

The `packager.ts` file currently tries to write these files to the package, but fails because those are not inside of the current package. So the logic to create a relative path for the file fails, and the definition will be copied to a location like:

```js
// Notice the double "bazel-out" here.
C:\Users\Paul\_bazel_Paul\kn4tsvyh\execroot\angular_material\bazel-out\x64_windows-fastbuild\bin\src\bazel-out\x64_windows-fastbuild\bin\src\cdk
```

[See logic that causes this](https://github.com/angular/angular/blob/4f9374951d67c75f67a31c110bd61ab72563db7d/packages/bazel/src/ng_package/packager.ts#L105-L124) (nothing wrong with that logic because it assumes that only paths from within the package are passed to it)

PR Close #27519

@mhevery mhevery closed this in 44dfa60 Dec 13, 2018

ngfelixl added a commit to ngfelixl/angular that referenced this pull request Jan 28, 2019

fix(bazel): ng_package writes unrelevant definitions to bazel out (an…
…gular#27519)

In order to keep the bazel bin directory as clean as possible, we should not write definition files that are not relevant to a `ng_package` to an undesired location in the bazel bin directory. This currently just happens because we only filter out external definition files while we also should filter out definitions that aren't just in the current package.

The `packager.ts` file currently tries to write these files to the package, but fails because those are not inside of the current package. So the logic to create a relative path for the file fails, and the definition will be copied to a location like:

```js
// Notice the double "bazel-out" here.
C:\Users\Paul\_bazel_Paul\kn4tsvyh\execroot\angular_material\bazel-out\x64_windows-fastbuild\bin\src\bazel-out\x64_windows-fastbuild\bin\src\cdk
```

[See logic that causes this](https://github.com/angular/angular/blob/4f9374951d67c75f67a31c110bd61ab72563db7d/packages/bazel/src/ng_package/packager.ts#L105-L124) (nothing wrong with that logic because it assumes that only paths from within the package are passed to it)

PR Close angular#27519
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.