Skip to content

Const eval all of version.rs#7923

Merged
youknowone merged 1 commit into
RustPython:mainfrom
joshuamegnauth54:const-eval-version-rs
May 19, 2026
Merged

Const eval all of version.rs#7923
youknowone merged 1 commit into
RustPython:mainfrom
joshuamegnauth54:const-eval-version-rs

Conversation

@joshuamegnauth54
Copy link
Copy Markdown
Contributor

@joshuamegnauth54 joshuamegnauth54 commented May 19, 2026

version.rs essentially consists of constants that can be baked in at compile time. I moved most of version.rs to build.rs. The constants are passed via rustc's environment then stored in the binary.

Summary by CodeRabbit

  • Refactor

    • Version and system metadata are now compile-time constants, reducing runtime work.
    • sys module attributes (version, winver, _git) are exposed as constants instead of computed at runtime.
  • Chores

    • Build now captures and embeds richer build metadata (Git identifier, build datetime/info, release level, compiler/version details) into the produced binaries.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: f9eda925-fb6b-4ccd-8e74-f19c2e0dcc14

📥 Commits

Reviewing files that changed from the base of the PR and between a6f57bb and 9f877e1.

📒 Files selected for processing (5)
  • crates/vm/Cargo.toml
  • crates/vm/build.rs
  • crates/vm/src/stdlib/sys.rs
  • crates/vm/src/version.rs
  • src/settings.rs
✅ Files skipped from review due to trivial changes (1)
  • src/settings.rs
🚧 Files skipped from review as they are similar to previous changes (3)
  • crates/vm/Cargo.toml
  • crates/vm/build.rs
  • crates/vm/src/stdlib/sys.rs

📝 Walkthrough

Walkthrough

Refactors version and Git metadata handling to build-time: build.rs now emits new environment variables and formatted timestamps; version.rs exposes compile-time constants derived from env!; sys and settings are updated to read the new constants instead of runtime helper functions.

Changes

Build-time version metadata refactoring

Layer / File(s) Summary
Build dependency and imports
crates/vm/Cargo.toml, crates/vm/build.rs
Added chrono as a workspace build-dependency and imported datetime/duration helpers for build-time local datetime formatting.
Build script version metadata generation
crates/vm/build.rs
Extended build script to emit RUSTPYTHON_GIT_IDENTIFIER, RUSTPYTHON_BUILD_INFO, and RUSTPYTHON_RELEASE_LEVEL_N. Implemented get_build_info()/git_identifier(), timestamp-to-local-datetime formatters (get_git_date, get_git_time, get_git_datetime), python_version() emissions, and updated get_version() composition including build info.
Compile-time version constants module
crates/vm/src/version.rs
Converted module to constants-only using a parse_consts! macro and env! values. Added RUSTPYTHON_BUILD_INFO, GIT_IDENTIFIER, and RUSTPYTHON_VERSION (compile-time concatenation of left/right with UTF-8 validation). Removed former runtime getter functions.
Runtime consumer updates
crates/vm/src/stdlib/sys.rs, src/settings.rs
sys._git now reads version::GIT_IDENTIFIER; sys.version and sys.winver are now compile-time #[pyattr] constants (RUSTPYTHON_VERSION, WINVER); settings::version() prints RUSTPYTHON_VERSION constant.

Sequence Diagram(s)

sequenceDiagram
  participant Build as build.rs (build script)
  participant Env as compile env (RUSTPYTHON_*)
  participant Version as crates::vm::version (compile-time)
  participant Runtime as sys/settings (runtime)

  Build->>Env: emit RUSTPYTHON_GIT_IDENTIFIER, RUSTPYTHON_BUILD_INFO, RUSTPYTHON_VERSION_LEFT/RIGHT, RUSTPYTHON_RELEASE_LEVEL_N
  Env-->>Version: env! reads (MAJOR/MINOR/..., GIT_IDENTIFIER, RUSTPYTHON_BUILD_INFO, VERSION_LEFT/RIGHT)
  Version->>Runtime: expose constants (RUSTPYTHON_VERSION, GIT_IDENTIFIER, WINVER)
  Runtime->>Version: read constants for sys.version, sys._git, settings::version()
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • RustPython/RustPython#7789: Overlaps at the crates/vm/Cargo.toml dependency style changes (workspace = true) similar to adding chrono as a workspace build-dependency.
  • RustPython/RustPython#7793: Related edits to crates/vm/src/version.rs and crates/vm/src/stdlib/sys.rs affecting how version/git metadata are surfaced as constants.

Suggested reviewers

  • ShaharNaveh
  • fanninpm

Poem

🐇 I baked the build at dawn,

envs set, the git tag drawn,
Constants stitched from left and right,
Now versions rest at compile-time light,
Hop, hop — the release is bright!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.89% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main objective: refactoring version.rs to use compile-time const evaluation instead of runtime computation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
crates/vm/build.rs (1)

73-81: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use SOURCE_DATE_EPOCH and UTC timezone for reproducible build metadata.

The git_timestamp() function uses SystemTime::now() as a fallback, and get_git_timestamp_datetime() converts that Unix epoch to DateTime<Local> (line 129), then formats it with patterns like "%b %e %Y" (line 136) and "%H:%M:%S" (line 141). This embeds local timezone information and current system time into RUSTPYTHON_BUILD_INFO, producing different formatted timestamps across equivalent builds in different timezones or at different times. The code already references reproducible-builds.org on line 98 but does not honor the spec. The reproducible-builds standard requires checking SOURCE_DATE_EPOCH and using stable (UTC) timezone rendering to ensure identical sources produce identical binaries.

Suggested fix
-use chrono::{Local, prelude::DateTime};
+use chrono::{Utc, prelude::DateTime};
 use core::time::Duration;
