-
Notifications
You must be signed in to change notification settings - Fork 143
[Build] Add a patch to check whether we have .NET 4.6 at build time. #164
Conversation
0558719
to
addd03d
Compare
Let's see how jenkins copes with this. |
6c76abb
to
4afb5d7
Compare
@@ -224,6 +230,9 @@ AM_CONDITIONAL(ENABLE_MONOGETOPTIONS, test "x$has_mono" = "xtrue") | |||
|
|||
CSFLAGS="$DEBUG_FLAGS $CSDEFINES $WIN64DEFINES -unsafe" | |||
|
|||
if text "x$missing_net_4_6" = "xno" ; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: s/text/test/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
PKG_CHECK_MODULES(MONO_DEPENDENCY, mono >= $MONO_REQUIRED_VERSION, has_mono=true, has_mono=false) | ||
if test "x$has_mono" = "xfalse" ; then | ||
PKG_CHECK_MODULES(MONO_DEPENDENCY, mono-2 >= $MONO_REQUIRED_VERSION, has_mono=true, has_mono=false) | ||
if "x$has_mono" = "xtrue" ; then | ||
PKG_CHECK_MODULES(MONO_DEPENDENCY, mono < $MONO_NET_4_6, missing_net_4_6=true, missing_net_4_6=false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather use NET_4_6_SUPPORT (uppercase).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
3a88086
to
e7bd823
Compare
@@ -88,15 +88,19 @@ static string Utf8PtrToStringFast (IntPtr ptr, int len) | |||
} | |||
} | |||
|
|||
#if HAVE_NET_4_6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're doing a check at buildtime then you don't need a reflection check at runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
@@ -106,9 +106,19 @@ OFF_T_FLAGS="-define:OFF_T_$ac_cv_sizeof_off_t" | |||
AC_SUBST(OFF_T_FLAGS) | |||
|
|||
MONO_REQUIRED_VERSION=1.0 | |||
MONO_NET_4_6=4.4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about "FIRST_MONO_VERSION_WITH_NET_4_6_SUPPORT"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
e7bd823
to
1ad5f63
Compare
if (hasFastGetStringOverload) | ||
return Utf8PtrToStringFast (ptr, len); | ||
|
||
#if HAVE_NET_4_6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about wrapping the old 3 lines with #if UTF8_SLOWPATH , and after them, return Utf8PtrToStringFast (ptr, len);
this way removing this build check in the future will be easier and cause less diff noise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see it done ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The diff is still here since you commented on the define, see below, the Utf8PtrStringFast is inlined now under the same HAVE_NET_4_6.
This way, you can use unifdef to remove the segments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant this way: #165
PKG_CHECK_MODULES(MONO_DEPENDENCY, mono >= $MONO_REQUIRED_VERSION, has_mono=true, has_mono=false) | ||
if test "x$has_mono" = "xfalse" ; then | ||
PKG_CHECK_MODULES(MONO_DEPENDENCY, mono-2 >= $MONO_REQUIRED_VERSION, has_mono=true, has_mono=false) | ||
if test "x$has_mono" = "xtrue" ; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beware, you placed this if test "x$has_mono" = "xtrue"
check inside a has_mono=xfalse block, so potentially this may always evaluate to false.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the above pkg check resets the value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh oops, disregard this.
1ad5f63
to
4e6206d
Compare
No description provided.