fix(bazel): ng_package cannot be run multiple times without clean (#27200)
Currently when building the `ng_package` multiple times, the old `ng_package` output will be copied over to the new `ng_package` content. Resulting in packages like `src/cdk/npm_package/npm_package/npm_package/AND_MORE`. This happens because currently all TypeScript definition files are resolved from within the `binDir`. This is just wrong because it could then take up the `d.ts` files from the previous `ng_package` output. All typescript definitions that belong to the target package, should be resolved through Bazel and copied based on that computation. Also fixes that `esm` files aren't written to the `ng_package` on Windows. This is because we try to flatten paths using the `path.delimiter` while the path is always using Posix delimiters (causing the paths to be incorrect) PR Close #27200
This commit is contained in:
		
							parent
							
								
									20a2bae1d3
								
							
						
					
					
						commit
						4f9374951d
					
				
							
								
								
									
										41
									
								
								packages/bazel/src/ng_package/collect-type-definitions.bzl
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										41
									
								
								packages/bazel/src/ng_package/collect-type-definitions.bzl
									
									
									
									
									
										Normal file
									
								
							| @ -0,0 +1,41 @@ | |||||||
|  | # Copyright Google LLC. All Rights Reserved. | ||||||
|  | # | ||||||
|  | # Use of this source code is governed by an MIT-style license that can be | ||||||
|  | # found in the LICENSE file at https://angular.io/license | ||||||
|  | 
 | ||||||
|  | """Collect TypeScript definition files from a rule context. | ||||||
|  | 
 | ||||||
|  | This is used to find all files that will be copied into a "ng_package". | ||||||
|  | """ | ||||||
|  | 
 | ||||||
|  | def _filter_typing_files(files): | ||||||
|  |     return [file for file in files if file.path.endswith(".d.ts")] | ||||||
|  | 
 | ||||||
|  | def collect_type_definitions(ctx): | ||||||
|  |     """Returns a file tree containing only TypeScript definition files. | ||||||
|  | 
 | ||||||
|  |     This is useful when packaging a "ng_package" where we only want to package specified | ||||||
|  |     definition files. | ||||||
|  | 
 | ||||||
|  |     Args: | ||||||
|  |       ctx: ctx. | ||||||
|  | 
 | ||||||
|  |     Returns: | ||||||
|  |       A file tree containing only TypeScript definition files. | ||||||
|  |     """ | ||||||
|  | 
 | ||||||
|  |     # Add all source files and filter for TypeScript definition files | ||||||
|  |     # See: https://docs.bazel.build/versions/master/skylark/lib/File.html#is_source | ||||||
|  |     collected_files = _filter_typing_files([d for d in ctx.files.deps if d.is_source]) | ||||||
|  | 
 | ||||||
|  |     # In case source files have been explicitly specified in the attributes, just collect | ||||||
|  |     # them and filter for definition files. | ||||||
|  |     if hasattr(ctx.attr, "srcs"): | ||||||
|  |         collected_files += _filter_typing_files(ctx.files.srcs) | ||||||
|  | 
 | ||||||
|  |     # Collect all TypeScript definition files from the specified dependencies. | ||||||
|  |     for dep in ctx.attr.deps: | ||||||
|  |         if hasattr(dep, "typescript"): | ||||||
|  |             collected_files += dep.typescript.transitive_declarations.to_list() | ||||||
|  | 
 | ||||||
|  |     return collected_files | ||||||
| @ -30,6 +30,7 @@ load( | |||||||
| load("@build_bazel_rules_nodejs//:internal/node.bzl", "sources_aspect") | load("@build_bazel_rules_nodejs//:internal/node.bzl", "sources_aspect") | ||||||
| load("@build_bazel_rules_nodejs//internal/common:node_module_info.bzl", "NodeModuleInfo") | load("@build_bazel_rules_nodejs//internal/common:node_module_info.bzl", "NodeModuleInfo") | ||||||
| load("//packages/bazel/src:esm5.bzl", "esm5_outputs_aspect", "esm5_root_dir", "flatten_esm5") | load("//packages/bazel/src:esm5.bzl", "esm5_outputs_aspect", "esm5_root_dir", "flatten_esm5") | ||||||
|  | load("//packages/bazel/src/ng_package:collect-type-definitions.bzl", "collect_type_definitions") | ||||||
| 
 | 
 | ||||||
| _DEFAULT_NG_PACKAGER = "@npm//@angular/bazel/bin:packager" | _DEFAULT_NG_PACKAGER = "@npm//@angular/bazel/bin:packager" | ||||||
| 
 | 
 | ||||||
| @ -153,11 +154,20 @@ def _flatten_paths(directory): | |||||||
|     return result |     return result | ||||||
| 
 | 
 | ||||||
| # takes an depset of files and returns an array that doesn't contain any generated files by ngc | # takes an depset of files and returns an array that doesn't contain any generated files by ngc | ||||||
| def _filter_out_generated_files(files): | def _filter_out_generated_files(files, extension, filter_external_files): | ||||||
|     result = [] |     result = [] | ||||||
|     for file in files: |     for file in files: | ||||||
|         if (not (file.path.endswith(".ngfactory.js") or file.path.endswith(".ngsummary.js") or file.path.endswith(".ngstyle.js"))): |         # If the "filter_external_files" parameter has been set to true, filter out files | ||||||
|  |         # that refer to external workspaces. | ||||||
|  |         if filter_external_files and file.short_path.startswith("../"): | ||||||
|  |             continue | ||||||
|  | 
 | ||||||
|  |         # Filter out files that are generated by the Angular Compiler CLI. | ||||||
|  |         if (not (file.path.endswith(".ngfactory.%s" % extension) or | ||||||
|  |                  file.path.endswith(".ngsummary.%s" % extension) or | ||||||
|  |                  file.path.endswith(".ngstyle.%s" % extension))): | ||||||
|             result.append(file) |             result.append(file) | ||||||
|  | 
 | ||||||
|     return depset(result) |     return depset(result) | ||||||
| 
 | 
 | ||||||
| def _esm2015_root_dir(ctx): | def _esm2015_root_dir(ctx): | ||||||
| @ -174,8 +184,9 @@ def _filter_js_inputs(all_inputs): | |||||||
| def _ng_package_impl(ctx): | def _ng_package_impl(ctx): | ||||||
|     npm_package_directory = ctx.actions.declare_directory("%s.ng_pkg" % ctx.label.name) |     npm_package_directory = ctx.actions.declare_directory("%s.ng_pkg" % ctx.label.name) | ||||||
| 
 | 
 | ||||||
|     esm_2015_files = _filter_out_generated_files(collect_es6_sources(ctx)) |     esm_2015_files = _filter_out_generated_files(collect_es6_sources(ctx), "js", False) | ||||||
|     esm5_sources = _filter_out_generated_files(flatten_esm5(ctx)) |     esm5_sources = _filter_out_generated_files(flatten_esm5(ctx), "js", False) | ||||||
|  |     type_definitions = _filter_out_generated_files(collect_type_definitions(ctx), "d.ts", True) | ||||||
| 
 | 
 | ||||||
|     # These accumulators match the directory names where the files live in the |     # These accumulators match the directory names where the files live in the | ||||||
|     # Angular package format. |     # Angular package format. | ||||||
| @ -284,11 +295,7 @@ def _ng_package_impl(ctx): | |||||||
|         ctx.files.srcs + |         ctx.files.srcs + | ||||||
|         ctx.files.data + |         ctx.files.data + | ||||||
|         esm5_sources.to_list() + |         esm5_sources.to_list() + | ||||||
|         depset(transitive = [ |         type_definitions.to_list() + | ||||||
|             d.typescript.transitive_declarations |  | ||||||
|             for d in ctx.attr.deps |  | ||||||
|             if hasattr(d, "typescript") |  | ||||||
|         ]).to_list() + |  | ||||||
|         [f.js for f in fesm2015 + fesm5 + esm2015 + esm5 + bundles] + |         [f.js for f in fesm2015 + fesm5 + esm2015 + esm5 + bundles] + | ||||||
|         [f.map for f in fesm2015 + fesm5 + esm2015 + esm5 + bundles if f.map] |         [f.map for f in fesm2015 + fesm5 + esm2015 + esm5 + bundles if f.map] | ||||||
|     ) |     ) | ||||||
| @ -321,15 +328,16 @@ def _ng_package_impl(ctx): | |||||||
|         # placeholder |         # placeholder | ||||||
|         packager_args.add("") |         packager_args.add("") | ||||||
| 
 | 
 | ||||||
