add github actions #195

N-R-K · 2021-12-23T00:21:06Z

this runs both default and minimal build with gcc and clang.
tcc seems to fail at link time for some reason.

I'm not familiar with github actions/workflow, simply hacked this together from the provided template. Seems to be working fine, here's some example:

https://github.com/N-R-K/nsxiv/runs/4614439272?check_suite_focus=true
(minimal build failing due to EXIT_SUCCESS not being defined)
fix: broken compilation when using HAVE_LIBFONTS=0 N-R-K/nsxiv#2
(PR fixing it)

this runs both default and minimal build with gcc and clang. tcc seems to fail at link time for some reason.

N-R-K · 2021-12-23T00:21:24Z

@TAAPArthur Any clue on why tcc is failing at linking?

TAAPArthur · 2021-12-23T00:23:16Z

@TAAPArthur Any clue on why tcc is failing at linking?

I'll take a look. Without looking, I'd guess because it is not to using a recent tcc version. The last "stable" release is really old and behind the git version.

TAAPArthur · 2021-12-23T00:40:35Z

Yeah it looks like 0.9.27-8 is used. Probably should just build tcc from mob (or some recent hash) if we want to use tcc.

Maybe something like

git clone git://repo.or.cz/tinycc.git
( cd tinycc && ./configure && make)
CC=$PWD/tinycc/tcc OPT_DEP_DEFAULT=1 make

Or of course we could find some binary somewhere.

Can confirm that tcc can build nsixv

N-R-K · 2021-12-23T00:57:43Z

Now it fails compilation and says something about not finding stdarg.h. I'm assuming it's environment related issue: https://github.com/N-R-K/nsxiv/runs/4613227345?check_suite_focus=true

edit: fixed in b39ae9f

N-R-K · 2021-12-23T03:51:49Z

Wonder if someone here is familiar with github actions and can take a look into this for best practices and whatnot.
@kdkasad Giving a ping since I recall you've mentioned github actions somewhere before.

