Skip to content

Fix for conflicting symbols in X.h and Windows.h#89

Open
chrstphrchvz wants to merge 2 commits into
eserte:masterfrom
chrstphrchvz:backport-9e31fd9449
Open

Fix for conflicting symbols in X.h and Windows.h#89
chrstphrchvz wants to merge 2 commits into
eserte:masterfrom
chrstphrchvz:backport-9e31fd9449

Conversation

@chrstphrchvz

Copy link
Copy Markdown
Contributor

Backported from Tcl/Tk 8.6.10: see https://core.tcl-lang.org/tk/info/9e31fd944934

Fixes #87
Alternative to #88

Comment thread pTk/mTk/xlib/X11/X.h Outdated
Comment thread pTk/mTk/xlib/X11/X.h
#define ControlMask (1<<2)
#ifndef _WIN32
# define ControlMask (1<<2) /* See bug [9e31fd9449] and below */
#endif

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means that on Win32 ControlMask won't be defined as a macro. But X.xs checks for #ifdef ControlMask, so I think perl -Mblib -wE "use Tk::X qw(ControlMask); say ControlMask()" won't work and output 4 on Windows.

Possible workaround:

#ifndef ControlMask
#define ControlMask ControlMask /* uses the enum below */
#endif

@chrstphrchvz chrstphrchvz Aug 14, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out, I failed to notice this earlier. It looks like the same issue might apply to None.

Why does X.xs check for whether ControlMask, None, and other macros are defined? I do not know for certain. But at the time X.xs was introduced (698f508), Perl/Tk did not bundle X.h. Presumably it instead relied on X.h from the system, and so the defensive approach in X.xs may have been to deal with inconsistencies between different platforms’ X11 implementations. But that seems like a historical concern. Can X.xs just assume ControlMask/None are defined, as apparently done by usage elsewhere in Perl/Tk?

@eserte

eserte commented Nov 5, 2023

Copy link
Copy Markdown
Owner

Hi @chrstphrchvz @mauke
to resolve this issue I think it's best to use commit 517fb27 and additionally define the ControlMask+None macros for _WIN32 only, as @mauke suggested.
Additionally I think there could be a small test for Tk::X to check that ControlMask+None are available on Windows, and also in other "controlled" systems (linux, freebsd) --- I also don't know if any of the constants may be missing on some platforms.

The proposed changes: master...gh-87

@chrstphrchvz

chrstphrchvz commented Nov 10, 2023

Copy link
Copy Markdown
Contributor Author

Hi @chrstphrchvz @mauke to resolve this issue I think it's best to use commit 517fb27 and additionally define the ControlMask+None macros for _WIN32 only, as @mauke suggested. Additionally I think there could be a small test for Tk::X to check that ControlMask+None are available on Windows, and also in other "controlled" systems (linux, freebsd) --- I also don't know if any of the constants may be missing on some platforms.

The proposed changes: master...gh-87

Those proposed changes seem fine. I notified upstream Tcl/Tk about this approach, and they accepted it: https://core.tcl-lang.org/tk/info/593eb0227cfa

@jgpuckering

jgpuckering commented Mar 13, 2024

Copy link
Copy Markdown

I applied the changes proposed in https://github.com/eserte/perl-tk/compare/master...gh-87 in situ but Tk would not launch a window. A simple script that instantiates a MainWindow and then creates a Label and Button ran for a brief moment, then silently exited.

I also downloaded Tk-804.036.tar.gz and applied the changes inside it. This allowed the build to complete, though the tests failed. I didn't apply the proposed changes to t\X.t so maybe those would have allowed it to complete. Instead I just ran cpanm --force Tk-804.036.tar.gz and let it install Tk despite the test failures. Same result.

Tk builds and installs and runs just fine in Strawberry perl 5.32, but not in 5.36 or 5.38.

A ticket for this problem has been submitted to RT Tracker https://rt.cpan.org/Ticket/Display.html?id=151625 but no one has taken it and its priority is set to Low. I think it should be a higher priority than that. Tk is a pretty important package, and as it stands it's completely unusable in Strawberry perl above 5.32. I haven't checked ActiveState.

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.

../pTk/mTk/xlib/X11/X.h:182:34: error: expected identifier or '(' before numeric constant

4 participants