|     packager_args.add_joined(_flatten_paths(fesm2015), join_with = ",") |     packager_args.add_joined(_flatten_paths(fesm2015), join_with = ",", omit_if_empty = False) | ||||||
|     packager_args.add_joined(_flatten_paths(fesm5), join_with = ",") |     packager_args.add_joined(_flatten_paths(fesm5), join_with = ",", omit_if_empty = False) | ||||||
|     packager_args.add_joined(_flatten_paths(esm2015), join_with = ",") |     packager_args.add_joined(_flatten_paths(esm2015), join_with = ",", omit_if_empty = False) | ||||||
|     packager_args.add_joined(_flatten_paths(esm5), join_with = ",") |     packager_args.add_joined(_flatten_paths(esm5), join_with = ",", omit_if_empty = False) | ||||||
|     packager_args.add_joined(_flatten_paths(bundles), join_with = ",") |     packager_args.add_joined(_flatten_paths(bundles), join_with = ",", omit_if_empty = False) | ||||||
|     packager_args.add_joined([s.path for s in ctx.files.srcs], join_with = ",") |     packager_args.add_joined([s.path for s in ctx.files.srcs], join_with = ",", omit_if_empty = False) | ||||||
|  |     packager_args.add_joined([s.path for s in type_definitions], join_with = ",", omit_if_empty = False) | ||||||
| 
 | 
 | ||||||
