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

Allow compilation with both MinGW and mingw-64 #82

Closed
wants to merge 2 commits into from

Conversation

GitMensch
Copy link
Collaborator

@GitMensch GitMensch commented Apr 15, 2018

Allow compilation with self-defined INFOEX, needed with old MINGW (and maybe old Watcom, too), by specifying -DHAVE_NO_INFOEX.

MinGW actually doesn't define this since years (see wmcbrine/PDCurses#28), which likely changes in some point of time (see https://osdn.net/projects/mingw/ticket/38187).

I'd say it is best to assume it is available on MinGW and WATCOM if not explicit requested otherwise.
I'm not sure about the used names, though...

@Bill-Gray Opinions?

Allow compilation with self-defined INFOEX, needed with old MINGW (and maybe old Watcom, too), by specifying `-DHAVE_NO_INFOEX`.
allow the C DEFINE added with ea517ce to be specified during Make
@GitMensch GitMensch changed the title Update pdcwin.h Allow compilation with both MinGW and mingw-64 Apr 15, 2018
@GitMensch
Copy link
Collaborator Author

For a discussion about a possibly more "clean" solution see https://sourceforge.net/p/open-cobol/discussion/help/thread/cae471ca

@GitMensch
Copy link
Collaborator Author

@Bill-Gray: With the renaming of the variant and the Makefile this PR has too much conflicts. Do you want me to redo it on current master, do it your own or aim for the "possibly more 'clean' solution"?

Note: PDCurses 3.4 worked on MinGW and likely on mingw-64, too; PDCurses 4.0.2 cannot be built with MinGW any more. The fastest version to get it working was the hack given above and manually specifying -DHAVE_NO_INFOEX when invoking make (I think I don't like this solution but I like it more than not being able to build in MinGW which itself works on older systems than mingw-64 and has less dependencies on MS dll's).

@Bill-Gray
Copy link
Owner

"With the renaming of the variant and the Makefile this PR has too much conflicts."

Though fortunately, not enough conflict to ruin your basic reasoning, i.e.: add an INFOEX=N flag for such compilers. I should have pulled it before the renaming occurred; I don't recall why I didn't.

I may have been thinking about the desirability of maintaining some level of compatibility with William McBrine's branch. He has, of course, now removed this capability. I'm not ready to do that yet (nor are you or others). But that does mean we no longer have to try to make our logic resemble that of "official" PDCurses; we can do it however we wish, and your method seems straightforward enough.

The one minor change I'd suggest is that instead of making a new PDCCFLAGS in the makefile (now makefile.mng), I'd just add the -DHAVE_INFOEX or -DHAVE_NO_INFOEX directly to the already existing CFLAGS. Do that (or tell me why we need a new variable), and I'll pull it in.

We do have the issue that William has incorporated quite a few nice improvements, particularly to the Windows console platform, and not all of them have percolated down to this fork. At some point, I'll have to go through the various commits to PDCurses/wmcbrine and add them to PDCurses/Bill-Gray (I've already done this with a few).

(Alternatively, the improvements in this fork would get pulled up into PDCurses/wmcbrine, and some already have been. But I don't expect most of them ever will; incorporating the wmcbrine improvements here seems more likely to occur.)

@GitMensch
Copy link
Collaborator Author

I'd just add the -DHAVE_INFOEX or -DHAVE_NO_INFOEX directly to the already existing CFLAGS. Do that (or tell me why we need a new variable), and I'll pull it in.

That could be done but there is no reason to set this (or possible other) PDCurses library only flags when generating the demos (and you also see those flags as the Makefile is not "silent").
If you still want the "quick and dirty" solution I'll redo a PR without an extra PDCCFLAGS.

GitMensch added a commit that referenced this pull request Aug 4, 2018
@GitMensch
Copy link
Collaborator Author

Makefile merged by Bill with ec1e87e, header by me with b475ed8

@GitMensch GitMensch closed this Aug 4, 2018
@GitMensch GitMensch deleted the GitMensch-patch-1 branch August 4, 2018 08:53
@GitMensch
Copy link
Collaborator Author

@Bill-Gray I currently don't have any old Watcom environment to test if the INFOEX=N changes should be integrated in its makefile, too - Do you?

@Bill-Gray
Copy link
Owner

Oldest (and only) OpenWATCOM I've got is 1.9, the last officially supported one. Since it's free, I'm hard pressed to think of an instance where we couldn't just say, "Go update your OpenWATCOM compiler".

I do have a machine with MSVC 5.0, which needs INFOEX=N (just tried it by adding #define HAVE_NO_INFOEX into pdcwin.h). And you can't freely update MSVC. But... 5.0 is about two decades old. I don't see much need to mangle Makefile.vc for the three people who use MSVC 5.0. (Though I can report that if you just add that line to pdcwin.h, the WinCon flavor compiles and works on a Windows ME machine.)

@GitMensch
Copy link
Collaborator Author

GitMensch commented Aug 5, 2018

Concerning OpenWatcom: Yes, let's stay with the Makefile as-is.
As the effort for VC is only minimal I'd still vote for including the change there, too (and add a second line to the MinGW and VC Makefile specifying something along "for very old systems specify INFOEX=N").

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.

None yet

2 participants