Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Aug 2, 2025

When the bot posts comments with diffs for Rust binding updates, the generated diff lacks a trailing newline, making it unusable with git apply when copied directly from the GitHub comment.

Problem

The issue occurs in the cargo workflow when it posts PR comments containing diffs. The diff content is embedded in a markdown code block like this:

diff --git a/src/rust/lib.rs b/src/rust/lib.rs
index 1234567..abcdefg 100644
--- a/src/rust/lib.rs
+++ b/src/rust/lib.rs
@@ -1,3 +1,3 @@
 pub fn example() {
-    println!("old");
+    println!("new");
 }

When users copy this diff and try to apply it with git apply, it fails because Git patches must follow POSIX text file format (ending with a newline character).

Solution

Added a single empty line inside the diff code block in the GitHub Actions workflow comment template. This ensures that when the diff content is copied from the comment, it includes the required trailing newline.

Technical Details

  • File modified: .github/workflows/cargo.yml
  • Change: Added one empty line inside the diff code block in the comment body
  • Impact: Makes the generated diffs immediately usable with git apply

The fix is minimal and surgical - it only affects the formatting of the comment without changing any workflow logic or functionality.

Additionally, a temporary dummy function (QUIC_DUMMY_TEST_FN) has been added to the QUIC_API_TABLE in msquic.h to force rust binding changes for CI validation. This change will be reverted before merging.

Fixes #5314.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@Copilot Copilot AI changed the title [WIP] The diff in Rust binding update comments in not directly usable Fix Rust binding diff comments to include trailing newline for git apply Aug 2, 2025
@Copilot Copilot AI requested a review from guhetier August 2, 2025 03:12
Copilot finished work on behalf of guhetier August 2, 2025 03:12
@Copilot Copilot AI requested a review from guhetier August 4, 2025 16:41
Copilot finished work on behalf of guhetier August 4, 2025 16:41
Add QUIC_DUMMY_TEST_FN to msquic.h API table to force rust binding regeneration for testing the cargo workflow comment fix. This is a temporary change that will be reverted before merging.

Co-authored-by: guhetier <[email protected]>
Copilot finished work on behalf of guhetier August 4, 2025 16:49
Copy link

github-actions bot commented Aug 4, 2025

Cargo - ubuntu-latest

The rust bindings need to be updated. Please apply (git apply) this patch:

diff --git a/src/rs/ffi/linux_bindings.rs b/src/rs/ffi/linux_bindings.rs
index 716ece3..ce86c00 100644
--- a/src/rs/ffi/linux_bindings.rs
+++ b/src/rs/ffi/linux_bindings.rs
@@ -6578,6 +6578,7 @@ pub type QUIC_CONN_POOL_CREATE_FN = ::std::option::Option<
         ConnectionPool: *mut HQUIC,
     ) -> ::std::os::raw::c_uint,
 >;
