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

Modifying the install script to default to /opt/pret/ #63

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Modifying the install script to default to /opt/pret/ #63

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5252940
Modifying the install script to default to /opt/pret
furrysalamander Mar 13, 2023
ee1420f
Modified the script so that the tools folder isn't part of the defaul…
furrysalamander Mar 13, 2023
202cf10
Fixed double path separators (not that it matters)
furrysalamander Mar 13, 2023
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
65 changes: 40 additions & 25 deletions install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,34 +1,49 @@
#!/bin/sh
set -e
if [ "$1" != "" ]; then
if [ -d "$1" ]; then
mkdir -p $1/tools/agbcc
mkdir -p $1/tools/agbcc/bin
mkdir -p $1/tools/agbcc/include
mkdir -p $1/tools/agbcc/lib
cp agbcc $1/tools/agbcc/bin/
cp old_agbcc $1/tools/agbcc/bin/
cp agbcc_arm $1/tools/agbcc/bin/
cp -R libc/include $1/tools/agbcc/ #drop include, because we don't want include/include
cp ginclude/* $1/tools/agbcc/include/
cp libgcc.a $1/tools/agbcc/lib/
cp libc.a $1/tools/agbcc/lib/
echo "agbcc successfully installed!"

if [ "$1" = "-h" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if [ "$1" = "-h" ]; then
if [ "$1" = -h -o "$1" = --help ]; then

When parsing arguments as flags, it's appropriate to have a -- flag to end the list of argument flags, after which the rest is interpreted as positional arguments, and fail when an unknown flag is passed. That said, this is only for -h and I doubt anyone would install to "--help", but keep it in mind and consider using getopt in the future.

echo "Usage: install.sh PATH"
exit 0
fi

# If there's no installation directory specified, the install script will default to
# installing agbcc into /opt/pret/agbcc
if [ "$1" = "" ]; then
BASE_INSTALL_DIRECTORY=/opt/pret
INSTALL_DIRECTORY=$BASE_INSTALL_DIRECTORY
mkdir -p $INSTALL_DIRECTORY
else
BASE_INSTALL_DIRECTORY=$1
INSTALL_DIRECTORY=$BASE_INSTALL_DIRECTORY/tools
fi

# The BASE_INSTALL_DIRECTORY nomenclature is so that the check for the existence of the
# directory doesn't fail just because there's no tools directory yet.
if [ -d "$BASE_INSTALL_DIRECTORY" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

This will fail when /opt/pret doesn't exist, forcing people to mkdir /opt/pret before running the script.
This is a bit of a significant caveat, and I'd rather the script just create necessary directories itself.

Copy link
Member

@mid-kid mid-kid Mar 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

If it's imperative to check that a target directory exists, add a -p flag or similar to enable implicit directory creation.
Personally, I think it's good form for an installer to create directories automatically, and at worst provide a helpful message when the user doesn't have permissions to create a directory, and a useful message telling the user the location where the files were installed.

mkdir -p $INSTALL_DIRECTORY/agbcc
mkdir -p $INSTALL_DIRECTORY/agbcc/bin
mkdir -p $INSTALL_DIRECTORY/agbcc/include
mkdir -p $INSTALL_DIRECTORY/agbcc/lib
cp agbcc $INSTALL_DIRECTORY/agbcc/bin/
cp old_agbcc $INSTALL_DIRECTORY/agbcc/bin/
cp agbcc_arm $INSTALL_DIRECTORY/agbcc/bin/
cp -R libc/include $INSTALL_DIRECTORY/agbcc/ #drop include, because we don't want include/include
cp ginclude/* $INSTALL_DIRECTORY/agbcc/include/
cp libgcc.a $INSTALL_DIRECTORY/agbcc/lib/
cp libc.a $INSTALL_DIRECTORY/agbcc/lib/
echo "agbcc successfully installed!"
Comment on lines +23 to +34
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
mkdir -p $INSTALL_DIRECTORY/agbcc
mkdir -p $INSTALL_DIRECTORY/agbcc/bin
mkdir -p $INSTALL_DIRECTORY/agbcc/include
mkdir -p $INSTALL_DIRECTORY/agbcc/lib
cp agbcc $INSTALL_DIRECTORY/agbcc/bin/
cp old_agbcc $INSTALL_DIRECTORY/agbcc/bin/
cp agbcc_arm $INSTALL_DIRECTORY/agbcc/bin/
cp -R libc/include $INSTALL_DIRECTORY/agbcc/ #drop include, because we don't want include/include
cp ginclude/* $INSTALL_DIRECTORY/agbcc/include/
cp libgcc.a $INSTALL_DIRECTORY/agbcc/lib/
cp libc.a $INSTALL_DIRECTORY/agbcc/lib/
echo "agbcc successfully installed!"
install -v -Dm755 -t "$INSTALL_DIRECTORY/bin" agbcc old_agbcc agbcc_arm
install -v -Dm644 -t "$INSTALL_DIRECTORY/lib" libgcc.a libc.a
install -v -Dm644 -t "$INSTALL_DIRECTORY/include" ginclude/*
cp -vR libc/include "$INSTALL_DIRECTORY/" #drop include, because we don't want include/include
echo "agbcc successfully installed!"

More verbosity, and explicit file permissions

else
if [ -d "../$BASE_INSTALL_DIRECTORY" ]; then
echo "Target directory does not exist. Did you mean to do \"./install.sh ../$1\"?"
else
if [ -d "../$1" ]; then
echo "Target directory does not exist. Did you mean to do \"./install.sh ../$1\"?"
if case $BASE_INSTALL_DIRECTORY in ".."*) true;; *) false;; esac; then
echo "Target directory does not exist. If you aren't familiar with relative paths, make sure that agbcc and the repository are in the same directory, and run \"./install.sh $1\" again."
else
if case $1 in ".."*) true;; *) false;; esac; then
echo "Target directory does not exist. If you aren't familiar with relative paths, make sure that agbcc and the repository are in the same directory, and run \"./install.sh $1\" again."
if echo "$BASE_INSTALL_DIRECTORY" | grep -qE '^[^/]*.$'; then
echo "Target directory does not exist. You probably meant to do \"./install.sh ../$1\", but agbcc and $1 do not exist in the same directory. Check your spelling, make sure that the repository has been cloned, ensure that agbcc and the repository are in the same directory, and run \"./install.sh ../$1\" again."
else
if echo "$1" | grep -qE '^[^/]*.$'; then
echo "Target directory does not exist. You probably meant to do \"./install.sh ../$1\", but agbcc and $1 do not exist in the same directory. Check your spelling, make sure that the repository has been cloned, ensure that agbcc and the repository are in the same directory, and run \"./install.sh ../$1\" again."
else
echo "Target directory does not exist. Check your spelling, re-read the instructions, and try again."
fi
echo "Target directory does not exist. Check your spelling, re-read the instructions, and try again."
fi
fi
fi
else
echo "Usage: install.sh PATH"
fi