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

Tk still broken on SrawberryPerl-5.38.2 #104

Open
sisyphus opened this issue Apr 13, 2024 · 5 comments
Open

Tk still broken on SrawberryPerl-5.38.2 #104

sisyphus opened this issue Apr 13, 2024 · 5 comments

Comments

@sisyphus
Copy link

There's absolutely nothing new about this issue - but it still hasn't been addressed.
The attached patch allows Tk to build on StrawberryPerl-5.38.2.

It's the same patch to X.h as provided at master...gh-87.
However, in addition to that, it also replaces (again in X.h) the following piece of non-portable code:

#if defined(_WIN64) && defined(_MSC_VER)
typedef __int64 XID;
#else
typedef unsigned long XID;
#endif

with a a rendition that ports correctly:

#if defined(_WIN64)
#   if defined(_MSC_VER)
    typedef __int64 XID;
#   else
    typedef unsigned long long XID;
#   endif
#else
typedef unsigned long XID;
#endif

I think that both of those fixes emanate from @chrstphrchvz. (The solution to the non-portability issue was identified in #92.)
They're not something for which I can claim any credit, and I'm not trying to stomp all over the excellent work of anyone else.
I just get very annoyed that this hasn't yet been fixed in https://github.com/eserte/perl-tk - and I am prepared to re-formulate this bug report as a PR, if that's what it's going to take.

Better still, of course, if we can also have a new cpan release that includes this fix, before perl-5.40 is released.
(This would greatly enhance the chances of having Tk included in the Strawberry Perl 5.40 release.)

win32_patch.txt

@Lamprecht
Copy link
Contributor

I confirm, that Tk builds on StrawberryPerl-5.38.2 with the patch attached above.
All tests are passing.

@shawnlaffan
Copy link

I've created a patched distribution from the latest Tk commit. For some reason git did not recognise the format so I manually applied them.

I was also unable to update the version to append _001 as it needs to be in several places. If someone can provide a patch for that then that would be helpful.

https://github.com/StrawberryPerl/Perl-Dist-Strawberry/releases/tag/patched_cpan_modules

Whether we include it in the full SP release needs discussion. The compressed blib dir is another 2MB so would increase the downloads commensurately. This is not really very much in the scheme of things these days but it is still non-zero, and I would suspect the majority of users do no need Tk, even though there are many who do.

@mohawk2
Copy link
Contributor

mohawk2 commented May 5, 2024

@eserte Are you open to sharing commit bits on this repo, and PAUSE co-maint, to do these various maintenance tasks?

@Lamprecht
Copy link
Contributor

I agree that it's desirable to get a version to cpan that builds on strawberry. However (correct me if i am wrong) we should also try to be in sync with tcl-tk, which can be found here: tcl-tk X.h.

Unfortunately the tcl-tk X.h also fails to compile with SP 5.38.2:

../pTk/mTk/xlib/X11/X.h:67:26: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'XID'
   67 | typedef unsigned __int64 XID;
      |                          ^~~

@sisyphus
Copy link
Author

sisyphus commented May 6, 2024

Unfortunately the tcl-tk X.h also fails to compile with SP 5.38.2

I think all it needs is for the offending typedeffing to be made portable.
Mingw-w64 ports of gcc understand the __int64 type, but only if _mingw.h has been included.
Apparently _mingw.h is not included for this particular compilation.

I grabbed https://github.com/tcltk/tk/blob/74b174eed61acafb5e8bc26bfe8d8d33fcadd7aa/xlib/X11/X.h, and applied this patch to it:

--- X.h_tcl_tk  2024-05-06 11:07:06.086219000 +1000
+++ X.h 2024-05-06 11:43:06.491410400 +1000
@@ -65,7 +65,11 @@
 #  ifndef _XTYPEDEF_XID
 #    define _XTYPEDEF_XID
 #    ifdef _WIN64
+#      if defined(_MSC_VER)
 typedef unsigned __int64 XID;
+#      else
+typedef unsigned long long XID;
+#      endif
 #    else
 typedef unsigned long XID;
 #    endif

I then replaced the existing X.h in the Tk github repo (https://github.com/eserte/perl-tk) with this patched version of X.h.
That Tk source then built fine for me on both SP-5.38.2-PDL and SP-5.39.10.

I have a strong aversion to making PRs, but I'll try to find the energy to make one to https://github.com/tcltk/tk/ later today.

UPDATE: I submitted tcltk/tk#37, but apparently they want me to instead provide a report
at https://core.tcl-lang.org/tk/tktnew - so now there's also https://core.tcl-lang.org/tk/tktview/ff5417505be90f5abbbf6ea4c7a3ac3bae6bc8b1

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

No branches or pull requests

4 participants