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.

Sign up for GitHub

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

Jump to bottom

shell_cmd_gnrc_udp: add missing netutils dependency #18798

Merged
merged 1 commit into from
Oct 25, 2022
Merged

shell_cmd_gnrc_udp: add missing netutils dependency #18798

merged 1 commit into from
Oct 25, 2022

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Oct 25, 2022

Contribution description

The gnrc_udp shell command uses the function netutils_get_ipv6() but does not include the corresponding module netutils. The only reason most applications that use shell_cmd_gnrc_udp link is because they also include the shell_cmd_gnrc_icmpv6_echo module (e.g. implicit via gnrc_ipcmpv6_echo), which includes this dependency.

Testing procedure

Remove the gnrc_icmpv6_echo module from examples/gnrc_networking:

diff --git a/examples/gnrc_networking/Makefile b/examples/gnrc_networking/Makefile
index df2cb0ddcc..e2f766d264 100644
--- a/examples/gnrc_networking/Makefile
+++ b/examples/gnrc_networking/Makefile
@@ -21,3 +21,2 @@ USEMODULE += auto_init_gnrc_rpl
 # Additional networking modules that can be dropped if not needed
-USEMODULE += gnrc_icmpv6_echo
 USEMODULE += shell_cmd_gnrc_udp

Now try to build this application for a platform of your choice. On current master, it will fail to link:

/usr/bin/ld: examples/gnrc_networking/bin/native/shell_cmds/gnrc_udp.o: in function `_send':
sys/shell/cmds/gnrc_udp.c:54: undefined reference to `netutils_get_ipv6'
collect2: error: ld returned 1 exit status

with this PR, it will link again.

Issues/PRs references

Noticed while trying to port the test of #16158 to using shell_cmd_gnrc_udp.

@miri64
shell_cmd_gnrc_udp: add missing netutils dependency
149aadc 
The `gnrc_udp` shell command uses the function `netutils_get_ipv6()` but
does not include the corresponding module `netutils`. The only reason
most applications that use `shell_cmd_gnrc_udp` link is because they
also include the `shell_cmd_gnrc_icmpv6_echo` module (e.g. implicit via
`gnrc_ipcmpv6_echo`), which includes this dependency.
@miri64 miri64 added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label Oct 25, 2022
@miri64 miri64 requested review from benpicco and maribu October 25, 2022 10:42
@github-actions github-actions bot added the Area: sys Area: System label Oct 25, 2022
@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 25, 2022
@benpicco benpicco added the Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer label Oct 25, 2022
@miri64 miri64 mentioned this pull request Oct 25, 2022
Merged
@riot-ci
Copy link

riot-ci commented Oct 25, 2022 •
edited
Loading

Murdock results

✔️ PASSED

149aadc shell_cmd_gnrc_udp: add missing netutils dependency

Success Failures Total Runtime
1991 0 1991 06m:48s

Artifacts

This only reflects a subset of all builds from https://ci-prod.riot-os.org. Please refer to https://ci.riot-os.org for a complete build for now.

This was referenced Oct 25, 2022
Merged
Merged
Merged
@maribu maribu enabled auto-merge October 25, 2022 11:41
@maribu maribu added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Oct 25, 2022
@kaspar030 kaspar030 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 25, 2022
@maribu maribu merged commit 1cf6d79 into RIOT-OS:master Oct 25, 2022
@miri64 miri64 deleted the shell_cmd_gnrc_udp/fix/missing-dep branch October 25, 2022 15:42
@kaspar030 kaspar030 added this to the Release 2023.01 milestone Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benpicco benpicco benpicco approved these changes

@maribu maribu Awaiting requested review from maribu

Assignees
No one assigned
Labels
Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
Release 2023.01
Development

Successfully merging this pull request may close these issues.
5 participants
@miri64 @riot-ci @benpicco @maribu @kaspar030