@@
 fn git_timestamp() -> String {
-    git(&["log", "-1", "--format=%ct"]).unwrap_or_else(|_| {
-        SystemTime::now()
-            .duration_since(UNIX_EPOCH)
-            .unwrap_or_default()
-            .as_secs()
-            .to_string()
-    })
+    env::var("SOURCE_DATE_EPOCH")
+        .ok()
+        .or_else(|| git(&["log", "-1", "--format=%ct"]).ok())
+        .unwrap_or_else(|| {
+            SystemTime::now()
+                .duration_since(UNIX_EPOCH)
+                .unwrap_or_default()
+                .as_secs()
+                .to_string()
+        })
 }
@@
-fn get_git_timestamp_datetime() -> DateTime<Local> {
+fn get_git_timestamp_datetime() -> DateTime<Utc> {
     let timestamp = git_timestamp().parse::<u64>().unwrap_or_default();
     let datetime = UNIX_EPOCH + Duration::from_secs(timestamp);
-    datetime.into()
+    DateTime::<Utc>::from(datetime)
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/vm/build.rs` around lines 73 - 81, The
git_timestamp()/get_git_timestamp_datetime() logic must honor SOURCE_DATE_EPOCH
and always render timestamps in UTC for reproducible builds: change
git_timestamp() to first look for the SOURCE_DATE_EPOCH env var and use that
value (as a Unix epoch string) if present; only fall back to the git log output
or SystemTime::now() when SOURCE_DATE_EPOCH is absent; update
get_git_timestamp_datetime() to parse the epoch into a UTC DateTime (use
chrono::Utc or equivalent) instead of DateTime<Local> and format with UTC-based
patterns so RUSTPYTHON_BUILD_INFO is produced deterministically across timezones
and builds.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/vm/build.rs`:
- Around line 207-214: The match arm in function release_to_n currently uses
unreachable!() for unknown release strings, causing an opaque panic; update
release_to_n to explicitly panic with a clear, actionable message (including the
received release value and that it came from RUSTPYTHON_RELEASE_LEVEL) so
typos/invalid values produce a descriptive error instead of an opaque
build-script panic.

---

Outside diff comments:
In `@crates/vm/build.rs`:
- Around line 73-81: The git_timestamp()/get_git_timestamp_datetime() logic must
honor SOURCE_DATE_EPOCH and always render timestamps in UTC for reproducible
builds: change git_timestamp() to first look for the SOURCE_DATE_EPOCH env var
and use that value (as a Unix epoch string) if present; only fall back to the
git log output or SystemTime::now() when SOURCE_DATE_EPOCH is absent; update
get_git_timestamp_datetime() to parse the epoch into a UTC DateTime (use
chrono::Utc or equivalent) instead of DateTime<Local> and format with UTC-based
patterns so RUSTPYTHON_BUILD_INFO is produced deterministically across timezones
and builds.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 049c90f6-085c-49b0-a376-3faee648595a

📥 Commits

Reviewing files that changed from the base of the PR and between 883ce9d and a6f57bb.

📒 Files selected for processing (5)
  • crates/vm/Cargo.toml
  • crates/vm/build.rs
  • crates/vm/src/stdlib/sys.rs
  • crates/vm/src/version.rs
  • src/settings.rs

Comment thread crates/vm/build.rs
Comment on lines +207 to +214
fn release_to_n(release: &str) -> usize {
match release {
"alpha" => 0xA,
"beta" => 0xB,
"candidate" => 0xC,
"final" => 0xD,
_ => unreachable!(),
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Make invalid release levels fail with an actionable message.

Line 213 turns a typo in RUSTPYTHON_RELEASE_LEVEL into an opaque build-script panic. An explicit panic message here is much easier to diagnose.

Suggested fix
 fn release_to_n(release: &str) -> usize {
     match release {
         "alpha" => 0xA,
         "beta" => 0xB,
         "candidate" => 0xC,
         "final" => 0xD,
-        _ => unreachable!(),
+        _ => panic!("unsupported RUSTPYTHON_RELEASE_LEVEL: {release}"),
     }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fn release_to_n(release: &str) -> usize {
match release {
"alpha" => 0xA,
"beta" => 0xB,
"candidate" => 0xC,
"final" => 0xD,
_ => unreachable!(),
}
fn release_to_n(release: &str) -> usize {
match release {
"alpha" => 0xA,
"beta" => 0xB,
"candidate" => 0xC,
"final" => 0xD,
_ => panic!("unsupported RUSTPYTHON_RELEASE_LEVEL: {release}"),
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/vm/build.rs` around lines 207 - 214, The match arm in function
release_to_n currently uses unreachable!() for unknown release strings, causing
an opaque panic; update release_to_n to explicitly panic with a clear,
actionable message (including the received release value and that it came from
RUSTPYTHON_RELEASE_LEVEL) so typos/invalid values produce a descriptive error
instead of an opaque build-script panic.

`version.rs` essentially consists of constants that can be baked in at
compile time. I moved most of `version.rs` to `build.rs`. The constants
are passed via rustc's environment then stored in the binary.
@joshuamegnauth54 joshuamegnauth54 force-pushed the const-eval-version-rs branch from a6f57bb to 9f877e1 Compare May 19, 2026 01:52
@youknowone youknowone requested a review from ShaharNaveh May 19, 2026 04:35
Copy link
Copy Markdown
Contributor

@ShaharNaveh ShaharNaveh left a comment

Choose a reason for hiding this comment

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

tysm!

@youknowone youknowone merged commit c5143aa into RustPython:main May 19, 2026
26 checks passed
@joshuamegnauth54 joshuamegnauth54 deleted the const-eval-version-rs branch May 19, 2026 17:48
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.

3 participants