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

Update build.sh to error-out on bad $BUILD or $BIN #1882

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

Conversation

wilsonjholmes
Copy link

I tried to install to a directory that did not exist, and I got some unexpected behavior...

build.sh did not fail, it just continued to run and spew-out errors.

I added a -e after the bash sh-bang to help with catching issues sooner (portion of man page pasted below), and I changed readlink -f to realpath --canonicalize-existing (man page pasted also):

-e Exit immediately if a pipeline (which may consist of a single simple command), a list, or a com‐
pound command (see SHELL GRAMMAR above), exits with a non-zero status. The shell does not exit if
the command that fails is part of the command list immediately following a while or until keyword,
part of the test following the if or elif reserved words, part of any command executed in a && or
|| list except the command following the final && or ||, any command in a pipeline but the last, or
if the command's return value is being inverted with !. If a compound command other than a sub‐
shell returns a non-zero status because a command failed while -e was being ignored, the shell does
not exit. A trap on ERR, if set, is executed before the shell exits. This option applies to the
shell environment and each subshell environment separately (see COMMAND EXECUTION ENVIRONMENT
above), and may cause subshells to exit before executing all the commands in the subshell.

READLINK(1) User Commands READLINK(1)

NAME
readlink - print resolved symbolic links or canonical
file names

SYNOPSIS
readlink [OPTION]... FILE...

DESCRIPTION
Note realpath(1) is the preferred command to use for
canonicalization functionality.

Honestly, not sure if this would work on all POSIX-like systems that the project is targeting, but it built for me on Pop!_OS 22.04 LTS and klayout launches.

I am open to suggestions or learnings if realpath or -e is not suitable for some reason.

@klayoutmatthias
Copy link
Collaborator

Thanks for the patch.

Regarding "readlink" vs. "realpath", the situation is confusing. I found this post: https://unix.stackexchange.com/questions/136494/whats-the-difference-between-realpath-and-readlink-f - my takeaway is that "readlink" is the older, yet more portable solution.

But I do not fully understand the original problem: you can run "build.sh" with non-existing bin and build folders, using

build.sh -bin doesnotexist-bin -build doesnotexist-build ...

This works as the build script does a "mkdir" on the build folder and the creation of the bin folder is manages my the install step. Problems may arise if mkdir is not able to create the folder.

I like the "-e" option in bash, but my experience is that sometimes it errors out unintentionally - for example on "grep". I think is somewhat safer to specifically test the exit code of "mkdir", like

if ! mkdir -p $BUILD; then
  echo "*** ERROR: unable to create target folder"
  exit 1
fi

Matthias

@wilsonjholmes
Copy link
Author

wilsonjholmes commented Oct 6, 2024

"Problems may arise if mkdir is not able to create the folder."

Ahh, I believe this may have been my actual problem. If I recall correctly, I passed in some paths that my user did not have write permissions for, which resulted in some initially confusing breakage.

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