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

Warn on stale build due to kernel changed #485

Merged
merged 6 commits into from
Nov 1, 2023
Merged

Warn on stale build due to kernel changed #485

Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c34308f
Use variables where already available
paulo-ferraz-oliveira Oct 26, 2023
47e1468
Allow for a more generic `warn` function
paulo-ferraz-oliveira Oct 26, 2023
cec1074
Save build options in a more readable fashion
paulo-ferraz-oliveira Oct 26, 2023
76e6cf5
Warn on stale build for different kernel release
paulo-ferraz-oliveira Oct 26, 2023
6372d76
Reuse functions and label them
paulo-ferraz-oliveira Oct 30, 2023
c0e7dc4
Act on review comment: try to prevent unwarranted edition
paulo-ferraz-oliveira Nov 1, 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
37 changes: 32 additions & 5 deletions kerl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,13 @@ error() {

warn() {
# $1: log entry
# $2: use 'noprefix' if you want to omit WARNING:

l=warn stderr "WARNING: $1"
if [ "$2" != "noprefix" ]; then
l=warn stderr "WARNING: $1"
else
l=warn stderr "$1"
fi
}

notice() {
Expand Down Expand Up @@ -985,6 +990,14 @@ fail_do_build() {
fi
}

uname_r_label() {
echo "uname -r"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we sure this is the right command on all of our platforms?? probably is - but for md5 and sed we had to detect platform before we used a command like this...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can check further. At the same time, I'm not sure:

  1. we're always using \ (sometimes we are) where commands are executed
    1.1. I can later pull request for this, maybe (?)
  2. we're using the sed options consistently (don't seem to be)
    2.1. I can later pull request for this, maybe (?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

uname should be POSIX, and -r is defined as "Write the current release level of the operating system implementation."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I could gather, uname -r is valid for Darwin, OpenBSD, FreeBSD, and a host of Linux systems.

Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. we're always using \ (sometimes we are) where commands are executed

I don't remember the exact commit where it was introduced (think it was some kind of shellcheck clean up???) but it is supposed to make it so the command is executed in its own environment without environment variables or something?

1.1. I can later pull request for this, maybe (?)

Yeah maybe? low priority tho

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't remember the exact commit where it was introduced

ShellCheck does not complain about this sort of stuff, AFAIK. If I remember correctly is was an actual bug somebody wanted to fix...

Yeah maybe? low priority tho

I have no real priorities (I'm kinda going, with kerl, with oldest issues first - if that's any type of priority order), but these should mostly be mechanical changes 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ref: #363

}

uname_r() {
eval "$(uname_r_label)"
}

_do_build() {
init_build_logfile "$1" "$2"
log_build_entry "*** $(date) - kerl build $1 ***"
Expand Down Expand Up @@ -1060,8 +1073,12 @@ _do_build() {

# Check to see if configuration options need to be stored or have changed
TMPOPT="${TMP_DIR}/kerloptions.$$"
echo "${CFLAGS:-}" >"$TMPOPT"
echo "$KERL_CONFIGURE_OPTIONS" >>"$TMPOPT"
{
echo "This is kerl's control file for build configuration."
jadeallenx marked this conversation as resolved.
Show resolved Hide resolved
echo "CFLAGS: ${CFLAGS:-}"
echo "KERL_CONFIGURE_OPTIONS: $KERL_CONFIGURE_OPTIONS"
echo "$(uname_r_label): $(uname_r)"
} >>"$TMPOPT"
SUM=$($MD5SUM "$TMPOPT" | cut -d ' ' -f "$MD5SUM_FIELD")
# Check for a .kerl_config.md5 file
if [ -e ./"$KERL_CONFIG_STORAGE_FILENAME".md5 ]; then
Expand All @@ -1078,8 +1095,8 @@ _do_build() {
fi
else
# no file exists, so write one
mv "$TMPOPT" .kerl_config
echo "$SUM" >.kerl_config.md5
mv "$TMPOPT" "$KERL_CONFIG_STORAGE_FILENAME"
echo "$SUM" >"$KERL_CONFIG_STORAGE_FILENAME".md5
paulo-ferraz-oliveira marked this conversation as resolved.
Show resolved Hide resolved
fi

# Don't apply patches to "custom" git builds. We have no idea if they will apply
Expand Down Expand Up @@ -1618,6 +1635,16 @@ do_install() {
ERL_TOP="$KERL_BUILD_DIR/$build_name/otp_src_$rel"
cd "$ERL_TOP" || exit_install
init_install_logfile "$rel" "$build_name"

prev_build_kernel_release=$(grep <"$ERL_TOP"/"$KERL_CONFIG_STORAGE_FILENAME" -o "^$(uname_r_label): \(.*\)\$" | sed -n "s|^$(uname_r_label): \(.*\)\$|\1|p")
if [ "$(uname_r)" != "$prev_build_kernel_release" ]; then
warn "this Erlang/OTP build appears to be stale. It was created with kernel release"
warn " '$prev_build_kernel_release' while currently your system's kernel release is" "noprefix"
warn " '$(uname_r)'." "noprefix"
warn " You should consider removing the build with 'kerl delete build ...' and" "noprefix"
warn " re-installing it with 'kerl build-install ...'" "noprefix"
fi
paulo-ferraz-oliveira marked this conversation as resolved.
Show resolved Hide resolved

if ! log_install_cmd "ERL_TOP=$ERL_TOP ./otp_build release -a $absdir && cd $absdir && ./Install $INSTALL_OPT $absdir"; then
show_install_logfile "install of Erlang/OTP $rel ($build_name), in $absdir, failed!"
exit_install
Expand Down