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.

Sign up for GitHub

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

Clean up the addition of extra directories and/or files #125

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Clean up the addition of extra directories and/or files #125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd77e58
run_qemu: Remove extra-scripts hidden option
weiny2 Mar 7, 2025
2d688f3
run_qemu: Add extra-dirs option
weiny2 Mar 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions parser_generator.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ exit 11 #)Created by argbash-init v2.9.0
# ARG_OPTIONAL_BOOLEAN([debug], [v], [Debug script problems (enables set -x)], )
# ARG_OPTIONAL_BOOLEAN([ndctl-build], , [Enable ndctl build in root image], [on])
# ARG_OPTIONAL_BOOLEAN([kern-selftests], , [Enable kernel selftest build in root image (Warning: This option can take a long time and requires many support packages on the host; including some 32 bit)], [off])
# ARG_OPTIONAL_SINGLE([extra-dirs], , [Specify a file with a list of directories (or files) to be copied to the /root directory within the image.], [])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to say that:

  • Relative directories inside that file are relative to --working-dir (which defaults to .)
  • symbolic links are resolved (NOT recursively like rsync -L does, but that's impossible to summarize clearly and fairly intuitive so don't say "not recursively")

# ARG_OPTIONAL_INCREMENTAL([quiet], [q], [quieten some output, can be repeated multiple times to quieten even more], )
# ARG_OPTIONAL_BOOLEAN([gdb], , [Wait for gdb to connect for kernel debug (port 10000)], )
# ARG_OPTIONAL_BOOLEAN([gdb-qemu], , [Start qemu with gdb], )
Expand Down
37 changes: 33 additions & 4 deletions run_qemu.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,21 @@ distro_vars="${script_dir}/${_distro}_vars.sh"

pushd "$_arg_working_dir" > /dev/null || fail "couldn't cd to $_arg_working_dir"

# save the working directory
working_dir=$(pwd)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why save it again? It's already in "$_arg_working_dir"


# make a path canonical to the working dir
make_canonical_path()
{
local p=$1

p=$(eval echo $p)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this line do?

pushd $working_dir > /dev/null
p=$(realpath $p)
popd > /dev/null
echo $p
}

set_valid_mkosi_ver()
{
"$mkosi_bin" --version
Expand Down Expand Up @@ -1125,6 +1140,7 @@ make_rootfs()
process_mkosi_template "$tmpl" > "$dst"
done

# Add user space items from host
rsync -a "${script_dir}"/mkosi/extra/ mkosi.extra/

# misc rootfs setup
Expand All @@ -1148,17 +1164,30 @@ make_rootfs()
if [ -f ~/.vimrc ]; then
rsync "${rsync_opts[@]}" ~/.vim* mkosi.extra/root/
fi
mkdir -p mkosi.extra/root/bin
if [ -d ~/git/extra-scripts ]; then
rsync "${rsync_opts[@]}" ~/git/extra-scripts/bin/* mkosi.extra/root/bin/
fi
if [[ $_arg_ndctl_build == "on" ]]; then
if [ -n "$ndctl" ]; then
rsync "${rsync_opts[@]}" "$ndctl/" mkosi.extra/root/ndctl
prepare_ndctl_build # create mkosi.postinst which compiles
fi
fi

if [[ "$_arg_extra_dirs" ]]; then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if [[ "$_arg_extra_dirs" ]]; then
if [[ -n "$_arg_extra_dirs" ]]; then

extra_dirs=$(make_canonical_path $_arg_extra_dirs)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think make_canonical_path should not be used here, otherwise it's incompatible with shell completion?

if [[ ! -f $extra_dirs ]]; then
fail "$extra_dirs not found"
fi
echo "Installing extra directories and files from: $extra_dirs"
for d in `cat "$extra_dirs"`; do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.shellcheck.net/wiki/SC2006

shellcheck -x run_qemu.sh

Don't forget the -x otherwise there is a ton of false positives.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for d in `cat "$extra_dirs"`; do
cat "$extra_dirs" | while read d; do

read is guaranteed to read line by line whereas for ... in ... splits on any whitespace.

d=$(make_canonical_path $d)
echo " $d"
if [[ -e $d ]]; then
rsync -a "$d" mkosi.extra/root/
else
fail "$d not found"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fail "$d not found"
fail "extra-dir directory %s not found" "$d"

fi
done
fi

# timedatectl defaults to UTC when /etc/localtime is missing
local bld_tz; bld_tz=$( timedatectl | awk '/zone:/ { print $3 }' )
# v15 commit f11325afa02c "Adopt systemd-firstboot"
Expand Down