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?

ritesh006 · 2025-09-18T10:31:33Z

I added a Cygwin workflow that installs required Cygwin packages, builds libstrophe, and runs autoreconf -fi, ./configure, and make.
I haven’t enabled make check as a blocking step yet — I can add a non-blocking test run that uploads logs so we can triage failures, then enable tests once they’re stable.

ritesh006 · 2025-09-18T10:41:56Z

Sounds good!

Regarding:

enable make check later if tests are stable on Cygwin,

The tests do not succeed there currently?

could you please help me here bacuse its failing at linux stage not at Cygwin I thing Cygwin not even started yet

jubalh · 2025-09-18T10:47:14Z

I haven’t enabled make check as a blocking step yet — I can add a non-blocking test run that uploads logs so we can triage failures, then enable tests once they’re stable.

So I suggest this:

You have the the first commit that adds the cygwin build (we can already merge at this stage, but you could continue with the rest)
You add a second commit which enables make check to run the tests
You add a third commit which prints the errors

I think the other CIs don't even run the tests. So these things are optional.

could you please help me here bacuse its failing at linux stage not at Cygwin I thing Cygwin not even started yet

I'll retrigger them or disable them. No need to add commits to retrigger it :)

``` #11 0.137 --2025-09-18 10:41:06-- https://aur.archlinux.org/cgit/aur.git/snapshot/libstrophe-git.tar.gz ``` This currently blocks us at #2066.

jubalh · 2025-09-18T11:02:37Z

I'm not sure why it doesn't use the new workflow file. Can you try cleaning up git history into 1 commit and force push?

jubalh · 2025-09-18T11:45:24Z

When you got rid of the 2 extra commits, please run git rebase. to get the latest changes from master. then the CI should omit Arch.

ritesh006 · 2025-09-19T08:35:07Z

Ready for review

sjaeckel · 2025-09-19T11:11:08Z

BTW there's https://github.com/nektos/act to run GH actions locally while working on them. Not sure how/if it works for Windows runners, but it works great for Linux CI jobs.

jubalh · 2025-09-21T21:07:27Z

.github/workflows/cygwin-build.yml

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


Everything looks great :)
But I still don't understand what you mean with the tests being unstable. Or what the problem with them is. Is there one?

If everything looks fine, can we merge these changes? After that, we can handle the remaining parts in the next commit

This comment will be on master and even though people in the past often promised to change something in the future, sometimes they didn't do it. So I would like to understand this comment first. Or removing the outcommented part and just adding the whole thing once its working.

Could you please provide me with the definition of done? That will help me understand your expectations better. Since no issue was created for this, I just want to clarify

And I assure you I will give my best

I am making this pr as draft. let me know you expectations. I would like to work on that.

The comment above mentions that the tests are "unstable". I don't know what you mean by that.

I called them “unstable” because on Cygwin some make check tests don’t always pass, even though they work fine on Linux. This can be due to line endings, timing, or other Windows/Cygwin differences. To avoid blocking the build, I suggested running the tests in non-blocking mode and just saving the logs.

I uncommented the test step and it’s passing now. I kept the note so if tests fail in the future we remember why. If you want, I can remove the comment.

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

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 · 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?

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

jubalh added a commit that referenced this pull request Sep 18, 2025

ci: disable arch

58d130a

``` #11 0.137 --2025-09-18 10:41:06-- https://aur.archlinux.org/cgit/aur.git/snapshot/libstrophe-git.tar.gz ``` This currently blocks us at #2066.

ritesh006 force-pushed the ci/cygwin-build branch 2 times, most recently from 9f6f2fe to c1fc275 Compare September 19, 2025 08:29

jubalh reviewed Sep 21, 2025

View reviewed changes

ritesh006 marked this pull request as draft September 26, 2025 07:28

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

ci: add Cygwin build workflow

2b39c84

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 39dde49 to 2b39c84 Compare October 21, 2025 13:53

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!

ritesh006 commented Sep 18, 2025

Uh oh!

ritesh006 commented Sep 18, 2025

Uh oh!

jubalh commented Sep 18, 2025

Uh oh!

jubalh commented Sep 18, 2025

Uh oh!

jubalh commented Sep 18, 2025

Uh oh!

ritesh006 commented Sep 19, 2025

Uh oh!

sjaeckel commented Sep 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

ritesh006 commented Sep 18, 2025 •

edited

Loading

sjaeckel Oct 14, 2025 •

edited

Loading