You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by "mmartell (GitHub)" <gi...@apache.org> on 2018/11/20 23:51:19 UTC
[GitHub] [geode-native] mmartell opened pull request #407:
GEODE-6054: build with vs 2017
[ Full content available at: https://github.com/apache/geode-native/pull/407 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on issue #407:
GEODE-6054: build with vs 2017
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Can you explain why you are changing the exception handling mode? How is it interacting with the .NET library since this exception mode is not supported for /clr?
https://msdn.microsoft.com/en-us/library/ffkc918h.aspx
> The following compiler options are not supported with /clr:
>/EHsc and /EHs (/clr implies /EHa (see /EH (Exception Handling Model))
Strikes me that mixing exception modes is dangerous.
[ Full content available at: https://github.com/apache/geode-native/pull/407 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] echobravopapa commented on issue #407:
GEODE-6054: build with vs 2017
Posted by "echobravopapa (GitHub)" <gi...@apache.org>.
@mmartell if this has stalled you might consider closing the PR until ready for review again...
[ Full content available at: https://github.com/apache/geode-native/pull/407 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] mmartell commented on pull request #407:
GEODE-6054: build with vs 2017
Posted by "mmartell (GitHub)" <gi...@apache.org>.
Good catch! We overwrote CMAKE_CXX_FLAGS inadvertently when adding /EHsc.
[ Full content available at: https://github.com/apache/geode-native/pull/407 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on issue #407:
GEODE-6054: build with vs 2017
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Can you explain why you are changing the exception handling mode? How is it interacting with the .NET library since this exception mode is not supported for /clr?
> The following compiler options are not supported with /clr:
>/EHsc and /EHs (/clr implies /EHa (see /EH (Exception Handling Model))
Strikes me that mixing exception modes is dangerous.
[ Full content available at: https://github.com/apache/geode-native/pull/407 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#407: GEODE-6054: build with vs 2017
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Delete
[ Full content available at: https://github.com/apache/geode-native/pull/407 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] mmartell commented on pull request #407:
GEODE-6054: build with vs 2017
Posted by "mmartell (GitHub)" <gi...@apache.org>.
Moved this to root.
[ Full content available at: https://github.com/apache/geode-native/pull/407 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] mmartell commented on issue #407: GEODE-6054:
build with vs 2017
Posted by "mmartell (GitHub)" <gi...@apache.org>.
This is being replaced by PR #411.
[ Full content available at: https://github.com/apache/geode-native/pull/407 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on issue #407:
GEODE-6054: build with vs 2017
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Can you explain why you are changing the exception handling mode? How is it interacting with the .NET library since this exception mode is not supported for /clr?
> The following compiler options are not supported with /clr:
>/EHsc and /EHs (/clr implies /EHa (see /EH (Exception Handling Model))
[ Full content available at: https://github.com/apache/geode-native/pull/407 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] mmartell commented on issue #407: GEODE-6054:
build with vs 2017
Posted by "mmartell (GitHub)" <gi...@apache.org>.
Reverted the change to a different exception model. All issues addressed with latest commit.
[ Full content available at: https://github.com/apache/geode-native/pull/407 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#407: GEODE-6054: build with vs 2017
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Delete
[ Full content available at: https://github.com/apache/geode-native/pull/407 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] mmartell commented on issue #407: GEODE-6054:
build with vs 2017
Posted by "mmartell (GitHub)" <gi...@apache.org>.
Thanks pivotal-jbarrett for a diligent review! All changes have been implemented.
[ Full content available at: https://github.com/apache/geode-native/pull/407 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] mmartell commented on issue #407: GEODE-6054:
build with vs 2017
Posted by "mmartell (GitHub)" <gi...@apache.org>.
I think some new Warnings as Errors shows up with the new (and likely stricter compiler). I will forward to you as soon as I finish this episode of Ozark. Not sure about using a different exception model between the managed and unmanaged sides.
> On Nov 21, 2018, at 7:21 PM, Jacob Barrett <no...@github.com> wrote:
>
> Can you explain why you are changing the exception handling mode? How is it interacting with the .NET library since this exception mode is not supported for /clr?
>
> The following compiler options are not supported with /clr:
> /EHsc and /EHs (/clr implies /EHa (see /EH (Exception Handling Model))
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub, or mute the thread.
[ Full content available at: https://github.com/apache/geode-native/pull/407 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#407: GEODE-6054: build with vs 2017
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Why -m64 here?
The /D would be handled in a target definitions call and not cxx flags.
[ Full content available at: https://github.com/apache/geode-native/pull/407 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#407: GEODE-6054: build with vs 2017
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Format
[ Full content available at: https://github.com/apache/geode-native/pull/407 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#407: GEODE-6054: build with vs 2017
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
I am 99% certain that is already defined. I know 100% the _WIN32 is.
[ Full content available at: https://github.com/apache/geode-native/pull/407 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] mmartell closed pull request #407:
GEODE-6054: build with vs 2017
Posted by "mmartell (GitHub)" <gi...@apache.org>.
[ pull request closed by mmartell ]
[ Full content available at: https://github.com/apache/geode-native/pull/407 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] mmartell commented on issue #407: GEODE-6054:
build with vs 2017
Posted by "mmartell (GitHub)" <gi...@apache.org>.
We were seeing a C4530 Warning:
https://msdn.microsoft.com/en-us/library/2axwkyt4.aspx
But I can't reproduce it. Trying a build without the /EHsc. Also, it seems
like this would be the default for unmanaged but I can't find docs on that.
On Wed, Nov 21, 2018 at 8:51 PM Michael Martell <mm...@pivotal.io> wrote:
> I think some new Warnings as Errors shows up with the new (and likely
> stricter compiler). I will forward to you as soon as I finish this episode
> of Ozark. Not sure about using a different exception model between the
> managed and unmanaged sides.
>
> On Nov 21, 2018, at 7:21 PM, Jacob Barrett <no...@github.com>
> wrote:
>
> Can you explain why you are changing the exception handling mode? How is
> it interacting with the .NET library since this exception mode is not
> supported for /clr?
>
> The following compiler options are not supported with /clr:
> /EHsc and /EHs (/clr implies /EHa (see /EH (Exception Handling Model))
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/geode-native/pull/407#issuecomment-440901142>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AP6jXyrJoR90QvPlCf1ADBanlXJp5qUNks5uxhgegaJpZM4YsIuG>
> .
>
>
[ Full content available at: https://github.com/apache/geode-native/pull/407 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode-native] pivotal-jbarrett commented on pull request
#407: GEODE-6054: build with vs 2017
Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
This should be set at the root project so all inherit now if it’s required on all. Otherwise I think there is a set target property for this. At the very least if you’re changing this variablenit should be appended to the current value and not replacing.
[ Full content available at: https://github.com/apache/geode-native/pull/407 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org