Skip to content

Conversation

@titusfortner
Copy link
Member

@titusfortner titusfortner commented Jan 23, 2026

User description

When running bazel query deps(//some:test) on Linux CI, the query tries to resolve all dependencies, including browsers for non-applicable OS.

This error:

Evaluation of query "deps(//dotnet/test/common:AlertsTest-edge)" failed: preloading transitive closure failed: no such target '@@+pin_browsers_extension+mac_edge//:Edge.app': target 'Edge.app' not declared in package '' defined by /home/runner/.bazel/external/+pin_browsers_extension+mac_edge/BUILD.bazel

This is coming from pkg_archive and dmg_archive rules which attempt to execute pkgutil and hdiutil respectively, neither of which are available on Linux.

💥 What does this PR do?

  • Exits the pkg_archive and dmg_archive implementation before the unavailable command executes
  • Set the file in that rule so it doesn't error
  • Let js_library know that the data can be empty if it doesn't apply

🔧 Implementation Notes

Verified this fixes my issue: https://github.com/SeleniumHQ/selenium/actions/runs/21306668754/job/61335755897#step:19:30

🔄 Types of changes

  • Bug fix (backwards compatible)

PR Type

Bug fix


Description

  • Skip macOS-only archive rules when required tools unavailable

  • Check for hdiutil and pkgutil before execution

  • Create empty BUILD file and return early on unsupported platforms

  • Prevents bazel query failures on Linux/Windows CI environments


Diagram Walkthrough

flowchart LR
  A["Repository Rule Execution"] --> B{"Tool Available?"}
  B -->|Yes| C["Download & Extract Archive"]
  B -->|No| D["Create Empty BUILD.bazel"]
  D --> E["Return Early"]
Loading

File Walkthrough

Relevant files
Bug fix
dmg_archive.bzl
Add hdiutil availability check to dmg_archive                       

common/private/dmg_archive.bzl

  • Added check for hdiutil availability using repository_ctx.which()
  • Creates empty BUILD.bazel file with skip comment if tool not found
  • Returns early to prevent execution on unsupported platforms
+5/-0     
pkg_archive.bzl
Add pkgutil availability check to pkg_archive                       

common/private/pkg_archive.bzl

  • Added check for pkgutil availability using repository_ctx.which()
  • Creates empty BUILD.bazel file with skip comment if tool not found
  • Returns early to prevent execution on unsupported platforms
+5/-0     

@titusfortner titusfortner requested a review from Copilot January 23, 2026 01:35
@selenium-ci selenium-ci added the B-build Includes scripting, bazel and CI integrations label Jan 23, 2026
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Jan 23, 2026

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Silent skip path: The new early-return path silently skips repository setup when hdiutil/pkgutil is missing
and may need an explicit warning/fail message to make the cause actionable in Bazel output
depending on intended behavior.

Referred Code
hdiutil = repository_ctx.which("hdiutil")
if not hdiutil:
    repository_ctx.file("BUILD.bazel", "# dmg_archive: skipped (hdiutil not available on this platform)")
    return

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Jan 23, 2026

PR Code Suggestions ✨

Latest suggestions up to 3e48adb

CategorySuggestion                                                                                                                                    Impact
Possible issue
Create placeholders when skipping

In _dmg_archive_impl, when skipping on non-macOS platforms, create a placeholder
for the output directory to prevent exports_files from failing during Bazel
analysis.

common/private/dmg_archive.bzl [2-6]

 repository_ctx.file("BUILD.bazel", repository_ctx.attr.build_file_content)
 
 if not repository_ctx.which("hdiutil"):
     # hdiutil is macOS-only; skip download on other platforms
+    if repository_ctx.attr.output:
+        repository_ctx.execute(["mkdir", "-p", repository_ctx.attr.output])
     return
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that exports_files will fail on non-macOS platforms because the expected files/directories are not created, which is a bug the PR introduces. The proposed fix is valid and necessary for the repository rule to work correctly across platforms.

Medium
Ensure exported paths exist

In _pkg_archive_impl, when skipping on non-macOS platforms, create placeholder
directories for each destination in the move attribute to prevent exports_files
from failing.

common/private/pkg_archive.bzl [2-6]

 repository_ctx.file("BUILD.bazel", repository_ctx.attr.build_file_content)
 
 if not repository_ctx.which("pkgutil"):
     # pkgutil is macOS-only; skip download on other platforms
+    for dst in repository_ctx.attr.move.values():
+        repository_ctx.execute(["mkdir", "-p", dst])
     return
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a bug introduced by the PR where exports_files would fail on non-macOS platforms due to missing files. The proposed fix of creating placeholder directories for move destinations is correct and essential for cross-platform compatibility.

Medium
Learned
best practice
Validate optional inputs before use

Guard against missing/empty build_file_content (it’s not marked mandatory)
before writing BUILD.bazel, and fall back to a minimal placeholder content to
avoid failures on unsupported platforms.

common/private/dmg_archive.bzl [2-6]

-repository_ctx.file("BUILD.bazel", repository_ctx.attr.build_file_content)
+build_content = repository_ctx.attr.build_file_content
+if not build_content:
+    build_content = "# dmg_archive: no build_file_content provided\n"
+repository_ctx.file("BUILD.bazel", build_content)
 
 if not repository_ctx.which("hdiutil"):
     # hdiutil is macOS-only; skip download on other platforms
     return
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Add explicit validation/guards at integration boundaries (e.g., optional rule attrs) before using them.

Low
  • More

Previous suggestions

✅ Suggestions up to commit ac1095c
CategorySuggestion                                                                                                                                    Impact
Learned
best practice
Centralize repeated skip logic

The same “tool missing -> write empty BUILD -> return” pattern is duplicated in
multiple archives; extract it into a shared helper (e.g.,
common/private/archive_utils.bzl) and call it from both implementations to keep
behavior consistent.

common/private/dmg_archive.bzl [2-5]

-hdiutil = repository_ctx.which("hdiutil")
-if not hdiutil:
-    repository_ctx.file("BUILD.bazel", "# dmg_archive: skipped (hdiutil not available on this platform)")
-    return
+load("//common/private:archive_utils.bzl", "skip_if_tool_missing")
 
+skip_if_tool_missing(repository_ctx, "hdiutil", "dmg_archive")
+
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Reduce duplication by centralizing shared behavior (helpers/shared utilities) instead of repeating the same availability-guard logic in multiple files.

Low
Make generated BUILD file well-formed
Suggestion Impact:The commit changed the same pkgutil-missing handling logic by ensuring a BUILD.bazel is always written up front and adding a clearer skip comment when pkgutil is unavailable. While it does not implement the exact suggested placeholder string with a trailing newline, it addresses the "well-formed/explicit skip" intent by restructuring BUILD generation and making the skip reason explicit.

code diff:

-    pkgutil = repository_ctx.which("pkgutil")
-    if not pkgutil:
-        # Create BUILD with expected targets so queries succeed; builds will fail with missing files
-        repository_ctx.file("BUILD.bazel", repository_ctx.attr.build_file_content)
+    repository_ctx.file("BUILD.bazel", repository_ctx.attr.build_file_content)
+
+    if not repository_ctx.which("pkgutil"):
+        # pkgutil is macOS-only; skip download on other platforms
         return

When generating a placeholder BUILD.bazel, write a well-formed file (include a
trailing newline and a clearer comment) to avoid odd formatting/tooling edge
cases and make the skip reason explicit.

common/private/pkg_archive.bzl [2-5]

 pkgutil = repository_ctx.which("pkgutil")
 if not pkgutil:
-    repository_ctx.file("BUILD.bazel", "# pkg_archive: skipped (pkgutil not available on this platform)")
+    repository_ctx.file(
+        "BUILD.bazel",
+        "# pkg_archive: skipped because pkgutil is not available on this platform\n",
+    )
     return

[Suggestion processed]

Suggestion importance[1-10]: 5

__

Why:
Relevant best practice - Add explicit validation/guards at integration boundaries by ensuring generated files are well-formed and unambiguous when skipping due to missing external tools.

Low
General
Add skip notification log

Add a repository_ctx.info log message to notify the user when the dmg_archive
rule is skipped due to the absence of hdiutil.

common/private/dmg_archive.bzl [2-5]

 hdiutil = repository_ctx.which("hdiutil")
 if not hdiutil:
     repository_ctx.file("BUILD.bazel", "# dmg_archive: skipped (hdiutil not available on this platform)")
+    repository_ctx.info("dmg_archive: skipping rule because hdiutil was not found on this platform")
     return
Suggestion importance[1-10]: 5

__

Why: The suggestion improves debuggability by adding a visible log message when the rule is skipped, which is more user-friendly than a comment in a generated file.

Low
Rename BUILD file for compatibility

Rename the generated BUILD.bazel file to BUILD to align with common Bazel
conventions.

common/private/dmg_archive.bzl [4]

-repository_ctx.file("BUILD.bazel", "# dmg_archive: skipped (hdiutil not available on this platform)")
+repository_ctx.file("BUILD", "# dmg_archive: skipped (hdiutil not available on this platform)")
Suggestion importance[1-10]: 3

__

Why: This is a valid stylistic suggestion as BUILD is the conventional filename, but BUILD.bazel is also perfectly acceptable, making the impact of this change minor.

Low

@titusfortner
Copy link
Member Author

I'm manually kicking off a full CI test: https://github.com/SeleniumHQ/selenium/actions/runs/21271391848

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes Bazel query failures on non-macOS platforms by adding platform detection to macOS-specific archive rules. The issue occurred when bazel query deps() tried to resolve macOS browser download rules on Linux/Windows, causing failures because macOS-specific tools (pkgutil, hdiutil) were not available.

Changes:

  • Added tool availability checks to pkg_archive and dmg_archive repository rules
  • Repository rules now create a minimal BUILD.bazel file and return early when required tools are unavailable
  • Allows cross-platform Bazel queries without failing on platform-specific repository rules

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
common/private/pkg_archive.bzl Added pkgutil availability check, creates stub BUILD.bazel on unsupported platforms
common/private/dmg_archive.bzl Added hdiutil availability check, creates stub BUILD.bazel on unsupported platforms

@titusfortner
Copy link
Member Author

This isn't doing what I need it to. Might have to take a different direction.

@titusfortner titusfortner force-pushed the mac_archives branch 2 times, most recently from 61c89a2 to 09d19d3 Compare January 24, 2026 01:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

@titusfortner titusfortner merged commit f40b8ba into trunk Jan 24, 2026
25 checks passed
@titusfortner titusfortner deleted the mac_archives branch January 24, 2026 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations Review effort 1/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants