USBDDOS icon indicating copy to clipboard operation
USBDDOS copied to clipboard

Set up CI/CD for this project, cross-compiling and publishing DOS executable using GitHub Actions Workflow

Open volkertb opened this issue 2 years ago • 17 comments

Closes #5

volkertb avatar Dec 28 '23 19:12 volkertb

@crazii While this PR is not merged yet, you can see the results of the pipelines in my fork: https://github.com/volkertb/USBDDOS/actions

A new pipeline (Actions run) should be triggered whenever new changes are pushed to this PR.

I also fixed some issues in the Makefile and in some #include directives in the sources that DJGPP as apparently tripping over.

Your most recent commits introduced new build failures with DJGPP, since I had this working locally until I updated my branch with your most recent commits. When this PR is merged, breakage like this will be noticed quicker, since you (understandably) don't check each commit with all supported compilers while you work on the project.

Right now it's failing in DJGPP with this error, both in my local development environment and in GitHub Actions:

 ../USBDDOS/HCD/uhci.c: In function 'UHCI_StartHC':
 ../USBDDOS/HCD/uhci.c:845:14: error: unused variable 'status' [-Werror=unused-variable]
   845 |     uint16_t status = inpw((uint16_t)(pHCI->dwBaseAddress + USBSTS));
       |              ^~~~~~
 cc1: all warnings being treated as errors
 make: *** [Makefile:52: output/uhci.o] Error 1

See the pipeline log at https://github.com/volkertb/USBDDOS/actions/runs/7350805100/job/20013107796

It's a weird error though, because that variable is in fact being referenced in the following two lines:

    _LOG("UHCI start HC status: %x\n", status);
    assert(!(status & USBHCHALTED));

I did noticed the _LOG statement being greyed out as an Empty statement in my IDE. And if I'm not mistaken, assert statements are not always actually included in compilation when debugging isn't enabled. Correct? Maybe that results in neither of those statements qualifying as actually "using" that variable status.

volkertb avatar Dec 28 '23 19:12 volkertb

The errors are fixed.

crazii avatar Dec 28 '23 21:12 crazii

Thanks. I just rebased this branch on your recent upstream commits, and indeed, compilation is successful again. :+1:

The build job passes, but the publish job is failing now, but that's due to a bug in my work, not in your source code.

I'm confident I can fix that without too much effort.

As for also setting up CI/CD for Open Watcom compilation: I'd still like to do that, but I'll open a separate PR for that, once this one is merged.

volkertb avatar Dec 28 '23 21:12 volkertb

Pipeline is now working, both the building and the publishing. See the following:

  • https://github.com/volkertb/USBDDOS/actions/runs/7351528342
  • https://github.com/volkertb/USBDDOS/releases/tag/UserBuild_2023.12.28_21-58

We might want to replace the TODO in the release notes with something else. @crazii feel free to suggest something to put there. Thanks! :slightly_smiling_face:

volkertb avatar Dec 28 '23 21:12 volkertb

The debug-enabled variant is failing with a compilation error in GitHub Actions, even though it builds fine locally.

See this post for more details about that.

volkertb avatar Dec 29 '23 14:12 volkertb

The relocation truncated to fit error while building the debug-enabled variant might be due to the GitHub build runners having not enough RAM available for the build. Perhaps I can work around that by enabling a swap file during the build or something.

volkertb avatar Jan 07 '24 15:01 volkertb

Then again, the standard GitHub-hosted runners apparently have 7GB of RAM, and I successfully ran the GitHub Action with act on a Linux PC with 8GB of RAM. So that makes it less likely that this was the cause. Unless that one extra GB of RAM made just the difference between success and failure when building USBDDOS with debug enabled.

volkertb avatar Jan 07 '24 15:01 volkertb

The relocation truncated to fit error while building the debug-enabled variant might be due to the GitHub build runners having not enough RAM available for the build. Perhaps I can work around that by enabling a swap file during the build or something.

Glad to hear that. Congrats!

crazii avatar Jan 08 '24 12:01 crazii

Sorry I just pushed my changes, I should've merge it first. the unstable connection to github makes my anxious, editing readme.md online or pushing codes. damn.

crazii avatar Jan 08 '24 12:01 crazii

No, that's okay. The problem with the debug builds failing with that relocation truncated to fit error still hasn't been resolved. There are two options at this point:

  • Merge it with the debug builds disabled in GitHub Actions for now (so the debug builds aren't built automatically on each commit or merge to master/main, only the "release" (non-debug) build. And then try to troubleshoot the failing debug builds and open a separate PR to re-enable it later, once fixed.
  • Spend some more time trying to get the debug builds to succeed in this PR, and wait with merging until then.

What do you think?

(By the way, don't worry about any changes you merge or commit in the meantime. It's trivial to rebase PRs, especially since the PR changes different files than the stuff you're merging or committing in the meantime.)

volkertb avatar Jan 08 '24 21:01 volkertb

Cool, thanks. I will also add git tags string into the make file.

crazii avatar Jan 08 '24 21:01 crazii

Now the git commit hash is added into build string, I'm not using tags I think the commit hash is good enough. I also added it for Open watcom, but it just need a shell wrapper, if you also setup for open watcom, please run the ./owcmake.sh directly without invoking wmake. The shell script will pass git commit hash to makefile.

crazii avatar Jan 09 '24 08:01 crazii

Pipeline is now working, both the building and the publishing. See the following:

  • https://github.com/volkertb/USBDDOS/actions/runs/7351528342
  • https://github.com/volkertb/USBDDOS/releases/tag/UserBuild_2023.12.28_21-58

We might want to replace the TODO in the release notes with something else. @crazii feel free to suggest something to put there. Thanks! 🙂

Todo is a development notes that is not for release purpose. It's a list that we needed not forget 'to do it in the future'.

crazii avatar Feb 13 '24 01:02 crazii

No, that's okay. The problem with the debug builds failing with that relocation truncated to fit error still hasn't been resolved. There are two options at this point:

  • Merge it with the debug builds disabled in GitHub Actions for now (so the debug builds aren't built automatically on each commit or merge to master/main, only the "release" (non-debug) build. And then try to troubleshoot the failing debug builds and open a separate PR to re-enable it later, once fixed.
  • Spend some more time trying to get the debug builds to succeed in this PR, and wait with merging until then.

What do you think?

(By the way, don't worry about any changes you merge or commit in the meantime. It's trivial to rebase PRs, especially since the PR changes different files than the stuff you're merging or committing in the meantime.)

Either way will be ok, the error is related to the building environment but we don't know what triggers it. We can remove all packages to get a clean env to see if it still happens. If it still happens then we have to leave it for now.

crazii avatar Feb 13 '24 01:02 crazii

And sorry that when I was in the middle of coding, I become ignoring because I was focusing on the coding problem (it uses too many of my brain cells. :)

So forgive me if I didn't reply you in time. I didn't mean it.

crazii avatar Feb 13 '24 02:02 crazii

Hi again!

No need to apologize, since I've been off the radar myself, these last two weeks. (A little bicycle accident, still typing with one hand :sweat_smile:)

I'll need to find some time to process your feedback and dust off this PR. Thanks for understanding. :slightly_smiling_face:

volkertb avatar Feb 21 '24 22:02 volkertb

OK, cool. Get well soon!

crazii avatar Feb 27 '24 06:02 crazii