hypervisor make should fail if arguments are missing in clean build

[dbkinder]

I called the hypervisor Makefile with:

   $ make board=../vecow.xml scenario=../shared.xml

and the system ran along just fine and ended with:

This build directory is configured with the settings below.
- BOARD = nuc11tnbi5
- SCENARIO = shared
- RELEASE = n

It built the hypervisor for the wrong board? I did figure out that I had not specified the board and scenario arguments as uppercase. The correct make command is:

   $ make BOARD=../vecow.xml SCENARIO=../shared.xml

What's odd is there is all this checking at the top of the Makefile for missing BOARD and SCENARIO arguments, and whether a name or XML file is given. But in my incorrect calling of the makefile with lowercase board and scenario names, it appears to make as if I didn't specify either BOARD or SCENARIO, the same as if I had just said make with no arguments.

I expect make with no arguments to complain that the BOARD and SCENARIO arguments are missing. There's a comment that if these parameters are missing a "default" board and scenario will be used, but that doesn't make sense.

[gvancuts]

What do you think would be the right behaviour here? Build whatever is already in the build folder from a previous build or error out if nothing is specified?

Note that in the case you could still run in the situation where you run the same command as in your example, but a previous build for a different board/scenario is in the build folder already and it rebuilds that. In that case the final output may be as confusing as it is today.

[dbkinder]

I started with a make clean, so there should be nothing previously built.

[gvancuts]

I started with a make clean, so there should be nothing previously built.

Yes, I knew that else you would have seen an error. But what do you think the right behaviour should have been in your case, error out because no board/scenario is specified?

[dbkinder]

I started with a make clean, so there should be nothing previously built.

Yes, I knew that else you would have seen an error. But what do you think the right behaviour should have been in your case, error out because no board/scenario is specified?

Yes. If you look through the Makefiles and .mk scripts, there are many and repeated checks for missing BOARD or SCENARIO or mixing BOARD and SCENARIO name and XML files. It's all rather complex and messy.

Looks like acrn-hypervisor/hypervisor/scripts/makefile/config.mk is where the decision is made to provide default values for BOARD and SCENARIO if they're not specified and there's no left-overs from a previous build to use. In this case (3) the default values are set:

$(eval $(call determine_config,BOARD,nuc11tnbi5))
$(eval $(call determine_config,SCENARIO,shared))
$(eval $(call determine_build_type,n))

To avoid surprise, I'd recommend not assuming defaults in this case: a clean build and either BOARD or SCENARIO are missing should report an error and stop with a nice message about missing arguments. Given the variety of boards being used I don't see a default board option that works.

[dbkinder]

@junjiemao1 As a DX issue, not specifying the BOARD and SCENARIO when making the hypervisor can lead to surprises if it's a clean work directory because a default board/scenario combination is built, but there's no reasonable values for these given the developer needs to configure the hypervisor. If the work directory artifacts already exist and you don't specify the BOARD and SCENARIO it will build using the last known BOARD and SCENARIO settings, which might be OK, but could also lead to surprises. It seems best to raise an error in the first (clean) case (as Geoffroy's patch does), and maybe even in the second (existing) case. Your thoughts?

[junjiemao1]

I'm fine to drop the default values if no reasonable can be identified.

As for the second case, could it be another DX issue to force users typing all those BOARD and SCENARIO settings (which can be looooooong paths).

[dbkinder]

@junjiemao1 @gvancuts Let's stick with this PR's changes then, and only complain for missing BOARD/SCENARIO when it's a "clean" working directory. Thanks.

[gvancuts]

As for the second case, could it be another DX issue to force users typing all those BOARD and SCENARIO settings (which can be looooooong paths).

My opinion on this is that I'd rather have the user be fully aware of what he's building even if that means he has to specify both parameters. This is only needed if you have a clean build folder, and in that case, chances are that the default are anyway not what you want. Maybe we can think about a mechanism to allow the user to set a default value though? Perhaps via some .makerc file that would be controlled by the user, that file would be sourced if it exists and default values could be set in it, such as BOARD and SCENARIO but also potentially the fact something would be built in a Docker container or now (that was the original idea but things deviated at some point...)

[junjiemao1]

@gvancuts The "second case" in my previous comment refers to "If the work directory artifacts already exist and you don't specify the BOARD and SCENARIO it will build using the last known BOARD and SCENARIO settings" in David's analysis.

I think we are all aligned that, when starting from a clean build folder, users have to specify BOARD and SCENARIO explicitly.

[dbkinder]

PR #7126 implements this change, thanks.