COIN-OR::LEMON - Graph Library

Opened 9 years ago

Closed 9 years ago

#215 closed defect (fixed)

windows.h should never be included by LEMON header files

Reported by: alpar Owned by: alpar
Priority: major Milestone: LEMON 1.1 release
Component: core Version: hg main
Keywords: Cc: ladanyi, jtapolcai
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 9 years ago.
df730c6c1298.patch (8.8 KB) - added by alpar 9 years ago.
879c55700cd4.patch (9.6 KB) - added by alpar 9 years ago.
28b3dec413f1_tj3.patch (1.9 KB) - added by jtapolcai 9 years ago.
this file seems to compile under windows and linux
ab2f0a7add1e_tj.patch (7.6 KB) - added by jtapolcai 9 years ago.
4b03fc4ad521_tj.patch (1.9 KB) - added by jtapolcai 9 years ago.
this one has unix line breaks
0316fe05c4ec.patch (1.7 KB) - added by alpar 9 years ago.

Download all attachments as: .zip

Change History (27)

comment:1 Changed 9 years ago by alpar

  • Cc ladanyi added

See also #214.

comment:2 Changed 9 years ago by alpar

  • Cc jtapolcai added

Changed 9 years ago by alpar

comment:3 Changed 9 years ago by alpar

[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 9 years ago by alpar

comment:4 Changed 9 years ago by alpar

  • Status changed from new to assigned

[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 follow-up: Changed 9 years ago by kpeter

I like this changeset.

comment:6 in reply to: ↑ 5 ; follow-up: Changed 9 years ago by alpar

Replying to kpeter:

I like this changeset.

Which one?

Changed 9 years ago by alpar

comment:7 Changed 9 years ago by alpar

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 ; follow-up: Changed 9 years ago by kpeter

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 9 years ago by alpar

  • Resolution set to fixed
  • Status changed from assigned to closed

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 9 years ago by alpar

  • Resolution fixed deleted
  • Status changed from closed to reopened

These are still some problems with VS2005 here

Changed 9 years ago by jtapolcai

this file seems to compile under windows and linux

comment:11 follow-ups: Changed 9 years ago by 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.

comment:12 in reply to: ↑ 11 Changed 9 years ago by alpar

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 9 years ago by alpar

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 9 years ago by jtapolcai

comment:14 Changed 9 years ago by jtapolcai

This patch compiles on

  • MinGW on Windows + cmake
  • VS2005 + cmake

Changed 9 years ago by jtapolcai

this one has unix line breaks

comment:15 follow-up: Changed 9 years ago by ladanyi

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

comment:16 in reply to: ↑ 15 Changed 9 years ago by alpar

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 follow-up: Changed 9 years ago by 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 9 years ago by alpar

comment:18 in reply to: ↑ 17 ; follow-up: Changed 9 years ago by alpar

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 9 years ago by jtapolcai

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 9 years ago by alpar

  • Resolution set to fixed
  • Status changed from reopened to closed

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

Note: See TracTickets for help on using tickets.