Enhancement: Create GitHub CI/CD workflow and/or client side commit hooks

[SirWumpus]

Each push to GitHub should run the Makefile test target (to ensure we only ever push working code) as part of a CI workflow.

Unclear if a CD workflow should be added (never done one) to regenerate manifests, tarballs, snowballs, etc.

[xexyl]

Each push to GitHub should run the Makefile test target (to ensure we only ever push working code) as part of a CI workflow.

Unclear if a CD workflow should be added (never done one) to regenerate manifests, tarballs, snowballs, etc.

Some tests (via workflow) are done but as for running make test I don't think it's even possible in full. And the real one that needs to be verified is make prep but that would require a lot.

On the other hand one can add a commit hook for their own that runs prior to committing.

... yes I brought this up before hence how I know about these things. Though of course that was some time ago so maybe things have changed.

I guess we'll have to wait to see what Landon thinks.

[lcn2]

Each push to GitHub should run the Makefile test target (to ensure we only ever push working code) as part of a CI workflow.
Unclear if a CD workflow should be added (never done one) to regenerate manifests, tarballs, snowballs, etc.

Some tests (via workflow) are done but as for running make test I don't think it's even possible in full. And the real one that needs to be verified is make prep but that would require a lot.

Agree.

On the other hand one can add a commit hook for their own that runs prior to committing.

That might be useful, @xexyl. Would that hook run make prep or make release or some other test?

[xexyl]

Each push to GitHub should run the Makefile test target (to ensure we only ever push working code) as part of a CI workflow.

Unclear if a CD workflow should be added (never done one) to regenerate manifests, tarballs, snowballs, etc.

Some tests (via workflow) are done but as for running make test I don't think it's even possible in full. And the real one that needs to be verified is make prep but that would require a lot.

Agree.

On the other hand one can add a commit hook for their own that runs prior to committing.

That might be useful, @xexyl. Would that hook run make prep or make release or some other test?

Just as a reminder I did this once but you thought it might be the wrong time. Is it a good time now?

I can look and see if I still have what I did or else rewrite it.

BUT it might be possible to only have an example for people to use if they want to.

Let me know what you think.

[lcn2]

Each push to GitHub should run the Makefile test target (to ensure we only ever push working code) as part of a CI workflow.

Unclear if a CD workflow should be added (never done one) to regenerate manifests, tarballs, snowballs, etc.

Some tests (via workflow) are done but as for running make test I don't think it's even possible in full. And the real one that needs to be verified is make prep but that would require a lot.

Agree.

On the other hand one can add a commit hook for their own that runs prior to committing.

That might be useful, @xexyl. Would that hook run make prep or make release or some other test?

Just as a reminder I did this once but you thought it might be the wrong time. Is it a good time now?

I can look and see if I still have what I did or else rewrite it.

BUT it might be possible to only have an example for people to use if they want to.

Let me know what you think.

It seems like now is the time. The code is reasonable stable.

If you propose a pull request, we can try it out before we declare (i.e., tag) the current contents "version 1.2" and make it a formal release.

[xexyl]

Each push to GitHub should run the Makefile test target (to ensure we only ever push working code) as part of a CI workflow.

Unclear if a CD workflow should be added (never done one) to regenerate manifests, tarballs, snowballs, etc.

Some tests (via workflow) are done but as for running make test I don't think it's even possible in full. And the real one that needs to be verified is make prep but that would require a lot.

Agree.

On the other hand one can add a commit hook for their own that runs prior to committing.

That might be useful, @xexyl. Would that hook run make prep or make release or some other test?

Just as a reminder I did this once but you thought it might be the wrong time. Is it a good time now?

I can look and see if I still have what I did or else rewrite it.

BUT it might be possible to only have an example for people to use if they want to.

Let me know what you think.

It seems like now is the time. The code is reasonable stable.

If you propose a pull request, we can try it out before we declare (i.e., tag) the current contents "version 1.2" and make it a formal release.

