ci: add Cygwin build workflow #2066

ritesh006 · 2025-09-13T05:12:37Z

I ran valgrind when using my new feature

How to test the functionality

step 1
step 2

jubalh · 2025-09-13T11:36:08Z

Awesome! This looks like a good start :)

You could have set this PR to draft PR and then to regular PR when you want us to review.
But it is fine as it is now. Just request a review from us once you are done and got everything to work :)

ritesh006 · 2025-09-13T12:15:39Z

Awesome! This looks like a good start :)

You could have set this PR to draft PR and then to regular PR when you want us to review. But it is fine as it is now. Just request a review from us once you are done and got everything to work :)

sorry I dont understand what is draft pr?

jubalh · 2025-09-13T12:17:25Z

It's just a way to signal whether your pull request is still work in progress or ready for review by maintainers.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-stage-of-a-pull-request

ritesh006 · 2025-09-13T12:44:58Z

cygwin-build build success @jubalh could you please review this

jubalh

Now you need to clean up your history.
The commit Merge branch 'master' into ci/cygwin-build is not needed and just clutters the history. The other 3 commits can be squashed into one.

.github/workflows/cygwin-build.yml

ritesh006 · 2025-09-18T02:43:23Z

Hi @jubalh due to family loss I am in hometown as soon as I reach to my place would like to work start again. Thank You

jubalh · 2025-09-18T06:35:34Z

My condolences. Take your time!

ritesh006 · 2025-09-18T09:56:26Z

Now you need to clean up your history. The commit Merge branch 'master' into ci/cygwin-build is not needed and just clutters the history. The other 3 commits can be squashed into one.

Thanks I cleaned up the branch and squashed the changes into a single commit.

What I did: added a GitHub Actions workflow to check that Profanity builds on Cygwin (runs on windows-latest). The job installs the needed Cygwin packages and runs autoreconf -fi, ./configure, and make. I also forced the steps to run under Cygwin’s bash and handled temp-script paths and CRLF issues so the job should run more reliably on the runner.

Next steps I’m thinking of (if you’re OK with this landing):

enable make check later if tests are stable on Cygwin,

add a MSYS2/MINGW64 job for native Windows builds,

upload build logs/artifacts on failures to make debugging easier.

I renamed the workflow to Cygwin to match the other job names. Please take a look and let me know if you want any changes.

jubalh · 2025-09-18T10:08:33Z

Sounds good!

Regarding:

enable make check later if tests are stable on Cygwin,

The tests do not succeed there currently?

jubalh

Thank your for the PR and your patience.
I'll wait for @sjaeckel review as well.

sjaeckel

Thanks for the PR! Basically LGTM, just some nitpicks and clarifications missing IMO.

General thing: one doesn't keep code "just in case" "if it's maybe needed in the future". YAGNI. Instead doing that will most likely confuse others in the future.

Also if you've questions regarding some parts of the code you're most likely to go to

git blame these lines
see that this originates from a PR
look into the discussions and see what changed and what was the reasoning.

Things like the 4 line comment about the wrapper is good! That comment sounds very reasonable, because I don't get that from the code.

sjaeckel · 2025-10-14T06:38:12Z

.github/workflows/cygwin-build.yml

+          if [ -x ./bootstrap.sh ]; then
+            ./bootstrap.sh
+          else
+            autoreconf -fi
+          fi


Why should the executable bit change?

sjaeckel · 2025-10-14T06:38:53Z

.github/workflows/cygwin-build.yml

+        run: |
+          cd "$GITHUB_WORKSPACE"
+          # don't fail if git is not present inside Cygwin
+          git config --global core.autocrlf false || true


Why is this required? I don't see any Git operations afterwards

sjaeckel · 2025-10-14T06:40:05Z

.github/workflows/cygwin-build.yml

+      - name: Check repo state
+        run: |
+          cd "$GITHUB_WORKSPACE"
+          pwd
+          ls -la
+          test -f configure.ac && echo "configure.ac present" || (echo "configure.ac missing"; exit 1)
+


Can't this step be removed?

sjaeckel · 2025-10-14T06:41:00Z

.github/workflows/cygwin-build.yml

