-
Notifications
You must be signed in to change notification settings - Fork 493
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
perl: build for cygwin #5177
perl: build for cygwin #5177
Conversation
The goal is for perl to report that it was built for cygwin ($^O gives "cygwin" for example) and to adjust all the code depending on that and remove all the patches for handing the "msys" case. Everything else should work as before. * In case cygwin check was extended with msys, remove msys case * In case msys check was added, replace with cygwin check * In case msys specific checks/code was added, merge that into the cygwin paths We still check/set the MSYS env var instead of the CYGWIN env var, since msys2-runtime doesn't handle both yet, so keep that as is for now.
@dscho since I've always copied the perl work from git4win, any thoughts/feedback on this? It should make updating perl easier in the future. |
perl/0006-perl-5.36.0-msys2.patch
Outdated
-# - eliminate -lc, implied by gcc and a symlink to libcygwin.a | ||
+# - eliminate -lc, implied by gcc and a symlink to libmsys-2.0.a | ||
libswanted=`echo " $libswanted " | sed -e 's/ c / /g'` | ||
-# - eliminate -lm, symlink to libcygwin.a | ||
+# - eliminate -lm, symlink to libmsys-2.0.a | ||
libswanted=`echo " $libswanted " | sed -e 's/ m / /g'` | ||
-# - eliminate -lutil, symbols are all in libcygwin.a | ||
+# - eliminate -lutil, symbols are all in libmsys-2.0.a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it really worth patching comments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point, but I just copied over the hints/msys.sh
changes to hints/cygwin.sh
(and adjusting the arch strings) to avoid any other side effects of this PR.
So the diff was there before, just in a different file
I'm fine with removing the comment changes though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so cygwin.sh probably diverged since msys.sh was created
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-test -z "$useithreads" && useithreads='define' | ||
+test -z "$usethreads" && usethreads='define' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above, not changed in this PR
- | ||
-# Seems that exporting _Thread_local doesn't work on cygwin. This 6 year old | ||
-# gcc bug suggests that maybe the problem really is binutils, but either way | ||
-# it still doesn't work, despite our probes looking good: | ||
-# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64697 | ||
-d_thread_local=undef | ||
- | ||
-# Broken: https://sourceware.org/pipermail/cygwin/2022-August/252043.html */ | ||
-d_newlocale=undef |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why remove these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above, not changed in this PR
That's a good goal! As for Git for Windows, I recognize that for i686 builds, we're stuck with requiring the Having said that, it's a good idea for MSYS2 to go ahead with this. In Git for Windows, I'll have to adjust a couple of lines:
Note how inconsistently the affected Git code is: Even the underlying theme is to handle DOS/Unix paths issues, the handling is ad-hoc and not standardized in tl;dr no objections from my side! |
Why would i686 builds still require the msys2 patches? I've been building i686 packages, including those that remove msys2 patches, without issues |
hmmm, I'd guess because the MSYSTEM=CYGWIN logic is not in the current i686 repo. I need to look into #4712 ...
yeah, that needs to be adjusted |
This is since this used to be a separate msys.sh file which was synced back. Let's keep things simple and only patch what we need to. TODO: check if the other changes in that file are really needed, or if they just diverged since the split for no good reason.
as pointed out here: msys2#5177 (comment)
It's a bit worse than that: Git for Windows' and MSYS2's commit histories of the |
The goal is for perl to report that it was built for cygwin ($^O gives "cygwin" for example) and to adjust all the code depending on that and remove all the patches for handing the "msys" case. Everything else should work as before.
We still check/set the MSYS env var instead of the CYGWIN env var, since msys2-runtime doesn't handle both yet, so keep that as is for now.
Part of #3012
edit: confirmed with the test suite that there are no changes in the test results