Skip to content
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

add github workflow for CYGWIN-like platform MSYS2 on Windows #9

Open
wants to merge 8,318 commits into
base: main
Choose a base branch
from

Conversation

jannick0
Copy link

This commit series serves the purpose to prepare for and add a github workflow for MSYS(2), a CYGWIN-like environment on Windows. It compiles fine, but reveals many test failures to be fixed.

NB:

  • The entanglement with the win dir appears to be unneccassary and thus is removed to allow a pure unix configuration and build for MSYS and CYGWIN.
  • As of now there are many failing tests, while one test (io-53.4.1) is omitted since it hangs as of now.
  • For reference as to how CYGWIN and MSYS2 see need for heavily patching the tcl package which effectively ignore most of the #ifdef __CYGWIN__ branches. I believe that MSYS2 mostly relies on the CYGWIN patch.
  • The msys2 workflow file added by the top of this branch is meant to be copied to the windows workflow (after testing)

I hope it is OK that this is a Github PR since this is about a github workflow which should be automatically triggered when opening this PR.

jan.nijtmans and others added 30 commits September 25, 2020 14:26
…HANNEL_VERSION_5. Not actually a change, since supported procs are the same. So all internal channels have the same type
…tring is list" & length %2 == 0 to check for a valid dict
…tring is list" & length %2 == 0 to check for a valid dict
jan.nijtmans and others added 9 commits November 26, 2020 14:43
Fix some more warnings, discovered in c20/c++20 mode
* unix/tcl.m4: add 'MSYS_' whenever 'CYGWIN_'
* unix/configure: same here.
The env variable MSYSTEM is used in the CYGWIN-like MSYS shell governing
the underlying platform type.
The removed code block is generated by the autoconf (2.69) macro AC_ARG_ENABLE.
So let's remove it since it works as an idempotent.

* unix/tcl.m4: here.
* unix/configure: same.
* win/tcl.m4: here.
* win/configure: same.
The entanglement is unneccessary and rather confusing.

* unix/tcl.m4:
  - remove call of win/configure when builing a shared CYGWIN library
* unix/configure: same.
* unix/Makefile.in:
  - remove call of make target 'winextensions' in dir 'win'
* unix/tcl.m4:
  - use cpp macro condition 'INTPTR_MAX == INT64_MAX' to portably check
    if platform is 64-bit.
* unix/configure: same.
This github workflow is meant to be copied to the windows workflow file.

NB: Ignore io-53.4.1 which currently hangs.
@nijtmans
Copy link
Member

nijtmans commented Nov 30, 2020

There's a "msys2-fixes-v001" branch in fossil now, with your PR. Two changes went right in Tcl now, the additional MSYSTEM environmentvariable, and the MSYS_ prefix in tcl.4: those don't harm any for the other platforms.

The entanglement with the "win" directory is only used to build the dde and registry extension. Although those 2 extensions are win32, they work fine with cygwin (and should work fine with msys2 too). Using dde, cygwin Tcl interpreters can communicate with win32 Tcl intepreters, that's usefull. And using registry, cygwin Tcl interpreters and access the Windows registry. I think that's usefull to keep, and is 100% separate from the build in the /unix directory.

Automatically determine --enable-64bit is a good idea, but it doesn't work yet together with the 'entaglement' with the "win" directory. Therefore I didn't take it over yet.

It's a pity that so many testcases still fail. This shows that msys isn't that compatible with unix (yet).

@jannick0
Copy link
Author

Many thanks for looking at that!

The entanglement with the "win" directory is only used to build the dde and registry extension. Although those 2 extensions are win32, they work fine with cygwin (and should work fine with msys2 too).

Would you be able to provide a pointer to where the tcl package compiles out of the box for CYGWIN including running the test cases? I would be courious to see the logs for config, build and tests.

Automatically determine --enable-64bit is a good idea, but it doesn't work yet together with the 'entaglement' with the "win" directory. Therefore I didn't take it over yet.

It could serve as global test to make --enable-64bit obsolete. I thought about that, too, when putting the test together, but I prefer to leave it with you to use the test globally. Potential additional test whether the macro INTPTR_MAX is really defined by stdint.h might be helpful for, e.g., very exotic or old systems, while the macro should usually exist, though.

It's a pity that so many testcases still fail. This shows that msys isn't that compatible with unix (yet).

Well, that might well be. On the other hand CYGWIN itself applies heavy patches to (cf. references in the first comment)

  • ignore the __CYGWIN__ branches in the code and
  • ignoring the winextension target in the win folder.
    A similar patch MSYS2 does use. This suggests another interpretation. I am not advocating for any of the systems.

Again, this is why I would be intrigued to see how the tcl built on CYGWIN compiles (with winextensions) and tests out of the box. Any pointer to such a build for a most recent tcl package version? Many thanks.

@nijtmans nijtmans self-assigned this Nov 30, 2020
@nijtmans
Copy link
Member

nijtmans commented Dec 1, 2020

out.txt

Here's the log when doing a cygwin "make test" for Tcl's unmodified core-8-6-branch (with -skip io-53.4.1, since this testcase fails on Cygwin too)

@jannick0
Copy link
Author

jannick0 commented Dec 1, 2020

Thanks - this is interesting. It looks like that similar tests fail for CYGWIN and MSYS2, so they are not so far apart from each other in this context I believe.

Would it be possible to see how the tcl package build was configured, too. Maybe in a Github Action which would be the most transparent way I guess.

@nijtmans
Copy link
Member

nijtmans commented Dec 8, 2020

The number of testcases failing on msys2 is now below 100. One of them because of this commit (https://core.tcl-lang.org/tcl/info/921cc1eb98e51b90). The fossil repository (which this GIT repository is a mirror or) now has a "cygwin" branch which contains all official cygwin patches (undoing most of the #ifdef CYGWIN stuff). And there is a "msys2-fixes-v001" branch, which contains the changes from this PR rebased to the "cygwin" branch (so, based on 8.6 too).

Most test-failures I see seem to be caused by lack of support for links on Windows. On cygwin it's a little bit better.

Thanks for your contribution.

B.T.W.: Is fossil ported to msys2?

@jannick0
Copy link
Author

jannick0 commented Dec 8, 2020

The number of testcases failing on msys2 is now below 100.

This is really good news - congratulations!

One of them because of this commit (https://core.tcl-lang.org/tcl/info/921cc1eb98e51b90).

I personally would use msys (or msys2) instead of msystem which would then be more in line with the unix nomenclature for MSYS2 animals.

Most test-failures I see seem to be caused by lack of support for links on Windows. On cygwin it's a little bit better.

This was exactly what I was expecting. The same thing for the sqlite testsuite on MSYS2.
Q: So would it be possible to have a tcl function which intrinsically checks at run-time if symbolic links work or not? (I am not sure if this gets into the realm of soft and hard links on Windows.) This function could then be used to test at check time if links work or not to decide if tests relying on links should be run or not.
See here a related suggestion for the sqlite testsuite. As far as I can see the ball is not rolling there yet.

B.T.W.: Is fossil ported to msys2?

Yes. NB: Call fossil either in the MINGW32 or MINGW64 shell (but not in the MSYS shell, though):

  • Note that there are the three shells MSYS, MINGW32 and MINGW64
  • info on the fossil package for, say, MINGW64; download the fossil package for MINGW64 like so: pacman -S mingw-w64-x86_64-fossil (NB: pacman can be called in any of the 3 shells).
  • how to use in Github Workflows CI and more detailed info

HTH - thanks again for your work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants