Support riscos build service #16
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cumulative merge of changes from Develop to Main repo
The code had only been compiled properly for GCC and was known to not work on Norcroft. This change introduces a Makefile (using my own makefile system, which will be updated later) and the necessary code changes to allow it to build on Norcroft. Specifically... * We use TCPIPLibs variable to find the network libraries. This makes it easier to include things in the exact same way on GCC and Norcroft. * Code is modified to work with pre-C99 format variables. My compiler doesn't support them, and requiring C99 format requires the developer to spend (more) money to get the later compiler. * Magic packet sending has been moved to a separate function because of this, to isolate the parsing from the packet sending. * Bug in leaking the socket due to not closing it is now fixed.
The standard style of VersionNum file can be used to give a programatically extractable version number in most tools. This is common in most RISC OS sources in some form. Tools such as the old RISC OS 'srccommit' tool, my own 'commit' tool, and more recently the 'riscos-vmanage' tool all use this format. The syntax output has been updated to match RISC OS tools better, including correcting the capitalisation of the tool.
The RISC OS build system can build most things, given a suitable set of scripts. Here we invoke the 'MkROBuild' command which will build things using the toolchain on the RISC OS build system. The artifacts generated are in the usual format, allowing users to copy the tool into their library as they see fit.
|
I'm not sure why it hasn't triggered the CI run, but maybe it's because the twitter notify failed? |
|
Hi Charles,Thanks a lot for the work and the info. Normally the twitter action works fine, but I believe the PR was made to merge directly to main, while normally we do PR to merge to develop, that may be why the action has failed.My apologies, I should have mentioned that. Let me see if I can disable the actions we already have and then I’ll resync develop from main.I am not home at the moment, so it will take a bit, sorry for making you wait.- PaoloOn 12 Dec 2022, at 11:04, Charles Ferguson ***@***.***> wrote:
I'm not sure why it hasn't triggered the CI run, but maybe it's because the twitter notify failed?
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
|
Hi again,
Ok so twitter sync workflow is disabled now, so you should not have any issue from it.
I also see that your’ workflow is still not it, which is probably why it did not trigger?
Again thank you so much for your help, cheers,
- Paolo
… On Dec 12, 2022, at 12:03 PM, P.Zaino @ ZFP ***@***.***> wrote:
Hi Charles,
Thanks a lot for the work and the info. Normally the twitter action works fine, but I believe the PR was made to merge directly to main, while normally we do PR to merge to develop, that may be why the action has failed.
My apologies, I should have mentioned that. Let me see if I can disable the actions we already have and then I’ll resync develop from main.
I am not home at the moment, so it will take a bit, sorry for making you wait.
- Paolo
> On 12 Dec 2022, at 11:04, Charles Ferguson ***@***.***> wrote:
>
>
>
> I'm not sure why it hasn't triggered the CI run, but maybe it's because the twitter notify failed?
>
> —
> Reply to this email directly, view it on GitHub <#16 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACDKMEOZYEFQNLAMF3KZ6PTWM4BEVANCNFSM6AAAAAAS3K2YAA>.
> You are receiving this because you are subscribed to this thread.
>
|
|
Oh sorry, I'd not noticed it should have been against develop. I can rebase to go against develop instead if that's helpful. As for the pipeline... I'm pretty sure I /had/ included the workflow. I shall investigate. |
The workflow isn't very specialised - it has been copied from the template repository with just the names changed to allow it to build properly. However, as we're focusing on pull requests, the conditions have been updated so that it triggers on the pull request not just on the branch - this is important as otherwise it wouldn't trigger when we create a PR from a fork.
gerph
force-pushed
the
support-riscos-build-service
branch
from
16d72a2
to
6c0a776
Compare
|
Ah-ha... spotted it. It was because I had said it only triggers on pushes to branches, but of course a PR isn't a branch push (strictly), so I have to use the
Not sure if that means that it's going to do that approval thing on every push though. |
|
Ok approved and ran it (sorry couldn't wait!) looks like it worked!!! 😄 |
3 tasks
|
@gerph , Sorry, had a lot of things going on and lost track of this one, and this is a very useful feature to have. Thanks, |
|
I need to return to it and replace the makefiles that use my build system with a regular makefile so that it's not special for the build service, I think. Then it'll be ok. Sorry I got distracted with the videos and then doing my 2022 write up. Maybe next week I'll have some time for it.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Description
This change introduces building through the RISC OS Build Service. It also reworks the code slightly to make it more likely to work with older compilers (it's C89 compatible), and fixes a socket leak. And fixes the header that never worked due to a poor compiler
#ifndefcheckThe CI build is a rudimentary process right now - we just run the build and copy a few files around.
We don't yet handle the application, which can come a little later.
There are no explicit tests present right now, but then there weren't originally either.
Outstanding work
Archive outputs
The archive contains the following structure (from the unzip tool), which isn't entirely nice: