Skip to content

change dllexport controlling macro to use _WIN32 #66

Merged
perlpunk merged 1 commit into
yaml:release/0.2.3from
marksheahan:master
Apr 11, 2020
Merged

change dllexport controlling macro to use _WIN32 #66
perlpunk merged 1 commit into
yaml:release/0.2.3from
marksheahan:master

Conversation

@marksheahan
Copy link
Copy Markdown
Contributor

use _WIN32 which is provided by msvc compiler, instead of WIN32 which is user configurable, and omitted by default on some x64 builds.

I'm still needing to do:
#define YAML_DECLARE_STATIC
#include <yaml.h>
in my own code on windows builds to avoid dllimport when using libyaml as a static library, but this is acceptable.

…mpiler) instead of WIN32 (which is user configurable, and omitted by default on some x64 builds); this fixes an issue with 64 bit windows static library builds
@ingydotnet ingydotnet mentioned this pull request Jan 7, 2018
@ingydotnet
Copy link
Copy Markdown
Member

@marksheahan Thanks for the PR. I don't have a way to test this out yet. I wonder if it can be done on Appveyor CI/CD?

Is there a no-cost toolchain that I can point one of our windows devs at to test/review this?

Glances at @hanetzer ...

@hanetzer
Copy link
Copy Markdown
Contributor

hanetzer commented Jan 7, 2018

@ingydotnet mingw-w64 is a very good toolchain, basically gcc for windows, no cost.
@marksheahan are you deliberately trying to use static libraries, or are you just using
them because prior to #88 's merge we were not producing shared windows libraries
(dll's)?

@ingydotnet
Copy link
Copy Markdown
Member

@hanetzer I meant a msvc free compiler to test with. I think MS offers such a thing, but it's been a long time...

@hanetzer
Copy link
Copy Markdown
Contributor

hanetzer commented Jan 8, 2018

@ingydotnet did you not notice that the appveyor cmake builds are happening with the windows toolchains?

@marksheahan
Copy link
Copy Markdown
Contributor Author

marksheahan commented Jan 11, 2018 via email

@perlpunk
Copy link
Copy Markdown
Member

perlpunk commented Apr 11, 2020

It seems this was fixed at some point, but the fix got lost:
Commit 52e34e2 from 2011 by Kirill changed it to _WIN32

Commit 119b7b6 changed it back to WIN32 accidentally, I assume.

So I think this should be merged.

@perlpunk perlpunk changed the base branch from master to release/0.2.3 April 11, 2020 14:06
@perlpunk perlpunk merged commit 9bc9f61 into yaml:release/0.2.3 Apr 11, 2020
@perlpunk
Copy link
Copy Markdown
Member

Thanks, merged!

perlpunk pushed a commit that referenced this pull request Apr 11, 2020
(provided by msvc compiler) instead of WIN32 (which is user configurable, and
omitted by default on some x64 builds); this fixes an issue with 64 bit windows
static library builds (#66)

Co-authored-by: Mark Sheahan <mark.sheahan@upguard.com>
egli added a commit to liblouis/liblouis that referenced this pull request Aug 25, 2020
for some (unknown) reason only this version will create a
lou_checkyaml.exe that has libyaml statically linked in.

Believe me, I tried with 0.2.1, 0.2.2 and 0.2.5 and none of them had
libyaml statically included.

There is a remark in
yaml/libyaml#66 (comment) which
indicates that at some point they weren't even shared windows
libraries.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants