Add support for the aarch64 Hook #104
Conversation
jacobweinstock
added
the
kind/feature
deploy/compose/osie/lastmile.sh
Outdated
| echo "extracting hook..." | ||
| hook_extract "${extract_dir}" "${source_dir}" "${filename}" |
There was a problem hiding this comment.
did you mean to get rid of the check if already extracted?
There was a problem hiding this comment.
yeah. This hinders switching between hook and osie as these files would need to be overwritten but wouldn't because of these checks.
There was a problem hiding this comment.
how about adding a stamp file like ${extract_dir}/.hook-successfully-extracted and similar for osie. Then the if would also be simpler too. I don't want to force anyone to re-download hook/osie unnecessarily.
There was a problem hiding this comment.
this is not for the download, just the archive extracting. The downloads will still be checked before trying again. see here
There was a problem hiding this comment.
oops, right. But then does mean that osie/hook is going to be re-extracted everytime we run sandbox? If not, then why were the already-extracted-checks around for in the first place.
There was a problem hiding this comment.
yeah, an extract will happen with every docker-compose up. I removed them because the check for extracting based on files inside the archive feels unnecessary and couples this basic script to knowing what's inside the archive. For Hook there are 2 archives with 2 files in each, but OSIE has a lot of files. I don't know if it's worth checking. In my tests, the extract is normally only a few seconds.
There was a problem hiding this comment.
you likely have a fast ssd/nvme machine right? IIRC this was put in because extracting osie over and over was very slow for some users and very much a waste of time. You don't need to know what the files are that exist in the archive if you add your own stamp file post-extraction and check for that.
There was a problem hiding this comment.
I originated this script a month ago with the idea to have it be as idempotent as possible, not to solve any slowness or other. I would like to see this pushed through so I have updated the PR to check based on the downloaded archive.
There was a problem hiding this comment.
hmm then I'm thinking of something else then. Thanks for that context. All good then!
deploy/compose/osie/lastmile.sh
Outdated
| else | ||
| echo "osie files already exist, not extracting" | ||
| fi | ||
| echo "extracting osie..." |
There was a problem hiding this comment.
ditto, did you mean to get rid of the already extracted check?
Hook repo just recently released support for aarch64. Signed-off-by: Jacob Weinstock <jakobweinstock@gmail.com>
jacobweinstock
force-pushed
the
add-aarch64-hook
branch
from
e71b52f
to
2bf87ec
Compare
Hook repo just recently released support for aarch64. Signed-off-by: Jacob Weinstock <jakobweinstock@gmail.com>
| fi | ||
| osie_move_helper_scripts "${source_dir}" "${dest_dir}" | ||
| else | ||
| echo "osie already downloaded" |
There was a problem hiding this comment.
do you want to update the message here to be osie/hook?
Description
Hook repo just recently released support for aarch64.
This change adds support for downloading these files.
Why is this needed
Fixes: #
How Has This Been Tested?
How are existing users impacted? What migration steps/scripts do we need?
Checklist:
I have: