On 06/24/14 17:39, Paolo Bonzini wrote:
> Check for the presence of a well-known file in the checkout,
> in order to detect an incorrect setting of $WORKSPACE.
>
> Only print the new message if sourcing BuildEnv succeeds.
>
> Suggested-by: Laszlo Ersek <[email protected]>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Paolo Bonzini <[email protected]>
> ---
> This patch is on top of "[PATCH] Look for BuildEnv
> under EDK_TOOLS_PATH".
> ---
> edksetup.sh | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/edksetup.sh b/edksetup.sh
> index 5b4d37e..6854188 100755
> --- a/edksetup.sh
> +++ b/edksetup.sh
> @@ -44,6 +44,20 @@ function SetupEnv()
> else
> . BaseTools/BuildEnv $*
> fi
> + if [ $? -gt 0 ]
> + then
> + return 1
> + fi
> + if [ ! -f $WORKSPACE/MdePkg/MdePkg.dsc ]
> + then
> + echo
> + echo WORKSPACE does not point to the top of the edk2 tree.
> + echo Please ensure that this file is sourced from the top
> + echo of the tree and that WORKSPACE is not set incorrectly
> + echo in your shell.
> + echo
> + return 1
> + fi
> }
>
> function SourceEnv()
>
(Hazarding serious bodily harm to myself:)
- (minor nit) I suggested MdePkg.dec, not MdePkg.dsc, but I guess their
availability is interchangeable for this purpose, so OK;
- more importantly, since the user is requested to source "edksetup.sh"
rather than execute it in a subshell, I think the above code (although
exiting with status 1) can introduce an incorrect WORKSPACE variable to
the user's environment. In addition, I think BuildEnv can capture that
incorrect WORKSPACE in a few other places too.
What is the reason for not doing the workspace sanity check before
sourcing BuildEnv? In BuildEnv this sanity check used to be an early step.
ScriptMain
SetWorkspace
check for already set WORKSPACE
if unset or empty, verify edk2 root
Ah, I think I understand. You don't want to duplicate (or exploit)
BuildEnv's
export WORKSPACE=`pwd`
logic in edksetup.sh. Namely, if you wanted to check in advance, before
calling BuildEnv, you'd have to know where BuildEnv itself would check
(ie. know that BuildEnv would assign WORKSPACE the cwd).
Not sure what to suggest. It's either duplicating this knowledge in
edksetup.sh, or rolling back all of BuildEnv's changes, or leaving the
check in BuildEnv's SetWorkspace (just with a different implementation,
ie. looking for MdePkg).
I hope I'm wrong though.
Thanks,
Laszlo
------------------------------------------------------------------------------
Open source business process management suite built on Java and Eclipse
Turn processes into business applications with Bonita BPM Community Edition
Quickly connect people, data, and systems into organized workflows
Winner of BOSSIE, CODIE, OW2 and Gartner awards
http://p.sf.net/sfu/Bonitasoft
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
