Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(918)

Issue 579340043: Fixes for cross-compilation to x86_64-w64-mingw32 (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 1 month ago by hahnjo
Modified:
3 years, 11 months ago
Reviewers:
benko.pal, lemzwerg, hanwenn
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Fixes for cross-compilation to x86_64-w64-mingw32 1) Include config.hh before cmath Recently commit 7396a28bde ("Add enabling extension definitions for `-std=c++11` option") added a few #defines to config.hh to make POSIX extensions available. Make sure that they are pulled in from config.hh before cmath is included. 2) Improve cast from pointer to integer When cross-compiling to x86_64-w64-mingw32, 'long' (32 bit) is smaller than 'void *' (64 bit). This leads to compiler errors because the cast truncates. After analysis I think that truncation is actually ok here, so make this explicit by first casting to a suitable integer and then truncate to long. Note that 'suitable integer' is quite a mess: Ideally we would use intptr_t, but it's optional per the C++ standard. So instead rely on intmax_t and hope that it works. 3) Use host triple when looking for windres --host=host-type the type of system on which the package runs. [...] The $target variable is meant for compilers. However, it is still needed for compatibility with GUB / i686-mingw32. 4) Rename DATADIR to CONFIG_DATADIR The header objidl.h from mingw has an enum called DATADIR. This conflicts with our definition and results in compilation errors because the preprocessor replaces the enum name with the configured string. This is apparently dealt with in mingw-compatibility.hh, but not in the configure checks. As a result, they return the wrong answer which leads to further compilation errors down the road.

Patch Set 1 #

Total comments: 3

Patch Set 2 : add FIXME wrt truncation #

Patch Set 3 : Rename DATADIR to CONFIG_DATADIR #

Patch Set 4 : Use config.hh #

Patch Set 5 : fixes for GUB #

Unified diffs Side-by-side diffs Delta from patch set Stats (+41 lines, -6 lines) Patch
M aclocal.m4 View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download
M config.hh.in View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M flower/include/interval.hh View 1 2 3 1 chunk +4 lines, -1 line 0 comments Download
M flower/include/real.hh View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M flower/offset.cc View 1 2 3 1 chunk +5 lines, -0 lines 0 comments Download
M flower/polynomial.cc View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download
M lily/accidental-engraver.cc View 1 2 chunks +8 lines, -1 line 0 comments Download
M lily/bezier-bow.cc View 1 2 3 1 chunk +5 lines, -0 lines 0 comments Download
M lily/stencil-integral.cc View 1 2 3 2 chunks +4 lines, -0 lines 0 comments Download
M lily/tuplet-bracket.cc View 1 2 3 2 chunks +5 lines, -0 lines 0 comments Download

Messages

Total messages: 13
lemzwerg
LGTM
4 years, 1 month ago (2020-02-28 06:25:11 UTC) #1
benko.pal
it may not matter, but actually long is 32 bit, void * is 64 bit ...
4 years, 1 month ago (2020-02-28 09:23:44 UTC) #2
hahnjo
On 2020/02/28 09:23:44, benko.pal wrote: > it may not matter, but actually long is 32 ...
4 years, 1 month ago (2020-02-28 09:32:57 UTC) #3
benko.pal
On 2020/02/28 09:32:57, hahnjo wrote: > On 2020/02/28 09:23:44, benko.pal wrote: > > it may ...
4 years, 1 month ago (2020-02-28 09:54:00 UTC) #4
hanwenn
https://codereview.appspot.com/579340043/diff/571760043/lily/accidental-engraver.cc File lily/accidental-engraver.cc (left): https://codereview.appspot.com/579340043/diff/571760043/lily/accidental-engraver.cc#oldcode338 lily/accidental-engraver.cc:338: (long) trans); ouch - this is terrible. could we ...
4 years, 1 month ago (2020-02-28 09:54:16 UTC) #5
hahnjo
https://codereview.appspot.com/579340043/diff/571760043/lily/accidental-engraver.cc File lily/accidental-engraver.cc (left): https://codereview.appspot.com/579340043/diff/571760043/lily/accidental-engraver.cc#oldcode338 lily/accidental-engraver.cc:338: (long) trans); On 2020/02/28 09:54:16, hanwenn wrote: > ouch ...
4 years, 1 month ago (2020-02-28 09:57:30 UTC) #6
hanwenn
https://codereview.appspot.com/579340043/diff/571760043/lily/accidental-engraver.cc File lily/accidental-engraver.cc (left): https://codereview.appspot.com/579340043/diff/571760043/lily/accidental-engraver.cc#oldcode338 lily/accidental-engraver.cc:338: (long) trans); On 2020/02/28 09:57:30, hahnjo wrote: > On ...
4 years, 1 month ago (2020-02-28 10:02:46 UTC) #7
hahnjo
add FIXME wrt truncation
4 years, 1 month ago (2020-02-28 10:07:09 UTC) #8
hahnjo
On 2020/02/28 10:02:46, hanwenn wrote: > https://codereview.appspot.com/579340043/diff/571760043/lily/accidental-engraver.cc > File lily/accidental-engraver.cc (left): > > https://codereview.appspot.com/579340043/diff/571760043/lily/accidental-engraver.cc#oldcode338 > ...
4 years, 1 month ago (2020-02-28 10:07:40 UTC) #9
hahnjo
Rename DATADIR to CONFIG_DATADIR
4 years ago (2020-02-29 10:37:28 UTC) #10
hahnjo
On 2020/02/29 10:37:28, hahnjo wrote: > Rename DATADIR to CONFIG_DATADIR I need this additional fix ...
4 years ago (2020-02-29 10:39:35 UTC) #11
hahnjo
Use config.hh
4 years ago (2020-03-02 09:51:56 UTC) #12
hahnjo
4 years ago (2020-03-02 16:23:31 UTC) #13
fixes for GUB
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b