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.

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

Stop using random-numgen, not needed in recent OpenWrt #1117

Merged
merged 3 commits into from
Aug 1, 2024

Conversation

ilario
Copy link
Member

@ilario ilario commented Jun 30, 2024

The random-numgen package was created in #991 as in OpenWrt 21 BusyBox did not have $RANDOM enabled by default (so it was not present in the official ImageBuilder), see #800.

But since OpenWrt 22 it is present by default, see openwrt/packages#20728 so there is no need to use it.

Maybe we can keep the random-numgen package for a few more years and then delete it.

This commit should be picked also in the 2024.1 branch.

Fix #1075

@@ -6,5 +6,5 @@ unique_append()
}

unique_append \
'*/20 * * * * ((sleep $(($(random-numgen) % 600)); check-date-http &> /dev/null)&)'\
'*/20 * * * * ((sleep $(($RANDOM % 600)); check-date-http &> /dev/null)&)'\
Copy link
Member

Choose a reason for hiding this comment

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

@ilario are you sure the $ is needed before RANDOM in this context? AFAIR inside $(()) arithmetic evaluation the plain variable name without $ should work, or it is like that only in bash ?

Copy link
Member

Choose a reason for hiding this comment

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

same question goes for all other similar occurrencies

Copy link
Member Author

Choose a reason for hiding this comment

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

Just checked on a virtualized image of LibreMesh 2024.1-rc1 and you remember right, it works also without the $.
Removing it is not going to make the code any better, but ok I am going to remove it from everywhere.

Copy link
Member

Choose a reason for hiding this comment

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

well less non useful chars to read makes it more readable IMHO, plus it uses a bit less bytes ;-)

@ilario
Copy link
Member Author

ilario commented Jul 5, 2024

@G10h4ck pls check if the changes are ok

@G10h4ck G10h4ck merged commit a3d751c into libremesh:master Aug 1, 2024
1 check passed
@G10h4ck
Copy link
Member

G10h4ck commented Aug 1, 2024

Great. Thanks!

@ilario
Copy link
Member Author

ilario commented Aug 1, 2024

Picked to branch 2024.1 https://github.com/libremesh/lime-packages/tree/2024.1

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.

lime-app still uses $RANDOM in crontab
2 participants