Skip to content

annotate(hypercall/lseek): make sure missing constants cause compile-time error - #1170

Merged
jounathaen merged 1 commit into
hermit-os:mainfrom
fogti:seek-harder
Nov 24, 2025
Merged

jounathaen merged 1 commit into
hermit-os:mainfrom
fogti:seek-harder

Conversation

@fogti

@fogti fogti commented Nov 19, 2025

Copy link
Copy Markdown
Contributor

Related: #1145 (comment)

In the Uhyve V2 Interface PR, the problem popped up that missing re-exports of SEEK_* constants only show up as warnings, and this gets fixed by this PR.

@fogti
fogti requested review from jounathaen and n0toose November 19, 2025 15:41

@mkroening mkroening left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Will this not error in CI anyway? Or is this about local development somehow? 🤔

@fogti

fogti commented Nov 19, 2025 •

Copy link
Copy Markdown
Contributor Author

It currently only produces a warning. Feel free to try.

@mkroening

mkroening commented Nov 19, 2025 •

Copy link
Copy Markdown
Member

It produces a warning locally, sure. That decision was intentional to not hinder local development by erroring on every warning. But in CI it should already be an error:

run: cargo clippy --all-targets ${{ matrix.flags }} -- -D warnings

@fogti

fogti commented Nov 19, 2025

Copy link
Copy Markdown
Contributor Author

The problem is that this tiny piece of code completely breaks on missing constants and relying on CI for that feels like a bad idea, as it runs a bit too late (i.e. causes unnecessary CI runs, which waste time and resources).

@mkroening

Copy link
Copy Markdown
Member

Okay so, yes, this is about local development. That answers my question. :D

@codecov

codecov Bot commented Nov 19, 2025 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.28%. Comparing base (f9de15f) to head (3f6898b).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1170      +/-   ##
==========================================
- Coverage   76.61%   76.28%   -0.34%     
==========================================
  Files          28       28              
  Lines        3605     3605              
==========================================
- Hits         2762     2750      -12     
- Misses        843      855      +12     

☔ 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.

Comment thread src/hypercall.rs
@fogti

fogti commented Nov 21, 2025

Copy link
Copy Markdown
Contributor Author

CI failure is unrelated (timeout).

@fogti
fogti requested a review from jounathaen November 21, 2025 10:49
@jounathaen

Copy link
Copy Markdown
Member

Thx!

@jounathaen
jounathaen added this pull request to the merge queue Nov 24, 2025
Merged via the queue into hermit-os:main with commit c63503d Nov 24, 2025
18 of 19 checks passed
@fogti
fogti deleted the seek-harder branch November 24, 2025 14:16
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.

4 participants