303574 - MinGW build problem in icalrecur.c

Status: RESOLVED WONTFIX

(Calendar :: Internal Components, defect)

Description

[Matthias Bockelkamp]

The build of the calendar as part of the suite fails because of a double
definition of intptr_t. Patch follows.

Comment 1

[Matthias Bockelkamp]

Attachment: Patch

Comment 2

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 191699 [details] [diff] [review]
Patch

So mingw doesn't define HAVE_INTPTR_T, but does have it? Sounds like a mingw
problem to me.

Comment 3

[Matthias Bockelkamp]

I just found
<http://sourceforge.net/tracker/index.php?func=detail&aid=1020042&group_id=5470&atid=305470>,
where the suggestion is to use 

#if HAVE_STDINT_H
#define HAVE_UINTPTR_T 1
#define HAVE_INTPTR_T 1
#endif

Since LXR sais HAVE_UINTPTR_T isn't used anywhere, that line can be removed.
Patch follows.

Comment 4

[Matthias Bockelkamp]

Attachment: Patch 2 (workaround stolen from python, untested)

Comment 5

[Matthias Bockelkamp]

Comment on attachment 191752 [details] [diff] [review]
Patch 2 (workaround stolen from python, untested)

That was totally wrong

Comment 6

[Matthias Bockelkamp]

Attachment: Patch 3

This works with MinGW but I'm not able to test other compilers.

Note: LXR sais icalrecur.c is the only place where HAVE_INTPTR_T is used, so
setting it in the "#ifdef HAVE_STDINT_H" block above doesn't pay.

Comment 7

[Simon Paquet [:sipaq]]

Comment on attachment 191753 [details] [diff] [review]
Patch 3

Moving review request to mvl, since he is still actively reviewing stuff and has also already commented in this bug.

Comment 8

[Reed Loden [:reed]]

The bugspam monkeys have been set free and are feeding on Calendar :: Internal Components. Be afraid for your sanity!

Comment 9

[David G King]

This patch seems to have bitrotted slightly due to some BeOS code.

Comment 10

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 191753 [details] [diff] [review]
Patch 3

you want to test for having intptr_t, not for some random other thing.

Comment 11

[Craig Watcham]

Attachment: Alternate patch

Comment 12

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 255212 [details] [diff] [review]
Alternate patch

This patch is exactly the same as attachment 191699 [details] [diff] [review], and does not address my comment in comment 2 (or explains why that comment is wrong)

Comment 13

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 255212 [details] [diff] [review]
Alternate patch

This is exactly the same patch as the first patch here, which i already rejected in comment 2.

Comment 14

[Matthias Bockelkamp]

MVL, please give me a hint how you want this to be fixed. This seems to be a trivial thing, since MinGW supports intptr_t but just doesn't define HAVE_INTPTR_T. Would a test that checks for MinGW first be more welcome?

Comment 15

[Daniel Boelzle [:dbo]]

Attachment: different approach for discussion

IMO we should not stick to HAVE_STDINT_H, HAVE_INTPTR_T at all. When including stdint.h, I assume intptr_t being defined. So we should IMO more cleanly just work around msvc using stddef.h (testing against _MSC_VER) while all other platforms use stdint.h. However, I don't know what compiler is used on XP_BEOS, but I suspect we could just use one of those two headers...

Comment 16

[Martin Schröder [:mschroeder]]

Can this bug be marked as resolved INVALID/WONTFIX/WORKSFORME? btw Such a fix should be made in libical directly (http://sourceforge.net/projects/freeassociation/) in the first place.

Comment 17

[Philipp Kewisch [:Fallen][📱📆]]

I think this should go into libical if it has not already, and since we haven't heard anything in 2 years, lets close the bug. Thanks for trying though! Feel free to reopen if someone is still experiencing this issue.