Skip to content

Fix 'shellcheck' complaints #193

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

Merged
sinnykumari merged 2 commits into from
Nov 9, 2018
Merged

Fix 'shellcheck' complaints #193

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
2779555
Fix 'shellcheck' complaints
miabbott Nov 5, 2018
0cc5e01
ignore pycache dir
miabbott Nov 6, 2018
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 .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/Cargo.lock
/target
**/*.rs.bk
src/__pycache__/*
32 changes: 22 additions & 10 deletions build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
#!/usr/bin/bash
set -xeuo pipefail

#!/usr/bin/env bash
set -euo pipefail

if [ $# -eq 0 ]; then
echo Usage: "build.sh CMD"
echo "Supported commands:"
echo " configure_user"
echo " configure_yum_repos"
echo " install_rpms"
echo " make_and_makeinstall"
exit 1
fi

set -x
srcdir=$(pwd)

configure_yum_repos() {
Expand All @@ -14,12 +25,13 @@ configure_yum_repos() {
# Until we fix https://github.com/rpm-software-management/libdnf/pull/149
excludes='exclude=ostree ostree-libs ostree-grub2 rpm-ostree'
for repo in /etc/yum.repos.d/fedora*.repo; do
cat ${repo} | (while read line; do if echo "$line" | grep -qE -e '^enabled=1'; then echo "${excludes}"; fi; echo $line; done) > ${repo}.new
mv ${repo}.new ${repo}
# reworked to remove useless `cat` - https://github.com/koalaman/shellcheck/wiki/SC2002
(while read -r line; do if echo "$line" | grep -qE -e '^enabled=1'; then echo "${excludes}"; fi; echo "$line"; done < "${repo}") > "${repo}".new
mv "${repo}".new "${repo}"
done

# enable `walters/buildtools-fedora` copr
# pulled from https://copr.fedorainfracloud.org/coprs/walters/buildtools-fedora/repo/fedora-28/walters-buildtools-fedora-fedora-28.repo
# pulled from https://copr.fedorainfracloud.org/coprs/walters/buildtools-fedora/repo/fedora-28/walters-buildtools-fedora-fedora-28.repo
cat > /etc/yum.repos.d/walters-buildtools-fedora-fedora-28.repo <<'EOF'
[walters-buildtools-fedora]
name=Copr repo for buildtools-fedora owned by walters
Expand All @@ -34,7 +46,7 @@ enabled_metadata=1
EOF

# enable `dustymabe/ignition` copr
# pulled from https://copr.fedorainfracloud.org/coprs/dustymabe/ignition/repo/fedora-28/dustymabe-ignition-fedora-28.repo
# pulled from https://copr.fedorainfracloud.org/coprs/dustymabe/ignition/repo/fedora-28/dustymabe-ignition-fedora-28.repo
cat > /etc/yum.repos.d/dustymabe-ignition-fedora-28.repo <<'EOF'
[dustymabe-ignition]
name=Copr repo for ignition owned by dustymabe
Expand Down Expand Up @@ -65,10 +77,10 @@ install_rpms() {
# to use the container as a development environment for itself.
# Down the line we may strip these out, or have a separate
# development version.
self_builddeps=$(grep -v '^#' ${srcdir}/build-deps.txt)
self_builddeps=$(grep -v '^#' "${srcdir}"/build-deps.txt)

# Process our base dependencies + build dependencies
(echo ${self_builddeps} && grep -v '^#' ${srcdir}/deps.txt) | xargs dnf -y install
(echo "${self_builddeps}" && grep -v '^#' "${srcdir}"/deps.txt) | xargs dnf -y install

# Commented out for now, see above
#dnf remove -y ${self_builddeps}
Expand All @@ -90,7 +102,7 @@ make_and_makeinstall() {
# TODO: install these as e.g.
# /usr/bin/ostree-releng-script-rsync-repos
mkdir -p /usr/app/
rsync -rlv ${srcdir}/ostree-releng-scripts/ /usr/app/ostree-releng-scripts/
rsync -rlv "${srcdir}"/ostree-releng-scripts/ /usr/app/ostree-releng-scripts/

if ! test -f mantle/README.md; then
echo "Run: git submodule update --init" 1>&2
Expand Down
10 changes: 5 additions & 5 deletions coreos-assembler
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ set -euo pipefail

# docker/podman don't run through PAM, but we want this set for the privileged
# (non-virtualized) path
export USER=${USER:-$(id -nu)}
export USER="${USER:-$(id -nu)}"

# When trying to connect to libvirt we get "Failed to find user record
# for uid" errors if there is no entry for our UID in /etc/passwd.
Expand All @@ -16,7 +16,7 @@ if ! whoami &> /dev/null; then
# We need to make sure we set $HOME in the /etc/passwd file because
# if we don't libvirt will try to use `/` and we will get permission
# issues
export HOME="/var/tmp/${USER_NAME:-default}" && mkdir -p $HOME
export HOME="/var/tmp/${USER_NAME:-default}" && mkdir -p "$HOME"
if [ -w /etc/passwd ]; then
echo "${USER_NAME:-default}:x:$(id -u):0:${USER_NAME:-default} user:${HOME}:/sbin/nologin" >> /etc/passwd
fi
Expand All @@ -26,15 +26,15 @@ fi
# the later umount doesn't affect the host potentially
if [ -e /sys/fs/selinux/status ]; then
if [ -z "${coreos_assembler_unshared:-}" ]; then
exec sudo -- env coreos_assembler_unshared=1 unshare -m -- runuser -u ${USER} -- $0 "$@"
exec sudo -- env coreos_assembler_unshared=1 unshare -m -- runuser -u "${USER}" -- "$0" "$@"
else
# Work around https://github.com/containers/libpod/issues/1448
sudo umount /sys/fs/selinux
fi
fi

cmd=${1:-}
build_commands="init fetch build run prune clean"
build_commands="init fetch build buildextend-ec2 run prune clean"
other_commands="shell"
utility_commands="gf-oemid virt-install oscontainer"
if [ -z "${cmd}" ]; then
Expand All @@ -59,7 +59,7 @@ shift

target=/usr/lib/coreos-assembler/cmd-${cmd}
if test -x "${target}"; then
exec ${target} "$@"
exec "${target}" "$@"
fi

echo "Unknown command: ${cmd}" 1>&2
Expand Down
97 changes: 47 additions & 50 deletions src/cmd-build
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
#!/usr/bin/env bash
set -euo pipefail

dn=$(dirname $0)
. ${dn}/cmdlib.sh
dn=$(dirname "$0")
# shellcheck source=src/cmdlib.sh
. "${dn}"/cmdlib.sh

print_help() {
cat 1>&2 <<'EOF'
Expand Down Expand Up @@ -61,15 +62,15 @@ ostree --version
rpm-ostree --version

previous_build=
if [ -L ${workdir}/builds/latest ]; then
previous_build=$(readlink ${workdir}/builds/latest)
previous_builddir=${workdir}/builds/${previous_build}
if [ -L "${workdir:?}"/builds/latest ]; then
previous_build=$(readlink "${workdir}"/builds/latest)
previous_builddir="${workdir}/builds/${previous_build}"
echo "Previous build: ${previous_build}"
fi

previous_commit=
if [ -n "${ref:-}" ]; then
previous_commit=$(ostree --repo=${workdir}/repo rev-parse ${ref} 2>/dev/null || true)
previous_commit=$(ostree --repo="${workdir}"/repo rev-parse "${ref}" 2>/dev/null || true)
fi
# If the ref was unset or missing, look at the previous build
if [ -z "${previous_commit}" ] && [ -n "${previous_build}" ]; then
Expand All @@ -82,22 +83,22 @@ sha256sum_str() {
}

# Calculate kickstart checksum now and gather previous image build variables if any
kickstart_input=${configdir}/image.ks
kickstart_checksum=$(cat ${kickstart_input} | sha256sum_str)
kickstart_input="${configdir:?}"/image.ks
kickstart_checksum=$(< "${kickstart_input}" sha256sum_str)
if [ -n "${previous_build}" ]; then
previous_image_input_checksum=$(jq -r '.["coreos-assembler.image-input-checksum"]' < "${previous_builddir}/meta.json")
previous_image_genver=$(jq -r '.["coreos-assembler.image-genver"]' < "${previous_builddir}/meta.json")
fi
echo "Kickstart checksum: ${kickstart_checksum}"

# Generate metadata that's *input* to the ostree commit
config_gitrev=$(cd ${configdir} && git describe --tags --always --abbrev=42)
config_gitrev=$(cd "${configdir}" && git describe --tags --always --abbrev=42)
config_dirty=false
if ! git -C ${configdir} diff --quiet --exit-code; then
if ! git -C "${configdir}" diff --quiet --exit-code; then
config_dirty=true
fi
commitmeta_input_json=$(pwd)/tmp/commit-metadata-input.json
cat >${commitmeta_input_json} <<EOF
cat >"${commitmeta_input_json}" <<EOF
{
"coreos-assembler.config-gitrev": "${config_gitrev}",
"coreos-assembler.config-dirty": ${config_dirty}
Expand All @@ -106,19 +107,19 @@ EOF
# These need to be absolute paths right now for rpm-ostree
composejson=$(pwd)/tmp/compose.json
# --cache-only is here since `fetch` is a separate verb.
runcompose --cache-only ${FORCE} --add-metadata-from-json ${commitmeta_input_json} \
--write-composejson-to ${composejson}
runcompose --cache-only ${FORCE} --add-metadata-from-json "${commitmeta_input_json}" \
--write-composejson-to "${composejson}"
# Very special handling for --write-composejson-to as rpm-ostree doesn't
# write it if the commit didn't change.
if [ -f "${changed_stamp}" ]; then
if [ -f "${changed_stamp:?}" ]; then
commit=$(jq -r '.["ostree-commit"]' < "${composejson}")
# Clean up prior versions
rm -f ${workdir}/tmp/compose-*.json
rm -f "${workdir}"/tmp/compose-*.json
# Save this in case the image build fails
cp -a --reflink=auto ${composejson} ${workdir}/tmp/compose-${commit}.json
cp -a --reflink=auto "${composejson}" "${workdir}"/tmp/compose-"${commit}".json
else
commit=${previous_commit}
image_input_checksum=$((echo ${commit} && echo ${kickstart_checksum}) | sha256sum_str)
image_input_checksum=$( (echo "${commit}" && echo "${kickstart_checksum}") | sha256sum_str)
# Note we may not actually have a previous build in the case of
# successfully composing an ostree but failing the image on the
# first build.
Expand All @@ -129,62 +130,55 @@ else

# Grab the previous treecompose JSON (local developer case: treecompose succeeded but
# image build failed) if possible, otherwise grab the previous build
cached_previous_composejson=${workdir}/tmp/compose-${previous_commit}.json
cached_previous_composejson="${workdir}"/tmp/compose-"${previous_commit}".json
if [ -f "${cached_previous_composejson}" ]; then
echo "Resuming partial build from: ${commit}"
cp -a --reflink=auto ${cached_previous_composejson} ${composejson}
cp -a --reflink=auto "${cached_previous_composejson}" "${composejson}"
else
if [ -z "${previous_build}" ]; then
fatal "compose tree had no changes, but no previous build or cached data"
fi
echo "Commit ${commit} unchanged; reusing previous build's rpm-ostree metadata"
# This will have all of the data from the previous build, but we'll
# overwrite things.
cp -a --reflink=auto ${previous_builddir}/meta.json ${composejson}
cp -a --reflink=auto "${previous_builddir}"/meta.json "${composejson}"
fi
fi

if [ -n "${previous_build}" ]; then
rpm-ostree --repo=${workdir}/repo db diff ${previous_commit} ${commit}
rpm-ostree --repo="${workdir}"/repo db diff "${previous_commit}" "${commit}"
fi

image_input_checksum=$((echo ${commit} && echo ${kickstart_checksum}) | sha256sum_str)
image_input_checksum=$( (echo "${commit}" && echo "${kickstart_checksum}") | sha256sum_str)
echo "New image input checksum: ${image_input_checksum}"
version=$(ostree --repo=${workdir}/repo show --print-metadata-key=version ${commit} | sed -e "s,',,g")
version=$(ostree --repo="${workdir}"/repo show --print-metadata-key=version "${commit}" | sed -e "s,',,g")
if [ "${previous_commit}" = "${commit}" ] && [ -n "${previous_image_genver:-}" ]; then
image_genver=$((${previous_image_genver} + 1))
buildid=${version}-${image_genver}
image_genver=$((previous_image_genver + 1))
buildid="${version}"-"${image_genver}"
else
image_genver=0
buildid=${version}
buildid="${version}"
fi
echo "New build ID: ${buildid}"

# Generate JSON
if [ -n "${previous_commit}" ]; then
previous_commit_json='"'"${previous_commit}"'"'
else
previous_commit_json=null
fi

imageprefix=${name}-${buildid}
imageprefix="${name:?}"-"${buildid}"
# Make these two verbose
set -x
mkdir -p tmp/anaconda
img_base=tmp/${imageprefix}-base.qcow2
img_qemu=${imageprefix}-qemu.qcow2
/usr/lib/coreos-assembler/virt-install --dest=$(pwd)/${img_base} \
--create-disk --kickstart ${kickstart_input} --kickstart-out $(pwd)/tmp/flattened.ks \
--ostree-remote=${name} --ostree-stateroot=${name} \
--ostree-ref=${ref:-${commit}} --ostree-repo=${workdir}/repo \
--location ${workdir}/installer/*.iso --console-log-file $(pwd)/install.log \
--logs $(pwd)/tmp/anaconda
/usr/lib/coreos-assembler/gf-oemid $(pwd)/${img_base} $(pwd)/${img_qemu} qemu
/usr/lib/coreos-assembler/virt-install --dest="$(pwd)"/"${img_base}" \
--create-disk --kickstart "${kickstart_input}" --kickstart-out "$(pwd)"/tmp/flattened.ks \
--ostree-remote="${name}" --ostree-stateroot="${name}" \
--ostree-ref="${ref:-${commit}}" --ostree-repo="${workdir}"/repo \
--location "${workdir}"/installer/*.iso --console-log-file "$(pwd)"/install.log \
--logs "$(pwd)"/tmp/anaconda
/usr/lib/coreos-assembler/gf-oemid "$(pwd)"/"${img_base}" "$(pwd)"/"${img_qemu}" qemu
set +x
# make a version-less symlink to have a stable path
ln -s ${img_qemu} ${name}-qemu.qcow2
ln -s "${img_qemu}" "${name}"-qemu.qcow2

img_qemu_sha256=$(sha256sum ${img_qemu} | cut -f 1 -d ' ')
img_qemu_sha256=$(sha256sum "${img_qemu}" | cut -f 1 -d ' ')

build_timestamp=$(date -u --iso-8601=seconds)

Expand All @@ -204,26 +198,29 @@ cat > tmp/meta.json <<EOF
EOF
# Merge all the JSON; note that we want ${composejson} first
# since we may be overriding data from a previous build.
cat ${composejson} tmp/meta.json ${commitmeta_input_json} | jq -s add > meta.json
cat "${composejson}" tmp/meta.json "${commitmeta_input_json}" | jq -s add > meta.json

# Clean up our temporary data
rm tmp -rf
# Back to the toplevel build directory, so we can rename this one
cd ${workdir}/builds
cd "${workdir}"/builds
# We create a .build-commit file to note that we're in the
# middle of a "commit". This may be useful in the future
# for having things be transactional. If for example we
# were interrupted between the rename() and linkat() below,
# things would be inconsistent and future builds would fail
# on the `mv`.
touch .build-commit
mv -T ${tmp_builddir} ${buildid}
mv -T "${tmp_builddir:?}" "${buildid}"
Copy link
Copy Markdown
Member

@cgwalters cgwalters Nov 9, 2018

Choose a reason for hiding this comment

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

Hmm, why the explicit :? here? We're using set -u so that behavior applies everywhere right?

Copy link
Copy Markdown
Member

@cgwalters cgwalters Nov 9, 2018

Choose a reason for hiding this comment

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

(It doesn't hurt though, I'm fine leaving this as is)

Copy link
Copy Markdown
Member Author

@miabbott miabbott Nov 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

https://github.com/koalaman/shellcheck/wiki/SC2154

I used the guidance at the bottom:

If you know for a fact that the variable is set, you can use ${var:?} to fail if the variable is unset (or empty), or explicitly initialize/declare it with var="" or declare var.

Copy link
Copy Markdown
Member Author

@miabbott miabbott Nov 9, 2018

Choose a reason for hiding this comment

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

Ah, so if I have the shellcheck directive at the top of the file # shellcheck source=src/cmdlib.sh, this warning doesn't get thrown any more. Leaving the :? doesn't seem to hurt it either.

Copy link
Copy Markdown
Contributor

@sinnykumari sinnykumari Nov 9, 2018

Choose a reason for hiding this comment

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

Ah ok!

# Replace the latest link
ln -Tsfr "${buildid}" latest
# Update builds.json
prune_args=""
if [ ${SKIP_PRUNE} = 1 ]; then
prune_args="--insert-only ${buildid}"
# the variables passed to `prune_builds` end up single quoted and
# python treats them as literals, so we workaround this by duplicating
# the command ¯\_(ツ)_/¯
if [ "${SKIP_PRUNE}" == 1 ]; then
"${dn}"/prune_builds --workdir "${workdir}" --insert-only "${buildid}"
else
"${dn}"/prune_builds --workdir "${workdir}"
fi
${dn}/prune_builds --workdir ${workdir} ${prune_args}
rm .build-commit
10 changes: 6 additions & 4 deletions src/cmd-clean
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
#!/usr/bin/env bash
set -xeuo pipefail
set -euo pipefail

dn=$(dirname $0)
. ${dn}/cmdlib.sh
dn=$(dirname "$0")
# shellcheck source=src/cmdlib.sh
. "${dn}"/cmdlib.sh

print_help() {
cat 1>&2 <<'EOF'
Expand Down Expand Up @@ -44,11 +45,12 @@ if [ $# -ne 0 ]; then
exit 1
fi

set -x
# This has some useful sanity checks
prepare_build

# But go back to the toplevel
cd ${workdir}
cd "${workdir:?}"
# Note we don't prune the cache.qcow2 or the objects
# in the repo. If you want that, just rm -rf them.
rm -rf repo/refs/heads/* builds/* tmp/*
Expand Down
5 changes: 3 additions & 2 deletions src/cmd-fetch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
#!/usr/bin/env bash
set -euo pipefail

dn=$(dirname $0)
. ${dn}/cmdlib.sh
dn=$(dirname "$0")
# shellcheck source=src/cmdlib.sh
. "${dn}"/cmdlib.sh

print_help() {
cat 1>&2 <<'EOF'
Expand Down
Loading