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

Fix install directory for FVP crypto plugin. #577

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

statham-arm
Copy link
Collaborator

Before this patch, running get_fvps.sh --non-interactive would make a weirdly named directory in the user's home, with a name like "". or "." (depending on circumstances), with the crypto plugins installed in it.

Now --non-interactive installs the plugins under fvp/install in your git checkout of this repository, which is where they should have been.

The problem was because of this construction in get_fvps.sh:

INSTALLER_FLAGS_CRYPTO="... --basepath "$(dirname "$0")""

which has multiple shell errors. Firstly, the " around $0 are taken literally, so dirname is handed a path beginning with a double quote, which propagates into the path it returns. Secondly, the " around $(dirname) are also literal, but not reprocessed when $INSTALLER_FLAGS_CRYPTO is expanded on to the installer command line (because quote processing happens before variable expansion). The net effect is to give setup.bin a --basepath directory containing lots of confusing double quotes. For extra confusion, setup.bin appears to change directory to $HOME before interpreting that path, so you don't even get a strangely named directory under the FVP install directory – it appears in your home dir instead.

Since this script already commits to bash rather than plain POSIX sh, the sensible fix for the quoting issues is to use a bash array variable to accumulate the extra arguments, and expand it via "${INSTALLER_FLAGS_CRYPTO[@]}".

Also, the --basepath will need to be absolute rather than relative, to avoid confusion when setup.bin changes directory to $HOME. The easiest approach to that is to use the fact that we were already issuing a cd $(dirname "$0") command to change into the fvp directory: do that earlier, and then we can use $PWD to retrieve the absolute path.

This fixes --non-interactive. But another problem is that if you run setup.bin interactively, you'll have to enter the pathname by hand, and we don't give the user any guidance on what path to enter. So I've also updated the docs to explain the interactive installation more fully.

This is all very error-prone, so I've also extended run_fvp.py so that it actually finds and loads the crypto plugin. That way, if it hasn't been installed in the right place, we find that out early, and can figure out what went wrong.

(Finally, in this commit, I've also fixed a spelling error in one of the script's internal variables: CORSTONE, not CORSONE.)

Before this patch, running get_fvps.sh --non-interactive would make a
weirdly named directory in the user's home, with a name like "". or
"." (depending on circumstances), with the crypto plugins installed in
it.

Now --non-interactive installs the plugins under fvp/install in your
git checkout of this repository, which is where they should have been.

The problem was because of this construction in get_fvps.sh:

  INSTALLER_FLAGS_CRYPTO="... --basepath \"$(dirname \"$0\")\""

which has multiple shell errors. Firstly, the \" around $0 are taken
literally, so dirname is handed a path beginning with a double quote,
which propagates into the path it returns. Secondly, the \" around
$(dirname) are also literal, but _not_ reprocessed when
$INSTALLER_FLAGS_CRYPTO is expanded on to the installer command
line (because quote processing happens before variable expansion). The
net effect is to give setup.bin a --basepath directory containing lots
of confusing double quotes. For extra confusion, setup.bin appears to
change directory to $HOME before interpreting that path, so you don't
even get a strangely named directory under the FVP install directory –
it appears in your home dir instead.

Since this script already commits to bash rather than plain POSIX sh,
the sensible fix for the quoting issues is to use a bash array
variable to accumulate the extra arguments, and expand it via
"${INSTALLER_FLAGS_CRYPTO[@]}".

Also, the --basepath will need to be absolute rather than relative, to
avoid confusion when setup.bin changes directory to $HOME. The easiest
approach to _that_ is to use the fact that we were already issuing a
`cd $(dirname "$0")` command to change into the fvp directory: do that
earlier, and then we can use `$PWD` to retrieve the absolute path.

This fixes --non-interactive. But another problem is that if you run
setup.bin interactively, you'll have to enter the pathname by hand,
and we don't give the user any guidance on what path to enter. So I've
also updated the docs to explain the interactive installation more
fully.

This is all very error-prone, so I've also extended run_fvp.py so that
it actually finds and loads the crypto plugin. That way, if it hasn't
been installed in the right place, we find that out early, and can
figure out what went wrong.

(Finally, in this commit, I've also fixed a spelling error in one of
the script's internal variables: CORSTONE, not CORSONE.)
@statham-arm statham-arm merged commit ed73480 into ARM-software:main Nov 26, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants