[PATCH] Enhance scripts/build to allow passing arguments to modules/apps

20 views
Skip to first unread message

Waldemar Kozaczuk

unread,
Jul 20, 2019, 4:41:45 PM7/20/19
to osv...@googlegroups.com, Waldemar Kozaczuk
This patch enhances scripts/build to allow passing arguments to modules/apps
makefiles like so:

./script/build m_JAVA_VERSION=11 m_JAVA_MODULES=ava.base,java.sql,java.logging,java.xml,jdk.unsupported image=openjdk-zulu-9-and-above

That way corresponding app makefile can use passed in m_* variable and customize
its build behavior. This new mechanism should let us remove redundant openjdk
apps.

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---
scripts/build | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/scripts/build b/scripts/build
index 2affe578..b30943c9 100755
--- a/scripts/build
+++ b/scripts/build
@@ -17,16 +17,26 @@ unset image modules

# Pass to "make" all the given args except "image=..." or "module=..."
declare -a args
+declare -a module_make_args
# Make is going to be invoked twice. At the first run ("stage1") we
# build all possible dependencies any app or module might require. At
# the second - the loader.elf is linked and the loader.img is
# produced. In the case of "make clean", the first invocation (and
# the only one) is used for cleaning up the kernel.
stage1_args="stage1"
+shopt -s extglob
for i
do
case $i in
image=*|modules=*|fs=*|usrskel=*|check|--append-manifest) ;;
+ # use pattern below to filter out arguments passed in form m_*=*
+ # and append them to module_make_args array in order to set them
+ # later as environment variables passed further to the modules/apps make files
+ # as a way to customize their build logic
+ # sanitize the arguments passed in form m_*=* to make sure eval
+ # is not going harm us in any way below
+ m_+([[:alnum:]_])=+([[:alnum:]_,]))
+ module_make_args[${#module_make_args[@]}]="$i" ;;
clean)
stage1_args=clean ;;
*) # yuck... Is there a prettier way to append to array?
@@ -161,7 +171,9 @@ if [[ ${vars[append_manifest]} == "true" && $modules == "!default" ]]; then
modules="empty"
fi

-fs_type=$fs_type jdkbase=$jdkbase ARCH=$arch mode=$mode OSV_BASE=$SRC OSV_BUILD_PATH=$OSV_BUILD_PATH scripts/module.py $j_arg build -c "$modules" $usrskel_arg $no_required_arg
+# the m_*=* arguments have been sanitized above so we deem it is safe enough
+# to use eval to set corresponding variables (is there a way to do it without eval?)
+eval ${module_make_args[@]} fs_type=$fs_type jdkbase=$jdkbase ARCH=$arch mode=$mode OSV_BASE=$SRC OSV_BUILD_PATH=$OSV_BUILD_PATH scripts/module.py $j_arg build -c "$modules" $usrskel_arg $no_required_arg

if [[ ${vars[append_manifest]} == "true" && -f "$OSV_BUILD_PATH/append.manifest" ]]; then
cat "$OSV_BUILD_PATH/append.manifest" >> "$OSV_BUILD_PATH/usr.manifest"
--
2.20.1

Nadav Har'El

unread,
Jul 21, 2019, 3:25:14 AM7/21/19
to Waldemar Kozaczuk, Osv Dev
On Sat, Jul 20, 2019 at 11:41 PM Waldemar Kozaczuk <jwkoz...@gmail.com> wrote:
This patch enhances scripts/build to allow passing arguments to modules/apps
makefiles like so:

./script/build m_JAVA_VERSION=11 m_JAVA_MODULES=ava.base,java.sql,java.logging,java.xml,jdk.unsupported image=openjdk-zulu-9-and-above

That way corresponding app makefile can use passed in m_* variable and customize
its build behavior. This new mechanism should let us remove redundant openjdk
apps.

Nice idea. But I wonder, would it hurt if we pass *all* the parameters (which we already store in the "args" array) same as we pass them to the kernel "make", instead of just those starting with "m_*"?



-fs_type=$fs_type jdkbase=$jdkbase ARCH=$arch mode=$mode OSV_BASE=$SRC OSV_BUILD_PATH=$OSV_BUILD_PATH scripts/module.py $j_arg build -c "$modules" $usrskel_arg $no_required_arg
+# the m_*=* arguments have been sanitized above so we deem it is safe enough
+# to use eval to set corresponding variables (is there a way to do it without eval?)
+eval ${module_make_args[@]} fs_type=$fs_type jdkbase=$jdkbase ARCH=$arch mode=$mode OSV_BASE=$SRC OSV_BUILD_PATH=$OSV_BUILD_PATH scripts/module.py $j_arg build -c "$modules" $usrskel_arg $no_required_arg

