Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork15.2k
Eliminate build warnings from the main project#13386
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…e generally, [allow(unused)], as static analysis apparently doesn't see the call sites.
…bbs_http/sync.rs to disable warning about it being deprecated.
rustdesk commentedNov 2, 2025
fix ci please. |
logiclrd commentedNov 2, 2025
Huh. There's some code thatdoes require it to be mutable, but that code is only enabled for the |
…d on whether the code that requires it to be mutable is enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
This PR addresses Rust compiler warnings by adding#[allow(...)] attributes to suppress false-positive dead code warnings for code that is actually used via FFI or in conditional compilation contexts. It also handles a deprecated base64 API warning and fixes a mutability issue with an event loop variable.
- Suppresses dead code warnings for FFI-accessible functions and types
- Handles deprecated
base64::encodeAPI with allow attribute - Corrects event loop mutability for non-macOS builds
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/updater.rs | Added#[allow(dead_code)] for session count tracking used via conditional compilation |
| src/ui_session_interface.rs | Added#[allow(unused_imports)] for conditionally used imports |
| src/ui_interface.rs | Added module-level#[allow(dead_code)] for FFI-accessible interface |
| src/ui.rs | Added#[allow(dead_code)] for HTTP request methods called from FFI |
| src/tray.rs | Fixed event loop mutability by making it non-mutable on non-macOS platforms |
| src/service.rs | Removed unused imports |
| src/server/input_service.rs | Added#[allow(unused)] for conditionally compiled code |
| src/hbbs_http/sync.rs | Scoped#[allow(deprecated)] for base64 API usage |
| src/hbbs_http/downloader.rs | Added module-level#[allow(dead_code)] for FFI-accessible downloader |
| src/hbbs_http.rs | Added#[allow(dead_code)] for HTTP response enum |
| src/client/file_trait.rs | Added#[allow(dead_code)] for trait used in flutter_ffi |
| src/client.rs | Added#[allow(dead_code)] for peer_online module |
| src/auth_2fa.rs | Added#[allow(dead_code)] for 2FA implementation and methods |
💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.
src/hbbs_http/sync.rs Outdated
| { | ||
| #![allow(deprecated)] | ||
| hash = hbb_common::base64::encode(&res[..]); | ||
| } |
CopilotAINov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The#![allow(deprecated)] inner attribute applies to the entire block scope. Consider using an outer attribute#[allow(deprecated)] on the statement instead for more precise scoping. Change to:#[allow(deprecated)] hash = hbb_common::base64::encode(&res[..]); and remove the block braces.
| { | |
| #![allow(deprecated)] | |
| hash = hbb_common::base64::encode(&res[..]); | |
| } | |
| #[allow(deprecated)] | |
| hash = hbb_common::base64::encode(&res[..]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is really 6 of one, half a dozen of the other 😛 I deliberately made a single-statement block to set it out, because I feel like allowing deprecated code isusually something that should be avoided, and where it's not avoided, it should be noticed. The intent needs to be clear. Would you prefer that this be written as an outer attribute instead, though? The style preference for the code-base carries the most weight here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
should change to encode64 in src/common.rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
can ask chatgpt cl (or cursor ci, cc, codex etc) to do this, it will be quite quick.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Ah, nice, centralization of the override. 🙂
Uh oh!
There was an error while loading.Please reload this page.
rustdesk commentedNov 6, 2025
Could you fix the CI? |
logiclrd commentedNov 6, 2025
There are some OS X-specific warnings that I have no idea what to do with: |
logiclrd commentedNov 6, 2025
Also, my initial changes and observation of no more build warnings was on a Windows machine. I'm at my Linux machine now, and see a number of warnings that don't appear on Windows presumably because of conditional compilation; those bits are entirely excluded on Windows. Checking into these... |
- Marked MouseButton and ScrollDirection in server/uinput.rs as allow(unused).- Removed the resolution field from RdpInputMouse, as nothing actually ever reads from it. (The constructor still takes it, as it derives instance data from its value.)- Updated server/display_service.rs to only generate the is_capturer_mag_supported function and the IS_CAPTURER_MAGNIFIER_SUPPORTED lazy static on the "windows" configuration.- Updated server/connection.rs to not import crate::clipboard_file::* for target_os "linux".- Updated the conditional attribute of enable_file_transfer in server/connection.rs to match the condition applied to its use (target_os "windows" | feature "unix-file-copy-paste").- Updated align_to_32 in server/audio_service.rs to not assign a value to buf that it then immediately overwrites.- Removed unnecessary assignment to local "conn" in a lambda within sync_and_watch_config_dir in server.rs.- Moved the is_opensuse() function in platform/linux.rs into the comment block wrapping function elevate, which is its only consumer.- Updated platform/linux.rs to use whoami::fallible::hostname instead of the deprecated whoami::hostname.- Updated platform/linux.rs to mark the AwakeHandle inside WakeLock with [allow(unused)], since nothing explicitly reads it; its purpose is simply its presence.- Updated flutter_ffi.rs to conditionally import PathBuf for the platforms that have code explicitly using it, and to not import time::Duration at all. I think it is unnecessary??
…get_os "windows" and "macos".
…uggested but was wrong anyway, with increasing the scope of the import of crate::clipboard_file::* to include feature "unix-file-copy-paste".
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/server/uinput.rs:773
- The module name 'mouce' appears to be a typo. It should be 'mouse' to match the actual functionality (mouse input handling). The comment on line 772 references 'https://github.com/emrebicer/mouce' which suggests this might be intentionally named after that library, but this is still confusing and should be clarified or corrected.
mod mouce {💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
rustdesk commentedNov 7, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
These are useful warnings, should not be removed. |
…e::clipboard_file::* in connection.rs. This might break the OS X build, in which case I will undo this change.
… fallible::hostname() in platform/linux.rs, because the value the functor was yielding was the default anyway.
Uh oh!
There was an error while loading.Please reload this page.
… in sync_and_watch_config_dir in server.rs, now that it is no longer unused because ofce7d794.
This PR makes minor adjustments so that
rustdeskbuilds without warnings.Before:
After:
In this PR, each type of warning is addressed in a separate commit.