Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to windows-bindgen #1202

Merged
merged 2 commits into from
Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ jobs:
- uses: Swatinem/rust-cache@v2
- run: |
cargo hack check --feature-powerset --optional-deps serde,rkyv \
--skip __internal_bench,__doctest,iana-time-zone,pure-rust-locales,libc \
--skip __internal_bench,__doctest,iana-time-zone,pure-rust-locales,libc,winapi \
--all-targets
# run using `bash` on all platforms for consistent
# line-continuation marks
Expand Down
6 changes: 5 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ name = "chrono"
default = ["clock", "std", "oldtime", "wasmbind"]
alloc = []
libc = []
winapi = ["windows-targets"]
djc marked this conversation as resolved.
Show resolved Hide resolved
std = []
clock = ["std", "winapi", "iana-time-zone", "android-tzdata"]
oldtime = ["time"]
Expand All @@ -44,7 +45,10 @@ js-sys = { version = "0.3", optional = true } # contains FFI bindings for


[target.'cfg(windows)'.dependencies]
winapi = { version = "0.3.0", features = ["std", "minwinbase", "minwindef", "timezoneapi", "sysinfoapi"], optional = true }
windows-targets = { version = "0.48", optional = true }

[target.'cfg(windows)'.dev-dependencies]
windows-bindgen = { version = "0.51" }

[target.'cfg(unix)'.dependencies]
iana-time-zone = { version = "0.1.45", optional = true, features = ["fallback"] }
Expand Down
4 changes: 4 additions & 0 deletions src/offset/local/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ mod inner;
#[path = "windows.rs"]
mod inner;

#[cfg(all(windows, feature = "clock"))]
#[allow(unreachable_pub)]
mod win_bindings;

#[cfg(all(
not(unix),
not(windows),
Expand Down
51 changes: 51 additions & 0 deletions src/offset/local/win_bindings.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Bindings generated by `windows-bindgen` 0.51.1

#![allow(non_snake_case, non_upper_case_globals, non_camel_case_types, dead_code, clippy::all)]
::windows_targets::link!("kernel32.dll" "system" fn SystemTimeToFileTime(lpsystemtime : *const SYSTEMTIME, lpfiletime : *mut FILETIME) -> BOOL);
::windows_targets::link!("kernel32.dll" "system" fn SystemTimeToTzSpecificLocalTime(lptimezoneinformation : *const TIME_ZONE_INFORMATION, lpuniversaltime : *const SYSTEMTIME, lplocaltime : *mut SYSTEMTIME) -> BOOL);
::windows_targets::link!("kernel32.dll" "system" fn TzSpecificLocalTimeToSystemTime(lptimezoneinformation : *const TIME_ZONE_INFORMATION, lplocaltime : *const SYSTEMTIME, lpuniversaltime : *mut SYSTEMTIME) -> BOOL);
pub type BOOL = i32;
#[repr(C)]
pub struct FILETIME {
pub dwLowDateTime: u32,
pub dwHighDateTime: u32,
}
impl ::core::marker::Copy for FILETIME {}
impl ::core::clone::Clone for FILETIME {
fn clone(&self) -> Self {
*self
}
}
#[repr(C)]
pub struct SYSTEMTIME {
pub wYear: u16,
pub wMonth: u16,
pub wDayOfWeek: u16,
pub wDay: u16,
pub wHour: u16,
pub wMinute: u16,
pub wSecond: u16,
pub wMilliseconds: u16,
}
impl ::core::marker::Copy for SYSTEMTIME {}
impl ::core::clone::Clone for SYSTEMTIME {
fn clone(&self) -> Self {
*self
}
}
#[repr(C)]
pub struct TIME_ZONE_INFORMATION {
pub Bias: i32,
pub StandardName: [u16; 32],
pub StandardDate: SYSTEMTIME,
pub StandardBias: i32,
pub DaylightName: [u16; 32],
pub DaylightDate: SYSTEMTIME,
pub DaylightBias: i32,
}
impl ::core::marker::Copy for TIME_ZONE_INFORMATION {}
impl ::core::clone::Clone for TIME_ZONE_INFORMATION {
fn clone(&self) -> Self {
*self
}
}
6 changes: 6 additions & 0 deletions src/offset/local/win_bindings.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
--out src/offset/local/win_bindings.rs
--config flatten sys
djc marked this conversation as resolved.
Show resolved Hide resolved
--filter
Windows.Win32.System.Time.SystemTimeToFileTime
Windows.Win32.System.Time.SystemTimeToTzSpecificLocalTime
Windows.Win32.System.Time.TzSpecificLocalTimeToSystemTime
5 changes: 2 additions & 3 deletions src/offset/local/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,9 @@ use std::io::Error;
use std::ptr;
use std::result::Result;

use winapi::shared::minwindef::FILETIME;
use winapi::um::minwinbase::SYSTEMTIME;
use winapi::um::timezoneapi::{
use super::win_bindings::{
SystemTimeToFileTime, SystemTimeToTzSpecificLocalTime, TzSpecificLocalTimeToSystemTime,
FILETIME, SYSTEMTIME,
};

use super::FixedOffset;
Expand Down
22 changes: 22 additions & 0 deletions tests/win_bindings.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#![cfg(all(windows, feature = "clock", feature = "std"))]

use std::fs;
use windows_bindgen::bindgen;

#[test]
fn gen_bindings() {
let input = "src/offset/local/win_bindings.txt";
let output = "src/offset/local/win_bindings.rs";
let existing = fs::read_to_string(output).unwrap();

let log = bindgen(["--etc", input]).unwrap();
eprintln!("{}", log);

// Check the output is the same as before.
// Depending on the git configuration the file may have been checked out with `\r\n` newlines or
// with `\n`. Compare line-by-line to ignore this difference.
let new = fs::read_to_string(output).unwrap();
if !new.lines().eq(existing.lines()) {
Copy link
Member

Choose a reason for hiding this comment

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

Why are we comparing this on a line-by-line basis? To get around the \r\n vs \n differences? If so, please add a comment to that effect.

Choose a reason for hiding this comment

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

Pretty sure it's that, all files should be checked in to the repo with \n but because the test runs on Windows (windows-bindgen runs just fine on Linux, fwiw!) the output will be \r\n.

Personally I'd just run this test in the CI with a helpful "how to regenerate" message and a git status --porcelain check for files that may not be checked in, as linked above:

Traverse-Research/amd-ext-d3d-rs@d5ba122#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fR62

(Note that this line changed a bit on main because we're back to cargo r -p api_gen.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(windows-bindgen runs just fine on Linux, fwiw!)

I just rebooted from Linux to Windows to work on this 🤣

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@MarijnS95 I have to admit I am pretty bad at all this 😄. Is it easy for you to change this PR to something better?
And thanks for helping out already!

Choose a reason for hiding this comment

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

At least you can execute the generated code that way, unless cross-compiling with --target x86_64-pc-windows-gnu and running it in wine :)

Is it easy for you to change this PR

Not at all, I'm a Linux user keeping windows-rs "in check" around our dependencies to support a primarily (exclusively?) Windows-based dev team, so I just happen to look at random PRs but otherwise have no reviewer/maintainer rights here whatsoever.

panic!("generated file `{}` is changed.", output);
}
}
Loading