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

sys/syscalls: make gettimeofday() implementation optional #17733

Merged
merged 1 commit into from Mar 24, 2022
Merged

sys/syscalls: make gettimeofday() implementation optional #17733

merged 1 commit into from Mar 24, 2022

Conversation

fjmolinas
Copy link
Contributor

@fjmolinas fjmolinas commented Mar 2, 2022 •
edited

Contribution description

A follow-up to #17732 to not force 64bit on all syscalls users.

With this PR the users of ztimer64 should be limited to:

Revisited list of apps using ztimer64:

for app in examples/* tests/*; do make -C ${app} info-modules | if grep -q ztimer64; then echo ${app};fi;done

examples/ccn-lite-relay
examples/cord_ep
examples/cord_lc
examples/dtls-echo
examples/dtls-sock
examples/dtls-wolfssl
examples/gcoap_dtls
examples/gnrc_networking_mac
examples/lua_basic
examples/lua_REPL
examples/openthread
examples/posix_select
examples/wasm
tests/cpp11_condition_variable
tests/cpp11_mutex
tests/cpp11_thread
tests/driver_ltc4150
tests/driver_pir
tests/gnrc_gomach
tests/lua_loader
tests/nimble_autoconn_ccnl
tests/pkg_arduino_sdi_12
tests/pkg_flatbuffers
tests/pkg_tensorflow-lite
tests/pkg_tinydtls_sock_async
tests/pkg_wolfcrypt-ed25519-verify
tests/pkg_wolfssl
tests/posix_semaphore
tests/pthread
tests/pthread_barrier
tests/pthread_cleanup
tests/pthread_condition_variable
tests/pthread_cooperation
tests/pthread_flood
tests/pthread_rwlock
tests/pthread_tls
tests/saul_drivers
tests/sema
tests/sntp
tests/sys_fido2_ctap
tests/unittests
tests/ztimer64_msg

Testing procedure

Green Murdock, although should probably wait for #17721 or cherry-pick f7f7a93 to test for missing dependencies.

Issues/PRs references

Follow up to #17732
Related to #17721 and #17365

@fjmolinas fjmolinas added the State: waiting for other PR State: The PR requires another PR to be merged first label Mar 2, 2022
@fjmolinas fjmolinas requested a review from kfessel March 2, 2022 09:03
@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Area: Kconfig Area: Kconfig integration Area: network Area: Networking Area: sys Area: System Area: timers Area: timer subsystems Platform: native Platform: This PR/issue effects the native platform labels Mar 2, 2022
@fjmolinas fjmolinas added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: waiting for other PR State: The PR requires another PR to be merged first labels Mar 4, 2022
@github-actions github-actions bot added Area: CI Area: Continuous Integration of RIOT components Area: network Area: Networking and removed Area: network Area: Networking labels Mar 4, 2022
@github-actions github-actions bot added Area: doc Area: Documentation Area: tests Area: tests and testing framework and removed Area: CI Area: Continuous Integration of RIOT components labels Mar 4, 2022
@fjmolinas fjmolinas 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 Mar 4, 2022
@github-actions github-actions bot removed the Area: timers Area: timer subsystems label Mar 10, 2022
@fjmolinas fjmolinas removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 10, 2022
@fjmolinas
Copy link
Contributor Author

I think I'll wait for #17719 and then rebase on #17721.

@fjmolinas fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 21, 2022
@github-actions github-actions bot added Area: timers Area: timer subsystems and removed Area: doc Area: Documentation labels Mar 21, 2022
@benpicco benpicco 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 Mar 21, 2022
@fjmolinas fjmolinas force-pushed the pr_gettimeofday_64_bit branch 3 times, most recently from 417b260 to 9b7ed6f Compare March 22, 2022 17:04
@github-actions github-actions bot removed the Area: timers Area: timer subsystems label Mar 22, 2022
@fjmolinas
Copy link
Contributor Author

@kaspar030 @kfessel all green here, would be good to get this in to reduce ztimer64 users!

kfessel
kfessel reviewed Mar 23, 2022
View reviewed changes
Copy link
Contributor

@kfessel kfessel left a comment

Choose a reason for hiding this comment

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

I think the pseudo module could have a shorter name. libc_gettimeofday

cpu/native/Makefile.dep Outdated Show resolved Hide resolved
makefiles/pseudomodules.inc.mk Outdated Show resolved Hide resolved
@fjmolinas fjmolinas force-pushed the pr_gettimeofday_64_bit branch 2 times, most recently from 618c761 to c7fce7c Compare March 24, 2022 07:07
@fjmolinas
Copy link
Contributor Author

@kfessel changes addressed!

kfessel
kfessel reviewed Mar 24, 2022
View reviewed changes
Copy link
Contributor

@kfessel kfessel left a comment •
edited

Choose a reason for hiding this comment

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

two small nitpicks in Makefile.dep other than that this is good to go

image

u can skip the murdock

sys/Makefile.dep Outdated Show resolved Hide resolved
sys/Makefile.dep Outdated Show resolved Hide resolved
@fjmolinas
sys/syscalls: add libc_gettimeofday
0819660 
Conditionally implement gettimeofday() if module is included, this
avoids including ztimer64 even when not needed
@fjmolinas fjmolinas added the CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs label Mar 24, 2022
@kfessel kfessel changed the title sys/syscalls: empty gettimeofday() unless 64bit present sys/syscalls: make gettimeofday() implementation optional Mar 24, 2022
kfessel
kfessel approved these changes Mar 24, 2022
View reviewed changes
Copy link
Contributor

@kfessel kfessel left a comment •
edited

Choose a reason for hiding this comment

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

green murdock and good change

@fjmolinas fjmolinas merged commit b91b984 into RIOT-OS:master Mar 24, 2022
@OlegHahm OlegHahm added this to the Release 2022.04 milestone Apr 25, 2022
@benpicco
Copy link
Contributor

This breaks esp32 😢
RIOT-OS/Release-Specs#247 (comment)

@fjmolinas fjmolinas deleted the pr_gettimeofday_64_bit branch April 28, 2022 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kfessel kfessel kfessel approved these changes

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@jia200x jia200x Awaiting requested review from jia200x jia200x is a code owner

@cgundogan cgundogan Awaiting requested review from cgundogan

@miri64 miri64 Awaiting requested review from miri64

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann

@kaspar030 kaspar030 Awaiting requested review from kaspar030

@smlng smlng Awaiting requested review from smlng

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@MichelRottleuthner MichelRottleuthner Awaiting requested review from MichelRottleuthner MichelRottleuthner is a code owner

Assignees
No one assigned
Labels
Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: Kconfig Area: Kconfig integration Area: pkg Area: External package ports Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs Platform: native Platform: This PR/issue effects the native platform
Projects
None yet
Milestone
Release 2022.04
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@fjmolinas @kfessel @benpicco @OlegHahm