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

Bazel: add codeql specific packaging library #16432

Merged
merged 48 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
60cf77b
Bazel: add codeql specific packaging library
redsun82 May 6, 2024
4d93e8a
Bazel: move codeql packaging rules away from some macros
redsun82 May 23, 2024
e8b857b
Bazel/Swift: add zip imports to packs
redsun82 May 24, 2024
1529b58
Swift: add resource dir updater
redsun82 May 24, 2024
8e132e9
Bazel: add executable attribute to `lfs_files`
redsun82 May 24, 2024
94d6fef
Swift: fix module
redsun82 May 24, 2024
175f0db
Swift: remove broken obsolete alias
redsun82 May 24, 2024
e694968
Fix change to `.gitattributes` done by mistake
redsun82 May 24, 2024
dcbf42d
Bazel: reorganize LFS files and add licensing information
redsun82 May 24, 2024
fa2c626
Bazel: add fat macOS ripunzip binary
redsun82 May 24, 2024
ea01ae6
Swift: fix integration test log upload
redsun82 May 24, 2024
e990d75
Bazel: use codeql platform as arch zip filename
redsun82 May 24, 2024
f35f077
Swift: cleanup tools scripts in pack
redsun82 May 24, 2024
b9064c5
Bazel: fail install on `ripunzip` failing
redsun82 May 24, 2024
5d4b61c
Bazel: replace prebuilt ripunzip from workflow
redsun82 May 24, 2024
0b7a425
Bazel: use `{CODEQL_PLATFORM}` as discriminant between arch and gener…
redsun82 May 27, 2024
546d644
Swift: do not use `codeql_pkg_files` needlessly
redsun82 May 27, 2024
2bec696
Merge branch 'main' into redsun82/pkg
redsun82 May 27, 2024
6bbad22
Codegen: make codegen work on windows
redsun82 May 27, 2024
2f53c0e
Bazel: fix `codeql_pack` installation on Windows
redsun82 May 27, 2024
2f95944
Bazel: add documentation to `install.py`
redsun82 May 27, 2024
cde71a9
Bazel: address review comments
redsun82 May 27, 2024
392ef09
Zipmerge: make lib public for internal testing
redsun82 May 27, 2024
fbf3b9a
Merge branch 'main' into redsun82/pkg
redsun82 May 27, 2024
fe9a153
Merge branch 'main' into redsun82/pkg
redsun82 May 28, 2024
f7bfe43
Swift: fix windows build again
redsun82 May 28, 2024
afadc1f
Merge branch 'main' into redsun82/pkg
redsun82 May 28, 2024
a8543d4
Zipmerge: port tests from internal repo
redsun82 May 28, 2024
6d79841
Bazel: add `--no-cleanup` to installer script
redsun82 May 28, 2024
76fbb52
Bazel: use pack name for zip file name
redsun82 May 28, 2024
6b97161
Bazel: rename `_process_path` to `_expand_path`, and make its use cle…
redsun82 May 28, 2024
e2206e6
Bazel: restrict `codeql_pack` `zips` to `.zip` files
redsun82 May 28, 2024
00ed00e
Bazel: avoid unneeded operations if no imported zips are present
redsun82 May 28, 2024
9c1efb9
Bazel: expose `compression_level` in `codeql_pack`
redsun82 May 28, 2024
67d622f
Bazel: actually run the zipmerge tests
redsun82 May 28, 2024
c3ccf4d
Zipmerge: substitute `CPython` archives with dummy ones
redsun82 May 28, 2024
5eb12b8
Zipmerge: substitute stripped down slf4j jars with dummy ones
redsun82 May 28, 2024
2a62455
Merge branch 'main' into redsun82/pkg
redsun82 May 28, 2024
de48477
Zipmerge: print test outputs on CI
redsun82 May 28, 2024
45f1fdf
Bazel: extract pack filtering logic out of `_zipmerge`
redsun82 May 28, 2024
4094db4
Merge branch 'main' into redsun82/pkg
redsun82 May 28, 2024
332d178
Zipmerge: allow test to be run from internal repo
redsun82 May 28, 2024
fbe1b56
Zipmerge: link test statically
redsun82 May 29, 2024
491e3a4
Merge branch 'main' into redsun82/pkg
redsun82 May 29, 2024
5672ddf
Fix bazel formatting
redsun82 May 29, 2024
e8061ec
Bazel: fix `_zipmerge` rule
redsun82 May 29, 2024
336ec08
Bazel: use `extend(...)` instead of `+= list(...)`
redsun82 May 29, 2024
1e6820b
Merge branch 'main' into redsun82/pkg
redsun82 May 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 15 additions & 12 deletions misc/bazel/internal/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,18 @@
help="The wrapped `pkg_install` installation script rlocation")
parser.add_argument("--build-file", required=True,
help="BUILD.bazel rlocation relative to which the installation should take place")
parser.add_argument("--ripunzip", required=True,
help="ripunzip executable rlocation")
parser.add_argument("--zip-manifest", required=True,
parser.add_argument("--ripunzip",
help="ripunzip executable rlocation. Must be provided if `--zip-manifest` is.")
parser.add_argument("--zip-manifest",
help="The rlocation of a file containing newline-separated `prefix:zip_file` entries")
parser.add_argument("--cleanup", action=argparse.BooleanOptionalAction, default=True,
help="Whether to wipe the destination directory before installing (true by default)")
opts = parser.parse_args()
if opts.zip_manifest and not opts.ripunzip:
parser.error("Provide `--ripunzip` when specifying `--zip-manifest`")

build_file = runfiles.Rlocation(opts.build_file)
script = runfiles.Rlocation(opts.pkg_install_script)
ripunzip = runfiles.Rlocation(opts.ripunzip)
zip_manifest = runfiles.Rlocation(opts.zip_manifest)
destdir = pathlib.Path(build_file).resolve().parent / opts.destdir

if destdir.exists() and opts.cleanup:
Expand All @@ -43,10 +43,13 @@
destdir.mkdir(parents=True, exist_ok=True)
subprocess.run([script, "--destdir", destdir], check=True)

with open(zip_manifest) as manifest:
for line in manifest:
prefix, _, zip = line.partition(":")
assert zip, f"missing prefix for {prefix}, you should use prefix:zip format"
dest = destdir / prefix
dest.mkdir(parents=True, exist_ok=True)
subprocess.run([ripunzip, "unzip-file", zip, "-d", dest], check=True)
if opts.zip_manifest:
ripunzip = runfiles.Rlocation(opts.ripunzip)
zip_manifest = runfiles.Rlocation(opts.zip_manifest)
with open(zip_manifest) as manifest:
for line in manifest:
prefix, _, zip = line.partition(":")
assert zip, f"missing prefix for {prefix}, you should use prefix:zip format"
dest = destdir / prefix
dest.mkdir(parents=True, exist_ok=True)
subprocess.run([ripunzip, "unzip-file", zip, "-d", dest], check=True)
116 changes: 78 additions & 38 deletions misc/bazel/pkg.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,34 @@ _PLAT_DETECTION_ATTRS = {

_PLAT_PLACEHOLDER = "{CODEQL_PLATFORM}"

def _process_path(path, platform):
def _expand_path(path, platform):
if _PLAT_PLACEHOLDER in path:
path = path.replace(_PLAT_PLACEHOLDER, platform)
return ("arch", path)
return ("generic", path)

def _detect_platform(ctx):
if ctx.target_platform_has_constraint(ctx.attr._windows[platform_common.ConstraintValueInfo]):
return "win64"
elif ctx.target_platform_has_constraint(ctx.attr._macos[platform_common.ConstraintValueInfo]):
return "osx64"
def _platform_select(
ctx = None,
*,
linux,
windows,
macos):
if ctx:
if ctx.target_platform_has_constraint(ctx.attr._windows[platform_common.ConstraintValueInfo]):
return windows
elif ctx.target_platform_has_constraint(ctx.attr._macos[platform_common.ConstraintValueInfo]):
return macos
else:
return linux
else:
return "linux64"
return select({
"@platforms//os:linux": linux,
"@platforms//os:macos": macos,
"@platforms//os:windows": windows,
})

def _detect_platform(ctx = None):
return _platform_select(ctx, linux = "linux64", macos = "osx64", windows = "win64")

def codeql_pkg_files(
*,
Expand Down Expand Up @@ -89,9 +104,9 @@ def _extract_pkg_filegroup_impl(ctx):
for pfi, origin in src.pkg_files:
dest_src_map = {}
for dest, file in pfi.dest_src_map.items():
file_kind, dest = _process_path(dest, platform)
file_kind, expanded_dest = _expand_path(dest, platform)
if file_kind == ctx.attr.kind:
dest_src_map[dest] = file
dest_src_map[expanded_dest] = file
if dest_src_map:
pkg_files.append((PackageFilesInfo(dest_src_map = dest_src_map, attributes = pfi.attributes), origin))

Expand Down Expand Up @@ -124,9 +139,10 @@ def _imported_zips_manifest_impl(ctx):
manifest = []
files = []
for zip, prefix in ctx.attr.zips.items():
_, prefix = _process_path(prefix, platform)
# we don't care about the kind here, as we're taking all zips together
_, expanded_prefix = _expand_path(prefix, platform)
zip_files = zip.files.to_list()
manifest += ["%s:%s" % (prefix, f.short_path) for f in zip_files]
manifest += ["%s:%s" % (expanded_prefix, f.short_path) for f in zip_files]
files += zip_files

output = ctx.actions.declare_file(ctx.label.name + ".params")
Expand All @@ -146,7 +162,10 @@ _imported_zips_manifest = rule(
{CODEQL_PLATFORM} can be used as zip prefixes and will be expanded to the relevant codeql platform.
""",
attrs = {
"zips": attr.label_keyed_string_dict(doc = "mapping from zip files to install prefixes", allow_files = True),
"zips": attr.label_keyed_string_dict(
doc = "mapping from zip files to install prefixes",
allow_files = [".zip"],
),
} | _PLAT_DETECTION_ATTRS,
)

Expand All @@ -156,11 +175,11 @@ def _zipmerge_impl(ctx):
platform = _detect_platform(ctx)
filename = "%s-%s.zip" % (ctx.attr.zip_name, platform if ctx.attr.kind == "arch" else "generic")
output = ctx.actions.declare_file(filename)
args = [output.path, "--prefix=%s" % ctx.attr.zip_prefix, ctx.file.base.path]
args = [output.path, "--prefix=%s" % ctx.attr.prefix, ctx.file.base.path]
for zip, prefix in ctx.attr.zips.items():
criemen marked this conversation as resolved.
Show resolved Hide resolved
zip_kind, prefix = _process_path(prefix, platform)
zip_kind, expanded_prefix = _expand_path(prefix, platform)
if zip_kind == ctx.attr.kind:
args.append("--prefix=%s/%s" % (ctx.attr.zip_prefix, prefix.rstrip("/")))
args.append("--prefix=%s/%s" % (ctx.attr.prefix, expanded_prefix.rstrip("/")))
args += [f.path for f in zip.files.to_list()]
zips.append(zip.files)
ctx.actions.run(
Expand Down Expand Up @@ -188,12 +207,15 @@ _zipmerge = rule(
attrs = {
"base": attr.label(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a need to expose this separately from zips?
Couldn't we just merge the zip files in zips? I don't see the benefit of a base argument, but happy to be convinced.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, after trying it again I remembered why I had done this. The distinction whether to select a zip or not depends on its prefix containing {CODEQL_PLATFORM}. The base zip would be added with an empty "" prefix, which would disqualify it from the arch zipmerge. So the base_zip is always included, while zips are included conditionally.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, then a comment to that effect would be great, to explain that design choice to future us.

doc = "Base zip file to which zips from `zips` will be merged with",
allow_single_file = True,
allow_single_file = [".zip"],
),
"zips": attr.label_keyed_string_dict(
doc = "mapping from zip files to install prefixes",
allow_files = [".zip"],
),
"zips": attr.label_keyed_string_dict(doc = "mapping from zip files to install prefixes", allow_files = True),
"zip_name": attr.string(doc = "Prefix to use for the output file name"),
"kind": attr.string(doc = "Which zip kind to consider", values = ["generic", "arch"]),
"zip_prefix": attr.string(doc = "Prefix posix path to add to the zip contents in the archive"),
"prefix": attr.string(doc = "Prefix posix path to add to the zip contents in the archive"),
"_zipmerge": attr.label(default = "//misc/bazel/internal/zipmerge", executable = True, cfg = "exec"),
} | _PLAT_DETECTION_ATTRS,
)
Expand All @@ -203,9 +225,10 @@ def codeql_pack(
name,
srcs = None,
zips = None,
zip_filename = "extractor",
zip_filename = None,
visibility = None,
install_dest = "extractor-pack",
compression_level = None,
**kwargs):
"""
Define a codeql pack. This macro accepts `pkg_files`, `pkg_filegroup` or their `codeql_*` counterparts as `srcs`.
Expand All @@ -221,6 +244,9 @@ def codeql_pack(
The distinction between arch-specific and generic contents is made based on whether the paths (including possible
prefixes added by rules) contain the special `{CODEQL_PLATFORM}` placeholder, which in case it is present will also
be replaced by the appropriate platform (`linux64`, `win64` or `osx64`).

`compression_level` can be used to tweak the compression level used when creating archives. Consider that this
does not affect the contents of `zips`, only `srcs`.
"""
internal = _make_internal(name)
zip_filename = zip_filename or name
Expand All @@ -238,25 +264,37 @@ def codeql_pack(
kind = kind,
visibility = ["//visibility:private"],
)
pkg_zip(
name = internal(kind + "-zip-base"),
srcs = [internal(kind)],
visibility = ["//visibility:private"],
)
_zipmerge(
name = internal(kind + "-zip"),
base = internal(kind + "-zip-base"),
if zips:
pkg_zip(
name = internal(kind + "-zip-base"),
srcs = [internal(kind)],
visibility = ["//visibility:private"],
compression_level = compression_level,
)
_zipmerge(
name = internal(kind + "-zip"),
base = internal(kind + "-zip-base"),
zips = zips,
zip_name = zip_filename,
kind = kind,
prefix = name,
visibility = visibility,
)
else:
pkg_zip(
name = internal(kind + "-zip"),
srcs = [internal(kind)],
visibility = ["//visibility:private"],
package_dir = name,
package_file_name = name + "-" + (_detect_platform() if kind == "arch" else "generic") + ".zip",
criemen marked this conversation as resolved.
Show resolved Hide resolved
compression_level = compression_level,
)
if zips:
_imported_zips_manifest(
name = internal("zip-manifest"),
zips = zips,
zip_name = zip_filename,
zip_prefix = name,
kind = kind,
visibility = visibility,
visibility = ["//visibility:private"],
)
_imported_zips_manifest(
name = internal("zip-manifest"),
zips = zips,
visibility = ["//visibility:private"],
)

pkg_install(
name = internal("script"),
Expand All @@ -276,18 +314,20 @@ def codeql_pack(
data = [
internal("build-file"),
internal("script"),
] + ([
internal("zip-manifest"),
"//misc/bazel/internal/ripunzip",
],
] if zips else []),
deps = ["@rules_python//python/runfiles"],
args = [
"--build-file=$(rlocationpath %s)" % internal("build-file"),
"--pkg-install-script=$(rlocationpath %s)" % internal("script"),
"--destdir",
install_dest,
] + ([
"--ripunzip=$(rlocationpath //misc/bazel/internal/ripunzip)",
"--zip-manifest=$(rlocationpath %s)" % internal("zip-manifest"),
],
] if zips else []),
visibility = visibility,
)
native.filegroup(
Expand Down
Loading