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

buildsystem: centralise package sourcing #3171

Merged

Conversation

@MilhouseVH
Copy link
Contributor

commented Dec 16, 2018

This is a clean up of the build system so that all package sourcing takes place using a single method, config/options <pkg-name>, removing the need to source naked packages for a second time in scripts/unpack, scripts/install and scripts/build.

Helper functions have been added to unset all package functions, and to source the package itself (with appropriate error handling).

The get_pkg_variable() function has been updated to call source_package() directly rather than via config/options, as the former is being called from within the context of the latter so it should not be necessary to incur the overhead of sourcing config/options each time a package variable is requested.

Where necessary, packages referencing PKG_* variables that are initialised after the package is sourced (eg. PKG_BUILD) have been updated to implement configure_package() where such variables will be available and valid.

scripts/uninstall is removed as it serves no current purpose - can be reinstated if required.

scripts/build, scripts/install and scripts/unpack have been reorganised to bail out early when called incorrectly, and remove redundant variables/code.

scripts/build will now execute setup_toolchain_$STARGET after the package has been sourced, and any packages that override or reference variables configured by setup_toolchain_$TARGET (eg. CC, CXX, CFLAGS, LDFLAGS, HOST_CC, HOST_CFLAGS etc.) must now override/reference these variables in a suitable pre-function, ie. pre_build_*, pre_configure_*, pre_make_* etc.

scripts/install will now output the CLR_INSTALL build message after dependencies have been installed, and not before.

@MilhouseVH MilhouseVH force-pushed the MilhouseVH:le90_buildsystem_source_packages-with-drop branch 4 times, most recently from 1aed783 to ac9f81d Dec 16, 2018

@@ -4,14 +4,14 @@
# Copyright (C) 2009-2016 Stephan Raue (stephan@openelec.tv)
# Copyright (C) 2018-present Team LibreELEC (https://libreelec.tv)

. config/options $1

if [ -z "$1" ]; then
die "usage: $0 package_name"

This comment has been minimized.

Copy link
@antonlacon

antonlacon Dec 17, 2018

Contributor

die will be unknown until config/functions gets sourced, so

echo -e .... >&2
exit 1

Same issue scripts/install and scripts/build.

This comment has been minimized.

Copy link
@MilhouseVH

MilhouseVH Dec 17, 2018

Author Contributor

ah yes... very true! will fix.

@MilhouseVH MilhouseVH force-pushed the MilhouseVH:le90_buildsystem_source_packages-with-drop branch from ac9f81d to 0554f4d Dec 17, 2018

@MilhouseVH

This comment has been minimized.

Copy link
Contributor Author

commented Dec 17, 2018

I've made a few additions after further testing - to simplify reviewing I've added 6 7 new commits which I'll squash eventually.

@MilhouseVH

This comment has been minimized.

Copy link
Contributor Author

commented Dec 17, 2018

The release change is required because the release script is being sourced here, and in the case of RPi and Amlogic (Rockchip is OK) it sources config/options $1 where $1 is whatever value has been used to execute scripts/image... this fails because we are validating the package name now, so when running make release you'd have the error:

        root (0)
Number of gids 2
        root (0)
        unknown (81)