Okay. Though like I said I don't know if it can be enforced.

Which rule should it run? Obviously not release but maybe prep?

[SirWumpus]

I suggest the CI workflow test as much as possible. A quick glance at the Makefile, I'd say targets: test, test-chkentry, check_man at the least.

Does make prep do all that?

[SirWumpus]

BTW my apologies if I put anyone's nose out of joint.

[xexyl]

BTW my apologies if I put anyone's nose out of joint.

Of course you didn't! Don't think about it. It's a good suggestion too.

It might be that one has to install it in their fork but I don't know. I am pretty sure that's how hooks work but maybe there's a way to do it so it's enforced.

[xexyl]

I suggest the CI workflow test as much as possible. A quick glance at the Makefile, I'd say targets: test, test-chkentry, check_man at the least.

Does make prep do all that?

Well make prep does quite a few things including running bug_report.sh (but with the option to delete the log file if no problems are detected - this is why I was asked to put it in iirc).

The release one cannot be put in because it forces one to have the right version of flex and bison but only people (Landon and me being the only ones .. at least so far?) working on the parser need that.

It's suggested that one runs make prep prior to committing though certainly we have neglected to do this only to cause a problem that had to be fixed.

I also am aware of a situation where the state of my fork let the prep process work but Landon got errors. I don't remember the details: that was quite some time back and lots has happened since then (not just here but other things in life of course).

But I don't really know much about GitHub workflows either.

I just got home from the doctor a short bit ago and I am going to turn the laptop on shortly. I can look into it I think though there are some other things I need to take care of first.

Please feel free to offer suggestions on how this might work!

[SirWumpus]

I'd prefer this as much of this as a GitHub workflow (vs a .git/hooks/pre-commit) so that pushes and pull-requests are always tested. Relying on each contributing developer to apply a pre-commit hook and not skip it (git commit -n) is taking a lot on trust.

[xexyl]

I'd prefer this as much of this as a GitHub workflow (vs a .git/hooks/pre-commit) so that pushes and pull-requests are always tested. Relying on each contributing developer to apply a pre-commit hook and not skip it (git commit -n) is taking a lot on trust.

It is indeed. The question is whether it will work. If it's run on GitHub it won't have access to all the tools needed.

There is a workflow but it doesn't run make prep or make test even and I am not sure how it could.

But if it's possible I don't know how. Do you have any idea?

Booting up laptop now and will take care of those other things then I will look at this.

[SirWumpus]

Well I just added to my https://github.com/SirWumpus/iocccsize repo a CI workflow, which is probably the simplest.

I'm now trying to add one to https://github.com/SirWumpus/post4, which wants OpenJDK 17+ if possible which is a little more complex. Add to the fact that the Ubuntu gcc used for the workflow complains about stuff I've not had issue with before (printf formatting- actually had issues and fixed, but now its even more pedantic and annoying and have to revisit it - I keep telling myself its all in the name of portablity).

[SirWumpus]

If it's run on GitHub it won't have access to all the tools needed.

It is possible to have tools added to the environment. I once had a to deal with Erlang testing.

Based on the setup-java example I've just been playing with, you should find one for parser tools, or tools in general. It will then require use: statement blocks and with: to control options. There should be some ready made related examples you can search for.

[xexyl]

If it's run on GitHub it won't have access to all the tools needed.

It is possible to have tools added to the environment. I once had a to deal with Erlang testing.

Based on the setup-java example I've just been playing with, you should find one for parser tools, or tools in general. It will then require use: statement blocks and with: to control options. There should be some ready made related examples you can search for.

Can you add tools that are part of the repo? They have to be compiled by the Makefile.

If that's possible that seems like a good idea; otherwise I don't know what to do. Is it? Did you find some documentation that can be looked at to do this? If there is documentation I can look into it and maybe figure it out.

Thanks!

[SirWumpus]

