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

Fix nightly CI failures #505

Merged
merged 1 commit into from
Jan 12, 2024
Merged

Fix nightly CI failures #505

merged 1 commit into from
Jan 12, 2024

Conversation

newAM
Copy link
Member

@newAM newAM commented Jan 11, 2024

The data_overflow test seems to have been malfunctioning, and changing to addr_of! fixes it, but for some reason this test takes 10's of minutes to compile in CI? I'm not sure what to make of this.

@newAM newAM force-pushed the fix-ci branch 6 times, most recently from ec42dd2 to c5e3dc1 Compare January 11, 2024 05:43
@newAM newAM marked this pull request as ready for review January 11, 2024 06:14
@newAM newAM requested a review from a team as a code owner January 11, 2024 06:14
@adamgreig
Copy link
Member

Huh, the 10s of minutes sounds bizarre and kind of horrifying. I think the addr_of change is due to a new lint about taking the address of static mut variables (rust-lang/rust#117556).

@adamgreig adamgreig added this pull request to the merge queue Jan 12, 2024
Merged via the queue into rust-embedded:master with commit 6b3a5b7 Jan 12, 2024
14 checks passed
@newAM newAM deleted the fix-ci branch January 12, 2024 03:08
newAM added a commit to newAM/cortex-m that referenced this pull request Jun 30, 2024
Long story:

* This test was introduced
* `ci/script.sh` did not correctly check for failures, [SC2251]
  * Thanks rust-embedded#404 for the tip!
* The FLASH size was increased, and the test incorrectly passed, but
  nobody noticed
* I modified the test in rust-embedded#505 which made it fail again, but for the
  wrong reason.  `ptr::read_volatile(ptr::addr_of!(RODATA))` reads the
  entire array, which is not equivalent to the original code
  `ptr::read_volatile(&RODATA as *const u8)` which read a single element
  of the array.
* The test now failed, but the stack related overflow takes rustc a LONG
  time to compile, and pushed our CI times to >30m.

These changes fix ci/scripts.sh to exit with a non-zero code if
data_overflow is passing, and makes data_overflow fail to compile for
the original reason, updating RODATA to reflect the increased FLASH
size.

[SC2251](https://www.shellcheck.net/wiki/SC2251)
newAM added a commit to newAM/cortex-m that referenced this pull request Jun 30, 2024
Long story:

* This test was introduced
* `ci/script.sh` did not correctly check for failures, [SC2251]
  * Thanks rust-embedded#404 for the tip!
* The FLASH size was increased, and the test incorrectly passed, but
  nobody noticed.
* I modified the test in rust-embedded#505 which made it fail again, but for the
  wrong reason.  `ptr::read_volatile(ptr::addr_of!(RODATA))` reads the
  entire array, which is not equivalent to the original code
  `ptr::read_volatile(&RODATA as *const u8)` which read a single element
  of the array.
* The test now failed, but the stack related overflow takes rustc a LONG
  time to compile, and pushed our CI times to >30m.

These changes fix ci/scripts.sh to exit with a non-zero code if
data_overflow is passing, and makes data_overflow fail to compile for
the original reason, updating RODATA to reflect the increased FLASH
size.

[SC2251]: https://www.shellcheck.net/wiki/SC2251
@newAM newAM mentioned this pull request Jun 30, 2024
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.

2 participants