-
Notifications
You must be signed in to change notification settings - Fork 335
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
Add interim GitHub Actions #1123
Conversation
fb726a2
to
8817f8c
Compare
A better way may be something like $ make -j4 -k 2>errors.txt Then you can display that file (with a nice big banner or whatever) if there was any errors... plus you get all the errors front and centre. |
If we used kernel style make output, any errors/warnings would stand out much more... |
It was being done so that the second one prints errors without races. However, the same thing can be done by passing -Orecurse to make(1). And this makes the logs even more readable, since there's no racy output at all. Fixes: 97f79e3 ("CI: Make build logs more readable") Link: <shadow-maint#702> Link: <nginx/unit#1123> Acked-by: Iker Pedrosa <[email protected]> Cc: Andrew Clayton <[email protected]> Cc: Konstantin Pavlov <[email protected]> Cc: Dylan Arbour <https://github.com/arbourd> Signed-off-by: Alejandro Colomar <[email protected]>
It was being done so that the second one prints errors without races. However, the same thing can be done by passing -Orecurse to make(1). And this makes the logs even more readable, since there's no racy output at all. Fixes: 97f79e3 ("CI: Make build logs more readable") Link: <shadow-maint#702> Link: <nginx/unit#1123> Acked-by: Iker Pedrosa <[email protected]> Cc: Andrew Clayton <[email protected]> Cc: Konstantin Pavlov <[email protected]> Cc: Dylan Arbour <https://github.com/arbourd> Signed-off-by: Alejandro Colomar <[email protected]>
It was being done so that the second one prints errors without races. However, the same thing can be achieved by passing -Orecurse to make(1). And this makes the logs even more readable, since there's no racy output at all. Fixes: 97f79e3 ("CI: Make build logs more readable") Link: <shadow-maint#702> Link: <nginx/unit#1123> Acked-by: Iker Pedrosa <[email protected]> Cc: Andrew Clayton <[email protected]> Cc: Konstantin Pavlov <[email protected]> Cc: Dylan Arbour <https://github.com/arbourd> Signed-off-by: Alejandro Colomar <[email protected]>
On Tue, 13 Feb 2024 08:33:46 -0800 Dylan Arbour ***@***.***> wrote:
> On Buildbot we use whatever is shipped on that OS (with a notable exception of Node, which we fetch from nodesource repos). This helps us ensure our software is compatible with what is broadly used by respective OS users.
In a past life I worked on / with Ruby, Python and Node applications at scale, all with VMs. In each case we were using some variation of Debian or CentOS with specific versions of the runtimes selected to match the expectation from the developers. I personally have never relied on the default OS' version of a Python to run a production Python workload; Ruby, Node, etc.
My experience is sort of the opposite, we used the OS provided packages
where possible. But then I also detest that all these things also
bring along their own package management that completely bypasses the OS
package manager.
[...]
Lastly, we don't want to interrupt the release process. This implementation doesn't account for that. Because of the limitations of Actions default runners and Ubuntu, we aren't testing for Alpine or FreeBSD.
Not using 'actions' I think, but this is what I do for unit-wasm
```yaml
name: Builds
on:
push:
branches: main
paths:
- Makefile
- shared.mk
- 'examples/**'
- 'src/**'
- '.github/workflows/build_tests.yaml'
pull_request:
branches: main
paths:
- Makefile
- shared.mk
- 'examples/**'
- 'src/**'
- '.github/workflows/build_tests.yaml'
jobs:
# GitHub Currently only supports running directly on Ubuntu,
# for any other Linux we need to use a container.
fedora-rawhide:
runs-on: ubuntu-latest
container:
image: fedora:rawhide
steps:
- name: Install tools/deps
run: |
dnf -y install git wget clang llvm compiler-rt lld make
wasi-libc-devel cargo rust rust-std-static-wasm32-unknown-unknown
rust-std-static-wasm32-wasi wget -O-
https://github.com/WebAssembly/wasi-sdk/releases/download/wasi-sdk-20/libclang_rt.builtins-wasm32-wasi-20.0.tar.gz
| tar --strip-components=1 -xvzf - -C $(dirname $(clang
-print-runtime-dir))
- uses: ***@***.***
with:
fetch-depth: "0"
- name: make
run: make V=1 E=1 all
debian-testing:
runs-on: ubuntu-latest
container:
image: debian:testing
steps:
- name: Install tools/deps
run: |
apt-get -y update
apt-get -y install git curl wget wasi-libc make clang llvm lld
wget -O-
https://github.com/WebAssembly/wasi-sdk/releases/download/wasi-sdk-20/libclang_rt.builtins-wasm32-wasi-20.0.tar.gz
| tar --strip-components=1 -xvzf - -C $(dirname $(clang
-print-runtime-dir)) curl https://sh.rustup.rs -sSf | sh -s -- -y .
"$HOME/.cargo/env" rustup target add wasm32-wasi wget -O-
https://github.com/WebAssembly/wasi-sdk/releases/download/wasi-sdk-20/wasi-sysroot-20.0.tar.gz
| tar -xzf - -C ${RUNNER_TEMP}
- uses: ***@***.***
with:
fetch-depth: "0"
- name: make
run: |
. "$HOME/.cargo/env"
make WASI_SYSROOT=${RUNNER_TEMP}/wasi-sysroot V=1 E=1 all
```
That uses Fedora Rawhide and Debian Testing containers. But many many
others are available.
[...]
This would also be a very good thing to add, it check for whitespace
damage
```yaml
name: check-whitespace
# Get the repo with the commits(+1) in the series.
# Process `git log --check` output to extract just the check errors.
# Add a comment to the pull request with the check errors.
on:
pull_request:
types: [ opened, synchronize ]
jobs:
check-whitespace:
runs-on: ubuntu-latest
steps:
- uses: ***@***.***
with:
fetch-depth: 0
- name: git log --check
id: check_out
run: |
log=
commit=
while read dash etc
do
case "${dash}" in
"---")
commit="${etc}"
;;
"")
;;
*)
if test -n "${commit}"
then
log="${log}\n${commit}"
echo ""
echo "--- ${commit}"
fi
commit=
log="${log}\n${dash} ${etc}"
echo "${dash} ${etc}"
;;
esac
done <<< $(git log --check --pretty=format:"--- %h %s" ${{github.event.pull_request.base.sha}}..)
if test -n "${log}"
then
exit 2
fi
```
Just stick it in .github/workflows/check-whitespace.yaml, you can grab
it directly from
<https://raw.githubusercontent.com/nginx/unit-wasm/main/.github/workflows/check-whitespace.yaml>
|
It was being done so that the second one prints errors without races. However, the same thing can be achieved by passing -Orecurse to make(1). And this makes the logs even more readable, since there's no racy output at all. Fixes: 97f79e3 ("CI: Make build logs more readable") Link: <shadow-maint#702> Link: <nginx/unit#1123> Acked-by: Iker Pedrosa <[email protected]> Cc: Andrew Clayton <[email protected]> Cc: Konstantin Pavlov <[email protected]> Cc: Dylan Arbour <https://github.com/arbourd> Signed-off-by: Alejandro Colomar <[email protected]>
ce190e1
to
301728f
Compare
Hmm, something's gone wrong here (email address wise...)
They should probably also be squashed... |
This comment was marked as duplicate.
This comment was marked as duplicate.
37eb6a4
to
4360560
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is clearly Good Enough to be useful; let's merge now and refine later.
At your discretion, consider adding a richer commit message / subject line. It could be useful to have some of the caveats spelled out there (mainly, that this is an initial pass and not intended to be comprehensive; the emphasis is on providing a Good Enough smoke test for development purposes) |
This commit adds GitHub Actions configuration, running tests on pull-requests and master push changes. This change is meant to be a first-pass at our evolving CI processes. - Tests run in parallel per language for speed and isolation - Test matrix is composed by a string list of languages and versions - `setup-${language}` actions are preferred over base (and changing) versions from `ubuntu-latest` operating system A few caveats with the current setup: - Only tests on Ubuntu (no FreeBSD or Alpine) - Unpriviledged tests only - No core dumps available on failure
4360560
to
f9aa4fb
Compare
It was being done so that the second one prints errors without races. However, the same thing can be achieved by passing -Orecurse to make(1). And this makes the logs even more readable, since there's no racy output at all. Fixes: 97f79e3 ("CI: Make build logs more readable") Link: <shadow-maint#702> Link: <nginx/unit#1123> Acked-by: Iker Pedrosa <[email protected]> Cc: Andrew Clayton <[email protected]> Cc: Konstantin Pavlov <[email protected]> Cc: Dylan Arbour <https://github.com/arbourd> Signed-off-by: Alejandro Colomar <[email protected]>
It was being done so that the second one prints errors without races. However, the same thing can be achieved by passing -Orecurse to make(1). And this makes the logs even more readable, since there's no racy output at all. Fixes: 97f79e3 ("CI: Make build logs more readable") Link: <shadow-maint#702> Link: <nginx/unit#1123> Acked-by: Iker Pedrosa <[email protected]> Cc: Andrew Clayton <[email protected]> Cc: Konstantin Pavlov <[email protected]> Cc: Dylan Arbour <https://github.com/arbourd> Signed-off-by: Alejandro Colomar <[email protected]>
It was being done so that the second one prints errors without races. However, the same thing can be achieved by passing -Orecurse to make(1). And this makes the logs even more readable, since there's no racy output at all. Fixes: 97f79e3 ("CI: Make build logs more readable") Link: <#702> Link: <nginx/unit#1123> Acked-by: Iker Pedrosa <[email protected]> Cc: Andrew Clayton <[email protected]> Cc: Konstantin Pavlov <[email protected]> Cc: Dylan Arbour <https://github.com/arbourd> Signed-off-by: Alejandro Colomar <[email protected]>
That doesn't show the errors together with the commands that produced them. Plus, that doesn't solve the races. |
On Wed, 13 Mar 2024 09:38:12 -0700 Alejandro Colomar ***@***.***> wrote:
> A better way may be something like
>
> ```shell
> $ make -j4 -k 2>errors.txt
> ```
>
> Then you can display that file (with a nice big banner or whatever) if there was any errors... plus you get _all_ the errors front and centre.
That doesn't show the errors together with the commands that produced them. Plus, that doesn't solve the races.
This whole thing is pretty much moot now we have prettified output...
|
Not really. You're still subject to races. And redirecting stderr still leaves you without the corresponding So, think that the following may happen:
And if you redirect stderr, you may end up with stuff like the following in error.log:
(and even if you don't happen to see bad output from races, I think having the error lines without the corresponding |
On Wed, 13 Mar 2024 09:53:49 -0700 Alejandro Colomar ***@***.***> wrote:
So, think that the following may happen:
```
CC foo.o
some error line from compiling foo.o
CC bar.o
another error line from compiling foo.o
```
But the error itself will show where the problem lies...
|
Yeah, but if several error reports are intertwined due to a race, it will be hard to read each of them. Think of:
I see no reasons to avoid |
This pull-request adds GitHub Actions configuration that allows tests to run on pull-requests and
master
branches.Details
go-1.21
will run a job that runs Go tests against Go 1.21, etcgo-1.20
to the list would add an additional test for Go 1.20.test/test_${module}*
Caveats