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

addons: fix building target images with openwrt imagebuilder #205

Conversation

PolynomialDivision
Copy link
Contributor

Check if we are on "real hardware" or compiling an image with imagebuilder. If we are using imagebuilder the /lib/functions.sh is not existing.

This should fix freifunk-berlin/firmware#810.

Copy link
Contributor

@SvenRoederer SvenRoederer left a comment

Choose a reason for hiding this comment

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

beside my comments on $IPKG_INSTROOT doing it your way, will stop the code to be executed when building with imagebuilder. But when will this script be executed on the real hardware? I think with this change it will prevent execution at all in the imagebuilder and on real hardware. Which will not enable the plugin in the final consequence.

EDIT: https://github.com/freifunk/openwrt-packages/pull/22/files shows a similar usecase.

@@ -1,5 +1,10 @@
#!/bin/sh

# check if we are on real hardware
if [ ! -f "/lib/functions.sh" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

The usual way is to check for $IPKG_INSTROOT (e.g.https://git.openwrt.org/?p=feed/packages.git;a=blob;f=admin/syslog-ng/Makefile;hb=453c5a9b36523ceb6b715a841f0c852b5994992c#l95). So we should stick to it also.

@@ -8,6 +8,11 @@
#
# All other config sections are overwritten with current settings

# check if we are on real hardware
if [ ! -f "/lib/functions.sh" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

The usual way is to check for $IPKG_INSTROOT (e.g.https://git.openwrt.org/?p=feed/packages.git;a=blob;f=admin/syslog-ng/Makefile;hb=453c5a9b36523ceb6b715a841f0c852b5994992c#l95). So we should stick to it also.

@PolynomialDivision
Copy link
Contributor Author

PolynomialDivision commented Jun 20, 2020

beside my comments on $IPKG_INSTROOT doing it your way, will stop the code to be executed when building with imagebuilder. But when will this script be executed on the real hardware? I think with this change it will prevent execution at all in the imagebuilder and on real hardware. Which will not enable the plugin in the final consequence.

EDIT: https://github.com/freifunk/openwrt-packages/pull/22/files shows a similar usecase.

When are this post-install scripts executed when using the freifunk firmware image builder? I thought this scripts are just for opkg?

Further you mean we should do something like:

define Package/freifunk-berlin-bbbdigger/postinst
[ -n "$$IPKG_INSTROOT" ] || {
#!/bin/sh
$${IPKG_INSTROOT}/tmp/freifunk-berlin-bbbdigger_postinst.sh
}
endef

Is that the correct bash syntax?

Check for $IPKG_INSTROOT to figure out if package is installed on real
hardware. If that is the case the installer should not run the postinst
script.

Signed-off-by: Nick Hainke <vincent@systemli.org>
Copy link
Member

@Akira25 Akira25 left a comment

Choose a reason for hiding this comment

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

This builds without any issues. I followed that method here.

@SvenRoederer
Copy link
Contributor

When are this post-install scripts executed when using the freifunk firmware image builder? I thought this scripts are just for opkg?

As running the imagebuilder fails when running this postinst directive, it's obvious that it runs these scripts. When doing a runtime-installation via opkg, the same scripts are triggered in its postinst-hook.

@Akira25 Removing the postinst, as by this test-conditon of this PR, it will fix the imagebuilder run, but I don't see a way this postinst-script will be called at all. So the setup / configuration will not happen and must be done manually be the user.

@PolynomialDivision
Copy link
Contributor Author

I made this PR without knowing that the package is only used to be installed via opkg. I'm not sure what the best solution is, but it should not break the installation via opkg? Or is this wrong syntax?

@SvenRoederer
Copy link
Contributor

But the package will not be correctly configured when using it in the imagebuilder (currently it fails "verbose", after the change it fails "silently"). As it stays broken either way, I prefer "failing verbose" than having the user to dig around why the setup is failing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

collectd-dnsmasq: compiling/installing fails on latest openwrt master
3 participants