Skip to content

Commit

Permalink
Fix/improve output on errors (#482)
Browse files Browse the repository at this point in the history 
* Remove return/exit distinctions where not used

We don't document exit code
We sometimes don't care for the return of a function
(e.g. 0 is the same as 1, though 1 indicates to the
developer that something should potentially be handled)

As for the change to list_add I don't actually see
how it could fail (attaching to a file, or creating one?)
and if it does is _very_ unexpected situations we
want it to fail louder

* Update .kerl/otp_releases inplace

This allow us to, at the same time:

* create the file
* return from inner functions, for exiting purposes
* writing to stderr
  * without the output being consumed by sub-shells

* Fail louder

In certain situations, I was getting execution stopping
silently (e.g. 404 on curl calls) because we're using
shortcuts such as || true, and || return, or even || exit
without prior error messages, which this commit attempts
to fix

* Error out on assertion site, not call site

When it's clear to do so, instead of bubbling up
return values until we're ready to exit

* Use less return/exit indirections

Reduces the number of functions

assert_ is ok to use, but then we have scattered exit
vs. return which makes it harder to reason on

Also, we move `assert_build_name_unused` to
`is_build_name_used` to avoid double negation

* Prevent return+exit in same function/scope

If the function is exiting we call exit
If it's returning we prefer return (and exiting higher up)

* Simplify `kerl active`

Previous changes allow us to prevent two calls to
`get_active_path` while making sure:

* we exit on error
* always continue to function calls otherwise
  (we reduce a nesting level with this change)

* Add to current execution logging

* Centralize curl calls to ease debug logging

* Act on self-review: name flag more explicitly

* Act on self-review: add README note on how otp_releases is built

* Act on self-review: don't assume files aren't somehow protected

Prefer failure with controlled messages

* Act on self-review: ease maintenance of do_build function return

We always show_build_logfile with a message
  then autoclean with a file "pointer"

With a function it'll be easier to not forget how to
do cleanup, if the function changes in the future

* Act on local tests: silence tee

* Act on self-review: be consistent in punctuation

* Act on self-review: run all CI elements with KERL_DEBUG=yes

* Act on local test results: don't output ".:" when outputting errors

* Act on CI results: truthy value should be one of [false, true]

* Act on CI results: don't have CI fail with a new exit code

Due to the recent changes we started exiting with 1 on
_kerl status_ when no installation is active

* Act on CI results: prevent "indication of CI" from breaking script

* Act on CI results: validate activation in a correct shell scope

* Act on CI results: choose warn instead error, sometimes

I think errors should always exit with a non-0 status

... but you might not always show an error when you exit
with a non-0 status (this is context dependent)

* Act on review comment: Kerl > `kerl`

* Act on review comment: all _curl calls are GETs for now
  • Loading branch information
@paulo-ferraz-oliveira
paulo-ferraz-oliveira committed Oct 26, 2023
1 parent f6aaa94 commit 3dc0383
Show file tree
Hide file tree
Showing 3 changed files with 235 additions and 140 deletions.
37 changes: 23 additions & 14 deletions .github/workflows/ci.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ name: CI
'on': [push, pull_request]
env:
ERLC_USE_SERVER: true
KERL_DEBUG: 'yes'
jobs:
ci:
name: CI OTP ${{matrix.otp_vsn}}, on ${{matrix.os}}
Expand Down Expand Up @@ -60,24 +61,28 @@ jobs:
run: |
echo "OpenSSL is $(openssl version)"
export MAKEFLAGS=-j$(($(nproc) + 2))
if ! KERL_DEBUG=yes ./kerl build ${_KERL_PREFIX_GIT} \
${_KERL_PREFIX_GIT_TARGET} \
"${_KERL_VSN}" \
"${_KERL_VSN}"; then
if ! ./kerl build ${_KERL_PREFIX_GIT} \
${_KERL_PREFIX_GIT_TARGET} \
"${_KERL_VSN}" \
"${_KERL_VSN}"; then
## Print build log if it fails
cat ~/.kerl/builds/*/*.log
exit 1
fi
# yamllint disable rule:line-length
- name: Install chosen version
run: KERL_DEBUG=yes ./kerl install "$_KERL_VSN" "install_$_KERL_VSN"
- name: Check installation status
run: ./kerl status
run: ./kerl install "$_KERL_VSN" "install_$_KERL_VSN"
- name: Check installation status (pre- activation)
run: ./kerl status || exit 0
- name: Validate installation
run: |
source $(./kerl path install_"$_KERL_VSN")/activate
erl -s crypto -s init stop
erl_call
- name: Check installation status (post- activation)
run: |
source $(./kerl path install_"$_KERL_VSN")/activate
./kerl status
- name: Test KERL_RELEASE_TARGET
# yamllint disable rule:line-length
run: |
Expand Down Expand Up @@ -122,19 +127,23 @@ jobs:
- name: Test build+install chosen version
run: |
export MAKEFLAGS="-j$(getconf _NPROCESSORS_ONLN)"
if ! KERL_DEBUG=yes ./kerl build-install ${_KERL_PREFIX_GIT} \
${_KERL_PREFIX_GIT_TARGET} \
"${_KERL_VSN}" \
"${_KERL_VSN}" \
"$PWD/build-install_${_KERL_VSN}"; then
if ! ./kerl build-install ${_KERL_PREFIX_GIT} \
${_KERL_PREFIX_GIT_TARGET} \
"${_KERL_VSN}" \
"${_KERL_VSN}" \
"$PWD/build-install_${_KERL_VSN}"; then
## Print build log if it fails
cat ~/.kerl/builds/*/*.log
exit 1
fi
- name: Check installation status (build+install)
run: ./kerl status
- name: Check installation status (pre- activation)
run: ./kerl status || exit 0
- name: Validate installation (build+install)
run: |
source $(./kerl path build-install_"${_KERL_VSN}")/activate
erl -s crypto -s init stop
erl_call
- name: Check installation status (post- activation)
run: |
source $(./kerl path build-install_"${_KERL_VSN}")/activate
./kerl status
10 changes: 5 additions & 5 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ You can also install directly from a raw Git repository by using the

List the available releases:

<!-- markdownlint-disable MD007 # line-length -->
```console
$ kerl list releases
17.5.6.10
Expand All @@ -103,7 +102,6 @@ Run './kerl update releases' to update this list.
Run './kerl list releases all' to view all available releases.
Note: * means "currently supported".
```
<!-- markdownlint-enable MD007 # line-length -->

Pick your choice and build it:

Expand Down Expand Up @@ -257,11 +255,11 @@ $ kerl_deactivate
It is possible to build Erlang/OTP from a GitHub fork, by using the `KERL_BUILD_BACKEND=git` and
setting `OTP_GITHUB_URL` to the URL of the fork. For example, to build `<orgname>'s` Erlang/OTP fork:

<!-- markdownlint-disable MD007 # line-length -->
```console
$ export KERL_BUILD_BACKEND=git
$ export OTP_GITHUB_URL='https://github.com/<orgname>/otp'
$ KERL_INCLUDE_RELEASE_CANDIDATES=yes kerl update releases
Getting releases from GitHub...
The available releases are:
24.0-rc1 *
24.0-rc1.1-orgname *
Expand All @@ -276,7 +274,9 @@ The available releases are:
...
26.0.2 *
```
<!-- markdownlint-enable MD007 # line-length -->

**Note**: this list, kept in a file managed by `kerl`, is different depending on the build backend
you use.

From here (provided the `KERL_BUILD_BACKEND` and `OTP_GITHUB_URL` variables remain in place), it is
possible to use `kerl` as before:
Expand Down Expand Up @@ -362,7 +362,7 @@ Directory in which `kerl` will clone Git repositories for building.
#### `KERL_CHECK_BUILD_PACKAGES`

Default: yes (Enabled)
Kerl will try to probe your Linux distro for build-required packages logging
`kerl` will try to probe your Linux distro for build-required packages logging
where the probes fail. You can turn off this behaviour by setting the
environment variable to something other than "yes".

Expand Down

0 comments on commit 3dc0383

Please sign in to comment.