|     # TODO: figure out a better way to gather runfiles providers from the transitive closure. |     # TODO: figure out a better way to gather runfiles providers from the transitive closure. | ||||||
|     packager_args.add_joined([d.path for d in ctx.files.data], join_with = ",") |     packager_args.add_joined([d.path for d in ctx.files.data], join_with = ",", omit_if_empty = False) | ||||||
| 
 | 
 | ||||||
|     if ctx.file.license_banner: |     if ctx.file.license_banner: | ||||||
|         packager_inputs.append(ctx.file.license_banner) |         packager_inputs.append(ctx.file.license_banner) | ||||||
|  | |||||||
| @ -73,6 +73,9 @@ function main(args: string[]): number { | |||||||
|       // List of all files in the ng_package rule's srcs.
 |       // List of all files in the ng_package rule's srcs.
 | ||||||
|       srcsArg, |       srcsArg, | ||||||
| 
 | 
 | ||||||
|  |       // List of all type definitions that need to packaged into the ng_package.
 | ||||||
|  |       typeDefinitionsArg, | ||||||
|  | 
 | ||||||
|       // List of all files in the ng_package rule's data.
 |       // List of all files in the ng_package rule's data.
 | ||||||
|       dataArg, |       dataArg, | ||||||
| 
 | 
 | ||||||
| @ -85,6 +88,7 @@ function main(args: string[]): number { | |||||||
|   const esm2015 = esm2015Arg.split(',').filter(s => !!s); |   const esm2015 = esm2015Arg.split(',').filter(s => !!s); | ||||||
|   const esm5 = esm5Arg.split(',').filter(s => !!s); |   const esm5 = esm5Arg.split(',').filter(s => !!s); | ||||||
|   const bundles = bundlesArg.split(',').filter(s => !!s); |   const bundles = bundlesArg.split(',').filter(s => !!s); | ||||||
|  |   const typeDefinitions = typeDefinitionsArg.split(',').filter(s => !!s); | ||||||
|   const srcs = srcsArg.split(',').filter(s => !!s); |   const srcs = srcsArg.split(',').filter(s => !!s); | ||||||
|   const dataFiles: string[] = dataArg.split(',').filter(s => !!s); |   const dataFiles: string[] = dataArg.split(',').filter(s => !!s); | ||||||
|   const modulesManifest = JSON.parse(modulesManifestArg); |   const modulesManifest = JSON.parse(modulesManifestArg); | ||||||
| @ -134,7 +138,8 @@ function main(args: string[]): number { | |||||||
|    * @param outDir path where we copy the file, relative to the out |    * @param outDir path where we copy the file, relative to the out | ||||||
|    */ |    */ | ||||||
|   function writeEsmFile(file: string, suffix: string, outDir: string) { |   function writeEsmFile(file: string, suffix: string, outDir: string) { | ||||||
|     const root = file.substr(0, file.lastIndexOf(suffix + path.sep) + suffix.length + 1); |     // Note that the specified file path is always using the posix path delimiter.
 | ||||||
|  |     const root = file.substr(0, file.lastIndexOf(`${suffix}/`) + suffix.length + 1); | ||||||
|     const rel = path.dirname(path.relative(path.join(root, srcDir), file)); |     const rel = path.dirname(path.relative(path.join(root, srcDir), file)); | ||||||
|     if (!rel.startsWith('..')) { |     if (!rel.startsWith('..')) { | ||||||
|       copyFile(file, path.join(out, outDir), rel); |       copyFile(file, path.join(out, outDir), rel); | ||||||
| @ -148,8 +153,9 @@ function main(args: string[]): number { | |||||||
|   fesm2015.forEach(file => { copyFile(file, out, 'fesm2015'); }); |   fesm2015.forEach(file => { copyFile(file, out, 'fesm2015'); }); | ||||||
|   fesm5.forEach(file => { copyFile(file, out, 'fesm5'); }); |   fesm5.forEach(file => { copyFile(file, out, 'fesm5'); }); | ||||||
| 
 | 
 | ||||||
|   const allsrcs = shx.find('-R', binDir); |   // Copy all type definitions into the package. This is necessary so that developers can use
 | ||||||
|   allsrcs.filter(hasFileExtension('.d.ts')).forEach((f: string) => { |   // the package with type definitions.
 | ||||||
|  |   typeDefinitions.forEach((f: string) => { | ||||||
|     const content = fs.readFileSync(f, 'utf-8') |     const content = fs.readFileSync(f, 'utf-8') | ||||||
|                         // Strip the named AMD module for compatibility with non-bazel users
 |                         // Strip the named AMD module for compatibility with non-bazel users
 | ||||||
|                         .replace(/^\/\/\/ <amd-module name=.*\/>\n/gm, ''); |                         .replace(/^\/\/\/ <amd-module name=.*\/>\n/gm, ''); | ||||||
| @ -235,12 +241,6 @@ function main(args: string[]): number { | |||||||
|     return `./${result}`; |     return `./${result}`; | ||||||
|   } |   } | ||||||
| 
 | 
 | ||||||
|   /** Gets a predicate function to filter non-generated files with a specified extension. */ |  | ||||||
|   function hasFileExtension(ext: string): (path: string) => boolean { |  | ||||||
|     return f => f.endsWith(ext) && !f.endsWith(`.ngfactory${ext}`) && |  | ||||||
|         !f.endsWith(`.ngsummary${ext}`); |  | ||||||
|   } |  | ||||||
| 
 |  | ||||||
|   function copyFile(file: string, baseDir: string, relative = '.') { |   function copyFile(file: string, baseDir: string, relative = '.') { | ||||||
|     const dir = path.join(baseDir, relative); |     const dir = path.join(baseDir, relative); | ||||||
|     shx.mkdir('-p', dir); |     shx.mkdir('-p', dir); | ||||||
|  | |||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user