Running /home/neil/projects/LibreELEC.tv/packages/tools/bcm2835-bootloader/release
FAILURE: release/package.mk does not exist
Makefile:9: recipe for target 'release' failed
make: *** [release] Error 1
mkdir -p $RELEASE_DIR/3rdparty/bootloader
cp -a $INSTALL/usr/share/bootloader/* $RELEASE_DIR/3rdparty/bootloader
cp -a $INSTALL/usr/share/bootloader/* $RELEASE_DIR/3rdparty/bootloader

@MilhouseVH MilhouseVH force-pushed the MilhouseVH:le90_buildsystem_source_packages-with-drop branch from 02b5f84 to ab54015 Dec 17, 2018

MilhouseVH added 12 commits Dec 18, 2018
packages: implement late variable binding
Move variable assignments into configure_package() if the assignments
depend on variables initialised after the package is sourced, ie.
$PKG_BUILD, $PKG_SOURCE_NAME etc.
packages: setup_toolchain $TARGET only called after package is sourced
Packages referencing variables defined in setup_toolchain such as CC, CXX,
AR, CFLAGS, LDFLAGS, HOST_CC etc. etc. must only reference these variables in
pre_build()/pre_configure()/pre_make() etc. functions, as the variables will not
be available when the package is sourced, but will be available after the call
to setup_toolchain() from scripts/build.

@MilhouseVH MilhouseVH force-pushed the MilhouseVH:le90_buildsystem_source_packages-with-drop branch from ab54015 to 094b499 Dec 18, 2018

@MilhouseVH

This comment has been minimized.

Copy link
Contributor Author

commented Dec 18, 2018

Squashed, and cleaned up. I've now completed a dozen clean builds (RPi/RPi2/Generic) without issue, and also built tvheadend42. Hopefully there won't be further changes... I'll kick off some Amlogic/Rockchip/WeTek builds overnight.

@MilhouseVH

This comment has been minimized.

Copy link
Contributor Author

commented Dec 18, 2018

LePotato, KVIM, S905, MiQi, Tinkerboard, WeTek_Core and WeTek_Hub builds all successful.

packages: set custom PKG_[CONFIGURE|CMAKE|MESON]_SCRIPT in configure_…
…package()

When setting a custom build script (which must be relative to $PKG_BUILD), the
value should be set in configure_package() so that $PKG_BUILD is known, and also
because the path to the script will be validated prior to calling
pre_configure_$TARGET(), so the script variable must be configured once the pacakge
is sourced.
config/functions Outdated Show resolved Hide resolved
@antonlacon

This comment has been minimized.

Copy link
Contributor

commented Dec 22, 2018

Config/path needs a copyright header. Looks good to me after that and the config/functions quibble.

@MilhouseVH MilhouseVH force-pushed the MilhouseVH:le90_buildsystem_source_packages-with-drop branch from 285f9d2 to 12db14f Dec 22, 2018

@MilhouseVH

This comment has been minimized.

Copy link
Contributor Author

commented Dec 22, 2018

Thanks @antonlacon - copyright and quibble addressed.

@MilhouseVH

This comment has been minimized.

Copy link
Contributor Author

commented Dec 25, 2018

Added fixes for a few more packages that reference variables initialised by setup_toolchain $TARGET.

@MilhouseVH MilhouseVH force-pushed the MilhouseVH:le90_buildsystem_source_packages-with-drop branch from f13e89d to a805d85 Dec 27, 2018

@MilhouseVH MilhouseVH referenced this pull request Dec 27, 2018

@MilhouseVH MilhouseVH force-pushed the MilhouseVH:le90_buildsystem_source_packages-with-drop branch from a805d85 to 2163386 Dec 28, 2018

@MilhouseVH MilhouseVH force-pushed the MilhouseVH:le90_buildsystem_source_packages-with-drop branch from 2163386 to 710d431 Dec 28, 2018

@CvH
CvH approved these changes Dec 29, 2018

@CvH CvH merged commit a98586b into LibreELEC:master Dec 29, 2018

5schatten added a commit to 5schatten/LibreELEC-RR that referenced this pull request Dec 30, 2018
5schatten added a commit to 5schatten/LibreELEC-RR that referenced this pull request Dec 30, 2018
5schatten added a commit to 5schatten/LibreELEC-RR that referenced this pull request Dec 30, 2018
5schatten added a commit to 5schatten/LibreELEC-RR that referenced this pull request Dec 30, 2018
5schatten added a commit to 5schatten/LibreELEC-RR that referenced this pull request Dec 30, 2018
5schatten added a commit to 5schatten/LibreELEC-RR that referenced this pull request Dec 30, 2018
MilhouseVH added a commit to MilhouseVH/LibreELEC.tv that referenced this pull request Feb 14, 2019
MilhouseVH added a commit to MilhouseVH/LibreELEC.tv that referenced this pull request Feb 15, 2019
CvH added a commit that referenced this pull request Feb 17, 2019
Merge pull request #3312 from MilhouseVH/le92_fix_debug
buildsystem: fix debug broken by #3171
CvH added a commit that referenced this pull request Feb 17, 2019
Merge pull request #3313 from MilhouseVH/le90_fix_debug
buildsystem: fix debug broken by #3171 [backport]
thoradia pushed a commit to thoradia/LibreELEC.tv that referenced this pull request Feb 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.