-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix the installable archive build. (#4064)
There was a file that got into the testing filegroups but not the actual built tarball. The result was that the installation failed to locate itself correctly. We also didn't have any testing to catch this. I'd like to add a bit more end-to-end testing long-term, but for now just add a Python test that validates the same set of files are in both. Sadly, building and testing the compressed release is really slow and not likely worthwhile outside of the actual release, but it would be good to catch this stuff earlier. I tried switching from `bz2` to `gz`, which made very little file size difference (so we should do it anyways), and it is still too slow to do routinely. Instead, I've added a Starlark macro that builds both the `pkg_tar` and the `py_test` to validate it for both uncompressed `tar` and compressed `tar.gz`. This lets us continuously test the `tar` version, and just test the `tar.gz` in the nightly release run. Updates the nightly release workflow to both test, run this specific test, and use `gz`. Last but not least, removes the `pkg_zip` as I don't think we have a use case for this yet and some TODOs that should have been removed when the version got added.
- Loading branch information
Showing
5 changed files
with
166 additions
and
54 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| # Part of the Carbon Language project, under the Apache License v2.0 with LLVM | ||
| # Exceptions. See /LICENSE for license information. | ||
| # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
|
|
||
| """Rule to create variables for package naming.""" | ||
|
|
||
| load("@rules_pkg//pkg:providers.bzl", "PackageVariablesInfo") | ||
| load("@rules_pkg//pkg:tar.bzl", "pkg_tar") | ||
| load("@rules_python//python:defs.bzl", "py_test") | ||
| load("//bazel/version:compute_version.bzl", "VERSION_ATTRS", "compute_version") | ||
|
|
||
| def _pkg_naming_variables_impl(ctx): | ||
| # TODO: Add support for digging the target CPU out of the toolchain here, | ||
| # remapping it to a more canonical name, and add that to the variables. The | ||
| # Bazel target CPU is already directly available, but it isn't likely | ||
| # canonical. | ||
| # TODO: Include the target OS as well as the target CPU. This likely needs | ||
| # similar re-mapping as the CPU does. | ||
| return PackageVariablesInfo(values = { | ||
| "version": compute_version(ctx), | ||
| }) | ||
|
|
||
| pkg_naming_variables = rule( | ||
| implementation = _pkg_naming_variables_impl, | ||
| attrs = VERSION_ATTRS, | ||
| ) | ||
|
|
||
| def pkg_tar_and_test(name_base, package_file_name_base, test_data, test_install_marker, **kwargs): | ||
| """Create a `pkg_tar` and a test for both `.tar` and `.tar.gz` extensions. | ||
| Args: | ||
| name_base: | ||
| The base name of the rules and tests. Will have `tar` or `tar_gz` added | ||
| and then `_rule` for the `pkg_tar` and `_test` for the test. | ||
| package_file_name_base: | ||
| The base of the `package_file_name` attribute to `pkg_tar`. The file | ||
| extensions will be appended after a `.`. | ||
| test_data: | ||
| The test data to verify the tar file against. | ||
| test_install_marker: | ||
| The install marker within the test data to locate the installation. | ||
| **kwargs: | ||
| Passed to `pkg_tar` for all the rest of its attributes. | ||
| """ | ||
| for file_ext in ["tar", "tar.gz"]: | ||
| target_ext = file_ext.replace(".", "_") | ||
| tar_target = name_base + "_" + target_ext + "_rule" | ||
| pkg_tar( | ||
| name = tar_target, | ||
| extension = file_ext, | ||
| package_file_name = package_file_name_base + "." + file_ext, | ||
| # The compressed tar is slow, exclude building and testing that. | ||
| tags = ["manual"] if file_ext == "tar.gz" else [], | ||
| **kwargs | ||
| ) | ||
|
|
||
| py_test( | ||
| name = name_base + "_" + target_ext + "_test", | ||
| size = "small", | ||
| srcs = ["toolchain_tar_test.py"], | ||
| data = [":" + tar_target, test_install_marker] + test_data, | ||
| args = [ | ||
| "$(location :{})".format(tar_target), | ||
| "$(location {})".format(test_install_marker), | ||
| ], | ||
| main = "toolchain_tar_test.py", | ||
| # The compressed tar is slow, exclude building and testing that. | ||
| tags = ["manual"] if file_ext == "tar.gz" else [], | ||
| ) |
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| #!/usr/bin/env python3 | ||
|
|
||
| """Check that a release tar contains the same files as a prefix root.""" | ||
|
|
||
| __copyright__ = """ | ||
| Part of the Carbon Language project, under the Apache License v2.0 with LLVM | ||
| Exceptions. See /LICENSE for license information. | ||
| SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
| """ | ||
|
|
||
| import argparse | ||
| import sys | ||
| from pathlib import Path | ||
| import tarfile | ||
|
|
||
|
|
||
| def main() -> None: | ||
| parser = argparse.ArgumentParser(description=__doc__) | ||
| parser.add_argument( | ||
| "tar_file", | ||
| type=Path, | ||
| help="The tar file to test.", | ||
| ) | ||
| parser.add_argument( | ||
| "install_marker", | ||
| type=Path, | ||
| help="The path of the install marker in a prefix root to test against.", | ||
| ) | ||
| args = parser.parse_args() | ||
|
|
||
| # Locate the prefix root from the install marker. | ||
| if not args.install_marker.exists(): | ||
| sys.exit("ERROR: No install marker: " + args.install_marker) | ||
| prefix_root_path = args.install_marker.parent.parent.parent | ||
|
|
||
| # First check that every file and directory in the tar file exists in our | ||
| # prefix root, and build a set of those paths. | ||
| installed_paths = set() | ||
| with tarfile.open(args.tar_file) as tar: | ||
| for tarinfo in tar: | ||
| relative_path = Path(*Path(tarinfo.name).parts[1:]) | ||
| installed_paths.add(relative_path) | ||
| if not prefix_root_path.joinpath(relative_path).exists(): | ||
| sys.exit( | ||
| "ERROR: File `{0}` is not in prefix root: `{1}`".format( | ||
| tarinfo.name, prefix_root_path | ||
| ) | ||
| ) | ||
|
|
||
| # If we found an empty tar file, it's always an error. | ||
| if len(installed_paths) == 0: | ||
| sys.exit("ERROR: Tar file `{0}` was empty.".format(args.tar_file)) | ||
|
|
||
| # Now check that every file and directory in the prefix root is in that set. | ||
| for prefix_path in prefix_root_path.glob("**/*"): | ||
| relative_path = prefix_path.relative_to(prefix_root_path) | ||
| if relative_path not in installed_paths: | ||
| sys.exit( | ||
| "ERROR: File `{0}` is not in tar file.".format(relative_path) | ||
| ) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() |