Clover General discussion

apianti · May 14, 2017

The Build_Clover.command script has nothing to do with the compiler flags/settings (except choosing the build toolkit: GCCxx or XCODE5/8). Besides, Build_Clover.command only passes options, that are already present in ebuild.sh, based on the user input (selection).

Nah, I know what it does. I didn't mean there was a bug in the script, I meant that it was a problem with using the script.

After some more digging, I see that this is actually because of CLANG38 and GCC44 compilation that is included in the tools_def.txt(.template). So the reason appears to be the tool definitions. I'm not sure why it's quoted... or only used for these two versions?

DEFINE GCC44_X64_CC_FLAGS            = DEF(GCC44_ALL_CC_FLAGS) -m64 -fno-stack-protector "-DEFIAPI=__attribute__((ms_abi))" -maccumulate-outgoing-args -mno-red-zone -Wno-address -mcmodel=small -fpie -fno-asynchronous-unwind-tables

DEBUG_CLANG38_X64_CC_FLAGS         = DEF(CLANG38_ALL_CC_FLAGS) -m64 "-DEFIAPI=__attribute__((ms_abi))" -mno-red-zone -mcmodel=small -fpie -Oz -flto DEF(CLANG38_X64_TARGET) -g
RELEASE_CLANG38_X64_CC_FLAGS       = DEF(CLANG38_ALL_CC_FLAGS) -m64 "-DEFIAPI=__attribute__((ms_abi))" -mno-red-zone -mcmodel=small -fpie -Oz -flto DEF(CLANG38_X64_TARGET)

EDIT: These should be escaped differently, like

-D'EFIAPI=__attribute__((ms_abi))'

EDIT2: Or maybe not at all? I think that the EDK2 build system might automatically escape parentheses with a backslash...

gills83 · May 14, 2017

You enabled Boot/Debug and there was no debug.log? Or you just assume?

No, there was nothing, I will follow your advice to get better information.

thank you

gills83 · May 14, 2017

You enabled Boot/Debug and there was no debug.log? Or you just assume?

No, there was nothing, I will follow your advice to get better information.

thank you

@apianti,

no files debug.log

thanks

"Debug

<key>Debug</key><false/>

Formerly known as "Log" before revision 3064

If you are unable to boot into the Clover you can use this setting to produces a debug output to /EFI/CLOVER/misc/debug.log. This log then can be assessed to find out what the problem is. !WARNING turning on this log will dramatically increase loading time as it has to write the log to the disk as it goes. So please be patient and give plenty of time before reseting your computer."

Debug

<key>Debug</key><false/>

Formerly known as "Log" before revision 3064

If you are unable to boot into the Clover you can use this setting to produces a debug output to /EFI/CLOVER/misc/debug.log. This log then can be assessed to find out what the problem is. !WARNING turning on this log will dramatically increase loading time as it has to write the log to the disk as it goes. So please be patient and give plenty of time before reseting your computer.

Zenith432 · May 14, 2017

@apianti:

FYI

Variadic EFIAPI should use VA_LIST macros.
Variadic non-EFIAPI should use __builtin_va_list (and corresponding builtins for MS/Intel compilers.)
This is set in include file. I reverted r4071. Don't use NO_MSABI_VA_FUNC as everything is set up right in MdePkg/Include/Base.h
In, r4072, the settings for XCODE were missing -DSIXTY_FOUR_BIT, etc.
It is not always necessary to make XCODE, since XCODE uses GCC if specific XCODE is not available.
No need for -mmmx, -msse as x64 always has this, and even for ia32 (which is deprecated), minimum target architecture supports these.
== vs = in INF means "replace" instead of "append". Whoever used == (Andy V??) must have been thinking of something.
So I reverted r4072 as well.
OTOH there was a duplicate symbol _fltused when compiling -D ENABLE_SECURE_BOOT, so eliminated dup with #ifndef.

TheRacerMaster · May 14, 2017

Install EfiCalls.efi to drivers64uefi. The log will be in EFI/CLOVER/misc.

apianti · May 14, 2017

@papiercul,

You need to set Boot/Debug=true.......... And make sure that the directory EFI/CLOVER/misc exists and is writable (i.e. FAT)

@apianti:

FYI

Variadic EFIAPI should use VA_LIST macros.

Variadic non-EFIAPI should use __builtin_va_list (and corresponding builtins for MS/Intel compilers.)

This is set in include file. I reverted r4071. Don't use NO_MSABI_VA_FUNC as everything is set up right in MdePkg/Include/Base.h

In, r4072, the settings for XCODE were missing -DSIXTY_FOUR_BIT, etc.

It is not always necessary to make XCODE, since XCODE uses GCC if specific XCODE is not available.

No need for -mmmx, -msse as x64 always has this, and even for ia32 (which is deprecated), minimum target architecture supports these.

== vs = in INF means "replace" instead of "append". Whoever used == (Andy V??) must have been thinking of something.