Note that "eval" evaluates twice not just the module_make_args but also everything else in this command line, which can cause a mess in various cases including when the paths have spaces. I'd very much prefer not to do this eval.

One thing you could do is to use a subshell, and in it *export* all these variables, and then run. For example, I didn't test the following as a whole, but I checked some of the details so I think it should work:

# The parentheses start a subshell. Whatever is exported there, doesn't effect the external shell
(
    # Note: the double-quotes and almost everything in the line below is important to correctly allow spaces
    for i in "${args[@]}"
    do
          # again, the double-quotes is important in case the variable's value contains spaces
          export "$i"
    done
    # Export the variables we already have. This makes it unnecessary to do "fs__type=$fstype jdkbase $jdkbase ..."
    export fs_type jdkbase mode OSV_BUILD_PATH
    # Other variables we wanted to rename, I don't know why
    export ARCH=$arch OSV_BASE=$SRC
    # Run what we wanted to run. It will inherit everything we exported above.
    scripts/module.py $j_arg build -c "$modules" $usrskel_arg $no_required_arg
)

 

 if [[ ${vars[append_manifest]} == "true" && -f "$OSV_BUILD_PATH/append.manifest" ]]; then
    cat "$OSV_BUILD_PATH/append.manifest" >> "$OSV_BUILD_PATH/usr.manifest"
--
2.20.1

--
You received this message because you are subscribed to the Google Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/20190720204132.9662-1-jwkozaczuk%40gmail.com.

Commit Bot

unread,
Jul 24, 2019, 1:06:57 AM7/24/19
to osv...@googlegroups.com, Waldemar Kozaczuk
From: Waldemar Kozaczuk <jwkoz...@gmail.com>
Committer: Waldemar Kozaczuk <jwkoz...@gmail.com>
Branch: master

Enhance scripts/build to allow passing arguments to modules/apps

This patch enhances scripts/build to allow passing arguments to modules/apps
makefiles like so:

./script/build JAVA_VERSION=11
JAVA_MODULES=java.base,java.sql,java.logging,java.xml

That way corresponding app makefile can receive passed in extra variables
and customize
its build behavior. This new mechanism should let us remove redundant
openjdk apps.

The author of this patch is Nadav Har'El. The idea of this feature comes
from Waldek Kozaczuk.

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>

---
diff --git a/scripts/build b/scripts/build
--- a/scripts/build
+++ b/scripts/build
@@ -161,10 +161,28 @@ if [[ ${vars[append_manifest]} == "true" && $modules
== "!default" ]]; then
modules="empty"
fi

-fs_type=$fs_type jdkbase=$jdkbase ARCH=$arch mode=$mode OSV_BASE=$SRC
OSV_BUILD_PATH=$OSV_BUILD_PATH scripts/module.py $j_arg build -c "$modules"
$usrskel_arg $no_required_arg
+# The parentheses start a subshell. Whatever is exported there, doesn't
effect the external shell
+(
+ # Note: the double-quotes and almost everything in the line below is
important to correctly allow spaces
+ # This specifically allows us to pass extra arguments to the modules/apps
makefiles
+ for i in "${args[@]}"
+ do
+ case $i in
+ *=*)
+ # again, the double-quotes is important in case the variable's value
contains spaces
+ export "$i" ;;
+ esac
+ done
+ # Export the variables we already have. This makes it unnecessary to
do "fs__type=$fstype jdkbase $jdkbase ..."
+ export fs_type jdkbase mode OSV_BUILD_PATH
+ # Other variables we wanted to rename, I don't know why
+ export ARCH=$arch OSV_BASE=$SRC
+ # Run what we wanted to run. It will inherit everything we exported above.
+ scripts/module.py $j_arg build -c "$modules" $usrskel_arg $no_required_arg
+)

if [[ ${vars[append_manifest]} == "true" &&
-f "$OSV_BUILD_PATH/append.manifest" ]]; then
- cat "$OSV_BUILD_PATH/append.manifest" >> "$OSV_BUILD_PATH/usr.manifest"
+ cat "$OSV_BUILD_PATH/append.manifest" >> "$OSV_BUILD_PATH/usr.manifest"
fi

bootfs_manifest=$manifest make "${args[@]}" | tee -a build.out
Reply all
Reply to author
Forward
0 new messages