fixes all -Wshadow related warnings (on gcc). this would allow us to use `-Wshadow` in github workflow (nsxiv#195). i've thought about adding `-Wshadow` to our Makefile as well, but decided against it to keep the Makefile CFLAGS barebore/minimal.

This reverts commit a12e10b. tcc will fail with -Werror due to a false positive: > commands.c:66: error: function might return no value: 'cg_quit'

kdkasad

I'm not very well-versed with GitHub Actions but I would assume that the building/installing TinyCC could be separated into a different step from that of building nsxiv.

It seems like that could even be extracted to a separate job, so that the building of TinyCC only happens once, then the full-build and minimal-build jobs run in parallel after that completes.

So far I've just added some comments on usage of commands, not the structure of the actions themselves.

Edit: It looks like we can even cache dependencies so we don't have to rebuild TinyCC and/or reinstall the debian packages every time: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows

.github/workflows/build.yml

N-R-K · 2021-12-25T01:10:00Z

@kdkasad Thanks for the review, I've pushed the --quiet change.

As for the cache, I'll look into it in a bit. But without looking, on our minimal-build we're explicitly uninstalling some optional deps to make sure everything compiles/links fine without the optional headers/deps being installed. So that might require some extra fiddling with the cache to get it to work, I presume.

N-R-K · 2021-12-25T13:37:17Z

Thinking again, since our build process is rather fast (25 seconds) I'm fine leaving things be and not bother with caching.

fixes all -Wshadow related warnings (on gcc). this would allow us to use `-Wshadow` in github workflow (nsxiv#195). i've thought about adding `-Wshadow` to our Makefile as well, but decided against it to keep the Makefile CFLAGS barebore/minimal.

N-R-K · 2021-12-27T14:17:59Z

Unless someone wants to bother with caching, I think this PR should be ready to be merged.

mamg22 · 2021-12-28T22:22:22Z

The first build failed, failing when executing actions/checkout@v2. It seems to be an issue with the action settings in the repo limiting what actions can be run, as indicated in this question.

N-R-K · 2021-12-29T10:54:11Z

I've enabled "allow all actions" in both the repo and the org. The workflow seems to be running now, though it failed at cloning tcc. #194

this runs both default and minimal build with gcc, clang and tcc.

N-R-K · 2021-12-29T11:17:17Z

The workflow fails on my repo as well,

Seems like the Tcc repo isn't reliable. I've switched to a github mirror, cleaned up the commit msg and forced push the changes.

N-R-K · 2021-12-29T11:28:44Z

Now everything is working as expected: https://github.com/nsxiv/nsxiv/runs/4658052056?check_suite_focus=true

The minimal build fails, but that's expected due to #194

mamg22 · 2021-12-29T11:47:20Z

Regarding re-triggering checks, there's a button in the checks tab of a PR that allows running all jobs again, which seems better than closing and reopening.

N-R-K · 2021-12-29T11:50:39Z

there's a button in the checks tab of a PR that allows running all jobs again

That only appears when a workflow has failed afaik. The workflow never ran on that PR to begin with, so had to close and reopen.

fixes all -Wshadow related warnings (on gcc). this would allow us to use `-Wshadow` in github workflow (nsxiv#195). i've thought about adding `-Wshadow` to our Makefile as well, but decided against it to keep the Makefile CFLAGS barebore/minimal.

fixes all -Wshadow related warnings (on gcc). this would allow us to use `-Wshadow` in github workflow (#195). i've thought about adding `-Wshadow` to our Makefile as well, but decided against it to keep the Makefile CFLAGS barebore/minimal.

add github actions

c0f535e

this runs both default and minimal build with gcc and clang. tcc seems to fail at link time for some reason.

N-R-K added the workflow Improvements or additions to workflow label Dec 23, 2021

fix tcc build

b39ae9f

N-R-K force-pushed the my_actions branch from ba939de to b39ae9f Compare December 23, 2021 03:07

N-R-K added 3 commits December 23, 2021 10:21

explicitly require xserver and libexif

f6257fb

use 2 jobs instead of 6

52801eb

add -Werror to CFLAGS

a12e10b

N-R-K mentioned this pull request Dec 24, 2021

Add additional warning flags in github actions #197

Merged

N-R-K added 3 commits December 24, 2021 12:46

Revert "add -Werror to CFLAGS"

3416372

This reverts commit a12e10b. tcc will fail with -Werror due to a false positive: > commands.c:66: error: function might return no value: 'cg_quit'

silence some non-useful output

ae52cfd

slience more non-useful info, add some useful ones

e11180a

kdkasad reviewed Dec 24, 2021

View reviewed changes

.github/workflows/build.yml Outdated Show resolved Hide resolved

.github/workflows/build.yml Outdated Show resolved Hide resolved

N-R-K added 2 commits December 25, 2021 07:06

use git clone --quiet

e161843

remove newlines

4a1f05b

put core deps on single line

21d9cfe

cosmetic change

d95c03f

TAAPArthur approved these changes Dec 27, 2021

View reviewed changes

prefer depth 1 clone

6436692

mamg22 approved these changes Dec 28, 2021

View reviewed changes

mamg22 merged commit ad7ccbf into nsxiv:master Dec 28, 2021

N-R-K deleted the my_actions branch December 29, 2021 10:53

N-R-K added a commit that referenced this pull request Dec 29, 2021

add github actions (#195)

1c8ea41

this runs both default and minimal build with gcc, clang and tcc.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add github actions #195

add github actions #195

N-R-K commented Dec 23, 2021 •

edited

Loading

N-R-K commented Dec 23, 2021

TAAPArthur commented Dec 23, 2021

TAAPArthur commented Dec 23, 2021

N-R-K commented Dec 23, 2021 •

edited

Loading

N-R-K commented Dec 23, 2021

kdkasad left a comment •

edited

Loading

N-R-K commented Dec 25, 2021

N-R-K commented Dec 25, 2021

N-R-K commented Dec 27, 2021

mamg22 commented Dec 28, 2021

N-R-K commented Dec 29, 2021

N-R-K commented Dec 29, 2021

N-R-K commented Dec 29, 2021

mamg22 commented Dec 29, 2021

N-R-K commented Dec 29, 2021

add github actions #195

add github actions #195

Conversation

N-R-K commented Dec 23, 2021 • edited Loading

N-R-K commented Dec 23, 2021

TAAPArthur commented Dec 23, 2021

TAAPArthur commented Dec 23, 2021

N-R-K commented Dec 23, 2021 • edited Loading

N-R-K commented Dec 23, 2021

kdkasad left a comment • edited Loading

Choose a reason for hiding this comment

N-R-K commented Dec 25, 2021

N-R-K commented Dec 25, 2021

N-R-K commented Dec 27, 2021

mamg22 commented Dec 28, 2021

N-R-K commented Dec 29, 2021

N-R-K commented Dec 29, 2021

N-R-K commented Dec 29, 2021

mamg22 commented Dec 29, 2021

N-R-K commented Dec 29, 2021

N-R-K commented Dec 23, 2021 •

edited

Loading

N-R-K commented Dec 23, 2021 •

edited

Loading

kdkasad left a comment •

edited

Loading