So I reverted r4072 as well.

OTOH there was a duplicate symbol _fltused when compiling -D ENABLE_SECURE_BOOT, so eliminated dup with #ifndef.

I didn't see that the EFIAPI was removed from those functions, still not sure why removed though, it should still be EFIAPI with VA_LIST - that's a change from CryptoPkg. Copied those options from CryptoPkg. As for the ==, that was probably Andy, I meant to remove all the extra non-sense but forgot. I don't think any of that is needed, as it's not in CryptoPkg.

tluck · May 14, 2017

fyi...

on the latest revision, during build am getting a bunch of messages like this:

Building Clover Install Package
-------------------------------

====================== Preinstall ======================
[bUILD] Pre
===================== Installation =====================
[bUILD] UEFI.only
package/buildpkg.sh: line 488: [: No: integer expression expected
================== Target ESP ==========================
[bUILD] Target.ESP
package/buildpkg.sh: line 513: [: No: integer expression expected
===================== Utils ============================
[bUILD] Utils
===================== EFI folder =======================
[bUILD] EFIFolder
package/buildpkg.sh: line 595: [: No: integer expression expected
package/buildpkg.sh: line 755: [: No: integer expression expected
package/buildpkg.sh: line 809: [: No: integer expression expected
package/buildpkg.sh: line 917: [: No: integer expression expected
package/buildpkg.sh: line 946: [: No: integer expression expected
=============== drivers64 UEFI mandatory ===============
[bUILD] AppleImageCodec-64.UEFI
[bUILD] AppleKeyAggregator-64.UEFI
[bUILD] AppleUITheme-64.UEFI
[bUILD] DataHubDxe-64.UEFI
[bUILD] FirmwareVolume-64.UEFI
[bUILD] FSInject-64.UEFI
[bUILD] OsxFatBinaryDrv-64.UEFI
[bUILD] SMCHelper-64.UEFI
[bUILD] VBoxHfs-64.UEFI
=================== drivers64 UEFI =====================
[bUILD] CsmVideoDxe-64
[bUILD] EmuVariableUefi-64
[bUILD] Fat-64
[bUILD] OsxAptioFix2Drv-64
[bUILD] OsxAptioFixDrv-64
[bUILD] OsxLowMemFixDrv-64
[bUILD] PartitionDxe-64
package/buildpkg.sh: line 1032: [: No: integer expression expected
package/buildpkg.sh: line 1124: [: No: integer expression expected
================= Post =================
[bUILD] Post

and the package was not the expected size.

apianti · May 14, 2017

Ah someone fix that!

EDIT: How did you call the script? This message means that 'No' was passed as the fourth argument....

[: No: integer expression expected

Philip Petev · May 14, 2017

fyi...

on the latest revision, during build am getting a bunch of messages like this:

Building Clover Install Package

-------------------------------

====================== Preinstall ======================

[bUILD] Pre

===================== Installation =====================

[bUILD] UEFI.only

package/buildpkg.sh: line 488: [: No: integer expression expected

================== Target ESP ==========================

[bUILD] Target.ESP

package/buildpkg.sh: line 513: [: No: integer expression expected

===================== Utils ============================

[bUILD] Utils

===================== EFI folder =======================

[bUILD] EFIFolder

package/buildpkg.sh: line 595: [: No: integer expression expected

package/buildpkg.sh: line 755: [: No: integer expression expected

package/buildpkg.sh: line 809: [: No: integer expression expected

package/buildpkg.sh: line 917: [: No: integer expression expected

package/buildpkg.sh: line 946: [: No: integer expression expected

=============== drivers64 UEFI mandatory ===============

[bUILD] AppleImageCodec-64.UEFI

[bUILD] AppleKeyAggregator-64.UEFI

[bUILD] AppleUITheme-64.UEFI

[bUILD] DataHubDxe-64.UEFI

[bUILD] FirmwareVolume-64.UEFI

[bUILD] FSInject-64.UEFI

[bUILD] OsxFatBinaryDrv-64.UEFI

[bUILD] SMCHelper-64.UEFI

[bUILD] VBoxHfs-64.UEFI

=================== drivers64 UEFI =====================

[bUILD] CsmVideoDxe-64

[bUILD] EmuVariableUefi-64

[bUILD] Fat-64

[bUILD] OsxAptioFix2Drv-64

[bUILD] OsxAptioFixDrv-64

[bUILD] OsxLowMemFixDrv-64

[bUILD] PartitionDxe-64

package/buildpkg.sh: line 1032: [: No: integer expression expected

package/buildpkg.sh: line 1124: [: No: integer expression expected

================= Post =================

[bUILD] Post

and the package was not the expected size.

What is your bash version?

tluck · May 14, 2017

fixed

i am building Clover with clovergrowerpro....

it was calling "makepkg No"

fixed it so it calls "makepkg" without an arg

gills83 · May 14, 2017

@papiercul,

You need to set Boot/Debug=true.......... And make sure that the directory EFI/CLOVER/misc exists and is writable (i.e. FAT)

I didn't see that the EFIAPI was removed from those functions, still not sure why removed though, it should still be EFIAPI with VA_LIST - that's a change from CryptoPkg. Copied those options from CryptoPkg. As for the ==, that was probably Andy, I meant to remove all the extra non-sense but forgot. I don't think any of that is needed, as it's not in CryptoPkg.

the files misc is empty with debug=true

thanks

apianti · May 14, 2017

Are you booting from a FAT volume or an HFS+ volume? You need to boot from a FAT volume, so you might have to create a USB. If the file is not created that seems to point to that it's not loading, could be that it's crashing trying to determine the TSC but that seems unlikely. You may have to enter the shell and load the driver to dump efi calls and then enter the gui.

gills83 · May 14, 2017

Are you booting from a FAT volume or an HFS+ volume? You need to boot from a FAT volume, so you might have to create a USB. If the file is not created that seems to point to that it's not loading, could be that it's crashing trying to determine the TSC but that seems unlikely. You may have to enter the shell and load the driver to dump efi calls and then enter the gui.

Yes, FAT but 32

attached small video

screen.zip

apianti · May 15, 2017

I pretty much gathered that was what was happening. The next step is to setup the efi shell to boot from EFI/BOOT/BOOTX64.efi and load the dump efi calls driver and then load EFI/CLOVER/CLOVERX64.efi. This should create another file in EFI/CLOVER/misc (or print to the screen at least) with details about what is happening.

gills83 · May 15, 2017

haf, no result, the file misc is empty and uefi, no screen , but legacy, there is the same screen as before.

apianti · May 15, 2017

Show me a video of you entering efi shell at boot and loading the driver and then clover.

Philip Petev · May 15, 2017

fixed

i am building Clover with clovergrowerpro....

it was calling "makepkg No"

fixed it so it calls "makepkg" without an arg

CloverGrowerPro... :w00t: That thing was decommissioned long time ago.

tluck · May 16, 2017

CloverGrowerPro... That thing was decommissioned long time ago.

i know! i know! certainly will move to build_clover soon -- looks like it has leveled out. I am just lazy to convert my customized make scripts. i build a customized package with just 2 themes so will look at your method for slim packages.

"Build_Clover.command, another Script to build standard Clover (or customized)" -- certainly a modest and humble description!

So maybe : "Build_Clover.command, the Best way to build Clover"

vector sigma · May 16, 2017

But apparently you should use it, to avoid errors Lol

i.e. passing "No" as argument just for nothing does not make sense (isn't even awaited in the make file that is responsible to call that script)

cecekpawon · May 17, 2017

I saw $NOEXTRAS was not properly initialized. Im not sure what correlation between 4th arguments and CloverGrowerPro, as previously ./makepkg accept argument. Here Im trying to sanitize 4th argument as integer only, and rename $NOEXTRAS to $EXTRAPKG to avoid confusions.

--- /Users/cecekpawon/Desktop/buildpkg/a.sh	2017-05-17 08:38:54.000000000 
+++ /Users/cecekpawon/Desktop/buildpkg/b.sh	2017-05-17 09:05:40.000000000 
@@ -15,29 +15,29 @@
 packagename="Clover"
 
 declare -r PKGROOT="${0%/*}"    # ie. edk2/Clover/CloverPackage/package
 declare -r SRCROOT="${1}"       # ie. edk2/Clover/CloverPackage
 declare -r SYMROOT="${2}"       # ie. edk2/Clover/CloverPackage/sym
 declare -r PKG_BUILD_DIR="${3}" # ie. edk2/Clover/CloverPackage/sym/package
-declare -r EXTRAPKG="${4}"    # ie. 1, 2 or 3
+declare -r EXTRAPKG="${4}"      # ie. 1, 2 or 3
 declare -r SCPT_TPL_DIR="${PKGROOT}/Scripts.templates"
 declare -r SCPT_LIB_DIR="${PKGROOT}/Scripts.libraries"
 
 if [[ $# -lt 3 ]];then
     echo "Too few arguments. Aborting..." >&2 && exit 1
 fi
 
 if [[ ! -d "$SYMROOT" ]];then
     echo "Directory ${SYMROOT} doesn't exit. Aborting..." >&2 && exit 1
 fi
 
-# NOEXTRAS, skip packaging something (Micky1979)
-[ "$EXTRAPKG" == "" ] && NOEXTRAS=0 || NOEXTRAS=${EXTRAPKG}
-# NOEXTRAS=1 skip themes
-# NOEXTRAS=2 skip themes, clover updater and prefpanel
-# NOEXTRAS=3 skip themes, clover updater, prefpanel, RC script and Clover EFI (slim package UEFI only)
+# EXTRAPKG, skip packaging something (Micky1979)
+! [[ "$EXTRAPKG" =~ ^-?[0-9]+$ ]] && EXTRAPKG=0
+# EXTRAPKG=1 skip themes
+# EXTRAPKG=2 skip themes, clover updater and prefpanel
+# EXTRAPKG=3 skip themes, clover updater, prefpanel, RC script and Clover EFI (slim package UEFI only)
 
 # ====== LANGUAGE SETUP ======
 export LANG='en_US.UTF-8'
 export LC_COLLATE='C'
 export LC_CTYPE='C'
 
@@ -450,13 +450,13 @@
     rm -R -f "${PKG_BUILD_DIR}"
     echo ""
     echo -e $COL_CYAN"  -------------------------------"$COL_RESET
     echo -e $COL_CYAN"  Building $packagename Install Package"$COL_RESET
     echo -e $COL_CYAN"  -------------------------------"$COL_RESET
     echo ""
-  case ${NOEXTRAS} in
+  case ${EXTRAPKG} in
     1) echo -e $COL_CYAN"  Excluded packages: Themes\n"$COL_RESET;;
     2) echo -e $COL_CYAN"  Excluded packages:\n  Themes\n  Clover Updater\n  Clover Prefpane\n"$COL_RESET;;
     3) echo -e $COL_CYAN"  Excluded packages:\n  Themes\n  Clover Updater\n  Clover Prefpane\n  RC scripts\n  Clover EFI\n"$COL_RESET;;
   esac
 
 # build Pre package
@@ -482,13 +482,13 @@
     mkdir -p ${PKG_BUILD_DIR}/${choiceId}/Root/EFI
     rsync -r --exclude=.svn --exclude="*~" --exclude='drivers*'   \
      ${SRCROOT}/CloverV2/EFI/BOOT ${PKG_BUILD_DIR}/${choiceId}/Root/EFI/
     addTemplateScripts --pkg-rootdir="${PKG_BUILD_DIR}/${choiceId}" \
                        --subst="INSTALLER_CHOICE=$packageRefId" MarkChoice
     buildpackage "$packageRefId" "${choiceId}" "${PKG_BUILD_DIR}/${choiceId}" "/EFIROOTDIR"
-    if [ "$NOEXTRAS" -lt 3 ]; then
+    if [ "$EXTRAPKG" -lt 3 ]; then
         addChoice --start-visible="true" --start-selected="choicePreviouslySelected('$packageRefId')"  \
                   --pkg-refs="$packageRefId" "${choiceId}"
     else
         addChoice --start-visible="false" --start-selected="true"  \
                   --pkg-refs="$packageRefId" "${choiceId}"
     fi
@@ -507,13 +507,13 @@
     addChoice --start-visible="true" --start-selected="choicePreviouslySelected('$packageRefId')"  \
               --selected="choices['UEFI.only'].selected || choices['Target.ESP'].selected"         \
               --pkg-refs="$packageRefId" "${choiceId}"
 # End build EFI target
 
 # build BiosBoot package
-if [ "$NOEXTRAS" -lt 3 ]; then
+if [ "$EXTRAPKG" -lt 3 ]; then
     echo "=================== BiosBoot ==========================="
     packagesidentity="$clover_package_identity"
     choiceId="BiosBoot"
 
     if [[ "$add_ia32" -eq 1 ]]; then
         ditto --noextattr --noqtn ${SRCROOT}/CloverV2/Bootloaders/ia32/boot? ${PKG_BUILD_DIR}/${choiceId}/Root/usr/standalone/i386/ia32/
@@ -589,13 +589,13 @@
     packageRefId=$(getPackageRefId "${packagesidentity}" "${choiceId}")
     buildpackage "$packageRefId" "${choiceId}" "${PKG_BUILD_DIR}/${choiceId}" "/EFIROOTDIR"
     addChoice --start-visible="false" --start-selected="true" --pkg-refs="$packageRefId" "${choiceId}"
 # End build EFI folder package
 
 # Create Bootloader Node
-if [ "$NOEXTRAS" -lt 3 ]; then
+if [ "$EXTRAPKG" -lt 3 ]; then
     addGroupChoices --enabled="!choices['UEFI.only'].selected" --exclusive_one_choice "Bootloader"
     echo "===================== BootLoaders ======================"
     packagesidentity="$clover_package_identity".bootloader
 
     # build alternative booting package
     choiceId="AltBoot"
@@ -749,13 +749,13 @@
         addChoice ${choiceOptions[@]} --pkg-refs="$packageBiosBootRefId $packageRefId" "${choiceId}"
     fi
     # End for chipset only NVIDIA NFORCE-MCP79 cloverEFI.64.blockio2 package
 fi
 
 # build theme packages
-if [ "$NOEXTRAS" -lt 1 ]; then
+if [ "$EXTRAPKG" -lt 1 ]; then
     echo "======================== Themes ========================"
     addGroupChoices "Themes"
     local specialThemes=('christmas' 'newyear')
 
     # Using themes section from Azi's/package branch.
     packagesidentity="${clover_package_identity}".themes
@@ -803,13 +803,13 @@
         buildpackage "$packageRefId" "${themeName}" "${PKG_BUILD_DIR}/${themeName}" "${themeDestDir}"
         addChoice --start-visible="false"  --start-selected="true"  --pkg-refs="$packageRefId" "${themeName}"
     done
 fi
 
 # build CloverThemeManager package
-if [[ -d "${SRCROOT}"/CloverThemeManager ]] && [ "$NOEXTRAS" -lt 1 ]; then
+if [[ -d "${SRCROOT}"/CloverThemeManager ]] && [ "$EXTRAPKG" -lt 1 ]; then
     local CTM_Dir="${SRCROOT}"/CloverThemeManager
     local CTM_Dest='/Applications'
 
     packagesidentity="${clover_package_identity}".CTM.themes
     choiceId="CloverThemeManager"
     packageRefId=$(getPackageRefId "${packagesidentity}" "${choiceId}")
@@ -826,13 +826,13 @@
     # end CloverThemeManager package
 # End build theme packages
 fi
  
 if [[ "$add_ia32" -eq 1 ]]; then
 # build mandatory drivers-ia32 packages
-    if [ "$NOEXTRAS" -lt 3 ]; then
+    if [ "$EXTRAPKG" -lt 3 ]; then
         echo "================= drivers32 mandatory =================="
         packagesidentity="${clover_package_identity}".drivers32.mandatory
         local drivers=($( find "${SRCROOT}/CloverV2/EFI/CLOVER/drivers32" -type f -name '*.efi' -depth 1 ))
         local driverDestDir='/EFIROOTDIR/EFI/CLOVER/drivers32'
         for (( i = 0 ; i < ${#drivers[@]} ; i++ ))
         do
@@ -911,13 +911,13 @@
         rm -R -f "${PKG_BUILD_DIR}/${driverChoice}"
     done
 # End mandatory drivers-ia32UEFI packages
 fi
 
 # build mandatory drivers-x64 packages
-if [[ -d "${SRCROOT}/CloverV2/EFI/CLOVER/drivers64"  ]] && [ "$NOEXTRAS" -lt 3 ]; then
+if [[ -d "${SRCROOT}/CloverV2/EFI/CLOVER/drivers64"  ]] && [ "$EXTRAPKG" -lt 3 ]; then
     echo "================= drivers64 mandatory =================="
     packagesidentity="${clover_package_identity}".drivers64.mandatory
     local drivers=($( find "${SRCROOT}/CloverV2/EFI/CLOVER/drivers64" -type f -name '*.efi' -depth 1 ))
     local driverDestDir='/EFIROOTDIR/EFI/CLOVER/drivers64'
     for (( i = 0 ; i < ${#drivers[@]} ; i++ ))
     do
@@ -940,13 +940,13 @@
         rm -R -f "${PKG_BUILD_DIR}/${driverChoice}"
     done
 fi
 # End mandatory drivers-x64 packages
 
 # build drivers-x64 packages
-if [[ -d "${SRCROOT}/CloverV2/drivers-Off/drivers64" ]] && [ "$NOEXTRAS" -lt 3 ]; then
+if [[ -d "${SRCROOT}/CloverV2/drivers-Off/drivers64" ]] && [ "$EXTRAPKG" -lt 3 ]; then
     echo "===================== drivers64 ========================"
     addGroupChoices --title="Drivers64" --description="Drivers64"  \
                     --enabled="!choices['UEFI.only'].selected"     \
                     "Drivers64"
     packagesidentity="${clover_package_identity}".drivers64
     local drivers=($( find "${SRCROOT}/CloverV2/drivers-Off/drivers64" -type f -name '*.efi' -depth 1 ))
@@ -1026,13 +1026,13 @@
         rm -R -f "${PKG_BUILD_DIR}/${driverName}"
     done
 fi
 # End build drivers-x64UEFI packages
 
 # build rc scripts package
-if [ "$NOEXTRAS" -lt 3 ]; then
+if [ "$EXTRAPKG" -lt 3 ]; then
     echo "===================== RC Scripts ======================="
     packagesidentity="$clover_package_identity"
 
 
     choiceId="rc.scripts.on.target"
     packageRefId=$(getPackageRefId "${packagesidentity}" "${choiceId}")
@@ -1118,13 +1118,13 @@
     done
 # End build optional rc scripts package
 fi
 
 local cloverUpdaterDir="${SRCROOT}"/CloverUpdater
 local cloverPrefpaneDir="${SRCROOT}"/CloverPrefpane
-if [[ -x "$cloverPrefpaneDir"/build/Clover.prefPane/Contents/MacOS/Clover ]] && [ "$NOEXTRAS" -lt 2 ]; then
+if [[ -x "$cloverPrefpaneDir"/build/Clover.prefPane/Contents/MacOS/Clover ]] && [ "$EXTRAPKG" -lt 2 ]; then
 # build CloverPrefpane package
     echo "==================== Clover Prefpane ==================="
     packagesidentity="$clover_package_identity"
     choiceId="CloverPrefpane"
     packageRefId=$(getPackageRefId "${packagesidentity}" "${choiceId}")
     # ditto --noextattr --noqtn "$cloverUpdaterDir"/CloverUpdaterUtility.plist  \

soto2080 · May 17, 2017

Lilu.kext 1.1.1

IntelGraphicsDVMTFixup 1.1.3

Tested with other user. Redownload kext and Try test again

나의 LG-F410S 의 Tapatalk에서 보냄

Still get kernel panic related with sklframebuffer

Sherlocks · May 17, 2017

Still get kernel panic related with sklframebuffer

Can you send me pm? Because here is clover thread. I want to check it in detail.

I reported some users success to boot recovery and installer from both kexts.

나의 LG-F410S 의 Tapatalk에서 보냄

Philip Petev · May 17, 2017

I saw $NOEXTRAS was not properly initialized. Im not sure what correlation between 4th arguments and CloverGrowerPro, as previously ./makepkg accept argument. Here Im trying to sanitize 4th argument as integer only, and rename $NOEXTRAS to $EXTRAPKG to avoid confusions.

--- /Users/cecekpawon/Desktop/buildpkg/a.sh	2017-05-17 08:38:54.000000000 
+++ /Users/cecekpawon/Desktop/buildpkg/b.sh	2017-05-17 09:05:40.000000000 
@@ -15,29 +15,29 @@
 packagename="Clover"
 
 declare -r PKGROOT="${0%/*}"    # ie. edk2/Clover/CloverPackage/package
 declare -r SRCROOT="${1}"       # ie. edk2/Clover/CloverPackage
 declare -r SYMROOT="${2}"       # ie. edk2/Clover/CloverPackage/sym
 declare -r PKG_BUILD_DIR="${3}" # ie. edk2/Clover/CloverPackage/sym/package
-declare -r EXTRAPKG="${4}"    # ie. 1, 2 or 3
+declare -r EXTRAPKG="${4}"      # ie. 1, 2 or 3
 declare -r SCPT_TPL_DIR="${PKGROOT}/Scripts.templates"
 declare -r SCPT_LIB_DIR="${PKGROOT}/Scripts.libraries"
 
 if [[ $# -lt 3 ]];then
     echo "Too few arguments. Aborting..." >&2 && exit 1
 fi
 
 if [[ ! -d "$SYMROOT" ]];then
     echo "Directory ${SYMROOT} doesn't exit. Aborting..." >&2 && exit 1
 fi
 
-# NOEXTRAS, skip packaging something (Micky1979)
-[ "$EXTRAPKG" == "" ] && NOEXTRAS=0 || NOEXTRAS=${EXTRAPKG}
-# NOEXTRAS=1 skip themes
-# NOEXTRAS=2 skip themes, clover updater and prefpanel
-# NOEXTRAS=3 skip themes, clover updater, prefpanel, RC script and Clover EFI (slim package UEFI only)
+# EXTRAPKG, skip packaging something (Micky1979)
+! [[ "$EXTRAPKG" =~ ^-?[0-9]+$ ]] && EXTRAPKG=0
+# EXTRAPKG=1 skip themes
+# EXTRAPKG=2 skip themes, clover updater and prefpanel
+# EXTRAPKG=3 skip themes, clover updater, prefpanel, RC script and Clover EFI (slim package UEFI only)
 
 # ====== LANGUAGE SETUP ======
 export LANG='en_US.UTF-8'
 export LC_COLLATE='C'
 export LC_CTYPE='C'
 
@@ -450,13 +450,13 @@
     rm -R -f "${PKG_BUILD_DIR}"
     echo ""
     echo -e $COL_CYAN"  -------------------------------"$COL_RESET
     echo -e $COL_CYAN"  Building $packagename Install Package"$COL_RESET
     echo -e $COL_CYAN"  -------------------------------"$COL_RESET
     echo ""
-  case ${NOEXTRAS} in
+  case ${EXTRAPKG} in
     1) echo -e $COL_CYAN"  Excluded packages: Themes\n"$COL_RESET;;
     2) echo -e $COL_CYAN"  Excluded packages:\n  Themes\n  Clover Updater\n  Clover Prefpane\n"$COL_RESET;;
     3) echo -e $COL_CYAN"  Excluded packages:\n  Themes\n  Clover Updater\n  Clover Prefpane\n  RC scripts\n  Clover EFI\n"$COL_RESET;;
   esac
 
 # build Pre package
@@ -482,13 +482,13 @@
     mkdir -p ${PKG_BUILD_DIR}/${choiceId}/Root/EFI
     rsync -r --exclude=.svn --exclude="*~" --exclude='drivers*'   \
      ${SRCROOT}/CloverV2/EFI/BOOT ${PKG_BUILD_DIR}/${choiceId}/Root/EFI/
     addTemplateScripts --pkg-rootdir="${PKG_BUILD_DIR}/${choiceId}" \
                        --subst="INSTALLER_CHOICE=$packageRefId" MarkChoice
     buildpackage "$packageRefId" "${choiceId}" "${PKG_BUILD_DIR}/${choiceId}" "/EFIROOTDIR"
-    if [ "$NOEXTRAS" -lt 3 ]; then
+    if [ "$EXTRAPKG" -lt 3 ]; then
         addChoice --start-visible="true" --start-selected="choicePreviouslySelected('$packageRefId')"  \
                   --pkg-refs="$packageRefId" "${choiceId}"
     else
         addChoice --start-visible="false" --start-selected="true"  \
                   --pkg-refs="$packageRefId" "${choiceId}"
     fi
@@ -507,13 +507,13 @@
     addChoice --start-visible="true" --start-selected="choicePreviouslySelected('$packageRefId')"  \
               --selected="choices['UEFI.only'].selected || choices['Target.ESP'].selected"         \
               --pkg-refs="$packageRefId" "${choiceId}"
 # End build EFI target
 
 # build BiosBoot package
-if [ "$NOEXTRAS" -lt 3 ]; then
+if [ "$EXTRAPKG" -lt 3 ]; then
     echo "=================== BiosBoot ==========================="
     packagesidentity="$clover_package_identity"
     choiceId="BiosBoot"
 
     if [[ "$add_ia32" -eq 1 ]]; then
         ditto --noextattr --noqtn ${SRCROOT}/CloverV2/Bootloaders/ia32/boot? ${PKG_BUILD_DIR}/${choiceId}/Root/usr/standalone/i386/ia32/
@@ -589,13 +589,13 @@
     packageRefId=$(getPackageRefId "${packagesidentity}" "${choiceId}")
     buildpackage "$packageRefId" "${choiceId}" "${PKG_BUILD_DIR}/${choiceId}" "/EFIROOTDIR"
     addChoice --start-visible="false" --start-selected="true" --pkg-refs="$packageRefId" "${choiceId}"
 # End build EFI folder package
 
 # Create Bootloader Node
-if [ "$NOEXTRAS" -lt 3 ]; then
+if [ "$EXTRAPKG" -lt 3 ]; then
     addGroupChoices --enabled="!choices['UEFI.only'].selected" --exclusive_one_choice "Bootloader"
     echo "===================== BootLoaders ======================"
     packagesidentity="$clover_package_identity".bootloader
 
     # build alternative booting package
     choiceId="AltBoot"
@@ -749,13 +749,13 @@
         addChoice ${choiceOptions[@]} --pkg-refs="$packageBiosBootRefId $packageRefId" "${choiceId}"
     fi
     # End for chipset only NVIDIA NFORCE-MCP79 cloverEFI.64.blockio2 package
 fi
 
 # build theme packages
-if [ "$NOEXTRAS" -lt 1 ]; then
+if [ "$EXTRAPKG" -lt 1 ]; then
     echo "======================== Themes ========================"
     addGroupChoices "Themes"
     local specialThemes=('christmas' 'newyear')
 
     # Using themes section from Azi's/package branch.
     packagesidentity="${clover_package_identity}".themes
@@ -803,13 +803,13 @@
         buildpackage "$packageRefId" "${themeName}" "${PKG_BUILD_DIR}/${themeName}" "${themeDestDir}"
         addChoice --start-visible="false"  --start-selected="true"  --pkg-refs="$packageRefId" "${themeName}"
     done
 fi
 
 # build CloverThemeManager package
-if [[ -d "${SRCROOT}"/CloverThemeManager ]] && [ "$NOEXTRAS" -lt 1 ]; then
+if [[ -d "${SRCROOT}"/CloverThemeManager ]] && [ "$EXTRAPKG" -lt 1 ]; then
     local CTM_Dir="${SRCROOT}"/CloverThemeManager
     local CTM_Dest='/Applications'
 
     packagesidentity="${clover_package_identity}".CTM.themes
     choiceId="CloverThemeManager"
     packageRefId=$(getPackageRefId "${packagesidentity}" "${choiceId}")
@@ -826,13 +826,13 @@
     # end CloverThemeManager package
 # End build theme packages
 fi
  
 if [[ "$add_ia32" -eq 1 ]]; then
 # build mandatory drivers-ia32 packages
-    if [ "$NOEXTRAS" -lt 3 ]; then
+    if [ "$EXTRAPKG" -lt 3 ]; then
         echo "================= drivers32 mandatory =================="
         packagesidentity="${clover_package_identity}".drivers32.mandatory
         local drivers=($( find "${SRCROOT}/CloverV2/EFI/CLOVER/drivers32" -type f -name '*.efi' -depth 1 ))
         local driverDestDir='/EFIROOTDIR/EFI/CLOVER/drivers32'
         for (( i = 0 ; i < ${#drivers[@]} ; i++ ))
         do
@@ -911,13 +911,13 @@
         rm -R -f "${PKG_BUILD_DIR}/${driverChoice}"
     done
 # End mandatory drivers-ia32UEFI packages
 fi
 
 # build mandatory drivers-x64 packages
-if [[ -d "${SRCROOT}/CloverV2/EFI/CLOVER/drivers64"  ]] && [ "$NOEXTRAS" -lt 3 ]; then
+if [[ -d "${SRCROOT}/CloverV2/EFI/CLOVER/drivers64"  ]] && [ "$EXTRAPKG" -lt 3 ]; then
     echo "================= drivers64 mandatory =================="
     packagesidentity="${clover_package_identity}".drivers64.mandatory
     local drivers=($( find "${SRCROOT}/CloverV2/EFI/CLOVER/drivers64" -type f -name '*.efi' -depth 1 ))
     local driverDestDir='/EFIROOTDIR/EFI/CLOVER/drivers64'
     for (( i = 0 ; i < ${#drivers[@]} ; i++ ))
     do
@@ -940,13 +940,13 @@
         rm -R -f "${PKG_BUILD_DIR}/${driverChoice}"
     done
 fi
 # End mandatory drivers-x64 packages
 
 # build drivers-x64 packages
-if [[ -d "${SRCROOT}/CloverV2/drivers-Off/drivers64" ]] && [ "$NOEXTRAS" -lt 3 ]; then
+if [[ -d "${SRCROOT}/CloverV2/drivers-Off/drivers64" ]] && [ "$EXTRAPKG" -lt 3 ]; then
     echo "===================== drivers64 ========================"
     addGroupChoices --title="Drivers64" --description="Drivers64"  \
                     --enabled="!choices['UEFI.only'].selected"     \
                     "Drivers64"
     packagesidentity="${clover_package_identity}".drivers64
     local drivers=($( find "${SRCROOT}/CloverV2/drivers-Off/drivers64" -type f -name '*.efi' -depth 1 ))
@@ -1026,13 +1026,13 @@
         rm -R -f "${PKG_BUILD_DIR}/${driverName}"
     done
 fi
 # End build drivers-x64UEFI packages
 
 # build rc scripts package
-if [ "$NOEXTRAS" -lt 3 ]; then
+if [ "$EXTRAPKG" -lt 3 ]; then
     echo "===================== RC Scripts ======================="
     packagesidentity="$clover_package_identity"
 
 
     choiceId="rc.scripts.on.target"
     packageRefId=$(getPackageRefId "${packagesidentity}" "${choiceId}")
@@ -1118,13 +1118,13 @@
     done
 # End build optional rc scripts package
 fi
 
 local cloverUpdaterDir="${SRCROOT}"/CloverUpdater
 local cloverPrefpaneDir="${SRCROOT}"/CloverPrefpane
-if [[ -x "$cloverPrefpaneDir"/build/Clover.prefPane/Contents/MacOS/Clover ]] && [ "$NOEXTRAS" -lt 2 ]; then
+if [[ -x "$cloverPrefpaneDir"/build/Clover.prefPane/Contents/MacOS/Clover ]] && [ "$EXTRAPKG" -lt 2 ]; then
 # build CloverPrefpane package
     echo "==================== Clover Prefpane ==================="
     packagesidentity="$clover_package_identity"
     choiceId="CloverPrefpane"
     packageRefId=$(getPackageRefId "${packagesidentity}" "${choiceId}")
     # ditto --noextattr --noqtn "$cloverUpdaterDir"/CloverUpdaterUtility.plist  \

That won't work, because EXTRAPKG is declared as read-only variable (declare -r), that should take the 4th parameter as value and carry that value till the end of the script, that's why a second variable is used. Also, the number check for that variable is correct, but the rest is missing.

[ "$EXTRAPKG" == "" ] && NOEXTRAS=0 || NOEXTRAS=${EXTRAPKG}

NOEXTRA is suppose to take the value of EXTRAPKG in case the second is non-empty, otherwise it should take value 0. So, the combination should become something like this:

[[ "$EXTRAPKG" != "" && "$EXTRAPKG" =~ ^-?[0-9]+$ ]] && NOEXTRAS=${EXTRAPKG} || NOEXTRAS=0

apianti · May 17, 2017

Yeah, the way the script is now is perfectly fine. If you want to change, the better approach would be to not rely on the positional passing of arguments in exchange for using argument flags instead.

EDIT: @Philip, I agree, but really it only needs to test whether the value is 1, 2, or 3 so you could just instead remove the first comparison of $EXTRAPKG and simplify the second.

Philip Petev · May 17, 2017

Of course...

[[ $EXTRAPKG == [1-3] ]] && NOEXTRAS=${EXTRAPKG} || NOEXTRAS=0

Sometimes a little push is all that everyone needs.

Clover General discussion

Recommended Posts

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites