COIN-OR::LEMON - Graph Library

Opened 10 years ago

Closed 10 years ago

#215 closed defect (fixed)

windows.h should never be included by LEMON header files

Reported by: Alpar Juttner Owned by: Alpar Juttner
Priority: major Milestone: LEMON 1.1 release
Component: core Version: hg main
Keywords: Cc: Akos Ladanyi, Tapolcai János
Revision id:

Description

For instance, Arc is used in the global namespace by some versions of windows.h and it may cause problems in the user's code, even if they do not use windows.h directly.

I suggest putting all stuff using windows.c into a separate .cc file instead.

Attachments (7)

f387d3227f87.patch (8.0 KB) - added by Alpar Juttner 10 years ago.
df730c6c1298.patch (8.8 KB) - added by Alpar Juttner 10 years ago.
879c55700cd4.patch (9.6 KB) - added by Alpar Juttner 10 years ago.
28b3dec413f1_tj3.patch (1.9 KB) - added by Tapolcai János 10 years ago.
this file seems to compile under windows and linux
ab2f0a7add1e_tj.patch (7.6 KB) - added by Tapolcai János 10 years ago.
4b03fc4ad521_tj.patch (1.9 KB) - added by Tapolcai János 10 years ago.
this one has unix line breaks
0316fe05c4ec.patch (1.7 KB) - added by Alpar Juttner 10 years ago.

Download all attachments as: .zip

Change History (27)

comment:1 Changed 10 years ago by Alpar Juttner

Cc: Akos Ladanyi added

See also #214.

comment:2 Changed 10 years ago by Alpar Juttner

Cc: Tapolcai János added

Changed 10 years ago by Alpar Juttner

Attachment: f387d3227f87.patch added

comment:3 Changed 10 years ago by Alpar Juttner

[f387d3227f87] is a preliminary implementation of this idea.

It should work using the CMAKE build env. I have no idea what to do with MinGW+autotools.

Changed 10 years ago by Alpar Juttner

Attachment: df730c6c1298.patch added

comment:4 Changed 10 years ago by Alpar Juttner

Status: newassigned

[df730c6c1298] is an improved version of the [f387d3227f87]. Now, lemon/bits/windows.cc is always compiled and put into the lib, both by autotools and by cmake. Moreover, the two functions there will do something meaningful both on unices and on win32. I hope it solves the MinGW+autotools issue, too.

Note that although the stuffs in windows.h would work also on UNIX, they are used by time_measure.h and by graph_to_eps.h only on WIN32. This leads to some code duplications, but it makes it possible to use these tools in a header-only fashion.

Can anyone test this patch with MinGW? It would be even greater if someone could try out whether the Linux->WIN32 cross compilation will work with MinGW.

comment:5 Changed 10 years ago by Peter Kovacs

I like this changeset.

comment:6 in reply to:  5 ; Changed 10 years ago by Alpar Juttner

Replying to kpeter:

I like this changeset.

Which one?

Changed 10 years ago by Alpar Juttner

Attachment: 879c55700cd4.patch added

comment:7 Changed 10 years ago by Alpar Juttner

Meantime, I noticed that random.h includes windows.h, too. The changeset [879c55700cd4] gets rid of this, as well. Now, there should not be more references to windows.h in the lib.

comment:8 in reply to:  6 ; Changed 10 years ago by Peter Kovacs

Replying to alpar:

Which one?

The third one, of course, which is attached three hours later than I wrote this comment. :)

comment:9 in reply to:  8 Changed 10 years ago by Alpar Juttner

Resolution: fixed
Status: assignedclosed

Replying to kpeter:

Replying to alpar:

Which one?

The third one, of course, which is attached three hours later than I wrote this comment. :)

Good. So, I merge it both into the main and the 1.0 branches.

comment:10 Changed 10 years ago by Alpar Juttner

Resolution: fixed
Status: closedreopened

These are still some problems with VS2005 here

Changed 10 years ago by Tapolcai János

Attachment: 28b3dec413f1_tj3.patch added

this file seems to compile under windows and linux

comment:11 Changed 10 years ago by Tapolcai János

I just added a new patch (28b3dec413f1_tj3.patch) upon the latest patch of alpar (879c55700cd4.patch). This version compiles under VS2005 and under MINGV. By the way I have tried generating an Eclipse CDT4 project file with CMAKE and it works fine, too.

comment:12 in reply to:  11 Changed 10 years ago by Alpar Juttner

Replying to jtapolcai:

I just added a new patch (28b3dec413f1_tj3.patch) upon the latest patch of alpar (879c55700cd4.patch). This version compiles under VS2005 and under MINGV. By the way I have tried generating an Eclipse CDT4 project file with CMAKE and it works fine, too.

This sounds good.

May I ask you all trying out this last path on as much platforms as possible to make sure that we can really close this annoying ticket forever. By "as much platforms as possible" I mean

  • Linux + autotools
  • Linux + cmake
  • Cygwin + autotools
  • Cygwin + cmake
  • MinGW on Windows + cmake
  • VS2005 + cmake
  • VS2008 + cmake

Could you please report here the successful builds. Thank you in advance.

comment:13 in reply to:  11 Changed 10 years ago by Alpar Juttner

Replying to jtapolcai:

I just added a new patch (28b3dec413f1_tj3.patch) upon the latest patch of alpar (879c55700cd4.patch). This version compiles under VS2005 and under MINGV. By the way I have tried generating an Eclipse CDT4 project file with CMAKE and it works fine, too.

Two comments.

  • the UNICODE and _T defines should be surrounded by #ifndef/#endif pairs
  • The _T should only be defined when WIN32 is defined, too.

Others, please suspend the tests until it is fixed.

Changed 10 years ago by Tapolcai János

Attachment: ab2f0a7add1e_tj.patch added

comment:14 Changed 10 years ago by Tapolcai János

This patch compiles on

  • MinGW on Windows + cmake
  • VS2005 + cmake

Changed 10 years ago by Tapolcai János

Attachment: 4b03fc4ad521_tj.patch added

this one has unix line breaks

comment:15 Changed 10 years ago by Akos Ladanyi

879c55700cd4.patch with 4b03fc4ad521_tj.patch compiles with MinGW and Cygwin.

comment:16 in reply to:  15 Changed 10 years ago by Alpar Juttner

Replying to ladanyi:

879c55700cd4.patch with 4b03fc4ad521_tj.patch compiles with MinGW and Cygwin.

Did you try Cygwin with CMAKE or with autotools?

comment:17 Changed 10 years ago by Akos Ladanyi

To extend and clarify my previous comment: 879c55700cd4.patch with 4b03fc4ad521_tj.patch compiles with:

  • MinGW + CMake,
  • Cygwin + autotools.

It does not compile with:

  • VS2008EE + CMake.
Compiling...
windows.cc
..\..\lemon\bits\windows.cc(102) : error C2664: 'GetDateFormatW' : cannot convert parameter 5 from 'char [11]' to 'LPWSTR'
        Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast
..\..\lemon\bits\windows.cc(104) : error C2664: 'GetTimeFormatW' : cannot convert parameter 5 from 'char [9]' to 'LPWSTR'
        Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast
..\..\lemon\bits\windows.cc(106) : error C2664: 'GetDateFormatW' : cannot convert parameter 5 from 'char [5]' to 'LPWSTR'
        Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast

Changed 10 years ago by Alpar Juttner

Attachment: 0316fe05c4ec.patch added

comment:18 in reply to:  17 ; Changed 10 years ago by Alpar Juttner

Replying to ladanyi:

It does not compile with:

  • VS2008EE + CMake.

Changeset [0316fe05c4ec] is another (hopefully the last) attempt to solve this annoying issue.

It compiles and runs correctly on:

  • Linux (cmake and autotools)
  • Cygwin (cmake and autotools)
  • MinGW (cmake)
  • Visual Studio 2008 Express Edition (cmake)

Could anyone check it with VS 2005? It would be nice to see it working on both the Express and the full editions.

If it compiles in order, please run the graph_to_eps_demo and check the %%CreationDate: field in the created files.

comment:19 in reply to:  18 Changed 10 years ago by Tapolcai János

Replying to alpar:

Replying to ladanyi: Could anyone check it with VS 2005? It would be nice to see it working on both the Express and the full editions.

compiles fine under VS 2005 full editions

comment:20 Changed 10 years ago by Alpar Juttner

Resolution: fixed
Status: reopenedclosed

The changeset [3f0ddf255524] should put an end to this story.

Note: See TracTickets for help on using tickets.