+        # explicit shell to be extra-safe for this step
+        shell: >
+          C:\tools\cygwin\bin\bash.EXE --login -eo pipefail -c
+          "p=$(/usr/bin/cygpath -au '{0}');
+           w=$(/usr/bin/cygpath -au '${{ github.workspace }}');
+           /usr/bin/sed -i 's/\r$//' \"$p\" || true;
+           set -o igncr;
+           cd \"$w\" || (/usr/bin/pwd; /usr/bin/ls -la; exit 2);
+           /usr/bin/bash -leo pipefail \"$p\""
+


To be extra-safe for what? Shouldn't this be handled by the default: shell in the preamble?

sjaeckel · 2025-10-14T06:45:29Z

.github/workflows/cygwin-build.yml

+          cd "$GITHUB_WORKSPACE"
+          make -j2
+
+      # Optional: non-blocking tests + upload logs for inspection (recommended while tests are unstable)


Please remove that comment, it's lying, since it's not optional.

sjaeckel · 2025-10-14T06:57:27Z

.github/workflows/cygwin-build.yml

+        shell: >
+          C:\tools\cygwin\bin\bash.EXE --login -eo pipefail -c
+          "p=$(/usr/bin/cygpath -au '{0}');
+           w=$(/usr/bin/cygpath -au '${{ github.workspace }}');
+           /usr/bin/sed -i 's/\r$//' \"$p\" || true;
+           set -o igncr;
+           cd \"$w\" || (/usr/bin/pwd; /usr/bin/ls -la; exit 2);
+           /usr/bin/bash -leo pipefail \"$p\""


Most likely a super stupid question: if I look at [0], it seems to me like this solution is convoluted, but I guess there's a simple reason why this is as it is!? Where does this originate from?

[0] https://docs.github.com/en/actions/how-tos/write-workflows/choose-what-workflows-do/set-default-values-for-jobs#setting-default-shell-and-working-directory

ritesh006 · 2025-10-21T13:53:34Z

please take a look

sjaeckel · 2025-10-27T09:06:08Z

please take a look

I took a look now. Could you please address all the comments?

jubalh · 2025-12-03T11:08:11Z

@ritesh006 did you see the comments?

ritesh006 · 2026-01-06T17:51:09Z

@ritesh006 did you see the comments?

sorry for late comment @jubalh actually i was travelling so didnt got the time but now i have addressed all issue please take a look @jubalh @sjaeckel

jubalh · 2026-01-07T09:22:51Z

@ritesh006 thanks for the update. Currently the CI test fails though.

sjaeckel · 2026-01-07T09:36:20Z

@ritesh006 thanks for the update. Currently the CI test fails though.

Thanks indeed! Looks like at least cmocka is missing!?

ritesh006 · 2026-01-07T16:53:24Z

@ritesh006 thanks for the update. Currently the CI test fails though.

@jubalh CI success please review.

jubalh · 2026-01-07T17:02:39Z

Unfortunately now there is again a merge commit. Like already mentioned in #2066 (review) we dont like them :)
A rebase on master would have been more appropriate.
Please get rid of that second commit.

ritesh006 · 2026-01-07T17:09:14Z

Unfortunately now there is again a merge commit. Like already mentioned in #2066 (review) we dont like them :) A rebase on master would have been more appropriate. Please get rid of that second commit.

Please check now is everything ok.

sjaeckel · 2026-01-07T17:22:29Z

You simply removed the unittests ... I'm not a big fan.

Is there no cmocka available or what prevents us from building and running them?

ritesh006 · 2026-01-07T17:28:49Z

On Cygwin the tests failed earlier because the cmocka dependency is not available in the job.
That is why make check failed, so I removed the step for now.
We can enable tests again once cmocka works properly on Cygwin, either by adding a package or building it from source. I’m happy to look into that in a follow-up PR.

Add a GitHub Actions workflow to build with Cygwin on Windows. Fixes issues by forcing Cygwin bash for all steps, handling temporary script paths, stripping CRLF line endings, and enabling igncr.

ritesh006 · 2026-01-07T17:52:40Z

I added libcmocka-devel so the tests can be supported in the future.
I also tried enabling make check, but Cygwin fails with:

make: *** No rule to make target 'check'. Stop.

So it seems this build configuration does not expose a make check target yet.
At the moment the workflow just verifies the build, and cmocka is installed so we can enable tests later.

What would you suggest, should we keep it as a build-only job for now, or look into wiring the tests on Cygwin in a follow-up PR?

ritesh006 force-pushed the ci/cygwin-build branch 12 times, most recently from 7c1c710 to ae479f8 Compare September 13, 2025 09:23

