Weed out warnings#4939
Draft
ricab wants to merge 15 commits into
Draft
Conversation
The rpc target already gets -Wno-pedantic from gRPC (declared with INTERFACE).
Link `rpc` against gRPC with PRIVATE scope, to avoid propagating compilation options which are specific to that module. Warning strictness may need to be reduced for external code, but that should not affect our own code. This breaks the build until the uncovered warnings get fixed.
Fix a few unused parameters, along with some redundant namespace qualifications.
Tweak clang-format's penalty to break on assignment, to prefer
```
auto a = foo(aaaaaaaaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbb,
ccccccccccccccccccc,
ddddddddddddddddddd);
```
to
```
auto a =
foo(aaaaaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbbbbbb, cccccccccccccc...
```
This is obsolete now, as there is only one type left.
Replace lambda with gmock action, removing another couple of unused-parameter cases.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens warning hygiene by avoiding propagation of permissive warning flags from third-party dependencies, then resolving newly surfaced warnings across production and unit-test code (mostly unused parameters), plus a small style/tooling adjustment.
Changes:
- Remove the now-redundant
FetchTypeplumbing from the VM image vault / factory APIs and update call sites and tests accordingly. - Fix uncovered warnings (primarily unused parameters) across daemon/client/platform/network code and extensive unit-test scaffolding.
- Adjust RPC proto naming and build/format configuration related to warning handling and formatting stability.
Reviewed changes
Copilot reviewed 72 out of 72 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/tracking_url_downloader.h | Silence unused-parameter warnings in test downloader stub. |
| tests/unit/test_url_downloader.cpp | Rename local variable to match “progress” semantics. |
| tests/unit/test_ubuntu_image_host.cpp | Mark unused lambda parameter. |
| tests/unit/test_sshfsmount.cpp | Remove unused lambda parameter names. |
| tests/unit/test_ssh_process.cpp | Remove unused lambda parameter names. |
| tests/unit/test_sftpserver.cpp | Remove unused lambda parameter names; minor cleanup. |
| tests/unit/test_sftp_client.cpp | Remove unused lambda parameter names in test fakes. |
| tests/unit/test_qemu_img_utils.cpp | Mark unused test helper parameter. |
| tests/unit/test_mount_handler.cpp | Remove unused override parameter names. |
| tests/unit/test_image_vault.cpp | Update tests for fetch_image/update_images signature changes; unused param cleanups. |
| tests/unit/test_exception.cpp | Remove unused formatter parameters. |
| tests/unit/test_daemon.cpp | Update mock expectations for fetch_image signature change. |
| tests/unit/test_daemon_find.cpp | Mark unused lambda parameter. |
| tests/unit/test_custom_image_host.cpp | Mark unused lambda parameter. |
| tests/unit/test_cloud_init_iso.cpp | Mark unused lambda parameter in mock reader. |
| tests/unit/test_cli_client.cpp | Remove unused grpc context / raw cmd parameter names in test hooks. |
| tests/unit/test_base_virtual_machine.cpp | Mark unused params; simplify lambdas to avoid unused captures. |
| tests/unit/test_base_virtual_machine_factory.cpp | Remove test for removed FetchType API. |
| tests/unit/stub_vm_image_vault.h | Update stub vault API; remove FetchType; unused param cleanups. |
| tests/unit/stub_virtual_machine.h | Mark unused parameters in stub VM overrides. |
| tests/unit/stub_virtual_machine_factory.h | Remove fetch_type() override; unused param cleanups. |
| tests/unit/stub_url_downloader.h | Remove unused parameter names from stub downloader. |
| tests/unit/stub_terminal.h | Remove unused parameter names from terminal stub. |
| tests/unit/stub_status_monitor.h | Expand empty overrides; mark unused params; remove stray semicolon. |
| tests/unit/stub_process_factory.cpp | Remove unused parameter names in stub process implementation. |
| tests/unit/stub_image_host.h | Mark unused parameters in stub image host. |
| tests/unit/stub_cert_store.h | Mark unused parameters in stub cert store. |
| tests/unit/stub_availability_zone.h | Mark unused parameters in availability zone stub. |
| tests/unit/stub_availability_zone_manager.h | Mark unused parameter in manager stub. |
| tests/unit/sftp_server_test_fixture.h | Remove unused lambda parameter names. |
| tests/unit/qemu/linux/test_qemu_platform_linux.cpp | Capture tap opts via gmock action rather than mutable local; unused var cleanup. |
| tests/unit/qemu/linux/test_dnsmasq_server.cpp | Mark unused logger parameter. |
| tests/unit/mock_vm_image_vault.h | Update mock signatures for vault API changes. |
| tests/unit/mock_virtual_machine_factory.h | Remove fetch_type() mock method. |
| tests/unit/mischievous_url_downloader.h | Rename param to progress_type for consistency. |
| tests/unit/mischievous_url_downloader.cpp | Propagate param rename through wrapper call. |
| tests/unit/daemon_test_fixture.cpp | Remove default fetch_type() behavior; unused param cleanup. |
| src/sshfs_mount/sftp_server.cpp | Remove unused SSHSession& from check_sshfs_status. |
| src/rpc/multipass.proto | Rename LaunchProgress enum type to singular form. |
| src/rpc/CMakeLists.txt | Stop applying global compile options in rpc subdir; adjust rpc↔gRPC linkage visibility. |
| src/platform/platform_unix.cpp | Mark unused is_dir parameter. |
| src/platform/platform_linux.cpp | Remove redundant qualification; mark unused alias definition parameter. |
| src/platform/backends/virtualbox/virtualbox_virtual_machine_factory.cpp | Mark unused networking parameter. |
| src/platform/backends/shared/base_virtual_machine.h | Remove unused parameter names in default throw implementations. |
| src/platform/backends/shared/base_virtual_machine_factory.h | Remove fetch_type() implementation; mark unused parameters in NotImplemented stubs. |
| src/network/url_downloader.cpp | Rename download type parameter to progress_type and propagate into monitor calls. |
| src/image_host/ubuntu_image_host.cpp | Remove redundant qualification; mark unused parameter. |
| src/image_host/custom_image_host.cpp | Mark unused allow_unsupported parameter. |
| src/daemon/default_vm_image_vault.h | Remove FetchType from vault interface; adjust helper signature. |
| src/daemon/default_vm_image_vault.cpp | Remove FetchType from implementation and internal call sites. |
| src/daemon/daemon.cpp | Remove fetch_type() usage; adjust helper signatures; update enum name usage. |
| src/daemon/daemon_rpc.cpp | Mark unused request/server parameters. |
| src/client/cli/cmd/wait_ready.cpp | Mark unused reply parameter. |
| src/client/cli/cmd/version.cpp | Mark unused status parameter. |
| src/client/cli/cmd/umount.cpp | Simplify lambda types; mark unused reply parameter. |
| src/client/cli/cmd/suspend.cpp | Mark unused reply parameter. |
| src/client/cli/cmd/stop.cpp | Mark unused reply parameter. |
| src/client/cli/cmd/restore.cpp | Mark unused reply parameter. |
| src/client/cli/cmd/recover.cpp | Mark unused reply parameter. |
| src/client/cli/cmd/mount.cpp | Mark unused reply parameter. |
| src/client/cli/cmd/launch.cpp | Update enum constant names for renamed proto enum; unused param cleanup. |
| src/client/cli/cmd/authenticate.cpp | Reduce redundant qualification; mark unused reply parameter. |
| include/multipass/xz_image_decoder.h | Add missing <filesystem> include. |
| include/multipass/vm_image_vault.h | Remove FetchType from public interface. |
| include/multipass/virtual_machine_factory.h | Remove FetchType dependency and fetch_type() from factory interface. |
| include/multipass/url_downloader.h | Rename parameter to progress_type for clarity/consistency. |
| include/multipass/progress_monitor.h | Rename parameter to progress_type (type name only). |
| include/multipass/fetch_type.h | Delete redundant enum (no longer used). |
| include/multipass/cli/command.h | Remove unused parameter names; reduce redundant namespace qualifications. |
| CMakeLists.txt | Remove -Wno-expansion-to-defined suppression probe/flag. |
| 3rd-party/CMakeLists.txt | Reformat gRPC warning-suppression options for readability. |
| .clang-format | Increase PenaltyBreakAssignment to reduce formatting churn. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # tooling won't complain. | ||
| target_include_directories(rpc SYSTEM PUBLIC ${GRPC_GENERATED_SOURCE_DIR}) | ||
| target_link_libraries(rpc gRPC) | ||
| target_link_libraries(rpc PRIVATE gRPC) |
Comment on lines
95
to
103
| message LaunchProgress { | ||
| enum ProgressTypes { | ||
| enum ProgressType { | ||
| IMAGE = 0; | ||
| EXTRACT = 1; | ||
| VERIFY = 2; | ||
| WAITING = 3; | ||
| } | ||
| ProgressTypes type = 1; | ||
| ProgressType type = 1; | ||
| string percent_complete = 2; |
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Stop propagating lax warning settings for 3rd-party compilation. Then, fix uncovered warnings. They were mostly unused params, fixed by
This is open for tweaks, of course, so suggestions welcome. One case of type 3 cascaded to the removal of the
FetchType. And I ended up not using[[maybe_unused]]because I found it both more verbose and less informative (a param that is maybe unused may still be used).Also adjusted a format penalty to split multi-arg calls and preserve existing style, minimizing the diff. And did a few other drive-by fixes (param renames, redundant namespace qualifications). See commit messages for further details.
Related Issue(s)
Closes #4980.
MULTI-2703
Checklist