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

Created utils/random-numgen package, an alternative to $RANDOM #20728

Closed
wants to merge 1 commit into from

Conversation

ilario
Copy link

@ilario ilario commented Mar 26, 2023

Maintainer: me / @ilario
Compile tested: OpenWrt 22.03 and OpenWrt 19.07, YouHua WR1200JS
Run tested: OpenWrt 22.03 and OpenWrt 19.07, YouHua WR1200JS

Description:
By default, the $RANDOM shell variable is not available as Busybox does not have the BUSYBOX_CONFIG_ASH_RANDOM_SUPPORT option activated.
This command gives an equivalent output, an integer from 0 to 65535.
Originally, we decided we needed such a command in LibreMesh to replace the usage of $RANDOM, as discussed here libremesh/lime-packages#800 and here libremesh/lime-packages#980.

I would be nice to have this also in the openwrt-22.03 branch.

@ilario
Copy link
Author

ilario commented Mar 27, 2023

Just saw the "Formalities" test failing, I am going to fix it ASAP

Copy link
Member

@BKPepe BKPepe left a comment

Choose a reason for hiding this comment

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

I am not sure... You should rather active BUSYBOX_CONFIG_ASH_RANDOM_SUPPORT in your build instead of adding your own custom package for it.

And no, this will not be backported to OpenWrt 22.03.

PKG_NAME:=random-numgen
PKG_VERSION:=0.1
PKG_RELEASE:=1
PKG_LICENSE:=GPL-2.0
Copy link
Member

Choose a reason for hiding this comment

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

Outdated LICENSE SPDX License Identifier.


include $(INCLUDE_DIR)/package.mk

define Package/$(PKG_NAME)
Copy link
Member

Choose a reason for hiding this comment

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

Why you are trying to add something here back, when it was removed?

Suggested change
define Package/$(PKG_NAME)
define Package/random-numgen

SECTION:=utils
CATEGORY:=Utilities
TITLE:=Generates a random number 0-65535
MAINTAINER:=Ilario Gelmetti <ilario@sindominio.net>
Copy link
Member

Choose a reason for hiding this comment

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

Should be elsewhere, please look how Makefiles are done in this repository. Also, this section should have 2 spaces as indentation not tab.


define Package/$(PKG_NAME)/install
$(INSTALL_DIR) $(1)/
$(CP) ./files/* $(1)/
Copy link
Member

Choose a reason for hiding this comment

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

  • Use INSTALL_BIN
  • Move only specific files not everything

@1715173329
Copy link
Member

since this is a very simple wrapper, imo it should go to somewhere like package/base-files/files/lib/functions.sh

@ilario
Copy link
Author

ilario commented Mar 30, 2023

Thanks @BKPepe for the corrections!
Is it better now?
Following @1715173329's suggestion, I opened an alternative pull request here: openwrt/openwrt#12291

You should rather active BUSYBOX_CONFIG_ASH_RANDOM_SUPPORT in your build instead of adding your own custom package for it.

That is what we were doing in LibreMesh, but that implies that we are not compatible with pristine OpenWrt's ImageBuilder.

And no, this will not be backported to OpenWrt 22.03.

Ok

@ilario
Copy link
Author

ilario commented Mar 30, 2023

@aparcar realized here that the range of random-numgen (0-65535) is different from the range of $RANDOM (0-32767).
I did not thought that before.
So before merging I have either to document this or to have random-numgen to output in the same range.

@champtar
Copy link
Member

@ilario what is the size increase of enabling BUSYBOX_CONFIG_ASH_RANDOM_SUPPORT ?

Signed-off-by: Ilario Gelmetti <iochesonome@gmail.com>
@ilario
Copy link
Author

ilario commented Mar 30, 2023

@ilario what is the size increase of enabling BUSYBOX_CONFIG_ASH_RANDOM_SUPPORT ?

The same was asked by @aparcar here: openwrt/openwrt#12291 (comment) :)

Is there a proper way to measure that?

I compiled and then run binwalk on the -sysupgrade.bin image.
The result is that the third part of the file is a Squashfs filesystem with a size of:

  • 3453220 bytes for clean OpenWrt (compiled for YouHua WR1200JS)
  • 3453332 bytes when selecting BUSYBOX_CUSTOM
  • 3453408 bytes when selecting also BUSYBOX_CONFIG_ASH_RANDOM_SUPPORT

So in total, it increases by only 188 bytes, if I measured it correctly.

@ilario
Copy link
Author

ilario commented Apr 26, 2023

Closing as fixed in openwrt/openwrt#12465 and openwrt/openwrt@8f427f1

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.

4 participants