From cad0db92adf68799663fbb627cd444670712b1bf Mon Sep 17 00:00:00 2001 From: Gera Shegalov Date: Tue, 1 Sep 2026 10:27:22 -0700 Subject: [PATCH 1/9] Add advisory NVIDIA deprecation audit Signed-off-by: Gera Shegalov --- .github/workflows/mvn-verify-check.yml | 31 ++ CONTRIBUTING.md | 18 ++ pom.xml | 22 +- scala2.13/pom.xml | 22 +- scripts/check_deprecation_policy.py | 268 +++++++++++++++++ scripts/deprecation_audit.py | 373 ++++++++++++++++++++++++ scripts/tests/test_deprecation_audit.py | 195 +++++++++++++ 7 files changed, 927 insertions(+), 2 deletions(-) create mode 100644 scripts/check_deprecation_policy.py create mode 100644 scripts/deprecation_audit.py create mode 100644 scripts/tests/test_deprecation_audit.py diff --git a/.github/workflows/mvn-verify-check.yml b/.github/workflows/mvn-verify-check.yml index 2afb1eebb7d..61370dbb7f2 100644 --- a/.github/workflows/mvn-verify-check.yml +++ b/.github/workflows/mvn-verify-check.yml @@ -36,6 +36,7 @@ env: -Drapids.secondaryCacheDir=$HOME/.m2/repository/.sbt/1.0/zinc/org.scala-sbt permissions: + actions: read contents: read jobs: @@ -430,3 +431,33 @@ jobs: fi } done + + nvidia-deprecation-audit: + name: NVIDIA deprecation audit + if: ${{ always() }} + continue-on-error: true + needs: + - package-tests + - package-tests-scala213 + - verify-213-modules + - verify-all-212-modules + - install-modules + runs-on: ubuntu-latest + steps: + - uses: NVIDIA/spark-rapids-common/checkout@main + + - name: Collect compiler deprecations from matrix logs + env: + GITHUB_TOKEN: ${{ github.token }} + run: | + python3 scripts/deprecation_audit.py \ + --repo-root "$GITHUB_WORKSPACE" \ + --raw-report "$RUNNER_TEMP/nvidia-deprecation-audit.json" + + - name: Upload deprecation report + if: ${{ always() }} + uses: actions/upload-artifact@v4 + with: + name: nvidia-deprecation-audit + path: ${{ runner.temp }}/nvidia-deprecation-audit.json + if-no-files-found: ignore diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c52cf4ca5f2..7c5a312f250 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -235,6 +235,24 @@ or similarly ./build/buildall --rebuild-dist-only --option="-Ddist.jar.compress=false -Drapids.jni.unpack.skip" ``` +### Cross-repository API deprecations + +API replacements in cuDF Java, cudf-spark-jni, and cudf-spark-private must allow cudf-spark time to +consume a published artifact before the old entry point becomes deprecated. Introduce the +replacement first while the old method remains supported and delegates to the same implementation. +After the updated snapshot is available, migrate cudf-spark callers in a separate change. Add the +deprecation annotation only after known callers have migrated, and retain the compatibility entry +point for at least one more release before removal. + +Scala deprecations originating in NVIDIA-owned `ai.rapids.cudf`, `com.nvidia.spark.rapids`, and +`org.apache.spark.sql.rapids` APIs are reported as compiler information instead of fatal warnings +during this migration window. The same applies to cudf-spark-private's +`org.apache.spark.sql.execution.aggregate.PartialAggUtils` bridge; other APIs in Apache Spark +namespaces are not exempt. Deprecations from other dependencies remain build errors. The +non-blocking NVIDIA deprecation audit in pull requests collects these diagnostics across the Maven +build matrix; findings should result in follow-up migration work even though the audit itself does +not fail the build. + ## Code contributions ### Source code layout diff --git a/pom.xml b/pom.xml index ac0296ce52a..ce8b60ccde0 100644 --- a/pom.xml +++ b/pom.xml @@ -1743,6 +1743,18 @@ This will force full Scala code rebuild in downstream modules. -Wconf:cat=unused-privates:e -Wunused:imports,locals,patvars,privates --> + + -Wconf:cat=deprecation&origin=ai\.rapids\.cudf\..*:iv + -Wconf:cat=deprecation&origin=com\.nvidia\.spark\.rapids\..*:iv + -Wconf:cat=deprecation&origin=org\.apache\.spark\.sql\.rapids\..*:iv + -Wconf:cat=deprecation&origin=org\.apache\.spark\.sql\.execution\.aggregate\.PartialAggUtils([.$].*|$):iv ${scala.javac.args} diff --git a/scala2.13/pom.xml b/scala2.13/pom.xml index a9d3442ef2b..b1ae3588ad9 100644 --- a/scala2.13/pom.xml +++ b/scala2.13/pom.xml @@ -1743,6 +1743,18 @@ This will force full Scala code rebuild in downstream modules. -Wconf:cat=unused-privates:e -Wunused:imports,locals,patvars,privates + + -Wconf:cat=deprecation&origin=ai\.rapids\.cudf\..*:iv + -Wconf:cat=deprecation&origin=com\.nvidia\.spark\.rapids\..*:iv + -Wconf:cat=deprecation&origin=org\.apache\.spark\.sql\.rapids\..*:iv + -Wconf:cat=deprecation&origin=org\.apache\.spark\.sql\.execution\.aggregate\.PartialAggUtils([.$].*|$):iv ${scala.javac.args} @@ -2038,7 +2050,15 @@ This will force full Scala code rebuild in downstream modules. - + + + + + + + + diff --git a/scripts/check_deprecation_policy.py b/scripts/check_deprecation_policy.py new file mode 100644 index 00000000000..8140b98ceed --- /dev/null +++ b/scripts/check_deprecation_policy.py @@ -0,0 +1,268 @@ +#!/usr/bin/env python3 + +# Copyright (c) 2026, NVIDIA CORPORATION. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Compile fixtures that enforce the repository's scoped deprecation policy.""" + +import argparse +import os +import shutil +import subprocess +import sys +import tempfile +import xml.etree.ElementTree as ElementTree +from pathlib import Path + + +MAVEN_NAMESPACE = {"m": "http://maven.apache.org/POM/4.0.0"} + + +def compiler_configuration(pom_path): + root = ElementTree.parse(pom_path).getroot() + scala_version = root.findtext("m:properties/m:scala.version", namespaces=MAVEN_NAMESPACE) + if not scala_version: + raise RuntimeError(f"Could not find scala.version in {pom_path}") + plugin_paths = ( + "m:build/m:plugins/m:plugin", + "m:build/m:pluginManagement/m:plugins/m:plugin", + ) + plugins = ( + plugin + for plugin_path in plugin_paths + for plugin in root.findall(plugin_path, MAVEN_NAMESPACE) + ) + for plugin in plugins: + artifact_id = plugin.findtext("m:artifactId", namespaces=MAVEN_NAMESPACE) + if artifact_id == "scala-maven-plugin": + args = [ + argument.text + for argument in plugin.findall("m:configuration/m:args/m:arg", MAVEN_NAMESPACE) + if argument.text + ] + if not args: + raise RuntimeError(f"scala-maven-plugin has no compiler arguments in {pom_path}") + return scala_version, args + raise RuntimeError(f"Could not find scala-maven-plugin in {pom_path}") + + +def scala_compiler_classpath(maven_repo, scala_version): + scala_root = Path(maven_repo) / "org" / "scala-lang" + jars = [ + scala_root / artifact / scala_version / f"{artifact}-{scala_version}.jar" + for artifact in ("scala-compiler", "scala-library", "scala-reflect") + ] + missing = [str(jar) for jar in jars if not jar.is_file()] + if missing: + raise RuntimeError("Missing Scala compiler dependencies: " + ", ".join(missing)) + return jars + + +def run_command(command): + return subprocess.run(command, text=True, stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, check=False) + + +def write_fixtures(root): + sources = { + "ai/rapids/cudf/fixture/NvidiaApi.java": """ +package ai.rapids.cudf.fixture; +public final class NvidiaApi { + private NvidiaApi() {} + @Deprecated public static void oldApi() {} +} +""", + "org/example/fixture/ThirdPartyApi.java": """ +package org.example.fixture; +public final class ThirdPartyApi { + private ThirdPartyApi() {} + @Deprecated public static void oldApi() {} +} +""", + "com/nvidia/spark/rapids/jni/fixture/JniApi.java": """ +package com.nvidia.spark.rapids.jni.fixture; +public final class JniApi { + private JniApi() {} + @Deprecated public static void oldApi() {} +} +""", + "com/nvidia/spark/rapids/optimizer/fixture/PrivateApi.java": """ +package com.nvidia.spark.rapids.optimizer.fixture; +public final class PrivateApi { + private PrivateApi() {} + @Deprecated public static void oldApi() {} +} +""", + "org/apache/spark/sql/rapids/internal/fixture/PrivateApi.java": """ +package org.apache.spark.sql.rapids.internal.fixture; +public final class PrivateApi { + private PrivateApi() {} + @Deprecated public static void oldApi() {} +} +""", + "org/apache/spark/sql/execution/aggregate/PartialAggUtils.java": """ +package org.apache.spark.sql.execution.aggregate; +public final class PartialAggUtils { + private PartialAggUtils() {} + @Deprecated public static void oldApi() {} +} +""", + "org/apache/spark/sql/execution/aggregate/PartialAggUtilsNeighbor.java": """ +package org.apache.spark.sql.execution.aggregate; +public final class PartialAggUtilsNeighbor { + private PartialAggUtilsNeighbor() {} + @Deprecated public static void oldApi() {} +} +""", + "org/apache/spark/sql/execution/aggregate/SparkApi.java": """ +package org.apache.spark.sql.execution.aggregate; +public final class SparkApi { + private SparkApi() {} + @Deprecated public static void oldApi() {} +} +""", + "NvidiaCall.scala": """ +object NvidiaCall { + def call(): Unit = ai.rapids.cudf.fixture.NvidiaApi.oldApi() +} +""", + "JniCall.scala": """ +object JniCall { + def call(): Unit = com.nvidia.spark.rapids.jni.fixture.JniApi.oldApi() +} +""", + "PrivateComNvidiaCall.scala": """ +object PrivateComNvidiaCall { + def call(): Unit = com.nvidia.spark.rapids.optimizer.fixture.PrivateApi.oldApi() +} +""", + "PrivateRapidsCall.scala": """ +object PrivateRapidsCall { + def call(): Unit = org.apache.spark.sql.rapids.internal.fixture.PrivateApi.oldApi() +} +""", + "PrivateSparkBridgeCall.scala": """ +object PrivateSparkBridgeCall { + def call(): Unit = org.apache.spark.sql.execution.aggregate.PartialAggUtils.oldApi() +} +""", + "PartialAggUtilsNeighborCall.scala": """ +object PartialAggUtilsNeighborCall { + def call(): Unit = org.apache.spark.sql.execution.aggregate.PartialAggUtilsNeighbor.oldApi() +} +""", + "SparkCall.scala": """ +object SparkCall { + def call(): Unit = org.apache.spark.sql.execution.aggregate.SparkApi.oldApi() +} +""", + "ThirdPartyCall.scala": """ +object ThirdPartyCall { + def call(): Unit = org.example.fixture.ThirdPartyApi.oldApi() +} +""", + } + for relative_path, source in sources.items(): + path = root / relative_path + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(source.lstrip(), encoding="utf-8") + + +def check_policy(pom_path, maven_repo): + scala_version, compiler_args = compiler_configuration(pom_path) + compiler_jar, library_jar, reflect_jar = scala_compiler_classpath( + maven_repo, scala_version) + javac = shutil.which("javac") + java = shutil.which("java") + if not javac or not java: + raise RuntimeError("Both java and javac are required for the deprecation policy check") + + with tempfile.TemporaryDirectory(prefix="cudf-spark-deprecation-policy-") as temp_dir: + fixture_root = Path(temp_dir) + classes = fixture_root / "classes" + classes.mkdir() + write_fixtures(fixture_root) + java_compile = run_command([ + javac, "-d", str(classes), + str(fixture_root / "ai/rapids/cudf/fixture/NvidiaApi.java"), + str(fixture_root / "com/nvidia/spark/rapids/jni/fixture/JniApi.java"), + str(fixture_root / "com/nvidia/spark/rapids/optimizer/fixture/PrivateApi.java"), + str(fixture_root / "org/apache/spark/sql/rapids/internal/fixture/PrivateApi.java"), + str(fixture_root / "org/apache/spark/sql/execution/aggregate/PartialAggUtils.java"), + str(fixture_root / + "org/apache/spark/sql/execution/aggregate/PartialAggUtilsNeighbor.java"), + str(fixture_root / "org/apache/spark/sql/execution/aggregate/SparkApi.java"), + str(fixture_root / "org/example/fixture/ThirdPartyApi.java"), + ]) + if java_compile.returncode: + raise RuntimeError("Could not compile Java fixtures:\n" + java_compile.stdout) + + compiler_classpath = os.pathsep.join(map(str, (compiler_jar, library_jar, reflect_jar))) + source_classpath = os.pathsep.join(map(str, (classes, library_jar))) + + def compile_scala(source): + return run_command([ + java, "-cp", compiler_classpath, "scala.tools.nsc.Main", + "-classpath", source_classpath, "-d", str(classes), + *compiler_args, str(fixture_root / source), + ]) + + nvidia_sources = ( + ("cuDF Java", "NvidiaCall.scala"), + ("cudf-spark-jni", "JniCall.scala"), + ("cudf-spark-private com.nvidia namespace", "PrivateComNvidiaCall.scala"), + ("cudf-spark-private RAPIDS namespace", "PrivateRapidsCall.scala"), + ("cudf-spark-private Spark-package bridge", "PrivateSparkBridgeCall.scala"), + ) + for api_name, source in nvidia_sources: + nvidia_compile = compile_scala(source) + if nvidia_compile.returncode or "deprecated" not in nvidia_compile.stdout.lower(): + raise RuntimeError( + f"{api_name} deprecation must be visible and nonfatal, " + "but compilation produced:\n" + nvidia_compile.stdout) + + fatal_sources = ( + ("Third-party", "ThirdPartyCall.scala"), + ("Apache Spark sibling", "SparkCall.scala"), + ("PartialAggUtils prefix neighbor", "PartialAggUtilsNeighborCall.scala"), + ) + for api_name, source in fatal_sources: + fatal_compile = compile_scala(source) + if fatal_compile.returncode == 0 or "deprecated" not in fatal_compile.stdout.lower(): + raise RuntimeError( + f"{api_name} deprecation must be visible and fatal, " + "but compilation produced:\n" + fatal_compile.stdout) + + print(f"Deprecation policy check passed for Scala {scala_version}") + + +def parse_args(argv): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--pom", required=True) + parser.add_argument("--maven-repo", required=True) + return parser.parse_args(argv) + + +def main(argv=None): + args = parse_args(argv) + try: + check_policy(args.pom, args.maven_repo) + except RuntimeError as error: + print(error, file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/deprecation_audit.py b/scripts/deprecation_audit.py new file mode 100644 index 00000000000..ce123ed347d --- /dev/null +++ b/scripts/deprecation_audit.py @@ -0,0 +1,373 @@ +#!/usr/bin/env python3 + +# Copyright (c) 2026, NVIDIA CORPORATION. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Collect compiler deprecation diagnostics from a GitHub Actions build matrix.""" + +import argparse +import io +import json +import os +import re +import sys +import time +import urllib.error +import urllib.parse +import urllib.request +import zipfile +from dataclasses import dataclass, field +from pathlib import Path + + +ANSI_ESCAPE = re.compile(r"\x1b\[[0-?]*[ -/]*[@-~]") +SOURCE_LOCATION = re.compile( + r"(?P(?:[A-Za-z]:)?[^\s\[\]]+?\.(?:scala|java))" + r"(?::(?P\d+)|:\[(?P\d+),\d+\])" +) +DEPRECATION = re.compile(r"\bdeprecated\b", re.IGNORECASE) +ORIGIN = re.compile(r"\borigin(?:=|:)\s*(?P[\w.$]+)") +DEFAULT_JOB_PATTERN = ( + r"^(?:package-tests(?:-scala213)?|verify-213-modules|" + r"verify-all-212-modules|install-modules)(?:\s|$)" +) +NVIDIA_ORIGIN_PREFIXES = ( + "ai.rapids.cudf.", + "com.nvidia.spark.rapids.", + "org.apache.spark.sql.rapids.", +) +NVIDIA_ORIGIN_SYMBOLS = ( + "org.apache.spark.sql.execution.aggregate.PartialAggUtils", +) + + +def is_nvidia_origin(origin): + if origin.startswith(NVIDIA_ORIGIN_PREFIXES): + return True + return any( + origin == symbol or origin.startswith((symbol + ".", symbol + "$")) + for symbol in NVIDIA_ORIGIN_SYMBOLS + ) + + +@dataclass +class Finding: + path: str + line: int + message: str + origin: str = "" + jobs: set[str] = field(default_factory=set) + + @property + def owner(self): + if is_nvidia_origin(self.origin): + return "NVIDIA" + return "third-party/unknown" + + def key(self): + diagnostic = self.origin or self.message + return self.path, self.line, diagnostic + + +def clean_line(line): + return ANSI_ESCAPE.sub("", line).rstrip() + + +def normalize_path(path, repo_root): + candidate = Path(path) + if not candidate.is_absolute(): + return candidate.as_posix() + root = Path(repo_root).resolve() + try: + return candidate.resolve().relative_to(root).as_posix() + except ValueError: + pass + parts = candidate.parts + for index in range(len(parts)): + suffix = Path(*parts[index:]) + if (root / suffix).exists(): + return suffix.as_posix() + return candidate.as_posix() + + +def parse_log(text, job_name, repo_root="."): + lines = [clean_line(line) for line in text.splitlines()] + findings = [] + for index, line in enumerate(lines): + if not DEPRECATION.search(line): + continue + location = SOURCE_LOCATION.search(line) + if location is None: + for previous in reversed(lines[max(0, index - 3):index]): + location = SOURCE_LOCATION.search(previous) + if location is not None: + break + if location is None: + continue + origin = "" + for context in lines[index:min(len(lines), index + 6)]: + origin_match = ORIGIN.search(context) + if origin_match is not None: + origin = origin_match.group("origin") + break + line_number = location.group("line") or location.group("bracket_line") + message = re.sub(r"^.*?\.(?:scala|java)(?::\d+|:\[\d+,\d+\])\s*:?[ ]*", "", line) + findings.append(Finding( + path=normalize_path(location.group("path"), repo_root), + line=int(line_number), + message=message.strip() or line.strip(), + origin=origin, + jobs={job_name}, + )) + return findings + + +def merge_findings(findings): + merged = {} + for finding in findings: + existing = merged.get(finding.key()) + if existing is None: + merged[finding.key()] = finding + else: + existing.jobs.update(finding.jobs) + return sorted(merged.values(), key=lambda finding: (finding.path, finding.line, finding.message)) + + +def request_json(url, token): + request = urllib.request.Request(url, headers={ + "Accept": "application/vnd.github+json", + "Authorization": f"Bearer {token}", + "X-GitHub-Api-Version": "2022-11-28", + }) + with urllib.request.urlopen(request, timeout=30) as response: + return json.load(response) + + +class JobLogRedirectError(RuntimeError): + """The GitHub job-log endpoint returned an unsafe or malformed redirect.""" + + +class NoRedirectHandler(urllib.request.HTTPRedirectHandler): + def redirect_request(self, request, file_pointer, code, message, headers, new_url): + return None + + +def request_bytes(url, token): + """Download a GitHub API resource without forwarding credentials on its redirect.""" + request = urllib.request.Request(url, headers={ + "Accept": "application/vnd.github+json", + "Authorization": f"Bearer {token}", + "X-GitHub-Api-Version": "2022-11-28", + }) + opener = urllib.request.build_opener(NoRedirectHandler()) + try: + with opener.open(request, timeout=30): + raise JobLogRedirectError( + "GitHub job-log endpoint did not return the expected redirect") + except urllib.error.HTTPError as error: + if error.code != 302: + error.close() + raise + location = error.headers.get("Location") + error.close() + if not location: + raise JobLogRedirectError( + "GitHub job-log redirect did not include a Location header") + parsed_location = urllib.parse.urlsplit(location) + if parsed_location.scheme != "https" or not parsed_location.netloc: + raise JobLogRedirectError( + "GitHub job-log redirect must use an absolute HTTPS URL") + + # The redirect is a short-lived signed URL. It authorizes itself, so use a fresh request + # without the repository-scoped GitHub token or GitHub-specific API headers. + signed_request = urllib.request.Request(location) + with urllib.request.urlopen(signed_request, timeout=30) as response: + return response.read() +def decode_job_log(payload): + if payload.startswith(b"PK"): + with zipfile.ZipFile(io.BytesIO(payload)) as archive: + return "\n".join( + archive.read(name).decode("utf-8", errors="replace") + for name in archive.namelist() + if not name.endswith("/") + ) + return payload.decode("utf-8", errors="replace") + + +def download_logs(api_url, repository, run_id, token, job_pattern): + matcher = re.compile(job_pattern) + jobs = [] + page = 1 + while True: + result = request_json( + f"{api_url}/repos/{repository}/actions/runs/{run_id}/jobs?per_page=100&page={page}", + token, + ) + page_jobs = result.get("jobs", []) + jobs.extend(page_jobs) + if len(page_jobs) < 100: + break + page += 1 + + logs = {} + failures = [] + for job in jobs: + name = job.get("name", "") + if job.get("status") != "completed" or matcher.search(name) is None: + continue + error = None + for delay in (0, 1, 2, 4): + if delay: + time.sleep(delay) + try: + payload = request_bytes( + f"{api_url}/repos/{repository}/actions/jobs/{job['id']}/logs", token) + logs[name] = decode_job_log(payload) + error = None + break + except (OSError, urllib.error.HTTPError, zipfile.BadZipFile, + JobLogRedirectError) as caught: + error = caught + if error is not None: + failures.append(f"{name}: {error}") + if not logs and not failures: + failures.append("no completed build-matrix job logs matched the configured job pattern") + return logs, failures + + +def markdown_escape(value): + return value.replace("|", "\\|").replace("\n", " ") + + +def render_summary(findings, failures): + lines = ["## NVIDIA deprecation audit", ""] + if findings: + lines.extend([ + f"Found {len(findings)} unique compiler deprecation diagnostic(s).", + "", + "| Owner | Location | Deprecated API | Matrix jobs |", + "| --- | --- | --- | --- |", + ]) + for finding in findings[:200]: + location = f"`{finding.path}:{finding.line}`" + api = finding.origin or finding.message + jobs = ", ".join(sorted(finding.jobs)) + lines.append( + f"| {finding.owner} | {location} | `{markdown_escape(api)}` | " + f"{markdown_escape(jobs)} |" + ) + if len(findings) > 200: + lines.extend(["", f"Report truncated; see the raw artifact for all {len(findings)} findings."]) + else: + lines.append("No compiler deprecation diagnostics were found in the selected matrix jobs.") + if failures: + lines.extend(["", "### Incomplete log collection", ""]) + lines.extend(f"- {markdown_escape(failure)}" for failure in failures) + lines.extend([ + "", + "This audit is advisory. Build-job failures remain authoritative.", + "", + ]) + return "\n".join(lines) + + +def command_escape(value): + return value.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") + + +def command_property_escape(value): + return command_escape(value).replace(":", "%3A").replace(",", "%2C") + + +def emit_annotations(findings): + for finding in findings[:50]: + message = finding.origin or finding.message + print( + f"::warning file={command_property_escape(finding.path)},line={finding.line}," + f"title=NVIDIA deprecation::{command_escape(message)}" + ) + if len(findings) > 50: + print(f"::warning title=NVIDIA deprecation::Only 50 of {len(findings)} findings were annotated") + + +def write_raw_report(path, findings, failures): + report = { + "findings": [ + { + "owner": finding.owner, + "path": finding.path, + "line": finding.line, + "message": finding.message, + "origin": finding.origin, + "jobs": sorted(finding.jobs), + } + for finding in findings + ], + "log_collection_failures": failures, + } + Path(path).write_text(json.dumps(report, indent=2) + "\n", encoding="utf-8") + + +def parse_args(argv): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--repository", default=os.environ.get("GITHUB_REPOSITORY")) + parser.add_argument("--run-id", default=os.environ.get("GITHUB_RUN_ID")) + parser.add_argument("--api-url", default=os.environ.get("GITHUB_API_URL", "https://api.github.com")) + parser.add_argument("--job-pattern", default=DEFAULT_JOB_PATTERN) + parser.add_argument("--logs-dir", help="Parse local *.log files instead of downloading job logs") + parser.add_argument("--repo-root", default=".") + parser.add_argument("--summary", default=os.environ.get("GITHUB_STEP_SUMMARY")) + parser.add_argument("--raw-report", default="nvidia-deprecation-audit.json") + return parser.parse_args(argv) + + +def main(argv=None): + args = parse_args(argv) + failures = [] + try: + if args.logs_dir: + logs = { + path.stem: path.read_text(encoding="utf-8", errors="replace") + for path in Path(args.logs_dir).glob("*.log") + } + else: + token = os.environ.get("GITHUB_TOKEN") + if not token or not args.repository or not args.run_id: + raise ValueError("GITHUB_TOKEN, repository, and run ID are required") + logs, failures = download_logs( + args.api_url, args.repository, args.run_id, token, args.job_pattern) + findings = merge_findings( + finding + for job_name, log in logs.items() + for finding in parse_log(log, job_name, args.repo_root) + ) + except Exception as error: # The audit must never mask the build result. + findings = [] + failures.append(f"audit failed: {error}") + + summary = render_summary(findings, failures) + print(summary) + emit_annotations(findings) + if args.summary: + with Path(args.summary).open("a", encoding="utf-8") as summary_file: + summary_file.write(summary) + try: + write_raw_report(args.raw_report, findings, failures) + except OSError as error: + print(f"::warning title=NVIDIA deprecation audit::Could not write raw report: {error}") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/tests/test_deprecation_audit.py b/scripts/tests/test_deprecation_audit.py new file mode 100644 index 00000000000..9a808c81f1b --- /dev/null +++ b/scripts/tests/test_deprecation_audit.py @@ -0,0 +1,195 @@ +#!/usr/bin/env python3 + +# Copyright (c) 2026, NVIDIA CORPORATION. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import importlib.util +import io +import sys +import unittest +import zipfile +from pathlib import Path +from unittest import mock + + +SCRIPT = Path(__file__).parents[1] / "deprecation_audit.py" +SPEC = importlib.util.spec_from_file_location("deprecation_audit", SCRIPT) +AUDIT = importlib.util.module_from_spec(SPEC) +assert SPEC.loader is not None +sys.modules[SPEC.name] = AUDIT +SPEC.loader.exec_module(AUDIT) + + +class DeprecationAuditSuite(unittest.TestCase): + def test_parses_scala_verbose_deprecation(self): + log = ( + "[INFO] /workspace/sql-plugin/src/main/scala/Test.scala:42: " + "[deprecation @ example.Test.run | " + "origin=ai.rapids.cudf.ColumnView.oldApi | version=] " + "method oldApi in class ColumnView is deprecated\n" + ) + findings = AUDIT.parse_log(log, "package-tests (330)") + self.assertEqual(1, len(findings)) + self.assertEqual(42, findings[0].line) + self.assertEqual("ai.rapids.cudf.ColumnView.oldApi", findings[0].origin) + self.assertEqual("NVIDIA", findings[0].owner) + + def test_parses_maven_bracket_location(self): + log = """ +[WARNING] /workspace/src/main/java/Test.java:[17,9] oldApi() has been deprecated +""" + findings = AUDIT.parse_log(log, "verify-all-212-modules (330, 17)") + self.assertEqual(17, findings[0].line) + self.assertEqual("third-party/unknown", findings[0].owner) + + def test_reads_scala_213_origin_from_following_line(self): + log = """ +[INFO] /workspace/sql-plugin/src/main/scala/Test.scala:42: method oldApi is deprecated +Applicable -Wconf filters: cat=deprecation, origin=com.nvidia.spark.rapids.jni.Api.oldApi +""" + findings = AUDIT.parse_log(log, "package-tests-scala213 (350)") + self.assertEqual("com.nvidia.spark.rapids.jni.Api.oldApi", findings[0].origin) + self.assertEqual("NVIDIA", findings[0].owner) + + def test_classifies_all_advisory_origins_as_nvidia(self): + origins = ( + "ai.rapids.cudf.ColumnView.oldApi", + "com.nvidia.spark.rapids.optimizer.OptimizerConf.oldApi", + "org.apache.spark.sql.rapids.internal.PrivateRapidsConfs.oldApi", + "org.apache.spark.sql.execution.aggregate.PartialAggUtils.oldApi", + "org.apache.spark.sql.execution.aggregate.PartialAggUtils$Helper.oldApi", + ) + for origin in origins: + with self.subTest(origin=origin): + finding = AUDIT.Finding("Test.scala", 1, "deprecated", origin) + self.assertEqual("NVIDIA", finding.owner) + + def test_partial_agg_utils_owner_match_has_symbol_boundary(self): + origins = ( + "org.apache.spark.sql.execution.aggregate.PartialAggUtilsNeighbor.oldApi", + "org.apache.spark.sql.execution.aggregate.SparkApi.oldApi", + "org.example.fixture.ThirdPartyApi.oldApi", + ) + for origin in origins: + with self.subTest(origin=origin): + finding = AUDIT.Finding("Test.scala", 1, "deprecated", origin) + self.assertEqual("third-party/unknown", finding.owner) + + def test_ignores_non_source_deprecation_text(self): + log = "[WARNING] This build plugin uses a deprecated Maven feature\n" + self.assertEqual([], AUDIT.parse_log(log, "install-modules (3.9.3)")) + + def test_merges_same_finding_across_matrix_jobs(self): + first = AUDIT.Finding( + "Test.scala", 1, "[deprecation] old is deprecated", "ai.rapids.cudf.Api.old", + {"330"}) + second = AUDIT.Finding( + "Test.scala", 1, "old is deprecated", "ai.rapids.cudf.Api.old", {"400"}) + merged = AUDIT.merge_findings([first, second]) + self.assertEqual({"330", "400"}, merged[0].jobs) + + def test_decodes_zip_job_log(self): + payload = io.BytesIO() + with zipfile.ZipFile(payload, "w") as archive: + archive.writestr("job/step.txt", "deprecated output") + self.assertEqual("deprecated output", AUDIT.decode_job_log(payload.getvalue())) + + def test_job_log_redirect_does_not_forward_authorization(self): + api_url = "https://api.github.com/repos/NVIDIA/cudf-spark/actions/jobs/123/logs" + signed_url = "https://results-receiver.example/job.log?signature=secret" + redirect = AUDIT.urllib.error.HTTPError( + api_url, 302, "Found", {"Location": signed_url}, None) + authenticated_opener = mock.Mock() + authenticated_opener.open.side_effect = redirect + + with mock.patch.object( + AUDIT.urllib.request, "build_opener", return_value=authenticated_opener), \ + mock.patch.object( + AUDIT.urllib.request, "urlopen", return_value=io.BytesIO(b"job log")) \ + as signed_open: + payload = AUDIT.request_bytes(api_url, "github-token") + + self.assertEqual(b"job log", payload) + authenticated_request = authenticated_opener.open.call_args.args[0] + self.assertEqual("Bearer github-token", + authenticated_request.get_header("Authorization")) + signed_request = signed_open.call_args.args[0] + self.assertEqual(signed_url, signed_request.full_url) + self.assertIsNone(signed_request.get_header("Authorization")) + self.assertIsNone(signed_request.get_header("X-GitHub-Api-Version")) + + def test_job_log_redirect_requires_location(self): + api_url = "https://api.github.com/repos/NVIDIA/cudf-spark/actions/jobs/123/logs" + authenticated_opener = mock.Mock() + authenticated_opener.open.side_effect = AUDIT.urllib.error.HTTPError( + api_url, 302, "Found", {}, None) + with mock.patch.object( + AUDIT.urllib.request, "build_opener", return_value=authenticated_opener), \ + self.assertRaisesRegex(AUDIT.JobLogRedirectError, "Location"): + AUDIT.request_bytes(api_url, "github-token") + + def test_job_log_redirect_rejects_non_https_location(self): + api_url = "https://api.github.com/repos/NVIDIA/cudf-spark/actions/jobs/123/logs" + authenticated_opener = mock.Mock() + authenticated_opener.open.side_effect = AUDIT.urllib.error.HTTPError( + api_url, 302, "Found", {"Location": "http://logs.example/job.log"}, None) + with mock.patch.object( + AUDIT.urllib.request, "build_opener", return_value=authenticated_opener), \ + self.assertRaisesRegex(AUDIT.JobLogRedirectError, "HTTPS"): + AUDIT.request_bytes(api_url, "github-token") + + def test_job_log_endpoint_rejects_non_redirect_response(self): + api_url = "https://api.github.com/repos/NVIDIA/cudf-spark/actions/jobs/123/logs" + authenticated_opener = mock.Mock() + authenticated_opener.open.return_value = io.BytesIO(b"unexpected direct response") + with mock.patch.object( + AUDIT.urllib.request, "build_opener", return_value=authenticated_opener), \ + self.assertRaisesRegex(AUDIT.JobLogRedirectError, "expected redirect"): + AUDIT.request_bytes(api_url, "github-token") + + def test_download_logs_preserves_partial_results_after_bad_redirect(self): + jobs = {"jobs": [ + {"id": 1, "name": "package-tests (330, false)", "status": "completed"}, + {"id": 2, "name": "package-tests (340, false)", "status": "completed"}, + ]} + bad_redirect = AUDIT.JobLogRedirectError("missing redirect location") + with mock.patch.object(AUDIT, "request_json", return_value=jobs), \ + mock.patch.object( + AUDIT, "request_bytes", + side_effect=[b"first job log", bad_redirect, bad_redirect, + bad_redirect, bad_redirect]) as request_bytes, \ + mock.patch.object(AUDIT.time, "sleep"): + logs, failures = AUDIT.download_logs( + "https://api.github.com", "NVIDIA/cudf-spark", "run-id", "token", + AUDIT.DEFAULT_JOB_PATTERN) + + self.assertEqual({"package-tests (330, false)": "first job log"}, logs) + self.assertEqual(5, request_bytes.call_count) + self.assertEqual(1, len(failures)) + self.assertIn("package-tests (340, false)", failures[0]) + self.assertIn("missing redirect location", failures[0]) + + def test_summary_reports_incomplete_collection(self): + summary = AUDIT.render_summary([], ["package-tests: log unavailable"]) + self.assertIn("No compiler deprecation diagnostics", summary) + self.assertIn("Incomplete log collection", summary) + self.assertIn("advisory", summary) + + def test_annotation_property_escaping(self): + self.assertEqual("path%3Awith%2Cpunctuation", AUDIT.command_property_escape( + "path:with,punctuation")) + + +if __name__ == "__main__": + unittest.main() From 4c28634a2b378cf9a629b996fd7500e261355cf2 Mon Sep 17 00:00:00 2001 From: Gera Shegalov Date: Tue, 1 Sep 2026 11:20:37 -0700 Subject: [PATCH 2/9] Fail optional audit when deprecations are found Signed-off-by: Gera Shegalov --- .github/workflows/mvn-verify-check.yml | 3 +- CONTRIBUTING.md | 6 ++-- scripts/deprecation_audit.py | 9 ++++-- scripts/tests/test_deprecation_audit.py | 41 ++++++++++++++++++++++++- 4 files changed, 50 insertions(+), 9 deletions(-) diff --git a/.github/workflows/mvn-verify-check.yml b/.github/workflows/mvn-verify-check.yml index 61370dbb7f2..b5f9ce0afad 100644 --- a/.github/workflows/mvn-verify-check.yml +++ b/.github/workflows/mvn-verify-check.yml @@ -433,9 +433,8 @@ jobs: done nvidia-deprecation-audit: - name: NVIDIA deprecation audit + name: NVIDIA deprecation audit (optional) if: ${{ always() }} - continue-on-error: true needs: - package-tests - package-tests-scala213 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7c5a312f250..e71e4111ee2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -249,9 +249,9 @@ Scala deprecations originating in NVIDIA-owned `ai.rapids.cudf`, `com.nvidia.spa during this migration window. The same applies to cudf-spark-private's `org.apache.spark.sql.execution.aggregate.PartialAggUtils` bridge; other APIs in Apache Spark namespaces are not exempt. Deprecations from other dependencies remain build errors. The -non-blocking NVIDIA deprecation audit in pull requests collects these diagnostics across the Maven -build matrix; findings should result in follow-up migration work even though the audit itself does -not fail the build. +optional NVIDIA deprecation audit in pull requests collects these diagnostics across the Maven +build matrix. Findings or incomplete log collection fail the audit check so contributors inspect +the result, but the audit is not a required build check; build-job results remain authoritative. ## Code contributions diff --git a/scripts/deprecation_audit.py b/scripts/deprecation_audit.py index ce123ed347d..72906708500 100644 --- a/scripts/deprecation_audit.py +++ b/scripts/deprecation_audit.py @@ -276,7 +276,8 @@ def render_summary(findings, failures): lines.extend(f"- {markdown_escape(failure)}" for failure in failures) lines.extend([ "", - "This audit is advisory. Build-job failures remain authoritative.", + "This check is optional. Findings or incomplete log collection fail only this audit " + "check; build-job results remain authoritative.", "", ]) return "\n".join(lines) @@ -352,7 +353,7 @@ def main(argv=None): for job_name, log in logs.items() for finding in parse_log(log, job_name, args.repo_root) ) - except Exception as error: # The audit must never mask the build result. + except Exception as error: # Report operational errors through this optional check. findings = [] failures.append(f"audit failed: {error}") @@ -362,11 +363,13 @@ def main(argv=None): if args.summary: with Path(args.summary).open("a", encoding="utf-8") as summary_file: summary_file.write(summary) + report_failed = False try: write_raw_report(args.raw_report, findings, failures) except OSError as error: + report_failed = True print(f"::warning title=NVIDIA deprecation audit::Could not write raw report: {error}") - return 0 + return 1 if findings or failures or report_failed else 0 if __name__ == "__main__": diff --git a/scripts/tests/test_deprecation_audit.py b/scripts/tests/test_deprecation_audit.py index 9a808c81f1b..ff4776cb943 100644 --- a/scripts/tests/test_deprecation_audit.py +++ b/scripts/tests/test_deprecation_audit.py @@ -17,6 +17,7 @@ import importlib.util import io import sys +import tempfile import unittest import zipfile from pathlib import Path @@ -184,7 +185,45 @@ def test_summary_reports_incomplete_collection(self): summary = AUDIT.render_summary([], ["package-tests: log unavailable"]) self.assertIn("No compiler deprecation diagnostics", summary) self.assertIn("Incomplete log collection", summary) - self.assertIn("advisory", summary) + self.assertIn("optional", summary) + + def test_main_fails_when_findings_are_present(self): + log = ( + "/workspace/sql-plugin/src/main/scala/Test.scala:42: " + "[deprecation @ example.Test.run | " + "origin=ai.rapids.cudf.ColumnView.oldApi | version=] deprecated\n" + ) + with tempfile.TemporaryDirectory() as temp_dir: + root = Path(temp_dir) + (root / "package-tests.log").write_text(log, encoding="utf-8") + result = AUDIT.main([ + "--logs-dir", str(root), + "--raw-report", str(root / "report.json"), + ]) + self.assertEqual(1, result) + + def test_main_succeeds_when_audit_is_clean(self): + with tempfile.TemporaryDirectory() as temp_dir: + root = Path(temp_dir) + (root / "package-tests.log").write_text("clean build\n", encoding="utf-8") + result = AUDIT.main([ + "--logs-dir", str(root), + "--raw-report", str(root / "report.json"), + ]) + self.assertEqual(0, result) + + def test_main_fails_when_log_collection_is_incomplete(self): + with tempfile.TemporaryDirectory() as temp_dir, \ + mock.patch.object( + AUDIT, "download_logs", + return_value=({}, ["package-tests: log unavailable"])), \ + mock.patch.dict(AUDIT.os.environ, {"GITHUB_TOKEN": "token"}): + result = AUDIT.main([ + "--repository", "NVIDIA/cudf-spark", + "--run-id", "123", + "--raw-report", str(Path(temp_dir) / "report.json"), + ]) + self.assertEqual(1, result) def test_annotation_property_escaping(self): self.assertEqual("path%3Awith%2Cpunctuation", AUDIT.command_property_escape( From 910bd0ddab830af9f3977961e5b15b70de352792 Mon Sep 17 00:00:00 2001 From: Gera Shegalov Date: Tue, 1 Sep 2026 11:46:37 -0700 Subject: [PATCH 3/9] Run deprecation checks with Jython Signed-off-by: Gera Shegalov --- .github/workflows/mvn-verify-check.yml | 5 +- pom.xml | 35 +++- scala2.13/pom.xml | 35 +++- scripts/check_deprecation_policy.py | 142 +++++++++----- scripts/deprecation_audit.py | 192 +++++++++++-------- scripts/tests/test_deprecation_audit.py | 242 +++++++++++++++++------- 6 files changed, 447 insertions(+), 204 deletions(-) diff --git a/.github/workflows/mvn-verify-check.yml b/.github/workflows/mvn-verify-check.yml index b5f9ce0afad..54c3d2b54e2 100644 --- a/.github/workflows/mvn-verify-check.yml +++ b/.github/workflows/mvn-verify-check.yml @@ -449,9 +449,8 @@ jobs: env: GITHUB_TOKEN: ${{ github.token }} run: | - python3 scripts/deprecation_audit.py \ - --repo-root "$GITHUB_WORKSPACE" \ - --raw-report "$RUNNER_TEMP/nvidia-deprecation-audit.json" + mvn --batch-mode -N antrun:run@nvidia-deprecation-audit \ + -Ddeprecation.audit.rawReport="$RUNNER_TEMP/nvidia-deprecation-audit.json" - name: Upload deprecation report if: ${{ always() }} diff --git a/pom.xml b/pom.xml index ce8b60ccde0..ff8be39f99d 100644 --- a/pom.xml +++ b/pom.xml @@ -1153,6 +1153,8 @@ 4.9.10 3.1.1 3.3.0 + 2.7.3 + ${project.build.directory}/nvidia-deprecation-audit.json 2.0.2 30.0-jre 2.0.0 @@ -1619,7 +1621,7 @@ This will force full Scala code rebuild in downstream modules. org.python jython-standalone - 2.7.3 + ${jython.version} net.sourceforge.pmd @@ -2005,6 +2007,29 @@ This will force full Scala code rebuild in downstream modules. false + + nvidia-deprecation-audit + none + run + + + + + + + + + + + + + + + + + 4.9.10 3.1.1 3.3.0 + 2.7.3 + ${project.build.directory}/nvidia-deprecation-audit.json 2.0.2 30.0-jre 2.0.0 @@ -1619,7 +1621,7 @@ This will force full Scala code rebuild in downstream modules. org.python jython-standalone - 2.7.3 + ${jython.version} net.sourceforge.pmd @@ -2005,6 +2007,29 @@ This will force full Scala code rebuild in downstream modules. false + + nvidia-deprecation-audit + none + run + + + + + + + + + + + + + + + + + 200: - lines.extend(["", f"Report truncated; see the raw artifact for all {len(findings)} findings."]) + lines.extend([ + u"", + u"Report truncated; see the raw artifact for all {0} findings.".format( + len(findings)), + ]) else: - lines.append("No compiler deprecation diagnostics were found in the selected matrix jobs.") + lines.append(u"No compiler deprecation diagnostics were found in the selected matrix jobs.") if failures: - lines.extend(["", "### Incomplete log collection", ""]) - lines.extend(f"- {markdown_escape(failure)}" for failure in failures) + lines.extend([u"", u"### Incomplete log collection", u""]) + lines.extend(u"- {0}".format(markdown_escape(failure)) for failure in failures) lines.extend([ - "", - "This check is optional. Findings or incomplete log collection fail only this audit " + u"", + u"This check is optional. Findings or incomplete log collection fail only this audit " "check; build-job results remain authoritative.", - "", + u"", ]) - return "\n".join(lines) + return u"\n".join(lines) def command_escape(value): @@ -295,11 +329,12 @@ def emit_annotations(findings): for finding in findings[:50]: message = finding.origin or finding.message print( - f"::warning file={command_property_escape(finding.path)},line={finding.line}," - f"title=NVIDIA deprecation::{command_escape(message)}" + u"::warning file={0},line={1},title=NVIDIA deprecation::{2}".format( + command_property_escape(finding.path), finding.line, command_escape(message)) ) if len(findings) > 50: - print(f"::warning title=NVIDIA deprecation::Only 50 of {len(findings)} findings were annotated") + print(u"::warning title=NVIDIA deprecation::Only 50 of {0} findings were annotated".format( + len(findings))) def write_raw_report(path, findings, failures): @@ -317,16 +352,20 @@ def write_raw_report(path, findings, failures): ], "log_collection_failures": failures, } - Path(path).write_text(json.dumps(report, indent=2) + "\n", encoding="utf-8") + with io.open(path, "w", encoding="utf-8") as report_file: + report_file.write(json.dumps(report, indent=2, ensure_ascii=False) + u"\n") def parse_args(argv): parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("--repository", default=os.environ.get("GITHUB_REPOSITORY")) parser.add_argument("--run-id", default=os.environ.get("GITHUB_RUN_ID")) - parser.add_argument("--api-url", default=os.environ.get("GITHUB_API_URL", "https://api.github.com")) + parser.add_argument( + "--api-url", default=os.environ.get("GITHUB_API_URL", "https://api.github.com")) parser.add_argument("--job-pattern", default=DEFAULT_JOB_PATTERN) - parser.add_argument("--logs-dir", help="Parse local *.log files instead of downloading job logs") + parser.add_argument( + "--logs-dir", default=os.environ.get("DEPRECATION_AUDIT_LOGS_DIR"), + help="Parse local *.log files instead of downloading job logs") parser.add_argument("--repo-root", default=".") parser.add_argument("--summary", default=os.environ.get("GITHUB_STEP_SUMMARY")) parser.add_argument("--raw-report", default="nvidia-deprecation-audit.json") @@ -339,8 +378,8 @@ def main(argv=None): try: if args.logs_dir: logs = { - path.stem: path.read_text(encoding="utf-8", errors="replace") - for path in Path(args.logs_dir).glob("*.log") + os.path.splitext(os.path.basename(path))[0]: read_text(path) + for path in glob.glob(os.path.join(args.logs_dir, "*.log")) } else: token = os.environ.get("GITHUB_TOKEN") @@ -355,20 +394,21 @@ def main(argv=None): ) except Exception as error: # Report operational errors through this optional check. findings = [] - failures.append(f"audit failed: {error}") + failures.append(u"audit failed: {0}".format(error)) summary = render_summary(findings, failures) print(summary) emit_annotations(findings) if args.summary: - with Path(args.summary).open("a", encoding="utf-8") as summary_file: + with io.open(args.summary, "a", encoding="utf-8") as summary_file: summary_file.write(summary) report_failed = False try: write_raw_report(args.raw_report, findings, failures) - except OSError as error: + except (IOError, OSError) as error: report_failed = True - print(f"::warning title=NVIDIA deprecation audit::Could not write raw report: {error}") + print(u"::warning title=NVIDIA deprecation audit::Could not write raw report: {0}".format( + error)) return 1 if findings or failures or report_failed else 0 diff --git a/scripts/tests/test_deprecation_audit.py b/scripts/tests/test_deprecation_audit.py index ff4776cb943..b6e20f820a0 100644 --- a/scripts/tests/test_deprecation_audit.py +++ b/scripts/tests/test_deprecation_audit.py @@ -1,5 +1,3 @@ -#!/usr/bin/env python3 - # Copyright (c) 2026, NVIDIA CORPORATION. # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -14,22 +12,114 @@ # See the License for the specific language governing permissions and # limitations under the License. -import importlib.util +from __future__ import print_function + +import contextlib import io +import os +import shutil import sys import tempfile import unittest import zipfile -from pathlib import Path -from unittest import mock -SCRIPT = Path(__file__).parents[1] / "deprecation_audit.py" -SPEC = importlib.util.spec_from_file_location("deprecation_audit", SCRIPT) -AUDIT = importlib.util.module_from_spec(SPEC) -assert SPEC.loader is not None -sys.modules[SPEC.name] = AUDIT -SPEC.loader.exec_module(AUDIT) +SCRIPT = os.path.join(os.path.dirname(os.path.dirname(__file__)), "deprecation_audit.py") +try: + import importlib.util + SPEC = importlib.util.spec_from_file_location("deprecation_audit", SCRIPT) + AUDIT = importlib.util.module_from_spec(SPEC) + sys.modules[SPEC.name] = AUDIT + SPEC.loader.exec_module(AUDIT) +except ImportError: # Jython 2.7 / Python 2.7 + import imp + AUDIT = imp.load_source("deprecation_audit", SCRIPT) + + +class CallRecorder(object): + def __init__(self, return_value=None, side_effect=None): + self.return_value = return_value + self.side_effect = side_effect + self.calls = [] + + @property + def call_count(self): + return len(self.calls) + + def __call__(self, *args, **kwargs): + self.calls.append((args, kwargs)) + effect = self.side_effect + if isinstance(effect, list): + effect = effect.pop(0) + if isinstance(effect, BaseException): + raise effect + if callable(effect): + return effect(*args, **kwargs) + return self.return_value if effect is None else effect + + +class FakeOpener(object): + def __init__(self): + self.open = CallRecorder() + + +class OutputSink(object): + def __init__(self): + self.parts = [] + + def write(self, value): + self.parts.append(value) + + def flush(self): + pass + + +@contextlib.contextmanager +def patch_attribute(target, name, value): + original = getattr(target, name) + setattr(target, name, value) + try: + yield value + finally: + setattr(target, name, original) + + +@contextlib.contextmanager +def patch_environment(updates): + original = dict(os.environ) + os.environ.update(updates) + try: + yield + finally: + os.environ.clear() + os.environ.update(original) + + +@contextlib.contextmanager +def temporary_directory(): + path = tempfile.mkdtemp() + try: + yield path + finally: + shutil.rmtree(path) + + +@contextlib.contextmanager +def captured_stdout(): + output = OutputSink() + with patch_attribute(sys, "stdout", output): + yield output + + +def request_url(request): + return request.get_full_url() + + +def assert_raises_regex(test_case, exception, pattern): + method = getattr(test_case, "assertRaisesRegex", None) + if method is None: + method = test_case.assertRaisesRegexp + return method(exception, pattern) class DeprecationAuditSuite(unittest.TestCase): @@ -72,9 +162,8 @@ def test_classifies_all_advisory_origins_as_nvidia(self): "org.apache.spark.sql.execution.aggregate.PartialAggUtils$Helper.oldApi", ) for origin in origins: - with self.subTest(origin=origin): - finding = AUDIT.Finding("Test.scala", 1, "deprecated", origin) - self.assertEqual("NVIDIA", finding.owner) + finding = AUDIT.Finding("Test.scala", 1, "deprecated", origin) + self.assertEqual("NVIDIA", finding.owner, origin) def test_partial_agg_utils_owner_match_has_symbol_boundary(self): origins = ( @@ -83,9 +172,8 @@ def test_partial_agg_utils_owner_match_has_symbol_boundary(self): "org.example.fixture.ThirdPartyApi.oldApi", ) for origin in origins: - with self.subTest(origin=origin): - finding = AUDIT.Finding("Test.scala", 1, "deprecated", origin) - self.assertEqual("third-party/unknown", finding.owner) + finding = AUDIT.Finding("Test.scala", 1, "deprecated", origin) + self.assertEqual("third-party/unknown", finding.owner, origin) def test_ignores_non_source_deprecation_text(self): log = "[WARNING] This build plugin uses a deprecated Maven feature\n" @@ -109,54 +197,56 @@ def test_decodes_zip_job_log(self): def test_job_log_redirect_does_not_forward_authorization(self): api_url = "https://api.github.com/repos/NVIDIA/cudf-spark/actions/jobs/123/logs" signed_url = "https://results-receiver.example/job.log?signature=secret" - redirect = AUDIT.urllib.error.HTTPError( + redirect = AUDIT.urllib_error.HTTPError( api_url, 302, "Found", {"Location": signed_url}, None) - authenticated_opener = mock.Mock() + authenticated_opener = FakeOpener() authenticated_opener.open.side_effect = redirect + build_opener = CallRecorder(return_value=authenticated_opener) + signed_open = CallRecorder(return_value=io.BytesIO(b"job log")) - with mock.patch.object( - AUDIT.urllib.request, "build_opener", return_value=authenticated_opener), \ - mock.patch.object( - AUDIT.urllib.request, "urlopen", return_value=io.BytesIO(b"job log")) \ - as signed_open: + with patch_attribute(AUDIT.urllib_request, "build_opener", build_opener), \ + patch_attribute(AUDIT.urllib_request, "urlopen", signed_open): payload = AUDIT.request_bytes(api_url, "github-token") self.assertEqual(b"job log", payload) - authenticated_request = authenticated_opener.open.call_args.args[0] + authenticated_request = authenticated_opener.open.calls[0][0][0] self.assertEqual("Bearer github-token", authenticated_request.get_header("Authorization")) - signed_request = signed_open.call_args.args[0] - self.assertEqual(signed_url, signed_request.full_url) + signed_request = signed_open.calls[0][0][0] + self.assertEqual(signed_url, request_url(signed_request)) self.assertIsNone(signed_request.get_header("Authorization")) self.assertIsNone(signed_request.get_header("X-GitHub-Api-Version")) def test_job_log_redirect_requires_location(self): api_url = "https://api.github.com/repos/NVIDIA/cudf-spark/actions/jobs/123/logs" - authenticated_opener = mock.Mock() - authenticated_opener.open.side_effect = AUDIT.urllib.error.HTTPError( + authenticated_opener = FakeOpener() + authenticated_opener.open.side_effect = AUDIT.urllib_error.HTTPError( api_url, 302, "Found", {}, None) - with mock.patch.object( - AUDIT.urllib.request, "build_opener", return_value=authenticated_opener), \ - self.assertRaisesRegex(AUDIT.JobLogRedirectError, "Location"): + with patch_attribute( + AUDIT.urllib_request, "build_opener", + CallRecorder(return_value=authenticated_opener)), \ + assert_raises_regex(self, AUDIT.JobLogRedirectError, "Location"): AUDIT.request_bytes(api_url, "github-token") def test_job_log_redirect_rejects_non_https_location(self): api_url = "https://api.github.com/repos/NVIDIA/cudf-spark/actions/jobs/123/logs" - authenticated_opener = mock.Mock() - authenticated_opener.open.side_effect = AUDIT.urllib.error.HTTPError( + authenticated_opener = FakeOpener() + authenticated_opener.open.side_effect = AUDIT.urllib_error.HTTPError( api_url, 302, "Found", {"Location": "http://logs.example/job.log"}, None) - with mock.patch.object( - AUDIT.urllib.request, "build_opener", return_value=authenticated_opener), \ - self.assertRaisesRegex(AUDIT.JobLogRedirectError, "HTTPS"): + with patch_attribute( + AUDIT.urllib_request, "build_opener", + CallRecorder(return_value=authenticated_opener)), \ + assert_raises_regex(self, AUDIT.JobLogRedirectError, "HTTPS"): AUDIT.request_bytes(api_url, "github-token") def test_job_log_endpoint_rejects_non_redirect_response(self): api_url = "https://api.github.com/repos/NVIDIA/cudf-spark/actions/jobs/123/logs" - authenticated_opener = mock.Mock() + authenticated_opener = FakeOpener() authenticated_opener.open.return_value = io.BytesIO(b"unexpected direct response") - with mock.patch.object( - AUDIT.urllib.request, "build_opener", return_value=authenticated_opener), \ - self.assertRaisesRegex(AUDIT.JobLogRedirectError, "expected redirect"): + with patch_attribute( + AUDIT.urllib_request, "build_opener", + CallRecorder(return_value=authenticated_opener)), \ + assert_raises_regex(self, AUDIT.JobLogRedirectError, "expected redirect"): AUDIT.request_bytes(api_url, "github-token") def test_download_logs_preserves_partial_results_after_bad_redirect(self): @@ -165,12 +255,12 @@ def test_download_logs_preserves_partial_results_after_bad_redirect(self): {"id": 2, "name": "package-tests (340, false)", "status": "completed"}, ]} bad_redirect = AUDIT.JobLogRedirectError("missing redirect location") - with mock.patch.object(AUDIT, "request_json", return_value=jobs), \ - mock.patch.object( - AUDIT, "request_bytes", - side_effect=[b"first job log", bad_redirect, bad_redirect, - bad_redirect, bad_redirect]) as request_bytes, \ - mock.patch.object(AUDIT.time, "sleep"): + request_bytes = CallRecorder( + side_effect=[b"first job log", bad_redirect, bad_redirect, + bad_redirect, bad_redirect]) + with patch_attribute(AUDIT, "request_json", CallRecorder(return_value=jobs)), \ + patch_attribute(AUDIT, "request_bytes", request_bytes), \ + patch_attribute(AUDIT.time, "sleep", CallRecorder()): logs, failures = AUDIT.download_logs( "https://api.github.com", "NVIDIA/cudf-spark", "run-id", "token", AUDIT.DEFAULT_JOB_PATTERN) @@ -189,40 +279,50 @@ def test_summary_reports_incomplete_collection(self): def test_main_fails_when_findings_are_present(self): log = ( - "/workspace/sql-plugin/src/main/scala/Test.scala:42: " + u"/workspace/sql-plugin/src/main/scala/Test.scala:42: " "[deprecation @ example.Test.run | " "origin=ai.rapids.cudf.ColumnView.oldApi | version=] deprecated\n" ) - with tempfile.TemporaryDirectory() as temp_dir: - root = Path(temp_dir) - (root / "package-tests.log").write_text(log, encoding="utf-8") - result = AUDIT.main([ - "--logs-dir", str(root), - "--raw-report", str(root / "report.json"), - ]) + with temporary_directory() as temp_dir: + log_path = os.path.join(temp_dir, "package-tests.log") + with io.open(log_path, "w", encoding="utf-8") as log_file: + log_file.write(log) + with captured_stdout(): + result = AUDIT.main([ + "--logs-dir", temp_dir, + "--raw-report", os.path.join(temp_dir, "report.json"), + ]) self.assertEqual(1, result) def test_main_succeeds_when_audit_is_clean(self): - with tempfile.TemporaryDirectory() as temp_dir: - root = Path(temp_dir) - (root / "package-tests.log").write_text("clean build\n", encoding="utf-8") - result = AUDIT.main([ - "--logs-dir", str(root), - "--raw-report", str(root / "report.json"), - ]) + with temporary_directory() as temp_dir: + log_path = os.path.join(temp_dir, "package-tests.log") + summary_path = os.path.join(temp_dir, "summary.md") + with io.open(log_path, "w", encoding="utf-8") as log_file: + log_file.write(u"clean build\n") + with captured_stdout(): + result = AUDIT.main([ + "--logs-dir", temp_dir, + "--summary", summary_path, + "--raw-report", os.path.join(temp_dir, "report.json"), + ]) + with io.open(summary_path, "r", encoding="utf-8") as summary_file: + self.assertIn("No compiler deprecation diagnostics", summary_file.read()) self.assertEqual(0, result) def test_main_fails_when_log_collection_is_incomplete(self): - with tempfile.TemporaryDirectory() as temp_dir, \ - mock.patch.object( + with temporary_directory() as temp_dir, \ + patch_attribute( AUDIT, "download_logs", - return_value=({}, ["package-tests: log unavailable"])), \ - mock.patch.dict(AUDIT.os.environ, {"GITHUB_TOKEN": "token"}): - result = AUDIT.main([ - "--repository", "NVIDIA/cudf-spark", - "--run-id", "123", - "--raw-report", str(Path(temp_dir) / "report.json"), - ]) + CallRecorder(return_value=({}, ["package-tests: log unavailable"]))), \ + patch_environment({"GITHUB_TOKEN": "token"}): + with captured_stdout(): + result = AUDIT.main([ + "--repository", "NVIDIA/cudf-spark", + "--run-id", "123", + "--logs-dir", "", + "--raw-report", os.path.join(temp_dir, "report.json"), + ]) self.assertEqual(1, result) def test_annotation_property_escaping(self): From 46edaea14413837b6a2dc2c7d19c7e2d3ea0be7e Mon Sep 17 00:00:00 2001 From: Gera Shegalov Date: Tue, 1 Sep 2026 12:17:33 -0700 Subject: [PATCH 4/9] Launch Jython from the plugin classpath Signed-off-by: Gera Shegalov --- pom.xml | 9 ++++++--- scala2.13/pom.xml | 9 ++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/pom.xml b/pom.xml index ff8be39f99d..41a757ba759 100644 --- a/pom.xml +++ b/pom.xml @@ -2013,12 +2013,14 @@ This will force full Scala code rebuild in downstream modules. run - - @@ -2077,7 +2079,8 @@ This will force full Scala code rebuild in downstream modules. - diff --git a/scala2.13/pom.xml b/scala2.13/pom.xml index 238f6916534..c1c82abe6da 100644 --- a/scala2.13/pom.xml +++ b/scala2.13/pom.xml @@ -2013,12 +2013,14 @@ This will force full Scala code rebuild in downstream modules. run - - @@ -2077,7 +2079,8 @@ This will force full Scala code rebuild in downstream modules. - From 7781efef90a92e5faaae9412957562e80e79ca3e Mon Sep 17 00:00:00 2001 From: Gera Shegalov Date: Tue, 1 Sep 2026 12:50:44 -0700 Subject: [PATCH 5/9] Keep deprecation audit on root POM Signed-off-by: Gera Shegalov --- pom.xml | 7 +++++-- scala2.13/pom.xml | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/pom.xml b/pom.xml index 41a757ba759..056c7744f95 100644 --- a/pom.xml +++ b/pom.xml @@ -2007,6 +2007,8 @@ This will force full Scala code rebuild in downstream modules. false + + nvidia-deprecation-audit none @@ -2024,14 +2026,15 @@ This will force full Scala code rebuild in downstream modules. fork="true" failonerror="true" dir="${project.basedir}"> - + - + + false + + + + - - - - - - - - + + + + - + + - + - + + diff --git a/scala2.13/pom.xml b/scala2.13/pom.xml index 4ef4fd8c08f..bb89e83b785 100644 --- a/scala2.13/pom.xml +++ b/scala2.13/pom.xml @@ -2072,16 +2072,17 @@ This will force full Scala code rebuild in downstream modules. - + - diff --git a/scripts/README.md b/scripts/README.md index b8c49752346..0077dd77ffc 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -51,4 +51,7 @@ Only hoist values that own their data. Views returned by methods such as `bitCas `getChildColumnView`, `replaceListChild`, and `splitAsViews` must not outlive their owning parent. Copy or convert a view to an owning resource before closing its parent. -The check and its unit tests run with the all-modules Scalastyle execution during `mvn verify`. +The script remains directly runnable with Python 3, but is also compatible with Jython 2.7. Maven +uses its managed Jython dependency to run the check and unit tests with the all-modules Scalastyle +execution during `mvn verify`; the generated Scala 2.13 reactor does not repeat this repository-wide +check. diff --git a/scripts/check_with_resource_nesting.py b/scripts/check_with_resource_nesting.py index 4cbc3a2ed38..1a7f4f39f54 100644 --- a/scripts/check_with_resource_nesting.py +++ b/scripts/check_with_resource_nesting.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python3 +#!/usr/bin/env python # Copyright (c) 2026, NVIDIA CORPORATION. # @@ -16,17 +16,16 @@ """Prevent new deeply nested withResource scopes in production Scala code.""" -from __future__ import annotations +from __future__ import print_function import argparse import collections -import dataclasses import hashlib +import io import json +import os import re import sys -from pathlib import Path -from typing import Iterable, Sequence DEFAULT_MAX_DEPTH = 4 @@ -39,50 +38,33 @@ r"(? tuple[str, str]: + def baseline_key(self): return (self.path, self.fingerprint) -@dataclasses.dataclass(frozen=True) -class ScanResult: - violations: tuple[Violation, ...] - directive_errors: tuple[str, ...] +ScanResult = collections.namedtuple("ScanResult", "violations directive_errors") -def _consume_quoted(source: str, start: int, quote: str) -> int: +def _consume_quoted(source, start, quote): """Return the first offset after a quoted Scala string or character literal.""" if quote == '"' and source.startswith('"""', start): end = source.find('"""', start + 3) @@ -103,7 +85,7 @@ def _consume_quoted(source: str, start: int, quote: str) -> int: return len(source) -def _is_interpolated_quote(source: str, quote_start: int) -> bool: +def _is_interpolated_quote(source, quote_start): if quote_start == 0: return False offset = quote_start - 1 @@ -114,7 +96,7 @@ def _is_interpolated_quote(source: str, quote_start: int) -> bool: return source[offset].isalpha() or source[offset] in "_$" -def _consume_block_comment(source: str, start: int) -> tuple[int, bool]: +def _consume_block_comment(source, start): depth = 1 offset = start + 2 while offset < len(source) and depth: @@ -129,7 +111,7 @@ def _consume_block_comment(source: str, start: int) -> tuple[int, bool]: return offset, depth == 0 -def _matching_interpolation_brace(source: str, open_brace: int) -> int: +def _matching_interpolation_brace(source, open_brace): depth = 1 offset = open_brace + 1 while offset < len(source): @@ -156,10 +138,10 @@ def _matching_interpolation_brace(source: str, open_brace: int) -> int: return len(source) -def _consume_interpolated(source: str, start: int) -> tuple[int, list[tuple[int, int]]]: +def _consume_interpolated(source, start): delimiter = '\"\"\"' if source.startswith('\"\"\"', start) else '"' offset = start + len(delimiter) - expressions: list[tuple[int, int]] = [] + expressions = [] while offset < len(source): if source.startswith(delimiter, offset): @@ -178,15 +160,15 @@ def _consume_interpolated(source: str, start: int) -> tuple[int, list[tuple[int, return len(source), expressions -def _tokenize(source: str) -> tuple[list[Token], list[LineComment]]: +def _tokenize(source): """Tokenize enough Scala syntax to match calls and lexical blocks. Comments and literal contents are deliberately opaque. This avoids counting braces or withResource text embedded in comments and literal text. Executable `${...}` expressions inside interpolated strings are tokenized recursively. """ - tokens: list[Token] = [] - line_comments: list[LineComment] = [] + tokens = [] + line_comments = [] offset = 0 line = 1 length = len(source) @@ -209,9 +191,10 @@ def _tokenize(source: str) -> tuple[list[Token], list[LineComment]]: offset, closed = _consume_block_comment(source, offset) line += source[comment_start:offset].count("\n") if not closed: - raise ValueError(f"unterminated block comment at offset {comment_start}") + raise ValueError( + "unterminated block comment at offset {0}".format(comment_start)) elif char in "\"'": - expressions: list[tuple[int, int]] = [] + expressions = [] if char == '"' and _is_interpolated_quote(source, offset): end, expressions = _consume_interpolated(source, offset) else: @@ -259,16 +242,11 @@ def _tokenize(source: str) -> tuple[list[Token], list[LineComment]]: return tokens, line_comments -def tokenize(source: str) -> list[Token]: +def tokenize(source): return _tokenize(source)[0] -def _matching_delimiter( - tokens: Sequence[Token], - open_index: int, - open_value: str, - close_value: str, -) -> int | None: +def _matching_delimiter(tokens, open_index, open_value, close_value): depth = 0 for index in range(open_index, len(tokens)): value = tokens[index].value @@ -281,18 +259,13 @@ def _matching_delimiter( return None -def _canonical_call(tokens: Sequence[Token], start: int, end: int) -> str: +def _canonical_call(tokens, start, end): return "".join(token.value for token in tokens[start:end + 1]) -def _directive_lines( - source: str, - path: str, - tokens: Sequence[Token], - line_comments: Sequence[LineComment], -) -> tuple[set[int], list[str]]: - exempt_lines: set[int] = set() - errors: list[str] = [] +def _directive_lines(source, path, tokens, line_comments): + exempt_lines = set() + errors = [] lines = source.splitlines() for comment in line_comments: @@ -302,13 +275,14 @@ def _directive_lines( line_number = comment.line if match is None or len(match.group(1).strip()) < 10: errors.append( - f"{path}:{line_number}: {ALLOW_DIRECTIVE} requires a reason of at least " - "10 characters after ' -- '") + "{0}:{1}: {2} requires a reason of at least " + "10 characters after ' -- '".format( + path, line_number, ALLOW_DIRECTIVE)) continue if ISSUE_PATTERN.search(match.group(1)) is None: errors.append( - f"{path}:{line_number}: {ALLOW_DIRECTIVE} reason must reference an " - "NVIDIA/cudf-spark GitHub issue by URL or #number") + "{0}:{1}: {2} reason must reference an NVIDIA/cudf-spark GitHub " + "issue by URL or #number".format(path, line_number, ALLOW_DIRECTIVE)) continue # The directive applies to a withResource call on the same line or the next nonblank line. @@ -326,15 +300,15 @@ def _directive_lines( return exempt_lines, errors -def scan_source(path: str, source: str, max_depth: int) -> ScanResult: +def scan_source(path, source, max_depth): try: tokens, line_comments = _tokenize(source) except ValueError as error: - return ScanResult((), (f"{path}: {error}",)) + return ScanResult((), ("{0}: {1}".format(path, error),)) exempt_lines, directive_errors = _directive_lines( source, path, tokens, line_comments) - resource_blocks: dict[int, ResourceCall] = {} + resource_blocks = {} for index, token in enumerate(tokens): if token.value != "withResource" or index + 1 >= len(tokens): @@ -363,8 +337,8 @@ def scan_source(path: str, source: str, max_depth: int) -> ScanResult: resource=canonical, exempt=token.line in exempt_lines) - violations: list[Violation] = [] - scope_stack: list[ResourceCall | None] = [] + violations = [] + scope_stack = [] for index, token in enumerate(tokens): if token.value in {"{", "("}: resource_call = resource_blocks.get(index) @@ -386,77 +360,90 @@ def scan_source(path: str, source: str, max_depth: int) -> ScanResult: return ScanResult(tuple(violations), tuple(directive_errors)) -def production_scala_files(root: Path) -> Iterable[Path]: - for path in root.rglob("*.scala"): - relative = path.relative_to(root) - parts = relative.parts - if "target" in parts or (parts and parts[0] == "scala2.13"): - continue - if any(parts[index:index + 2] == ("src", "main") - for index in range(len(parts) - 1)): - yield path - - -def scan_tree(root: Path, max_depth: int) -> ScanResult: - violations: list[Violation] = [] - directive_errors: list[str] = [] +def production_scala_files(root): + for directory, directory_names, file_names in os.walk(root): + relative_directory = os.path.relpath(directory, root) + parts = (() if relative_directory == "." else + tuple(relative_directory.split(os.sep))) + directory_names[:] = sorted( + name for name in directory_names + if name != "target" and not (not parts and name == "scala2.13")) + in_production_source = any( + parts[index:index + 2] == ("src", "main") + for index in range(len(parts) - 1)) + if in_production_source: + for file_name in sorted(file_names): + if file_name.endswith(".scala"): + yield os.path.join(directory, file_name) + + +def scan_tree(root, max_depth): + violations = [] + directive_errors = [] for path in sorted(production_scala_files(root)): - relative = path.relative_to(root).as_posix() - result = scan_source(relative, path.read_text(encoding="utf-8"), max_depth) + relative = os.path.relpath(path, root).replace(os.sep, "/") + with io.open(path, "r", encoding="utf-8") as source_file: + result = scan_source(relative, source_file.read(), max_depth) violations.extend(result.violations) directive_errors.extend(result.directive_errors) return ScanResult(tuple(violations), tuple(directive_errors)) -def load_baseline(path: Path) -> tuple[int, collections.Counter[tuple[str, str]]]: - data = json.loads(path.read_text(encoding="utf-8")) +def _fullmatch(pattern, value): + match = pattern.match(value) + return match is not None and match.end() == len(value) + + +def load_baseline(path): + with io.open(path, "r", encoding="utf-8") as baseline_file: + data = json.loads(baseline_file.read()) if data.get("version") != BASELINE_VERSION: raise ValueError( - f"unsupported baseline version {data.get('version')}; expected {BASELINE_VERSION}") + "unsupported baseline version {0}; expected {1}".format( + data.get("version"), BASELINE_VERSION)) max_depth = data.get("maxDepth") if not isinstance(max_depth, int) or max_depth < 1: raise ValueError("baseline maxDepth must be a positive integer") tracking_issue = data.get("trackingIssue") - if not isinstance(tracking_issue, str) or ISSUE_PATTERN.fullmatch(tracking_issue) is None: + if not isinstance(tracking_issue, STRING_TYPES) or not _fullmatch( + ISSUE_PATTERN, tracking_issue): raise ValueError("baseline trackingIssue must link to an NVIDIA/cudf-spark GitHub issue") - entries: collections.Counter[tuple[str, str]] = collections.Counter() + entries = collections.Counter() for entry in data.get("entries", []): key = (entry["path"], entry["fingerprint"]) entries[key] += entry.get("count", 1) return max_depth, entries -def baseline_json(violations: Sequence[Violation], max_depth: int) -> str: - grouped: dict[tuple[str, str], list[Violation]] = collections.defaultdict(list) +def baseline_json(violations, max_depth): + grouped = collections.defaultdict(list) for violation in violations: grouped[violation.baseline_key].append(violation) entries = [] for (path, fingerprint), matches in sorted(grouped.items()): - entry = { - "path": path, - "fingerprint": fingerprint, - "resource": matches[0].resource[:160], - } + entry = collections.OrderedDict(( + ("path", path), + ("fingerprint", fingerprint), + ("resource", matches[0].resource[:160]), + )) if len(matches) > 1: entry["count"] = len(matches) entries.append(entry) - return json.dumps({ - "version": BASELINE_VERSION, - "maxDepth": max_depth, - "trackingIssue": DEFAULT_TRACKING_ISSUE, - "entries": entries, - }, indent=2) + "\n" + baseline = collections.OrderedDict(( + ("version", BASELINE_VERSION), + ("maxDepth", max_depth), + ("trackingIssue", DEFAULT_TRACKING_ISSUE), + ("entries", entries), + )) + return TEXT_TYPE(json.dumps(baseline, indent=2, separators=(",", ": "))) + "\n" -def new_violations( - violations: Sequence[Violation], - baseline: collections.Counter[tuple[str, str]], -) -> list[Violation]: +def new_violations(violations, baseline): remaining = baseline.copy() - result: list[Violation] = [] + result = [] for violation in violations: key = violation.baseline_key if remaining[key] > 0: @@ -466,20 +453,17 @@ def new_violations( return result -def stale_baseline_entries( - violations: Sequence[Violation], - baseline: collections.Counter[tuple[str, str]], -) -> collections.Counter[tuple[str, str]]: +def stale_baseline_entries(violations, baseline): current = collections.Counter(violation.baseline_key for violation in violations) return baseline - current -def parse_args(args: Sequence[str]) -> argparse.Namespace: +def parse_args(args): parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("--root", type=Path, default=Path.cwd(), + parser.add_argument("--root", default=os.getcwd(), help="repository root (default: current directory)") - parser.add_argument("--baseline", type=Path, - default=Path("scripts/with_resource_nesting_baseline.json")) + parser.add_argument("--baseline", + default="scripts/with_resource_nesting_baseline.json") parser.add_argument("--max-depth", type=int, default=None, help="override maximum allowed depth") parser.add_argument("--print-baseline", action="store_true", @@ -489,20 +473,21 @@ def parse_args(args: Sequence[str]) -> argparse.Namespace: return parser.parse_args(args) -def main(argv: Sequence[str] | None = None) -> int: +def main(argv=None): args = parse_args(sys.argv[1:] if argv is None else argv) - root = args.root.resolve() + root = os.path.abspath(args.root) baseline_path = args.baseline - if not baseline_path.is_absolute(): - baseline_path = root / baseline_path + if not os.path.isabs(baseline_path): + baseline_path = os.path.join(root, baseline_path) - baseline: collections.Counter[tuple[str, str]] = collections.Counter() + baseline = collections.Counter() baseline_depth = DEFAULT_MAX_DEPTH - if baseline_path.exists(): + if os.path.exists(baseline_path): try: baseline_depth, baseline = load_baseline(baseline_path) - except (KeyError, TypeError, ValueError, json.JSONDecodeError) as error: - print(f"Invalid withResource nesting baseline: {error}", file=sys.stderr) + except (KeyError, TypeError, ValueError) as error: + print("Invalid withResource nesting baseline: {0}".format(error), + file=sys.stderr) return 2 max_depth = args.max_depth if args.max_depth is not None else baseline_depth @@ -521,16 +506,18 @@ def main(argv: Sequence[str] | None = None) -> int: print(generated_baseline, end="") return 0 if args.update_baseline: - baseline_path.write_text(generated_baseline, encoding="utf-8") - print(f"Updated {baseline_path} with {len(scan.violations)} violations") + with io.open(baseline_path, "w", encoding="utf-8") as baseline_file: + baseline_file.write(generated_baseline) + print("Updated {0} with {1} violations".format( + baseline_path, len(scan.violations))) return 0 unexpected = new_violations(scan.violations, baseline) stale = stale_baseline_entries(scan.violations, baseline) if not unexpected and not stale: print( - f"withResource nesting lint passed ({len(scan.violations)} baselined violations, " - f"maximum allowed depth {max_depth})") + "withResource nesting lint passed ({0} baselined violations, maximum " + "allowed depth {1})".format(len(scan.violations), max_depth)) return 0 for violation in unexpected: @@ -538,14 +525,14 @@ def main(argv: Sequence[str] | None = None) -> int: if len(resource) > 120: resource = resource[:117] + "..." print( - f"{violation.path}:{violation.line}: withResource nesting depth " - f"{violation.depth} exceeds {max_depth}\n resource: {resource}", + "{0}:{1}: withResource nesting depth {2} exceeds {3}\n resource: {4}".format( + violation.path, violation.line, violation.depth, max_depth, resource), file=sys.stderr) if unexpected: print( - f"Found {len(unexpected)} new deep withResource scope(s). Shorten resource lifetimes " - f"or place '// {ALLOW_DIRECTIVE} -- ' immediately before a " - "scope whose overlap is necessary.", + "Found {0} new deep withResource scope(s). Shorten resource lifetimes or " + "place '// {1} -- ' immediately before a scope " + "whose overlap is necessary.".format(len(unexpected), ALLOW_DIRECTIVE), file=sys.stderr) if unexpected and stale: print( @@ -556,8 +543,8 @@ def main(argv: Sequence[str] | None = None) -> int: if stale: stale_count = sum(stale.values()) print( - f"The baseline contains {stale_count} resolved violation(s). Run this check with " - "--update-baseline to ratchet it down.", + "The baseline contains {0} resolved violation(s). Run this check with " + "--update-baseline to ratchet it down.".format(stale_count), file=sys.stderr) return 1 diff --git a/scripts/tests/test_check_with_resource_nesting.py b/scripts/tests/test_check_with_resource_nesting.py index 3410c676dbc..43b1983a587 100644 --- a/scripts/tests/test_check_with_resource_nesting.py +++ b/scripts/tests/test_check_with_resource_nesting.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python3 +#!/usr/bin/env python # Copyright (c) 2026, NVIDIA CORPORATION. # @@ -14,29 +14,82 @@ # See the License for the specific language governing permissions and # limitations under the License. +from __future__ import print_function + import collections import contextlib -import importlib.util import io import json +import os +import shutil import sys import tempfile import unittest -from pathlib import Path -SCRIPT = Path(__file__).parents[1] / "check_with_resource_nesting.py" -SPEC = importlib.util.spec_from_file_location("check_with_resource_nesting", SCRIPT) -LINT = importlib.util.module_from_spec(SPEC) -assert SPEC.loader is not None -sys.modules[SPEC.name] = LINT -SPEC.loader.exec_module(LINT) +try: + TEXT_TYPE = unicode +except NameError: # Python 3 + TEXT_TYPE = str + + +SCRIPT = os.path.join(os.path.dirname(os.path.dirname(__file__)), + "check_with_resource_nesting.py") +try: + import importlib.util + SPEC = importlib.util.spec_from_file_location("check_with_resource_nesting", SCRIPT) + LINT = importlib.util.module_from_spec(SPEC) + sys.modules[SPEC.name] = LINT + SPEC.loader.exec_module(LINT) +except ImportError: # Jython 2.7 / Python 2.7 + import imp + LINT = imp.load_source("check_with_resource_nesting", SCRIPT) + + +@contextlib.contextmanager +def temporary_directory(): + path = tempfile.mkdtemp() + try: + yield path + finally: + shutil.rmtree(path) + + +class OutputSink(object): + def __init__(self): + self.parts = [] + + def write(self, value): + self.parts.append(value) + + def flush(self): + pass + + def getvalue(self): + return TEXT_TYPE("").join(self.parts) + + +@contextlib.contextmanager +def captured_stream(name): + output = OutputSink() + original = getattr(sys, name) + setattr(sys, name, output) + try: + yield output + finally: + setattr(sys, name, original) + + +def write_text(path, value): + with io.open(path, "w", encoding="utf-8") as output_file: + output_file.write(TEXT_TYPE(value)) def nested_source(depth): body = "result" for index in reversed(range(depth)): - body = f"withResource(make{index}()) {{ resource{index} =>\n{body}\n}}" + body = "withResource(make{0}()) {{ resource{0} =>\n{1}\n}}".format( + index, body) return body @@ -192,69 +245,124 @@ def test_fingerprint_does_not_depend_on_depth(self): shallower = LINT.scan_source("Test.scala", nested_source(5), 3).violations[-1] self.assertEqual(deep.fingerprint, shallower.fingerprint) + def test_baseline_json_is_stable_across_runtimes(self): + violation = LINT.scan_source( + "Test.scala", "withResource(make()) { resource => result }", 0).violations[0] + self.assertEqual("ba163e5cbee380205ebb", violation.fingerprint) + self.assertEqual("""{ + "version": 1, + "maxDepth": 0, + "trackingIssue": "https://github.com/NVIDIA/cudf-spark/issues/11713", + "entries": [ + { + "path": "Test.scala", + "fingerprint": "ba163e5cbee380205ebb", + "resource": "withResource(make())" + } + ] +} +""", LINT.baseline_json((violation,), 0)) + + def test_production_source_discovery_excludes_generated_and_test_trees(self): + with temporary_directory() as root: + relative_paths = ( + "module/src/main/scala/Keep.scala", + "module/src/test/scala/IgnoreTest.scala", + "module/target/generated/src/main/scala/IgnoreTarget.scala", + "scala2.13/module/src/main/scala/IgnoreGeneratedPomTree.scala", + ) + for relative_path in relative_paths: + path = os.path.join(root, *relative_path.split("/")) + parent = os.path.dirname(path) + if not os.path.isdir(parent): + os.makedirs(parent) + write_text(path, "object Fixture\n") + + discovered = [ + os.path.relpath(path, root).replace(os.sep, "/") + for path in LINT.production_scala_files(root) + ] + self.assertEqual(["module/src/main/scala/Keep.scala"], discovered) + def test_command_fails_for_new_violation(self): - with tempfile.TemporaryDirectory() as temp_dir: - root = Path(temp_dir) - source_dir = root / "module" / "src" / "main" / "scala" - source_dir.mkdir(parents=True) - (source_dir / "Test.scala").write_text(nested_source(5), encoding="utf-8") - baseline = root / "baseline.json" - baseline.write_text(json.dumps({ + with temporary_directory() as root: + source_dir = os.path.join(root, "module", "src", "main", "scala") + os.makedirs(source_dir) + write_text(os.path.join(source_dir, "Test.scala"), nested_source(5)) + baseline = os.path.join(root, "baseline.json") + write_text(baseline, json.dumps({ "version": 1, "maxDepth": 4, "trackingIssue": "https://github.com/NVIDIA/cudf-spark/issues/11713", "entries": [], - }), encoding="utf-8") + })) - stderr = io.StringIO() - with contextlib.redirect_stderr(stderr): + with captured_stream("stderr") as stderr: exit_code = LINT.main([ - "--root", str(root), - "--baseline", str(baseline), + "--root", root, + "--baseline", baseline, ]) self.assertEqual(1, exit_code) self.assertIn("nesting depth 5 exceeds 4", stderr.getvalue()) def test_command_accepts_justified_exemption(self): - with tempfile.TemporaryDirectory() as temp_dir: - root = Path(temp_dir) - source_dir = root / "module" / "src" / "main" / "scala" - source_dir.mkdir(parents=True) + with temporary_directory() as root: + source_dir = os.path.join(root, "module", "src", "main", "scala") + os.makedirs(source_dir) source = ( "// with-resource-lint: allow-deep-nesting -- required by " "https://github.com/NVIDIA/cudf-spark/issues/11713\n" + nested_source(5)) - (source_dir / "Test.scala").write_text(source, encoding="utf-8") - baseline = root / "baseline.json" - baseline.write_text(json.dumps({ + write_text(os.path.join(source_dir, "Test.scala"), source) + baseline = os.path.join(root, "baseline.json") + write_text(baseline, json.dumps({ "version": 1, "maxDepth": 4, "trackingIssue": "https://github.com/NVIDIA/cudf-spark/issues/11713", "entries": [], - }), encoding="utf-8") + })) - stdout = io.StringIO() - with contextlib.redirect_stdout(stdout): + with captured_stream("stdout") as stdout: exit_code = LINT.main([ - "--root", str(root), - "--baseline", str(baseline), + "--root", root, + "--baseline", baseline, ]) self.assertEqual(0, exit_code) self.assertIn("lint passed", stdout.getvalue()) + def test_command_updates_baseline(self): + with temporary_directory() as root: + source_dir = os.path.join(root, "module", "src", "main", "scala") + os.makedirs(source_dir) + write_text(os.path.join(source_dir, "Test.scala"), nested_source(5)) + baseline = os.path.join(root, "baseline.json") + + with captured_stream("stdout") as stdout: + exit_code = LINT.main([ + "--root", root, + "--baseline", baseline, + "--update-baseline", + ]) + + self.assertEqual(0, exit_code) + self.assertIn("Updated", stdout.getvalue()) + with io.open(baseline, "r", encoding="utf-8") as baseline_file: + generated = baseline_file.read() + scan = LINT.scan_tree(root, 4) + self.assertEqual(LINT.baseline_json(scan.violations, 4), generated) + def test_command_explains_fingerprint_changes(self): - with tempfile.TemporaryDirectory() as temp_dir: - root = Path(temp_dir) - source_dir = root / "module" / "src" / "main" / "scala" - source_dir.mkdir(parents=True) + with temporary_directory() as root: + source_dir = os.path.join(root, "module", "src", "main", "scala") + os.makedirs(source_dir) source = nested_source(5) - source_path = source_dir / "Test.scala" - source_path.write_text(source, encoding="utf-8") + source_path = os.path.join(source_dir, "Test.scala") + write_text(source_path, source) violation = LINT.scan_source("module/src/main/scala/Test.scala", source, 4).violations[0] - baseline = root / "baseline.json" - baseline.write_text(json.dumps({ + baseline = os.path.join(root, "baseline.json") + write_text(baseline, json.dumps({ "version": 1, "maxDepth": 4, "trackingIssue": "https://github.com/NVIDIA/cudf-spark/issues/11713", @@ -263,14 +371,13 @@ def test_command_explains_fingerprint_changes(self): "fingerprint": violation.fingerprint, "resource": violation.resource, }], - }), encoding="utf-8") - source_path.write_text(source.replace("make4", "renamedMake4"), encoding="utf-8") + })) + write_text(source_path, source.replace("make4", "renamedMake4")) - stderr = io.StringIO() - with contextlib.redirect_stderr(stderr): + with captured_stream("stderr") as stderr: exit_code = LINT.main([ - "--root", str(root), - "--baseline", str(baseline), + "--root", root, + "--baseline", baseline, ]) self.assertEqual(1, exit_code) From f298d29a941f49303c82a1549414409c843a4588 Mon Sep 17 00:00:00 2001 From: Gera Shegalov Date: Wed, 2 Sep 2026 11:55:39 -0700 Subject: [PATCH 7/9] Publish withResource nesting audit report Signed-off-by: Gera Shegalov --- .github/workflows/mvn-verify-check.yml | 27 +- CONTRIBUTING.md | 18 - pom.xml | 70 +-- scala2.13/pom.xml | 70 +-- scripts/README.md | 10 +- scripts/check_deprecation_policy.py | 318 ------------- scripts/check_with_resource_nesting.py | 141 +++++- scripts/deprecation_audit.py | 416 ------------------ .../tests/test_check_with_resource_nesting.py | 72 +++ scripts/tests/test_deprecation_audit.py | 334 -------------- 10 files changed, 250 insertions(+), 1226 deletions(-) delete mode 100644 scripts/check_deprecation_policy.py delete mode 100644 scripts/deprecation_audit.py delete mode 100644 scripts/tests/test_deprecation_audit.py diff --git a/.github/workflows/mvn-verify-check.yml b/.github/workflows/mvn-verify-check.yml index 54c3d2b54e2..21aa7edfe92 100644 --- a/.github/workflows/mvn-verify-check.yml +++ b/.github/workflows/mvn-verify-check.yml @@ -36,7 +36,6 @@ env: -Drapids.secondaryCacheDir=$HOME/.m2/repository/.sbt/1.0/zinc/org.scala-sbt permissions: - actions: read contents: read jobs: @@ -432,30 +431,22 @@ jobs: } done - nvidia-deprecation-audit: - name: NVIDIA deprecation audit (optional) - if: ${{ always() }} - needs: - - package-tests - - package-tests-scala213 - - verify-213-modules - - verify-all-212-modules - - install-modules + with-resource-nesting-audit: + name: withResource nesting audit runs-on: ubuntu-latest steps: - uses: NVIDIA/spark-rapids-common/checkout@main - - name: Collect compiler deprecations from matrix logs - env: - GITHUB_TOKEN: ${{ github.token }} + - name: Report withResource nesting violations run: | - mvn --batch-mode -N antrun:run@nvidia-deprecation-audit \ - -Ddeprecation.audit.rawReport="$RUNNER_TEMP/nvidia-deprecation-audit.json" + mvn --batch-mode -N antrun:run@with-resource-nesting-audit \ + -DwithResource.audit.rawReport="$RUNNER_TEMP/with-resource-nesting-audit.json" \ + -DwithResource.audit.summary="$GITHUB_STEP_SUMMARY" - - name: Upload deprecation report + - name: Upload withResource nesting report if: ${{ always() }} uses: actions/upload-artifact@v4 with: - name: nvidia-deprecation-audit - path: ${{ runner.temp }}/nvidia-deprecation-audit.json + name: with-resource-nesting-audit + path: ${{ runner.temp }}/with-resource-nesting-audit.json if-no-files-found: ignore diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e71e4111ee2..c52cf4ca5f2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -235,24 +235,6 @@ or similarly ./build/buildall --rebuild-dist-only --option="-Ddist.jar.compress=false -Drapids.jni.unpack.skip" ``` -### Cross-repository API deprecations - -API replacements in cuDF Java, cudf-spark-jni, and cudf-spark-private must allow cudf-spark time to -consume a published artifact before the old entry point becomes deprecated. Introduce the -replacement first while the old method remains supported and delegates to the same implementation. -After the updated snapshot is available, migrate cudf-spark callers in a separate change. Add the -deprecation annotation only after known callers have migrated, and retain the compatibility entry -point for at least one more release before removal. - -Scala deprecations originating in NVIDIA-owned `ai.rapids.cudf`, `com.nvidia.spark.rapids`, and -`org.apache.spark.sql.rapids` APIs are reported as compiler information instead of fatal warnings -during this migration window. The same applies to cudf-spark-private's -`org.apache.spark.sql.execution.aggregate.PartialAggUtils` bridge; other APIs in Apache Spark -namespaces are not exempt. Deprecations from other dependencies remain build errors. The -optional NVIDIA deprecation audit in pull requests collects these diagnostics across the Maven -build matrix. Findings or incomplete log collection fail the audit check so contributors inspect -the result, but the audit is not a required build check; build-job results remain authoritative. - ## Code contributions ### Source code layout diff --git a/pom.xml b/pom.xml index e5acfd9bee0..45cf4840373 100644 --- a/pom.xml +++ b/pom.xml @@ -1153,8 +1153,8 @@ 4.9.10 3.1.1 3.3.0 - 2.7.3 - ${project.build.directory}/nvidia-deprecation-audit.json + ${project.build.directory}/with-resource-nesting-audit.json + ${project.build.directory}/with-resource-nesting-summary.md 2.0.2 30.0-jre 2.0.0 @@ -1621,7 +1621,7 @@ This will force full Scala code rebuild in downstream modules. org.python jython-standalone - ${jython.version} + 2.7.3 net.sourceforge.pmd @@ -1745,18 +1745,6 @@ This will force full Scala code rebuild in downstream modules. -Wconf:cat=unused-privates:e -Wunused:imports,locals,patvars,privates --> - - -Wconf:cat=deprecation&origin=ai\.rapids\.cudf\..*:iv - -Wconf:cat=deprecation&origin=com\.nvidia\.spark\.rapids\..*:iv - -Wconf:cat=deprecation&origin=org\.apache\.spark\.sql\.rapids\..*:iv - -Wconf:cat=deprecation&origin=org\.apache\.spark\.sql\.execution\.aggregate\.PartialAggUtils([.$].*|$):iv ${scala.javac.args} false - + - nvidia-deprecation-audit - none + with-resource-nesting-audit + verify run @@ -2019,17 +2008,19 @@ This will force full Scala code rebuild in downstream modules. classpathref="maven.plugin.classpath" fork="true" failonerror="true" dir="${project.basedir}"> - + - - + + - + + + @@ -2072,41 +2063,6 @@ This will force full Scala code rebuild in downstream modules. - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/scala2.13/pom.xml b/scala2.13/pom.xml index bb89e83b785..c0540689dfc 100644 --- a/scala2.13/pom.xml +++ b/scala2.13/pom.xml @@ -1153,8 +1153,8 @@ 4.9.10 3.1.1 3.3.0 - 2.7.3 - ${project.build.directory}/nvidia-deprecation-audit.json + ${project.build.directory}/with-resource-nesting-audit.json + ${project.build.directory}/with-resource-nesting-summary.md 2.0.2 30.0-jre 2.0.0 @@ -1621,7 +1621,7 @@ This will force full Scala code rebuild in downstream modules. org.python jython-standalone - ${jython.version} + 2.7.3 net.sourceforge.pmd @@ -1745,18 +1745,6 @@ This will force full Scala code rebuild in downstream modules. -Wconf:cat=unused-privates:e -Wunused:imports,locals,patvars,privates - - -Wconf:cat=deprecation&origin=ai\.rapids\.cudf\..*:iv - -Wconf:cat=deprecation&origin=com\.nvidia\.spark\.rapids\..*:iv - -Wconf:cat=deprecation&origin=org\.apache\.spark\.sql\.rapids\..*:iv - -Wconf:cat=deprecation&origin=org\.apache\.spark\.sql\.execution\.aggregate\.PartialAggUtils([.$].*|$):iv ${scala.javac.args} @@ -2007,11 +1995,12 @@ This will force full Scala code rebuild in downstream modules. false - + - - - - - - - - - - - diff --git a/scripts/README.md b/scripts/README.md index 0077dd77ffc..00aa34a78eb 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -52,6 +52,10 @@ Only hoist values that own their data. Views returned by methods such as `bitCas Copy or convert a view to an owning resource before closing its parent. The script remains directly runnable with Python 3, but is also compatible with Jython 2.7. Maven -uses its managed Jython dependency to run the check and unit tests with the all-modules Scalastyle -execution during `mvn verify`; the generated Scala 2.13 reactor does not repeat this repository-wide -check. +uses its managed Jython dependency to run the check and unit tests during root `mvn verify`; the +generated Scala 2.13 reactor does not repeat this repository-wide check. + +The pull-request audit publishes every deep scope in the job summary and a JSON artifact. Existing +baseline entries are reported as debt but do not fail the check. New violations, stale baseline +entries, invalid exemptions, or report-generation errors fail it. GitHub source annotations show +the first 50 entries, with the complete set retained in the summary and artifact. diff --git a/scripts/check_deprecation_policy.py b/scripts/check_deprecation_policy.py deleted file mode 100644 index f9d21f00d7c..00000000000 --- a/scripts/check_deprecation_policy.py +++ /dev/null @@ -1,318 +0,0 @@ -# Copyright (c) 2026, NVIDIA CORPORATION. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -"""Compile deprecation-policy fixtures using Jython 2.7-compatible code.""" - -from __future__ import print_function - -import argparse -import io -import os -import shutil -import subprocess -import sys -import tempfile -import xml.etree.ElementTree as ElementTree - - -MAVEN_NAMESPACE = "{http://maven.apache.org/POM/4.0.0}" -try: - TEXT_TYPE = unicode -except NameError: # Python 3 - TEXT_TYPE = str - - -class CommandResult(object): - def __init__(self, returncode, stdout): - self.returncode = returncode - self.stdout = stdout - - -def maven_tag(name): - return MAVEN_NAMESPACE + name - - -def find_executable(name): - extensions = [""] - if os.name == "nt": - extensions.extend(os.environ.get("PATHEXT", ".EXE").split(os.pathsep)) - for directory in os.environ.get("PATH", "").split(os.pathsep): - for extension in extensions: - candidate = os.path.join(directory, name + extension) - if os.path.isfile(candidate) and os.access(candidate, os.X_OK): - return candidate - return None - - -def compiler_configuration(pom_path): - root = ElementTree.parse(pom_path).getroot() - scala_version = root.findtext( - "{0}/{1}".format(maven_tag("properties"), maven_tag("scala.version"))) - if not scala_version: - raise RuntimeError("Could not find scala.version in {0}".format(pom_path)) - plugin_paths = ( - "{0}/{1}/{2}".format( - maven_tag("build"), maven_tag("plugins"), maven_tag("plugin")), - "{0}/{1}/{2}/{3}".format( - maven_tag("build"), maven_tag("pluginManagement"), - maven_tag("plugins"), maven_tag("plugin")), - ) - plugins = ( - plugin - for plugin_path in plugin_paths - for plugin in root.findall(plugin_path) - ) - for plugin in plugins: - artifact_id = plugin.findtext(maven_tag("artifactId")) - if artifact_id == "scala-maven-plugin": - args = [ - argument.text - for argument in plugin.findall("{0}/{1}/{2}".format( - maven_tag("configuration"), maven_tag("args"), maven_tag("arg"))) - if argument.text - ] - if not args: - raise RuntimeError( - "scala-maven-plugin has no compiler arguments in {0}".format(pom_path)) - return scala_version, args - raise RuntimeError("Could not find scala-maven-plugin in {0}".format(pom_path)) - - -def scala_compiler_classpath(maven_repo, scala_version): - scala_root = os.path.join(maven_repo, "org", "scala-lang") - jars = [ - os.path.join( - scala_root, artifact, scala_version, - "{0}-{1}.jar".format(artifact, scala_version)) - for artifact in ("scala-compiler", "scala-library", "scala-reflect") - ] - missing = [jar for jar in jars if not os.path.isfile(jar)] - if missing: - raise RuntimeError("Missing Scala compiler dependencies: " + ", ".join(missing)) - return jars - - -def run_command(command): - process = subprocess.Popen( - command, universal_newlines=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) - stdout, _ = process.communicate() - return CommandResult(process.returncode, stdout) - - -def write_fixtures(root): - sources = { - "ai/rapids/cudf/fixture/NvidiaApi.java": """ -package ai.rapids.cudf.fixture; -public final class NvidiaApi { - private NvidiaApi() {} - @Deprecated public static void oldApi() {} -} -""", - "org/example/fixture/ThirdPartyApi.java": """ -package org.example.fixture; -public final class ThirdPartyApi { - private ThirdPartyApi() {} - @Deprecated public static void oldApi() {} -} -""", - "com/nvidia/spark/rapids/jni/fixture/JniApi.java": """ -package com.nvidia.spark.rapids.jni.fixture; -public final class JniApi { - private JniApi() {} - @Deprecated public static void oldApi() {} -} -""", - "com/nvidia/spark/rapids/optimizer/fixture/PrivateApi.java": """ -package com.nvidia.spark.rapids.optimizer.fixture; -public final class PrivateApi { - private PrivateApi() {} - @Deprecated public static void oldApi() {} -} -""", - "org/apache/spark/sql/rapids/internal/fixture/PrivateApi.java": """ -package org.apache.spark.sql.rapids.internal.fixture; -public final class PrivateApi { - private PrivateApi() {} - @Deprecated public static void oldApi() {} -} -""", - "org/apache/spark/sql/execution/aggregate/PartialAggUtils.java": """ -package org.apache.spark.sql.execution.aggregate; -public final class PartialAggUtils { - private PartialAggUtils() {} - @Deprecated public static void oldApi() {} -} -""", - "org/apache/spark/sql/execution/aggregate/PartialAggUtilsNeighbor.java": """ -package org.apache.spark.sql.execution.aggregate; -public final class PartialAggUtilsNeighbor { - private PartialAggUtilsNeighbor() {} - @Deprecated public static void oldApi() {} -} -""", - "org/apache/spark/sql/execution/aggregate/SparkApi.java": """ -package org.apache.spark.sql.execution.aggregate; -public final class SparkApi { - private SparkApi() {} - @Deprecated public static void oldApi() {} -} -""", - "NvidiaCall.scala": """ -object NvidiaCall { - def call(): Unit = ai.rapids.cudf.fixture.NvidiaApi.oldApi() -} -""", - "JniCall.scala": """ -object JniCall { - def call(): Unit = com.nvidia.spark.rapids.jni.fixture.JniApi.oldApi() -} -""", - "PrivateComNvidiaCall.scala": """ -object PrivateComNvidiaCall { - def call(): Unit = com.nvidia.spark.rapids.optimizer.fixture.PrivateApi.oldApi() -} -""", - "PrivateRapidsCall.scala": """ -object PrivateRapidsCall { - def call(): Unit = org.apache.spark.sql.rapids.internal.fixture.PrivateApi.oldApi() -} -""", - "PrivateSparkBridgeCall.scala": """ -object PrivateSparkBridgeCall { - def call(): Unit = org.apache.spark.sql.execution.aggregate.PartialAggUtils.oldApi() -} -""", - "PartialAggUtilsNeighborCall.scala": """ -object PartialAggUtilsNeighborCall { - def call(): Unit = org.apache.spark.sql.execution.aggregate.PartialAggUtilsNeighbor.oldApi() -} -""", - "SparkCall.scala": """ -object SparkCall { - def call(): Unit = org.apache.spark.sql.execution.aggregate.SparkApi.oldApi() -} -""", - "ThirdPartyCall.scala": """ -object ThirdPartyCall { - def call(): Unit = org.example.fixture.ThirdPartyApi.oldApi() -} -""", - } - for relative_path, source in sources.items(): - path = os.path.join(root, relative_path) - parent = os.path.dirname(path) - if not os.path.isdir(parent): - os.makedirs(parent) - with io.open(path, "w", encoding="utf-8") as source_file: - source_file.write(TEXT_TYPE(source.lstrip())) - - -def check_policy(pom_path, maven_repo): - scala_version, compiler_args = compiler_configuration(pom_path) - compiler_jar, library_jar, reflect_jar = scala_compiler_classpath( - maven_repo, scala_version) - javac = find_executable("javac") - java = find_executable("java") - if not javac or not java: - raise RuntimeError("Both java and javac are required for the deprecation policy check") - - temp_dir = tempfile.mkdtemp(prefix="cudf-spark-deprecation-policy-") - try: - fixture_root = temp_dir - classes = os.path.join(fixture_root, "classes") - os.mkdir(classes) - write_fixtures(fixture_root) - java_compile = run_command([ - javac, "-d", classes, - os.path.join(fixture_root, "ai/rapids/cudf/fixture/NvidiaApi.java"), - os.path.join(fixture_root, "com/nvidia/spark/rapids/jni/fixture/JniApi.java"), - os.path.join( - fixture_root, "com/nvidia/spark/rapids/optimizer/fixture/PrivateApi.java"), - os.path.join( - fixture_root, "org/apache/spark/sql/rapids/internal/fixture/PrivateApi.java"), - os.path.join( - fixture_root, - "org/apache/spark/sql/execution/aggregate/PartialAggUtils.java"), - os.path.join( - fixture_root, - "org/apache/spark/sql/execution/aggregate/PartialAggUtilsNeighbor.java"), - os.path.join( - fixture_root, "org/apache/spark/sql/execution/aggregate/SparkApi.java"), - os.path.join(fixture_root, "org/example/fixture/ThirdPartyApi.java"), - ]) - if java_compile.returncode: - raise RuntimeError("Could not compile Java fixtures:\n" + java_compile.stdout) - - compiler_classpath = os.pathsep.join((compiler_jar, library_jar, reflect_jar)) - source_classpath = os.pathsep.join((classes, library_jar)) - - def compile_scala(source): - command = [ - java, "-cp", compiler_classpath, "scala.tools.nsc.Main", - "-classpath", source_classpath, "-d", classes, - ] - command.extend(compiler_args) - command.append(os.path.join(fixture_root, source)) - return run_command(command) - - nvidia_sources = ( - ("cuDF Java", "NvidiaCall.scala"), - ("cudf-spark-jni", "JniCall.scala"), - ("cudf-spark-private com.nvidia namespace", "PrivateComNvidiaCall.scala"), - ("cudf-spark-private RAPIDS namespace", "PrivateRapidsCall.scala"), - ("cudf-spark-private Spark-package bridge", "PrivateSparkBridgeCall.scala"), - ) - for api_name, source in nvidia_sources: - nvidia_compile = compile_scala(source) - if nvidia_compile.returncode or "deprecated" not in nvidia_compile.stdout.lower(): - raise RuntimeError( - "{0} deprecation must be visible and nonfatal, ".format(api_name) + - "but compilation produced:\n" + nvidia_compile.stdout) - - fatal_sources = ( - ("Third-party", "ThirdPartyCall.scala"), - ("Apache Spark sibling", "SparkCall.scala"), - ("PartialAggUtils prefix neighbor", "PartialAggUtilsNeighborCall.scala"), - ) - for api_name, source in fatal_sources: - fatal_compile = compile_scala(source) - if fatal_compile.returncode == 0 or "deprecated" not in fatal_compile.stdout.lower(): - raise RuntimeError( - "{0} deprecation must be visible and fatal, ".format(api_name) + - "but compilation produced:\n" + fatal_compile.stdout) - finally: - shutil.rmtree(temp_dir) - - print("Deprecation policy check passed for Scala {0}".format(scala_version)) - - -def parse_args(argv): - parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("--pom", required=True) - parser.add_argument("--maven-repo", required=True) - return parser.parse_args(argv) - - -def main(argv=None): - args = parse_args(argv) - try: - check_policy(args.pom, args.maven_repo) - except RuntimeError as error: - print(error, file=sys.stderr) - return 1 - return 0 - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/scripts/check_with_resource_nesting.py b/scripts/check_with_resource_nesting.py index 1a7f4f39f54..b198e13a715 100644 --- a/scripts/check_with_resource_nesting.py +++ b/scripts/check_with_resource_nesting.py @@ -62,6 +62,8 @@ def baseline_key(self): ScanResult = collections.namedtuple("ScanResult", "violations directive_errors") +ClassifiedViolation = collections.namedtuple( + "ClassifiedViolation", "violation status") def _consume_quoted(source, start, quote): @@ -458,6 +460,110 @@ def stale_baseline_entries(violations, baseline): return baseline - current +def classify_violations(violations, baseline): + remaining = baseline.copy() + classified = [] + for violation in violations: + key = violation.baseline_key + status = "baselined" if remaining[key] > 0 else "new" + if remaining[key] > 0: + remaining[key] -= 1 + classified.append(ClassifiedViolation(violation, status)) + return classified + + +def markdown_escape(value): + return (value.replace("&", "&").replace("<", "<").replace(">", ">") + .replace("|", "\\|").replace("\r", " ").replace("\n", " ")) + + +def render_summary(classified, stale, directive_errors, max_depth): + new_count = sum(1 for item in classified if item.status == "new") + baselined_count = len(classified) - new_count + lines = [ + "## withResource nesting audit", + "", + ("Found {0} scope(s) deeper than {1}: {2} baselined, {3} new.".format( + len(classified), max_depth, baselined_count, new_count)), + "", + ] + if classified: + lines.extend([ + "| Status | Depth | Location | Resource |", + "| --- | ---: | --- | --- |", + ]) + for item in classified: + violation = item.violation + lines.append("| {0} | {1} | `{2}:{3}` | {4} |".format( + item.status, violation.depth, markdown_escape(violation.path), + violation.line, markdown_escape(violation.resource))) + else: + lines.append("No deep withResource scopes were found.") + + if stale: + lines.extend(["", "### Stale baseline entries", ""]) + for (path, fingerprint), count in sorted(stale.items()): + lines.append("- `{0}` (`{1}`), count {2}".format( + markdown_escape(path), fingerprint, count)) + if directive_errors: + lines.extend(["", "### Invalid exemption directives", ""]) + lines.extend("- {0}".format(markdown_escape(error)) + for error in directive_errors) + lines.extend([ + "", + ("Baselined scopes are reported as existing debt and do not fail this check. " + "New scopes, stale baseline entries, and invalid directives fail the audit."), + "", + ]) + return "\n".join(lines) + + +def command_escape(value): + return value.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") + + +def command_property_escape(value): + return command_escape(value).replace(":", "%3A").replace(",", "%2C") + + +def emit_annotations(classified): + for item in classified[:50]: + violation = item.violation + level = "error" if item.status == "new" else "warning" + message = "depth {0}: {1} ({2})".format( + violation.depth, violation.resource, item.status) + print("::{0} file={1},line={2},title=withResource nesting::{3}".format( + level, command_property_escape(violation.path), violation.line, + command_escape(message))) + if len(classified) > 50: + print("::warning title=withResource nesting::Only 50 of {0} scopes were annotated; " + "see the job summary and raw report for all findings".format(len(classified))) + + +def write_raw_report(path, classified, stale, directive_errors, max_depth): + report = collections.OrderedDict(( + ("version", BASELINE_VERSION), + ("maxDepth", max_depth), + ("violations", [collections.OrderedDict(( + ("status", item.status), + ("path", item.violation.path), + ("line", item.violation.line), + ("depth", item.violation.depth), + ("fingerprint", item.violation.fingerprint), + ("resource", item.violation.resource), + )) for item in classified]), + ("staleBaselineEntries", [collections.OrderedDict(( + ("path", path), + ("fingerprint", fingerprint), + ("count", count), + )) for (path, fingerprint), count in sorted(stale.items())]), + ("directiveErrors", list(directive_errors)), + )) + with io.open(path, "w", encoding="utf-8") as report_file: + report_file.write(TEXT_TYPE(json.dumps( + report, indent=2, separators=(",", ": "))) + "\n") + + def parse_args(args): parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("--root", default=os.getcwd(), @@ -470,6 +576,10 @@ def parse_args(args): help="print a baseline for the current source tree and exit") parser.add_argument("--update-baseline", action="store_true", help="replace the baseline with the current source tree") + parser.add_argument("--summary", default=os.environ.get("GITHUB_STEP_SUMMARY"), + help="write a Markdown report containing every deep scope") + parser.add_argument("--raw-report", + help="write a JSON report containing every deep scope") return parser.parse_args(args) @@ -496,10 +606,6 @@ def main(argv=None): return 2 scan = scan_tree(root, max_depth) - if scan.directive_errors: - for error in scan.directive_errors: - print(error, file=sys.stderr) - return 1 generated_baseline = baseline_json(scan.violations, max_depth) if args.print_baseline: @@ -514,11 +620,36 @@ def main(argv=None): unexpected = new_violations(scan.violations, baseline) stale = stale_baseline_entries(scan.violations, baseline) + classified = classify_violations(scan.violations, baseline) + report_failed = False + if args.summary: + try: + with io.open(args.summary, "w", encoding="utf-8") as summary_file: + summary_file.write(TEXT_TYPE(render_summary( + classified, stale, scan.directive_errors, max_depth))) + except (IOError, OSError) as error: + report_failed = True + print("Could not write withResource summary: {0}".format(error), file=sys.stderr) + if args.raw_report: + try: + write_raw_report( + args.raw_report, classified, stale, scan.directive_errors, max_depth) + except (IOError, OSError) as error: + report_failed = True + print("Could not write withResource raw report: {0}".format(error), file=sys.stderr) + if os.environ.get("GITHUB_ACTIONS") == "true": + emit_annotations(classified) + + if scan.directive_errors: + for error in scan.directive_errors: + print(error, file=sys.stderr) + return 1 + if not unexpected and not stale: print( "withResource nesting lint passed ({0} baselined violations, maximum " "allowed depth {1})".format(len(scan.violations), max_depth)) - return 0 + return 1 if report_failed else 0 for violation in unexpected: resource = violation.resource diff --git a/scripts/deprecation_audit.py b/scripts/deprecation_audit.py deleted file mode 100644 index c4353118a14..00000000000 --- a/scripts/deprecation_audit.py +++ /dev/null @@ -1,416 +0,0 @@ -# Copyright (c) 2026, NVIDIA CORPORATION. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -"""Collect compiler deprecations from GitHub Actions logs using Jython 2.7-compatible code.""" - -from __future__ import print_function - -import argparse -import glob -import io -import json -import os -import re -import sys -import time -import zipfile - -try: - import urllib.error as urllib_error - import urllib.parse as urllib_parse - import urllib.request as urllib_request -except ImportError: # Jython 2.7 / Python 2.7 - import urllib2 as urllib_error - import urllib2 as urllib_request - import urlparse as urllib_parse - - -ANSI_ESCAPE = re.compile(r"\x1b\[[0-?]*[ -/]*[@-~]") -SOURCE_LOCATION = re.compile( - r"(?P(?:[A-Za-z]:)?[^\s\[\]]+?\.(?:scala|java))" - r"(?::(?P\d+)|:\[(?P\d+),\d+\])" -) -DEPRECATION = re.compile(r"\bdeprecated\b", re.IGNORECASE) -ORIGIN = re.compile(r"\borigin(?:=|:)\s*(?P[\w.$]+)") -DEFAULT_JOB_PATTERN = ( - r"^(?:package-tests(?:-scala213)?|verify-213-modules|" - r"verify-all-212-modules|install-modules)(?:\s|$)" -) -NVIDIA_ORIGIN_PREFIXES = ( - "ai.rapids.cudf.", - "com.nvidia.spark.rapids.", - "org.apache.spark.sql.rapids.", -) -NVIDIA_ORIGIN_SYMBOLS = ( - "org.apache.spark.sql.execution.aggregate.PartialAggUtils", -) - - -def is_nvidia_origin(origin): - if origin.startswith(NVIDIA_ORIGIN_PREFIXES): - return True - return any( - origin == symbol or origin.startswith((symbol + ".", symbol + "$")) - for symbol in NVIDIA_ORIGIN_SYMBOLS - ) - - -BAD_ZIP_ERROR = getattr(zipfile, "BadZipFile", zipfile.BadZipfile) - - -class Finding(object): - def __init__(self, path, line, message, origin="", jobs=None): - self.path = path - self.line = line - self.message = message - self.origin = origin - self.jobs = set(jobs or ()) - - @property - def owner(self): - if is_nvidia_origin(self.origin): - return "NVIDIA" - return "third-party/unknown" - - def key(self): - diagnostic = self.origin or self.message - return self.path, self.line, diagnostic - - -def clean_line(line): - return ANSI_ESCAPE.sub("", line).rstrip() - - -def read_text(path): - with io.open(path, "r", encoding="utf-8", errors="replace") as input_file: - return input_file.read() - - -def normalize_path(path, repo_root): - candidate = os.path.normpath(path) - if not os.path.isabs(candidate): - return candidate.replace(os.sep, "/") - root = os.path.realpath(repo_root) - resolved = os.path.realpath(candidate) - relative = os.path.relpath(resolved, root) - if relative != os.pardir and not relative.startswith(os.pardir + os.sep): - return relative.replace(os.sep, "/") - parts = candidate.split(os.sep) - for index in range(len(parts)): - suffix = os.path.join(*parts[index:]) - if os.path.exists(os.path.join(root, suffix)): - return suffix.replace(os.sep, "/") - return candidate.replace(os.sep, "/") - - -def parse_log(text, job_name, repo_root="."): - lines = [clean_line(line) for line in text.splitlines()] - findings = [] - for index, line in enumerate(lines): - if not DEPRECATION.search(line): - continue - location = SOURCE_LOCATION.search(line) - if location is None: - for previous in reversed(lines[max(0, index - 3):index]): - location = SOURCE_LOCATION.search(previous) - if location is not None: - break - if location is None: - continue - origin = "" - for context in lines[index:min(len(lines), index + 6)]: - origin_match = ORIGIN.search(context) - if origin_match is not None: - origin = origin_match.group("origin") - break - line_number = location.group("line") or location.group("bracket_line") - message = re.sub(r"^.*?\.(?:scala|java)(?::\d+|:\[\d+,\d+\])\s*:?[ ]*", "", line) - findings.append(Finding( - path=normalize_path(location.group("path"), repo_root), - line=int(line_number), - message=message.strip() or line.strip(), - origin=origin, - jobs={job_name}, - )) - return findings - - -def merge_findings(findings): - merged = {} - for finding in findings: - existing = merged.get(finding.key()) - if existing is None: - merged[finding.key()] = finding - else: - existing.jobs.update(finding.jobs) - return sorted( - merged.values(), key=lambda finding: (finding.path, finding.line, finding.message)) - - -def request_json(url, token): - request = urllib_request.Request(url, headers={ - "Accept": "application/vnd.github+json", - "Authorization": "Bearer {0}".format(token), - "X-GitHub-Api-Version": "2022-11-28", - }) - response = urllib_request.urlopen(request, timeout=30) - try: - return json.load(response) - finally: - response.close() - - -class JobLogRedirectError(RuntimeError): - """The GitHub job-log endpoint returned an unsafe or malformed redirect.""" - - -class NoRedirectHandler(urllib_request.HTTPRedirectHandler): - def redirect_request(self, request, file_pointer, code, message, headers, new_url): - return None - - -def request_bytes(url, token): - """Download a GitHub API resource without forwarding credentials on its redirect.""" - request = urllib_request.Request(url, headers={ - "Accept": "application/vnd.github+json", - "Authorization": "Bearer {0}".format(token), - "X-GitHub-Api-Version": "2022-11-28", - }) - opener = urllib_request.build_opener(NoRedirectHandler()) - try: - response = opener.open(request, timeout=30) - try: - raise JobLogRedirectError( - "GitHub job-log endpoint did not return the expected redirect") - finally: - response.close() - except urllib_error.HTTPError as error: - if error.code != 302: - error.close() - raise - headers = getattr(error, "headers", None) - if headers is None: - headers = error.hdrs - location = headers.get("Location") - error.close() - if not location: - raise JobLogRedirectError( - "GitHub job-log redirect did not include a Location header") - parsed_location = urllib_parse.urlsplit(location) - if parsed_location.scheme != "https" or not parsed_location.netloc: - raise JobLogRedirectError( - "GitHub job-log redirect must use an absolute HTTPS URL") - - # The redirect is a short-lived signed URL. It authorizes itself, so use a fresh request - # without the repository-scoped GitHub token or GitHub-specific API headers. - signed_request = urllib_request.Request(location) - response = urllib_request.urlopen(signed_request, timeout=30) - try: - return response.read() - finally: - response.close() - - -def decode_job_log(payload): - if payload.startswith(b"PK"): - with zipfile.ZipFile(io.BytesIO(payload)) as archive: - return u"\n".join( - archive.read(name).decode("utf-8", errors="replace") - for name in archive.namelist() - if not name.endswith("/") - ) - return payload.decode("utf-8", errors="replace") - - -def download_logs(api_url, repository, run_id, token, job_pattern): - matcher = re.compile(job_pattern) - jobs = [] - page = 1 - while True: - result = request_json( - "{0}/repos/{1}/actions/runs/{2}/jobs?per_page=100&page={3}".format( - api_url, repository, run_id, page), - token, - ) - page_jobs = result.get("jobs", []) - jobs.extend(page_jobs) - if len(page_jobs) < 100: - break - page += 1 - - logs = {} - failures = [] - for job in jobs: - name = job.get("name", "") - if job.get("status") != "completed" or matcher.search(name) is None: - continue - error = None - for delay in (0, 1, 2, 4): - if delay: - time.sleep(delay) - try: - payload = request_bytes( - "{0}/repos/{1}/actions/jobs/{2}/logs".format( - api_url, repository, job["id"]), token) - logs[name] = decode_job_log(payload) - error = None - break - except (OSError, urllib_error.HTTPError, BAD_ZIP_ERROR, - JobLogRedirectError) as caught: - error = caught - if error is not None: - failures.append(u"{0}: {1}".format(name, error)) - if not logs and not failures: - failures.append("no completed build-matrix job logs matched the configured job pattern") - return logs, failures - - -def markdown_escape(value): - return value.replace("|", "\\|").replace("\n", " ") - - -def render_summary(findings, failures): - lines = [u"## NVIDIA deprecation audit", u""] - if findings: - lines.extend([ - u"Found {0} unique compiler deprecation diagnostic(s).".format(len(findings)), - u"", - u"| Owner | Location | Deprecated API | Matrix jobs |", - u"| --- | --- | --- | --- |", - ]) - for finding in findings[:200]: - location = u"`{0}:{1}`".format(finding.path, finding.line) - api = finding.origin or finding.message - jobs = u", ".join(sorted(finding.jobs)) - lines.append( - u"| {0} | {1} | `{2}` | {3} |".format( - finding.owner, location, markdown_escape(api), markdown_escape(jobs)) - ) - if len(findings) > 200: - lines.extend([ - u"", - u"Report truncated; see the raw artifact for all {0} findings.".format( - len(findings)), - ]) - else: - lines.append(u"No compiler deprecation diagnostics were found in the selected matrix jobs.") - if failures: - lines.extend([u"", u"### Incomplete log collection", u""]) - lines.extend(u"- {0}".format(markdown_escape(failure)) for failure in failures) - lines.extend([ - u"", - u"This check is optional. Findings or incomplete log collection fail only this audit " - "check; build-job results remain authoritative.", - u"", - ]) - return u"\n".join(lines) - - -def command_escape(value): - return value.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") - - -def command_property_escape(value): - return command_escape(value).replace(":", "%3A").replace(",", "%2C") - - -def emit_annotations(findings): - for finding in findings[:50]: - message = finding.origin or finding.message - print( - u"::warning file={0},line={1},title=NVIDIA deprecation::{2}".format( - command_property_escape(finding.path), finding.line, command_escape(message)) - ) - if len(findings) > 50: - print(u"::warning title=NVIDIA deprecation::Only 50 of {0} findings were annotated".format( - len(findings))) - - -def write_raw_report(path, findings, failures): - report = { - "findings": [ - { - "owner": finding.owner, - "path": finding.path, - "line": finding.line, - "message": finding.message, - "origin": finding.origin, - "jobs": sorted(finding.jobs), - } - for finding in findings - ], - "log_collection_failures": failures, - } - with io.open(path, "w", encoding="utf-8") as report_file: - report_file.write(json.dumps(report, indent=2, ensure_ascii=False) + u"\n") - - -def parse_args(argv): - parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("--repository", default=os.environ.get("GITHUB_REPOSITORY")) - parser.add_argument("--run-id", default=os.environ.get("GITHUB_RUN_ID")) - parser.add_argument( - "--api-url", default=os.environ.get("GITHUB_API_URL", "https://api.github.com")) - parser.add_argument("--job-pattern", default=DEFAULT_JOB_PATTERN) - parser.add_argument( - "--logs-dir", default=os.environ.get("DEPRECATION_AUDIT_LOGS_DIR"), - help="Parse local *.log files instead of downloading job logs") - parser.add_argument("--repo-root", default=".") - parser.add_argument("--summary", default=os.environ.get("GITHUB_STEP_SUMMARY")) - parser.add_argument("--raw-report", default="nvidia-deprecation-audit.json") - return parser.parse_args(argv) - - -def main(argv=None): - args = parse_args(argv) - failures = [] - try: - if args.logs_dir: - logs = { - os.path.splitext(os.path.basename(path))[0]: read_text(path) - for path in glob.glob(os.path.join(args.logs_dir, "*.log")) - } - else: - token = os.environ.get("GITHUB_TOKEN") - if not token or not args.repository or not args.run_id: - raise ValueError("GITHUB_TOKEN, repository, and run ID are required") - logs, failures = download_logs( - args.api_url, args.repository, args.run_id, token, args.job_pattern) - findings = merge_findings( - finding - for job_name, log in logs.items() - for finding in parse_log(log, job_name, args.repo_root) - ) - except Exception as error: # Report operational errors through this optional check. - findings = [] - failures.append(u"audit failed: {0}".format(error)) - - summary = render_summary(findings, failures) - print(summary) - emit_annotations(findings) - if args.summary: - with io.open(args.summary, "a", encoding="utf-8") as summary_file: - summary_file.write(summary) - report_failed = False - try: - write_raw_report(args.raw_report, findings, failures) - except (IOError, OSError) as error: - report_failed = True - print(u"::warning title=NVIDIA deprecation audit::Could not write raw report: {0}".format( - error)) - return 1 if findings or failures or report_failed else 0 - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/scripts/tests/test_check_with_resource_nesting.py b/scripts/tests/test_check_with_resource_nesting.py index 43b1983a587..bd5b39bfc29 100644 --- a/scripts/tests/test_check_with_resource_nesting.py +++ b/scripts/tests/test_check_with_resource_nesting.py @@ -284,6 +284,39 @@ def test_production_source_discovery_excludes_generated_and_test_trees(self): ] self.assertEqual(["module/src/main/scala/Keep.scala"], discovered) + def test_report_classifies_and_lists_every_violation(self): + violations = LINT.scan_source("Test.scala", nested_source(6), 4).violations + baseline = collections.Counter({violations[0].baseline_key: 1}) + classified = LINT.classify_violations(violations, baseline) + self.assertEqual(["baselined", "new"], [item.status for item in classified]) + + summary = LINT.render_summary(classified, collections.Counter(), (), 4) + self.assertIn("Found 2 scope(s) deeper than 4: 1 baselined, 1 new", summary) + for violation in violations: + self.assertIn(violation.resource, summary) + + with temporary_directory() as root: + report_path = os.path.join(root, "report.json") + LINT.write_raw_report( + report_path, classified, collections.Counter(), (), 4) + with io.open(report_path, "r", encoding="utf-8") as report_file: + report = json.loads(report_file.read()) + self.assertEqual(2, len(report["violations"])) + self.assertEqual(["baselined", "new"], [ + violation["status"] for violation in report["violations"]]) + self.assertEqual("<literal> & value \\| next", + LINT.markdown_escape(" & value | next")) + + def test_annotations_distinguish_baselined_and_new_violations(self): + violations = LINT.scan_source("Test:File.scala", nested_source(6), 4).violations + baseline = collections.Counter({violations[0].baseline_key: 1}) + classified = LINT.classify_violations(violations, baseline) + with captured_stream("stdout") as stdout: + LINT.emit_annotations(classified) + output = stdout.getvalue() + self.assertIn("::warning file=Test%3AFile.scala", output) + self.assertIn("::error file=Test%3AFile.scala", output) + def test_command_fails_for_new_violation(self): with temporary_directory() as root: source_dir = os.path.join(root, "module", "src", "main", "scala") @@ -353,6 +386,45 @@ def test_command_updates_baseline(self): scan = LINT.scan_tree(root, 4) self.assertEqual(LINT.baseline_json(scan.violations, 4), generated) + def test_command_writes_complete_reports(self): + with temporary_directory() as root: + source_dir = os.path.join(root, "module", "src", "main", "scala") + os.makedirs(source_dir) + write_text(os.path.join(source_dir, "Test.scala"), nested_source(6)) + scan = LINT.scan_tree(root, 4) + baseline = os.path.join(root, "baseline.json") + write_text(baseline, LINT.baseline_json(scan.violations, 4)) + summary = os.path.join(root, "summary.md") + report = os.path.join(root, "report.json") + + with captured_stream("stdout"): + exit_code = LINT.main([ + "--root", root, + "--baseline", baseline, + "--summary", summary, + "--raw-report", report, + ]) + + self.assertEqual(0, exit_code) + with io.open(summary, "r", encoding="utf-8") as summary_file: + self.assertIn("2 baselined, 0 new", summary_file.read()) + with io.open(report, "r", encoding="utf-8") as report_file: + report_data = json.loads(report_file.read()) + self.assertEqual(2, len(report_data["violations"])) + + def test_command_fails_when_report_cannot_be_written(self): + with temporary_directory() as root: + baseline = os.path.join(root, "baseline.json") + write_text(baseline, LINT.baseline_json((), 4)) + with captured_stream("stderr") as stderr, captured_stream("stdout"): + exit_code = LINT.main([ + "--root", root, + "--baseline", baseline, + "--raw-report", root, + ]) + self.assertEqual(1, exit_code) + self.assertIn("Could not write withResource raw report", stderr.getvalue()) + def test_command_explains_fingerprint_changes(self): with temporary_directory() as root: source_dir = os.path.join(root, "module", "src", "main", "scala") diff --git a/scripts/tests/test_deprecation_audit.py b/scripts/tests/test_deprecation_audit.py deleted file mode 100644 index b6e20f820a0..00000000000 --- a/scripts/tests/test_deprecation_audit.py +++ /dev/null @@ -1,334 +0,0 @@ -# Copyright (c) 2026, NVIDIA CORPORATION. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from __future__ import print_function - -import contextlib -import io -import os -import shutil -import sys -import tempfile -import unittest -import zipfile - - -SCRIPT = os.path.join(os.path.dirname(os.path.dirname(__file__)), "deprecation_audit.py") -try: - import importlib.util - SPEC = importlib.util.spec_from_file_location("deprecation_audit", SCRIPT) - AUDIT = importlib.util.module_from_spec(SPEC) - sys.modules[SPEC.name] = AUDIT - SPEC.loader.exec_module(AUDIT) -except ImportError: # Jython 2.7 / Python 2.7 - import imp - AUDIT = imp.load_source("deprecation_audit", SCRIPT) - - -class CallRecorder(object): - def __init__(self, return_value=None, side_effect=None): - self.return_value = return_value - self.side_effect = side_effect - self.calls = [] - - @property - def call_count(self): - return len(self.calls) - - def __call__(self, *args, **kwargs): - self.calls.append((args, kwargs)) - effect = self.side_effect - if isinstance(effect, list): - effect = effect.pop(0) - if isinstance(effect, BaseException): - raise effect - if callable(effect): - return effect(*args, **kwargs) - return self.return_value if effect is None else effect - - -class FakeOpener(object): - def __init__(self): - self.open = CallRecorder() - - -class OutputSink(object): - def __init__(self): - self.parts = [] - - def write(self, value): - self.parts.append(value) - - def flush(self): - pass - - -@contextlib.contextmanager -def patch_attribute(target, name, value): - original = getattr(target, name) - setattr(target, name, value) - try: - yield value - finally: - setattr(target, name, original) - - -@contextlib.contextmanager -def patch_environment(updates): - original = dict(os.environ) - os.environ.update(updates) - try: - yield - finally: - os.environ.clear() - os.environ.update(original) - - -@contextlib.contextmanager -def temporary_directory(): - path = tempfile.mkdtemp() - try: - yield path - finally: - shutil.rmtree(path) - - -@contextlib.contextmanager -def captured_stdout(): - output = OutputSink() - with patch_attribute(sys, "stdout", output): - yield output - - -def request_url(request): - return request.get_full_url() - - -def assert_raises_regex(test_case, exception, pattern): - method = getattr(test_case, "assertRaisesRegex", None) - if method is None: - method = test_case.assertRaisesRegexp - return method(exception, pattern) - - -class DeprecationAuditSuite(unittest.TestCase): - def test_parses_scala_verbose_deprecation(self): - log = ( - "[INFO] /workspace/sql-plugin/src/main/scala/Test.scala:42: " - "[deprecation @ example.Test.run | " - "origin=ai.rapids.cudf.ColumnView.oldApi | version=] " - "method oldApi in class ColumnView is deprecated\n" - ) - findings = AUDIT.parse_log(log, "package-tests (330)") - self.assertEqual(1, len(findings)) - self.assertEqual(42, findings[0].line) - self.assertEqual("ai.rapids.cudf.ColumnView.oldApi", findings[0].origin) - self.assertEqual("NVIDIA", findings[0].owner) - - def test_parses_maven_bracket_location(self): - log = """ -[WARNING] /workspace/src/main/java/Test.java:[17,9] oldApi() has been deprecated -""" - findings = AUDIT.parse_log(log, "verify-all-212-modules (330, 17)") - self.assertEqual(17, findings[0].line) - self.assertEqual("third-party/unknown", findings[0].owner) - - def test_reads_scala_213_origin_from_following_line(self): - log = """ -[INFO] /workspace/sql-plugin/src/main/scala/Test.scala:42: method oldApi is deprecated -Applicable -Wconf filters: cat=deprecation, origin=com.nvidia.spark.rapids.jni.Api.oldApi -""" - findings = AUDIT.parse_log(log, "package-tests-scala213 (350)") - self.assertEqual("com.nvidia.spark.rapids.jni.Api.oldApi", findings[0].origin) - self.assertEqual("NVIDIA", findings[0].owner) - - def test_classifies_all_advisory_origins_as_nvidia(self): - origins = ( - "ai.rapids.cudf.ColumnView.oldApi", - "com.nvidia.spark.rapids.optimizer.OptimizerConf.oldApi", - "org.apache.spark.sql.rapids.internal.PrivateRapidsConfs.oldApi", - "org.apache.spark.sql.execution.aggregate.PartialAggUtils.oldApi", - "org.apache.spark.sql.execution.aggregate.PartialAggUtils$Helper.oldApi", - ) - for origin in origins: - finding = AUDIT.Finding("Test.scala", 1, "deprecated", origin) - self.assertEqual("NVIDIA", finding.owner, origin) - - def test_partial_agg_utils_owner_match_has_symbol_boundary(self): - origins = ( - "org.apache.spark.sql.execution.aggregate.PartialAggUtilsNeighbor.oldApi", - "org.apache.spark.sql.execution.aggregate.SparkApi.oldApi", - "org.example.fixture.ThirdPartyApi.oldApi", - ) - for origin in origins: - finding = AUDIT.Finding("Test.scala", 1, "deprecated", origin) - self.assertEqual("third-party/unknown", finding.owner, origin) - - def test_ignores_non_source_deprecation_text(self): - log = "[WARNING] This build plugin uses a deprecated Maven feature\n" - self.assertEqual([], AUDIT.parse_log(log, "install-modules (3.9.3)")) - - def test_merges_same_finding_across_matrix_jobs(self): - first = AUDIT.Finding( - "Test.scala", 1, "[deprecation] old is deprecated", "ai.rapids.cudf.Api.old", - {"330"}) - second = AUDIT.Finding( - "Test.scala", 1, "old is deprecated", "ai.rapids.cudf.Api.old", {"400"}) - merged = AUDIT.merge_findings([first, second]) - self.assertEqual({"330", "400"}, merged[0].jobs) - - def test_decodes_zip_job_log(self): - payload = io.BytesIO() - with zipfile.ZipFile(payload, "w") as archive: - archive.writestr("job/step.txt", "deprecated output") - self.assertEqual("deprecated output", AUDIT.decode_job_log(payload.getvalue())) - - def test_job_log_redirect_does_not_forward_authorization(self): - api_url = "https://api.github.com/repos/NVIDIA/cudf-spark/actions/jobs/123/logs" - signed_url = "https://results-receiver.example/job.log?signature=secret" - redirect = AUDIT.urllib_error.HTTPError( - api_url, 302, "Found", {"Location": signed_url}, None) - authenticated_opener = FakeOpener() - authenticated_opener.open.side_effect = redirect - build_opener = CallRecorder(return_value=authenticated_opener) - signed_open = CallRecorder(return_value=io.BytesIO(b"job log")) - - with patch_attribute(AUDIT.urllib_request, "build_opener", build_opener), \ - patch_attribute(AUDIT.urllib_request, "urlopen", signed_open): - payload = AUDIT.request_bytes(api_url, "github-token") - - self.assertEqual(b"job log", payload) - authenticated_request = authenticated_opener.open.calls[0][0][0] - self.assertEqual("Bearer github-token", - authenticated_request.get_header("Authorization")) - signed_request = signed_open.calls[0][0][0] - self.assertEqual(signed_url, request_url(signed_request)) - self.assertIsNone(signed_request.get_header("Authorization")) - self.assertIsNone(signed_request.get_header("X-GitHub-Api-Version")) - - def test_job_log_redirect_requires_location(self): - api_url = "https://api.github.com/repos/NVIDIA/cudf-spark/actions/jobs/123/logs" - authenticated_opener = FakeOpener() - authenticated_opener.open.side_effect = AUDIT.urllib_error.HTTPError( - api_url, 302, "Found", {}, None) - with patch_attribute( - AUDIT.urllib_request, "build_opener", - CallRecorder(return_value=authenticated_opener)), \ - assert_raises_regex(self, AUDIT.JobLogRedirectError, "Location"): - AUDIT.request_bytes(api_url, "github-token") - - def test_job_log_redirect_rejects_non_https_location(self): - api_url = "https://api.github.com/repos/NVIDIA/cudf-spark/actions/jobs/123/logs" - authenticated_opener = FakeOpener() - authenticated_opener.open.side_effect = AUDIT.urllib_error.HTTPError( - api_url, 302, "Found", {"Location": "http://logs.example/job.log"}, None) - with patch_attribute( - AUDIT.urllib_request, "build_opener", - CallRecorder(return_value=authenticated_opener)), \ - assert_raises_regex(self, AUDIT.JobLogRedirectError, "HTTPS"): - AUDIT.request_bytes(api_url, "github-token") - - def test_job_log_endpoint_rejects_non_redirect_response(self): - api_url = "https://api.github.com/repos/NVIDIA/cudf-spark/actions/jobs/123/logs" - authenticated_opener = FakeOpener() - authenticated_opener.open.return_value = io.BytesIO(b"unexpected direct response") - with patch_attribute( - AUDIT.urllib_request, "build_opener", - CallRecorder(return_value=authenticated_opener)), \ - assert_raises_regex(self, AUDIT.JobLogRedirectError, "expected redirect"): - AUDIT.request_bytes(api_url, "github-token") - - def test_download_logs_preserves_partial_results_after_bad_redirect(self): - jobs = {"jobs": [ - {"id": 1, "name": "package-tests (330, false)", "status": "completed"}, - {"id": 2, "name": "package-tests (340, false)", "status": "completed"}, - ]} - bad_redirect = AUDIT.JobLogRedirectError("missing redirect location") - request_bytes = CallRecorder( - side_effect=[b"first job log", bad_redirect, bad_redirect, - bad_redirect, bad_redirect]) - with patch_attribute(AUDIT, "request_json", CallRecorder(return_value=jobs)), \ - patch_attribute(AUDIT, "request_bytes", request_bytes), \ - patch_attribute(AUDIT.time, "sleep", CallRecorder()): - logs, failures = AUDIT.download_logs( - "https://api.github.com", "NVIDIA/cudf-spark", "run-id", "token", - AUDIT.DEFAULT_JOB_PATTERN) - - self.assertEqual({"package-tests (330, false)": "first job log"}, logs) - self.assertEqual(5, request_bytes.call_count) - self.assertEqual(1, len(failures)) - self.assertIn("package-tests (340, false)", failures[0]) - self.assertIn("missing redirect location", failures[0]) - - def test_summary_reports_incomplete_collection(self): - summary = AUDIT.render_summary([], ["package-tests: log unavailable"]) - self.assertIn("No compiler deprecation diagnostics", summary) - self.assertIn("Incomplete log collection", summary) - self.assertIn("optional", summary) - - def test_main_fails_when_findings_are_present(self): - log = ( - u"/workspace/sql-plugin/src/main/scala/Test.scala:42: " - "[deprecation @ example.Test.run | " - "origin=ai.rapids.cudf.ColumnView.oldApi | version=] deprecated\n" - ) - with temporary_directory() as temp_dir: - log_path = os.path.join(temp_dir, "package-tests.log") - with io.open(log_path, "w", encoding="utf-8") as log_file: - log_file.write(log) - with captured_stdout(): - result = AUDIT.main([ - "--logs-dir", temp_dir, - "--raw-report", os.path.join(temp_dir, "report.json"), - ]) - self.assertEqual(1, result) - - def test_main_succeeds_when_audit_is_clean(self): - with temporary_directory() as temp_dir: - log_path = os.path.join(temp_dir, "package-tests.log") - summary_path = os.path.join(temp_dir, "summary.md") - with io.open(log_path, "w", encoding="utf-8") as log_file: - log_file.write(u"clean build\n") - with captured_stdout(): - result = AUDIT.main([ - "--logs-dir", temp_dir, - "--summary", summary_path, - "--raw-report", os.path.join(temp_dir, "report.json"), - ]) - with io.open(summary_path, "r", encoding="utf-8") as summary_file: - self.assertIn("No compiler deprecation diagnostics", summary_file.read()) - self.assertEqual(0, result) - - def test_main_fails_when_log_collection_is_incomplete(self): - with temporary_directory() as temp_dir, \ - patch_attribute( - AUDIT, "download_logs", - CallRecorder(return_value=({}, ["package-tests: log unavailable"]))), \ - patch_environment({"GITHUB_TOKEN": "token"}): - with captured_stdout(): - result = AUDIT.main([ - "--repository", "NVIDIA/cudf-spark", - "--run-id", "123", - "--logs-dir", "", - "--raw-report", os.path.join(temp_dir, "report.json"), - ]) - self.assertEqual(1, result) - - def test_annotation_property_escaping(self): - self.assertEqual("path%3Awith%2Cpunctuation", AUDIT.command_property_escape( - "path:with,punctuation")) - - -if __name__ == "__main__": - unittest.main() From 761ca16f381b0887c88aedb58cc13ae290d1c1ea Mon Sep 17 00:00:00 2001 From: Gera Shegalov Date: Tue, 8 Sep 2026 22:38:09 -0700 Subject: [PATCH 8/9] Address withResource audit review feedback Signed-off-by: Gera Shegalov --- scripts/check_with_resource_nesting.py | 13 ++--- .../tests/test_check_with_resource_nesting.py | 50 +++++++++++++++++++ 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/scripts/check_with_resource_nesting.py b/scripts/check_with_resource_nesting.py index b198e13a715..49fc20c4d4e 100644 --- a/scripts/check_with_resource_nesting.py +++ b/scripts/check_with_resource_nesting.py @@ -527,7 +527,9 @@ def command_property_escape(value): def emit_annotations(classified): - for item in classified[:50]: + prioritized = ([item for item in classified if item.status == "new"] + + [item for item in classified if item.status != "new"]) + for item in prioritized[:50]: violation = item.violation level = "error" if item.status == "new" else "warning" message = "depth {0}: {1} ({2})".format( @@ -606,6 +608,10 @@ def main(argv=None): return 2 scan = scan_tree(root, max_depth) + if scan.directive_errors: + for error in scan.directive_errors: + print(error, file=sys.stderr) + return 1 generated_baseline = baseline_json(scan.violations, max_depth) if args.print_baseline: @@ -640,11 +646,6 @@ def main(argv=None): if os.environ.get("GITHUB_ACTIONS") == "true": emit_annotations(classified) - if scan.directive_errors: - for error in scan.directive_errors: - print(error, file=sys.stderr) - return 1 - if not unexpected and not stale: print( "withResource nesting lint passed ({0} baselined violations, maximum " diff --git a/scripts/tests/test_check_with_resource_nesting.py b/scripts/tests/test_check_with_resource_nesting.py index bd5b39bfc29..87e4dab8a74 100644 --- a/scripts/tests/test_check_with_resource_nesting.py +++ b/scripts/tests/test_check_with_resource_nesting.py @@ -317,6 +317,17 @@ def test_annotations_distinguish_baselined_and_new_violations(self): self.assertIn("::warning file=Test%3AFile.scala", output) self.assertIn("::error file=Test%3AFile.scala", output) + def test_annotations_prioritize_new_violations(self): + violations = LINT.scan_source("Test.scala", nested_source(55), 4).violations + baseline = collections.Counter( + violation.baseline_key for violation in violations[:50]) + classified = LINT.classify_violations(violations, baseline) + with captured_stream("stdout") as stdout: + LINT.emit_annotations(classified) + output = stdout.getvalue() + self.assertEqual(1, output.count("::error file=")) + self.assertEqual(49, output.count("::warning file=")) + def test_command_fails_for_new_violation(self): with temporary_directory() as root: source_dir = os.path.join(root, "module", "src", "main", "scala") @@ -386,6 +397,45 @@ def test_command_updates_baseline(self): scan = LINT.scan_tree(root, 4) self.assertEqual(LINT.baseline_json(scan.violations, 4), generated) + def test_command_does_not_print_baseline_with_invalid_directive(self): + with temporary_directory() as root: + source_dir = os.path.join(root, "module", "src", "main", "scala") + os.makedirs(source_dir) + source = ("// with-resource-lint: allow-deep-nesting -- short\n" + + nested_source(5)) + write_text(os.path.join(source_dir, "Test.scala"), source) + + with captured_stream("stdout") as stdout, captured_stream("stderr") as stderr: + exit_code = LINT.main(["--root", root, "--print-baseline"]) + + self.assertEqual(1, exit_code) + self.assertEqual("", stdout.getvalue()) + self.assertIn("requires a reason of at least", stderr.getvalue()) + + def test_command_does_not_update_baseline_with_invalid_directive(self): + with temporary_directory() as root: + source_dir = os.path.join(root, "module", "src", "main", "scala") + os.makedirs(source_dir) + source = ("// with-resource-lint: allow-deep-nesting -- short\n" + + nested_source(5)) + write_text(os.path.join(source_dir, "Test.scala"), source) + baseline = os.path.join(root, "baseline.json") + original_baseline = LINT.baseline_json((), 4) + write_text(baseline, original_baseline) + + with captured_stream("stdout") as stdout, captured_stream("stderr") as stderr: + exit_code = LINT.main([ + "--root", root, + "--baseline", baseline, + "--update-baseline", + ]) + + self.assertEqual(1, exit_code) + self.assertEqual("", stdout.getvalue()) + self.assertIn("requires a reason of at least", stderr.getvalue()) + with io.open(baseline, "r", encoding="utf-8") as baseline_file: + self.assertEqual(original_baseline, baseline_file.read()) + def test_command_writes_complete_reports(self): with temporary_directory() as root: source_dir = os.path.join(root, "module", "src", "main", "scala") From b1735b66f419fa1b1afff3415bf45a89f963fb7d Mon Sep 17 00:00:00 2001 From: Gera Shegalov Date: Thu, 10 Sep 2026 23:40:33 -0700 Subject: [PATCH 9/9] Fix withResource audit reporting Signed-off-by: Gera Shegalov --- .github/workflows/mvn-verify-check.yml | 8 +++++- pom.xml | 4 ++- scala2.13/pom.xml | 4 ++- scripts/check_with_resource_nesting.py | 5 ++-- .../tests/test_check_with_resource_nesting.py | 28 +++++++++++++++++++ 5 files changed, 44 insertions(+), 5 deletions(-) diff --git a/.github/workflows/mvn-verify-check.yml b/.github/workflows/mvn-verify-check.yml index 21aa7edfe92..a20f207d3dc 100644 --- a/.github/workflows/mvn-verify-check.yml +++ b/.github/workflows/mvn-verify-check.yml @@ -439,9 +439,15 @@ jobs: - name: Report withResource nesting violations run: | + audit_status=0 mvn --batch-mode -N antrun:run@with-resource-nesting-audit \ -DwithResource.audit.rawReport="$RUNNER_TEMP/with-resource-nesting-audit.json" \ - -DwithResource.audit.summary="$GITHUB_STEP_SUMMARY" + -DwithResource.audit.summary="$GITHUB_STEP_SUMMARY" \ + -DwithResource.audit.annotations="$RUNNER_TEMP/with-resource-nesting-annotations.txt" || audit_status=$? + if [[ -f "$RUNNER_TEMP/with-resource-nesting-annotations.txt" ]]; then + cat "$RUNNER_TEMP/with-resource-nesting-annotations.txt" + fi + exit "$audit_status" - name: Upload withResource nesting report if: ${{ always() }} diff --git a/pom.xml b/pom.xml index 45cf4840373..d69ad971c57 100644 --- a/pom.xml +++ b/pom.xml @@ -1155,6 +1155,7 @@ 3.3.0 ${project.build.directory}/with-resource-nesting-audit.json ${project.build.directory}/with-resource-nesting-summary.md + ${project.build.directory}/with-resource-nesting-annotations.txt 2.0.2 30.0-jre 2.0.0 @@ -2012,7 +2013,8 @@ This will force full Scala code rebuild in downstream modules. + fork="true" failonerror="true" dir="${project.basedir}" + output="${withResource.audit.annotations}"> diff --git a/scala2.13/pom.xml b/scala2.13/pom.xml index c0540689dfc..c3e44fcb3f6 100644 --- a/scala2.13/pom.xml +++ b/scala2.13/pom.xml @@ -1155,6 +1155,7 @@ 3.3.0 ${project.build.directory}/with-resource-nesting-audit.json ${project.build.directory}/with-resource-nesting-summary.md + ${project.build.directory}/with-resource-nesting-annotations.txt 2.0.2 30.0-jre 2.0.0 @@ -2012,7 +2013,8 @@ This will force full Scala code rebuild in downstream modules. + fork="true" failonerror="true" dir="${project.basedir}" + output="${withResource.audit.annotations}"> diff --git a/scripts/check_with_resource_nesting.py b/scripts/check_with_resource_nesting.py index 49fc20c4d4e..8b0ee0b5a57 100644 --- a/scripts/check_with_resource_nesting.py +++ b/scripts/check_with_resource_nesting.py @@ -611,7 +611,8 @@ def main(argv=None): if scan.directive_errors: for error in scan.directive_errors: print(error, file=sys.stderr) - return 1 + if args.print_baseline or args.update_baseline: + return 1 generated_baseline = baseline_json(scan.violations, max_depth) if args.print_baseline: @@ -646,7 +647,7 @@ def main(argv=None): if os.environ.get("GITHUB_ACTIONS") == "true": emit_annotations(classified) - if not unexpected and not stale: + if not unexpected and not stale and not scan.directive_errors: print( "withResource nesting lint passed ({0} baselined violations, maximum " "allowed depth {1})".format(len(scan.violations), max_depth)) diff --git a/scripts/tests/test_check_with_resource_nesting.py b/scripts/tests/test_check_with_resource_nesting.py index 87e4dab8a74..76234a8db3a 100644 --- a/scripts/tests/test_check_with_resource_nesting.py +++ b/scripts/tests/test_check_with_resource_nesting.py @@ -462,6 +462,34 @@ def test_command_writes_complete_reports(self): report_data = json.loads(report_file.read()) self.assertEqual(2, len(report_data["violations"])) + def test_command_reports_invalid_directive(self): + with temporary_directory() as root: + source_dir = os.path.join(root, "module", "src", "main", "scala") + os.makedirs(source_dir) + source = ("// with-resource-lint: allow-deep-nesting -- short\n" + + nested_source(5)) + write_text(os.path.join(source_dir, "Test.scala"), source) + summary = os.path.join(root, "summary.md") + report = os.path.join(root, "report.json") + + with captured_stream("stdout") as stdout, captured_stream("stderr") as stderr: + exit_code = LINT.main([ + "--root", root, + "--summary", summary, + "--raw-report", report, + ]) + + self.assertEqual(1, exit_code) + self.assertNotIn("lint passed", stdout.getvalue()) + self.assertIn("requires a reason of at least", stderr.getvalue()) + with io.open(summary, "r", encoding="utf-8") as summary_file: + self.assertIn("Invalid exemption directives", summary_file.read()) + with io.open(report, "r", encoding="utf-8") as report_file: + report_data = json.loads(report_file.read()) + self.assertEqual(1, len(report_data["directiveErrors"])) + self.assertIn("requires a reason of at least", + report_data["directiveErrors"][0]) + def test_command_fails_when_report_cannot_be_written(self): with temporary_directory() as root: baseline = os.path.join(root, "baseline.json")