Would you like me to take this task on (since I've touch it a little in the past)? It should allow you to get on with other things and give me something else to do other than "proof documentation" and raise tickets.

[xexyl]

Would you like me to take this task on (since I've touch it a little in the past)? It should allow you to get on with other things and give me something else to do other than "proof documentation" and raise tickets.

If you'd like to you certainly could and it'd be helpful indeed.

The right one to run is make prep. You might try running it on your system first to see what it does. It's pretty extensive.

If you have any questions Landon or I can happily answer and if it proves to be a problem and you can link to some documentation I can tackle it later on.

Thanks!

[SirWumpus]

Right then, assign it to me and I'll get stuck in.

[xexyl]

Right then, assign it to me and I'll get stuck in.

I of course cannot do that. Landon?

[lcn2]

Just as a reminder I did this once but you thought it might be the wrong time. Is it a good time now?

As that time, the code was under significant development and did not pass tests. Moreover the test was under development as well. One could argue that the tests should have been ready from the start: that wasn't the case back then. :-)

Comments for @SirWumpus

make prep and make release

As far as doing "GitHub CI / CD workflow", @SirWumpus, we have the make prep and make release rule such that if it exit 0 then all is well and it it exit != 0 then something went wrong. The tree is left in a compiled state, so perhaps if all is well then a make clobber needs to run in order to restore the tree to a "just the source" state.

One difference between make prep differs from make release is that prep will note errors and fail at the end whereas release will exit at the first error. Both make rules are driven by test_ioccc/prep.sh, and so:

$ test_ioccc/prep.sh -h
usage: test_ioccc/prep.sh [-h] [-v level] [-V] [-e] [-o] [-m make] [-M Makefile] [-l logfile]

    -h              print help and exit
    -v level        flag ignored
    -V              print version and exit

    -e		    exit on first make action error (def: exit only at end)
    -o		    do NOT use backup files, fail if bison or flex cannot be used (def: use)
    -m make	    make command (def: make)
    -M Makefile	    path to Makefile (def: ./Makefile)
    -l logfile      write details of actions to logfile (def: send to stdout)

Exit codes:
     0   all OK
     1   -h and help string printed or -V and version string printed
     2	 command line error
     3	 Makefile not a readable file
     4	 could not make writable log file
 >= 10   some make action exited non-zero

prep.sh version: 1.0.1 2024-03-02

Another key difference is between make prep differs from make release is if -o is supplied to test_ioccc/prep.sh. Under release -o is supplied while under prep it is not.

And what is the deal about -o? We do not require someone to have the proper level of flex(1) or lex(1) NOR do we require someone to have proper level of bison(1) or yacc(1). In both cases a minimum version is required to process jparse/jparse.l and jparse/jparse.y. We pre-build the result so that if someone lacks proper level of flex(1) or lex(1) and/or lacks the proper level of bison(1) or yacc(1) the code will simply C compile the pre-built parser code.

Given that, make release only is useful when run from a "reference development platform" where, among other things, we have a reasonably up to date flex(1) or lex(1) and a reasonably up to date bison(1) or yacc(1).

Development vs. Build

There are other dependencies that "reference development platform" needs to use make release, such as a reasonably up to date version of bash(1), etc. We also require a GNU Make-style make command as well as bash.

Passing make release is NOT supported on all platforms: only on "development platforms" that meet the requirements as noted above, among other things.

Passing make release is only needed by mkiocccentry repo developers and not someone who wants to submit the IOCCC.

See also comment-2205419620.

On client side or server side test hooks

This brings up the question of what mkiocccentry repo developer needs to do to react to make prep or make release failure: a mkiocccentry repo developer would need to read the logs, to make the corrections and to try again.

What sort of context (OS release, platform, etc.) would the "GitHub CI / CD workflow" be run under? We could see an advantage in having a commit tested under some platforms as part of the workflow. And how would a mkiocccentry repo developer make use / access the logs that were generated? And what if the problem occurs on the GitHub for a platform that is not interested in being supported?

In general, what does a GitHub CI / CD workflow buy us?

[xexyl]

Just as a reminder I did this once but you thought it might be the wrong time. Is it a good time now?

As that time, the code was under significant development and did not pass tests. Moreover the test was under development as well. One could argue that the tests should have been ready from the start: that wasn't the case back then. :-)

Comments for @SirWumpus

In general, what does a GitHub CI / CD workflow buy us?

I of course said some of that. Indeed very few people need make release. And some of the other tools that make prep run might not be strictly necessary for everyone.

I wonder very much also what the workflow might buy us. If it is not thought worth having them after all I can get the hooks I wrote and maybe SUGGEST people use them but not require them. That's also a possibility I guess.

But I must go to sleep now. Can barely see. Good night!

[lcn2]

Unclear if a CD workflow should be added (never done one) to regenerate manifests, tarballs, snowballs, etc.

This repo does not have manifests, nor tarballs.

Perhaps you are thinking of some other repo?

[xexyl]

Unclear if a CD workflow should be added (never done one) to regenerate manifests, tarballs, snowballs, etc.

This repo does not have manifests, nor tarballs.

Perhaps you are thinking of some other repo?

I wish it had some snowballs though!

'I don't like this at all,' panted Cody just behind. 'Snow's all right on a fine morning, but I like to be in bed while it's falling. I wish this lot would go off to Hobbiton! Folk might welcome it there.'

Okay I'm not in Hobbiton and I replaced Sam with Cody but I'm in a part of the world that has awful temperatures but I'm not willing to say exactly where that is so it'll have to be Hobbiton :-) ... actually I haven't seen snow touch the ground here since the 1989-1990 winter or maybe it was the 1990-1991 winter ... and it was there for not even an hour and it was hardly any at all. I don't want it to snow but I'd certainly prefer colder weather than what we have now!

[xexyl]

And since I'm into 'The Ring Goes South' I might as well add another amusing thing Sam says:

'And it is no good going back while the storm holds,' said Aragorn. 'We have passed no place on the way up that offered more shelter than this cliff-wall we are under now.'

'Shelter!' muttered Sam. 'If this is shelter, then one wall and no roof make a house.'

.. okay I'll stop spouting LR references now :-)

[xexyl]

Okay last thing: I guess that 0ecac76 broke it. That's what seems to have changed and I know from the past that parallel builds do not work. I don't know why the file would do it. I would try it now but I must leave now. Sorry.

.. although it worked earlier today?

[lcn2]

Oh! I see something funny. Look at this:

 cpp/autobuilder: trying to run make -j 4 [current dir: /home/runner/work/mkiocccentry/mkiocccentry]

Parallel build does not work with our system. That might be why?

The -j 4 is probably a problem. The parallel make system is generally problematic and should be avoided.

[lcn2]

Okay last thing: I guess that 0ecac76 broke it. That's what seems to have changed and I know from the past that parallel builds do not work. I don't know why the file would do it. I would try it now but I must leave now. Sorry.

.. although it worked earlier today?

Correct: before that change we were using deprecated test code. So it doesn't surprise that the older code failed to find issues.

[SirWumpus]

So was this suggestion of implementing workflows for CI and umm CodeQL worth the investment? I know you're trying to push towards an official release, sorry if I caused a significant delay.

Didn't think it would take this long.

[xexyl]

So was this suggestion of implementing workflows for CI and umm CodeQL worth the investment? I know you're trying to push towards an official release, sorry if I caused a significant delay.

Didn't think it would take this long.

It was worth it Anthony so don't worry about that. It's better to find problems sooner.

.. I will reply to other comments later today or else tomorrow morning.

[lcn2]

With commit 1398372 we added .NOTPARALLEL: to several Makefiles.

As a result, it appears that the workflow was successful. ‼️

[SirWumpus]

That is wonderful.

[lcn2]

The codeql.yml did expose a problems that our dependencies were complex enough to confuse parallel make. So adding the .NOTPARALLEL: serializer was a helpful thing that codeql.yml provided.

[lcn2]

Thank you.

[xexyl]

The codeql.yml did expose a problems that our dependencies were complex enough to confuse parallel make. So adding the .NOTPARALLEL: serializer was a helpful thing that codeql.yml provided.

As soon as I saw that option in the build I knew it was the problem. Maybe I should have reported it earlier on.

[xexyl]

That is wonderful.

For what it's worth: the reason I replied is it seemed like you might have been questioning or doubting yourself with it (since it seemed to cause a problem[0]) and I know from personal experience what that kind of thing can feel like.

I am not saying that is it but I wanted to make sure that you knew that I think it's a great thing you did and I really appreciate it too. I know Landon does also.

I will actually use this when the jparse repo is populated as well!

[0] I actually have experienced this kind of feeling a lot and I know it can feel quite awful so when I saw it I wanted to make sure to reply even though I was in the middle of something - just in case.

Either way it's an excellent improvement and I really appreciate that you did it. I wouldn't have known how to do it and I was able to work on other things because of your help.

I hope you weren't feeling this way of course but I felt I had to address it just in case.

It's bedtime here. Hope you have a great night, the both of you!

[xexyl]

So was this suggestion of implementing workflows for CI and umm CodeQL worth the investment? I know you're trying to push towards an official release, sorry if I caused a significant delay.

Didn't think it would take this long.

As for the last part: I can even take the blame for not addressing the fact that parallel building does not work. I never anticipated this would happen. For me I just figured 'okay - don't use -j !'

Definitely not your fault anyway.

Well sleep time for me. Good night!

[xexyl]

As far as niggle it reminded me of Leaf by Niggle, one of the few (relatively speaking) books by Tolkien I do not have. It's an interesting one in that it's allegorical which he was not really in favour of and did not usually do. The Letter they cite in that link is indeed legitimate as far as I remember: and I have a rather good memory of it. There are a few letters, at the least, about the story, iirc.

Anyway it's good that this issue is solved. I hope to continue with where I left off a few hours ago though I might have to pause again not too long after that. It's been a frustrating day but I hope to finish that task and that would mean that issue could be closed in the other repo.

[xexyl]

With xexyl/jparse@0c9ba66 I just successfully set this up in the jparse repo for when the code is actually populated! It works great. I just had to put in the Makefile some rules that would simply exit 0 and that way it would work. All good. When the code is actually populated then we can worry about updating those rules to 'do the right thing' but for now it's all good.

Thanks again Anthony!

UPDATE 0

dependabot.yml added and disabled parallel builds too and all seems good now (the former did not work before maybe because of no actions defined - but not sure).

[xexyl]

After I fixed an issue with txzchk wrt the -q option (not being honoured if issues are found - and I found another issue in make prep wrt this but that's not fixed yet and it is not clear if it needs to be) it dawned on me that maybe we could improve this a bit.

Would it be good to have a slow version of make release so that it force builds the parser? We would have to make sure that the bison and flex installed in GitHub are sufficient but assuming they are it would be beneficial .. at least without thinking deeply about it, it would.

I can do this if you think this is a good idea. What do you think, Landon?

[SirWumpus]

Couldn't you simple remove the generated files before make slow_prep to force them to regenerate?

[SirWumpus]

I would have added them as dependencies to the prep and slow_prep targets.

[xexyl]

Couldn't you simple remove the generated files before make slow_prep to force them to regenerate?

Well there are two sets of files. One set is the backup files - that which we generate in case someone cannot use flex/bison. That is what I am referring to.

I would have added them as dependencies to the prep and slow_prep targets.

Well release will fail if the parser cannot be generated as it uses the -o option which fails if flex or bison cannot be used or fail for some reason - but the others will use the backup files. So the idea is that this forces the build of the backup files in case one of the .l/.y files are modified but the backup files are not rebuilt.

It's been a long time and I'm too tired even to check but I'm pretty sure that the Makefile tries to run flex/bison but if it cannot then the backup files are used.

Does that make sense? If not I can try again with my apologies. It was a rough night.

[xexyl]

So .. what do you think @lcn2? Should I update the workflow to install flex and bison and then run make release (maybe adding a slow version to not write to a log file so it can be seen in GitHub)?

It seems like it might be a good idea but if you can think of some reason it s should not be done I will not.

[lcn2]

So .. what do you think @lcn2? Should I update the workflow to install flex and bison and then run make release (maybe adding a slow version to not write to a log file so it can be seen in GitHub)?

It seems like it might be a good idea but if you can think of some reason it s should not be done I will not.

That sounds like a reasonable idea.

[xexyl]

So .. what do you think @lcn2? Should I update the workflow to install flex and bison and then run make release (maybe adding a slow version to not write to a log file so it can be seen in GitHub)?
It seems like it might be a good idea but if you can think of some reason it s should not be done I will not.

That sounds like a reasonable idea.

Okay to not cause problems I'll wait on the other pull request and then look into it. Hopefully that one is taken care of now though.

[xexyl]

However ... slow_release rule just added so that this can be done easily. I'll look at the workflow next but not commit until that other one is resolved.

[xexyl]

Okay the work for this is done (unless there is an error in the test.yml file but I would look at actions before opening a pull request .. or trying to) but until the other pull request is resolved I will not open a new one due to GitHub's issue with multiple pull requests if one is closed or problematic.

[xexyl]

Perhaps this will answer the problem of why the workflows are not running automatically ? Or maybe better put how to fix it so they will? I'm not sure: I didn't read it in detail but it talks about approving them (that's the entire point of it) and maybe automating it. Oddly they do run automatically in the other repo so why not this one?

[SirWumpus]

Ok. Reading workflow approval if there are changes made to a workflow submitted in a pull-request, then the repo owner must approve the changes before they will run; its a security thing against someone submitting something naughty within the workflow.

The repo. owner can "configure workflow approval requirements", see above link as to where this is done.

This is something @lcn2 will have to address. Have you offered him any chocolate frosted sugar bombs lately?

---

Make your own Chocolate Frosted Sugar Bombs

https://wonderlandrecipes.com/2018/08/23/chocolate-frosted-sugar-bombs/

[xexyl]

Ok. Reading workflow approval if there are changes made to a workflow submitted in a pull-request, then the repo owner must approve the changes before they will run; its a security thing against someone submitting something naughty within the workflow.

The repo. owner can "configure workflow approval requirements", see above link as to where this is done.

That was the impression I got.

This is something @lcn2 will have to address. Have you offered him any chocolate frosted sugar bombs lately?

Hahaha. No but my mum told me to tell him that if he was round here she'd make her famous Double-layered Chocolate Fudge Cake in 2020/ferguson1: the best chocolate cake ever!

Make your own Chocolate Frosted Sugar Bombs

https://wonderlandrecipes.com/2018/08/23/chocolate-frosted-sugar-bombs/

I guess that Landon would want it enabled without having to entice him with chocolate: esp as he made an enquiry of how to make it happen and because the other repo does it already .. but it's a nice idea I guess. I mean who wouldn't want chocolate ? :-) .. quite a few people but there might be something wrong with them :-) (though it is possible there are some 'things wrong with me' and I love chocolate so maybe it's the other way round? :-) ).

[SirWumpus]

Well its question of how much @lcn2 trusts us. Can he enable it by user, group, or world permissions? I suspect it might be an all or nothing option, which means being very careful if new contributors come along.

[xexyl]

Well its question of how much @lcn2 trusts us. Can he enable it by user, group, or world permissions? I suspect it might be an all or nothing option, which means being very careful if new contributors come along.

Hmm .. but wouldn't it depend on if the workflow was installed? Or perhaps he can enable on push only but the other one, on pull request, not doing it? I don't know.