jubalh assigned ritesh006 Sep 13, 2025

jubalh added the tests label Sep 13, 2025

jubalh added this to the next milestone Sep 13, 2025

ritesh006 force-pushed the ci/cygwin-build branch 3 times, most recently from c9cc0c4 to 178d3f4 Compare September 13, 2025 12:34

jubalh requested changes Sep 15, 2025

View reviewed changes

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

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

ritesh006 force-pushed the ci/cygwin-build branch from f8d1ae0 to 0a47fd4 Compare September 18, 2025 09:55

ritesh006 force-pushed the ci/cygwin-build branch from 0a47fd4 to 7b2ee66 Compare September 18, 2025 10:00

ritesh006 force-pushed the ci/cygwin-build branch from 76a621b to e8c9cc0 Compare September 18, 2025 10:31

ritesh006 marked this pull request as ready for review September 26, 2025 07:32

ritesh006 marked this pull request as draft September 27, 2025 06:47

ritesh006 force-pushed the ci/cygwin-build branch 3 times, most recently from d36e5db to 39dde49 Compare October 1, 2025 11:43

ritesh006 marked this pull request as ready for review October 1, 2025 11:54

ritesh006 requested a review from jubalh October 1, 2025 11:58

jubalh approved these changes Oct 9, 2025

View reviewed changes

sjaeckel requested changes Oct 14, 2025

View reviewed changes

ritesh006 force-pushed the ci/cygwin-build branch from 39dde49 to 2b39c84 Compare October 21, 2025 13:53

ritesh006 force-pushed the ci/cygwin-build branch from 2b39c84 to 1f1be5b Compare January 6, 2026 17:49

ritesh006 force-pushed the ci/cygwin-build branch from ebb1f69 to 190ef95 Compare January 6, 2026 18:00

ritesh006 force-pushed the ci/cygwin-build branch from 190ef95 to a071b71 Compare January 7, 2026 16:37

ritesh006 force-pushed the ci/cygwin-build branch from 144407e to 789de1d Compare January 7, 2026 17:08

ci: add Cygwin build workflow

60c4dc6

Add a GitHub Actions workflow to build with Cygwin on Windows. Fixes issues by forcing Cygwin bash for all steps, handling temporary script paths, stripping CRLF line endings, and enabling igncr.

ritesh006 force-pushed the ci/cygwin-build branch from 789de1d to 60c4dc6 Compare January 7, 2026 17:40

ritesh006 requested review from jubalh and sjaeckel January 7, 2026 17:52

Uh oh!

ci: add Cygwin build workflow #2066

Are you sure you want to change the base?

ci: add Cygwin build workflow #2066

Uh oh!

Conversation

ritesh006 commented Sep 13, 2025

How to test the functionality

Uh oh!

jubalh commented Sep 13, 2025

Uh oh!

ritesh006 commented Sep 13, 2025

Uh oh!

jubalh commented Sep 13, 2025

Uh oh!

ritesh006 commented Sep 13, 2025

Uh oh!

jubalh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ritesh006 commented Sep 18, 2025

Uh oh!

jubalh commented Sep 18, 2025

Uh oh!

ritesh006 commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jubalh commented Sep 18, 2025

Uh oh!

jubalh left a comment

Choose a reason for hiding this comment

Uh oh!

sjaeckel left a comment

Choose a reason for hiding this comment

Uh oh!

sjaeckel Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sjaeckel Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

sjaeckel Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

sjaeckel Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

sjaeckel Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

sjaeckel Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

ritesh006 commented Oct 21, 2025

Uh oh!

sjaeckel commented Oct 27, 2025

Uh oh!

jubalh commented Dec 3, 2025

Uh oh!

ritesh006 commented Jan 6, 2026

Uh oh!

jubalh commented Jan 7, 2026

Uh oh!

sjaeckel commented Jan 7, 2026

Uh oh!

ritesh006 commented Jan 7, 2026

Uh oh!

jubalh commented Jan 7, 2026

Uh oh!

ritesh006 commented Jan 7, 2026

Uh oh!

sjaeckel commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ritesh006 commented Jan 7, 2026

Uh oh!

ritesh006 commented Jan 7, 2026

ritesh006 commented Sep 18, 2025 •

edited

Loading

sjaeckel Oct 14, 2025 •

edited

Loading

sjaeckel commented Jan 7, 2026 •

edited

Loading