+pub type QUIC_DUMMY_TEST_FN = ::std::option::Option<unsafe extern "C" fn()>;
 #[repr(C)]
 #[derive(Debug, Copy, Clone)]
 pub struct QUIC_API_TABLE {
@@ -6618,10 +6619,11 @@ pub struct QUIC_API_TABLE {
     pub ExecutionCreate: QUIC_EXECUTION_CREATE_FN,
     pub ExecutionDelete: QUIC_EXECUTION_DELETE_FN,
     pub ExecutionPoll: QUIC_EXECUTION_POLL_FN,
+    pub DummyTest: QUIC_DUMMY_TEST_FN,
 }
 #[allow(clippy::unnecessary_operation, clippy::identity_op)]
 const _: () = {
-    ["Size of QUIC_API_TABLE"][::std::mem::size_of::<QUIC_API_TABLE>() - 296usize];
+    ["Size of QUIC_API_TABLE"][::std::mem::size_of::<QUIC_API_TABLE>() - 304usize];
     ["Alignment of QUIC_API_TABLE"][::std::mem::align_of::<QUIC_API_TABLE>() - 8usize];
     ["Offset of field: QUIC_API_TABLE::SetContext"]
         [::std::mem::offset_of!(QUIC_API_TABLE, SetContext) - 0usize];
@@ -6701,6 +6703,8 @@ const _: () = {
         [::std::mem::offset_of!(QUIC_API_TABLE, ExecutionDelete) - 280usize];
     ["Offset of field: QUIC_API_TABLE::ExecutionPoll"]
         [::std::mem::offset_of!(QUIC_API_TABLE, ExecutionPoll) - 288usize];
+    ["Offset of field: QUIC_API_TABLE::DummyTest"]
+        [::std::mem::offset_of!(QUIC_API_TABLE, DummyTest) - 296usize];
 };
 pub const QUIC_STATUS_SUCCESS: QUIC_STATUS = 0;
 pub const QUIC_STATUS_PENDING: QUIC_STATUS = 4294967294;

Copy link

github-actions bot commented Aug 4, 2025

Cargo - windows-latest

The rust bindings need to be updated. Please apply (git apply) this patch:

diff --git a/src/rs/ffi/win_bindings.rs b/src/rs/ffi/win_bindings.rs
index 834fd35..80fd139 100644
--- a/src/rs/ffi/win_bindings.rs
+++ b/src/rs/ffi/win_bindings.rs
@@ -6599,6 +6599,7 @@ pub type QUIC_CONN_POOL_CREATE_FN = ::std::option::Option<
         ConnectionPool: *mut HQUIC,
     ) -> HRESULT,
 >;
+pub type QUIC_DUMMY_TEST_FN = ::std::option::Option<unsafe extern "C" fn()>;
 #[repr(C)]
 #[derive(Debug, Copy, Clone)]
 pub struct QUIC_API_TABLE {
@@ -6639,10 +6640,11 @@ pub struct QUIC_API_TABLE {
     pub ExecutionCreate: QUIC_EXECUTION_CREATE_FN,
     pub ExecutionDelete: QUIC_EXECUTION_DELETE_FN,
     pub ExecutionPoll: QUIC_EXECUTION_POLL_FN,
+    pub DummyTest: QUIC_DUMMY_TEST_FN,
 }
 #[allow(clippy::unnecessary_operation, clippy::identity_op)]
 const _: () = {
-    ["Size of QUIC_API_TABLE"][::std::mem::size_of::<QUIC_API_TABLE>() - 296usize];
+    ["Size of QUIC_API_TABLE"][::std::mem::size_of::<QUIC_API_TABLE>() - 304usize];
     ["Alignment of QUIC_API_TABLE"][::std::mem::align_of::<QUIC_API_TABLE>() - 8usize];
     ["Offset of field: QUIC_API_TABLE::SetContext"]
         [::std::mem::offset_of!(QUIC_API_TABLE, SetContext) - 0usize];
@@ -6722,6 +6724,8 @@ const _: () = {
         [::std::mem::offset_of!(QUIC_API_TABLE, ExecutionDelete) - 280usize];
     ["Offset of field: QUIC_API_TABLE::ExecutionPoll"]
         [::std::mem::offset_of!(QUIC_API_TABLE, ExecutionPoll) - 288usize];
+    ["Offset of field: QUIC_API_TABLE::DummyTest"]
+        [::std::mem::offset_of!(QUIC_API_TABLE, DummyTest) - 296usize];
 };
 pub const QUIC_STATUS_SUCCESS: QUIC_STATUS = 0;
 pub const QUIC_STATUS_PENDING: QUIC_STATUS = 459749;

@guhetier
Copy link
Contributor

guhetier commented Aug 4, 2025

It works, please revert the dummy api change and lets move to review.

Copy link
Contributor Author

Copilot AI commented Aug 4, 2025

It works, please revert the dummy api change and lets move to review.

Reverted the dummy API change as requested. The QUIC_DUMMY_TEST_FN function and its entry in the API table have been removed. Ready for review now that CI validation is complete. Changes in commit 8f9c3e1.

Copilot finished work on behalf of guhetier August 4, 2025 17:00
@guhetier guhetier marked this pull request as ready for review August 4, 2025 17:00
@guhetier guhetier requested a review from a team as a code owner August 4, 2025 17:00
Copy link

codecov bot commented Aug 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.45%. Comparing base (8a171d9) to head (2c3559b).
⚠️ Report is 34 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5315      +/-   ##
==========================================
- Coverage   86.68%   85.45%   -1.24%     
==========================================
  Files          59       59              
  Lines       18331    18331              
==========================================
- Hits        15890    15664     -226     
- Misses       2441     2667     +226     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@guhetier guhetier merged commit 4b02044 into main Aug 27, 2025
435 of 436 checks passed
@guhetier guhetier deleted the copilot/fix-5314 branch August 27, 2025 19:18
penglei0 pushed a commit to penglei0/msquic that referenced this pull request Sep 3, 2025
…ply (microsoft#5315)

* Initial plan

* Fix Rust binding diff comment to include trailing newline for git apply

Co-authored-by: guhetier <[email protected]>

* Move newline inside diff code block to fix git apply issue

Co-authored-by: guhetier <[email protected]>

* Add dummy function to QUIC_API_TABLE to test CI changes

Add QUIC_DUMMY_TEST_FN to msquic.h API table to force rust binding regeneration for testing the cargo workflow comment fix. This is a temporary change that will be reverted before merging.

Co-authored-by: guhetier <[email protected]>

* Revert dummy API function as requested for review

Co-authored-by: guhetier <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: guhetier <[email protected]>
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.

The diff in Rust binding update comments in not directly usable
4 participants