Skip to content

Conversation

@dcarp
Copy link
Collaborator

@dcarp dcarp commented Oct 6, 2025

  • feat: add 'env' (location and make variable expansion) and 'data' attributes
  • fix: typo in 'resolved_toolchain' definition
  • chore: improve curl downloader
  • chore: refactor compiler versions generator

@dcarp dcarp force-pushed the env-data-attributes branch from fa3a0b7 to 95cd4c8 Compare October 6, 2025 22:05
* feat: add 'env' (location and make variable expansion) and 'data' attributes
* fix: typo in 'resolved_toolchain' definition
* chore: improve curl downloader
* chore: refactor compiler versions generator
@dcarp dcarp force-pushed the env-data-attributes branch from 95cd4c8 to 07836c2 Compare October 6, 2025 22:09
@dcarp dcarp requested a review from Copilot October 6, 2025 22:16
Copy link

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 enhances D binary and test rules with runtime data and environment variable support, along with several improvements to the codebase structure and tooling. The primary purpose is to add 'env' and 'data' attributes to d_binary and d_test rules, allowing for better runtime configuration and data dependencies.

  • Added env and data attributes to both d_binary and d_test rules with location and make variable expansion
  • Fixed a typo in the resolved_toolchain definition from dinfo to d_toolchain_info
  • Improved the curl downloader implementation with better error handling and timeout configurations

Reviewed Changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tools/integrity_hash.d Added new utility module for computing SHA integrity hashes of files
tools/generate_compiler_versions_bzl.d Refactored to use new integrity hash module and improved string handling
tools/curl_downloader.d Enhanced with better error handling, timeouts, and per-request curl initialization
tools/BUILD.bazel Restructured to create a shared d_utils library for common utilities
e2e/smoke/tests/std_lib/BUILD.bazel Added new test file demonstrating D binary and test usage
e2e/smoke/WORKSPACE.bazel Added aspect_bazel_lib dependency for make variable expansion
e2e/smoke/BUILD.bazel Refactored to move test definitions to dedicated subdirectory
docs/rules.md Updated documentation to include new env and data attributes
d/private/rules/test.bzl Updated to use new runnable_attrs instead of common_attrs
d/private/rules/library.bzl Updated to use new library_attrs instead of common_attrs
d/private/rules/common.bzl Added support for env and data attributes with proper expansion
d/private/rules/binary.bzl Updated to use new runnable_attrs instead of common_attrs
d/private/rules/BUILD.bazel Added dependencies for make variable expansion functionality
d/private/resolved_toolchain.bzl Fixed typo from dinfo to d_toolchain_info
MODULE.bazel Moved aspect_bazel_lib from dev dependency to regular dependency

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +101 to +105
curl_easy_setopt(curl, CurlOption.followlocation, 1L); // follow redirects
curl_easy_setopt(curl, CurlOption.maxredirs, 5L); // maximum redirects to follow
curl_easy_setopt(curl, CurlOption.connecttimeout, 10L); // 10 seconds connection timeout
curl_easy_setopt(curl, CurlOption.timeout, 600L); // 10 minutes
curl_easy_setopt(curl, CurlOption.failonerror, 1L); // fail on HTTP errors
Copy link

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

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

[nitpick] The 600-second (10 minutes) timeout may be excessive for most use cases. Consider using a more reasonable default timeout (e.g., 120 seconds) or making it configurable.

Copilot uses AI. Check for mistakes.
@dcarp dcarp merged commit 6cd1d1f into bazel-contrib:main Oct 6, 2025
11 checks passed
@dcarp dcarp deleted the env-data-attributes branch October 6, 2025 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant