-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
"Hide most of non-public..." PR breaks Windows build #1046
Comments
Can you be more specific? Appveyor says it's still passing:
https://ci.appveyor.com/project/jbeder/yaml-cpp
…On Mon, Oct 11, 2021 at 3:42 PM noelotterness ***@***.***> wrote:
PR #984 <#984> broke the Windows
use of the yaml-cpp branch. It was fixed when it was backed out on Sept
28th and it has broken the Windows usage again with the revert of the
revert.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1046>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAICUBVZ7DAPPXU3M5QCQY3UGND5BANCNFSM5FZA37IQ>
.
|
Here is the part of my GitHub action that pulls down the yaml-ccp (ignore the "ref:" section as I had to add that to get the build to work).
Then the files that use the yaml-ccp just include:
Prior to the PR, the latest all worked and then after the PR, the build is broken. |
I should have included the following compile line:
|
I am using Visual Studio 2019, version 16.11.4 and it does not look like your test suites include that. |
Can you post the full error logs?
…On Mon, Oct 11, 2021 at 4:30 PM noelotterness ***@***.***> wrote:
I am using Visual Studio 2019, version 16.11.4 and it does not look like
your test suites include that.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1046 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAICUBSDNBQ7LFF3Y75Q2VTUGNJQDANCNFSM5FZA37IQ>
.
|
|
Can you please also post a link to the source code, especially the build system files? There are different ways to embed another project inside your own, and it's hard for me to provide proper feedback if I don't know what's going on. Also, see my answer in a previous ticket on this: #1036 (comment) |
First I cannot provide the source code since it is not a public repository. The build system files are what are generated by Visual Studio to build a native C++ DLL for IIS. I include the directories where the generated source is placed. The problem is that there is something in the yaml-cpp source that requires the dll.h file and that is no longer being generated for the Windows 2019 build. |
So you are embedding yaml-cpp are part of your sources?
|
what about anyone not using cmake? |
Considering the build system is cmake, it seems fair to assume that the build system will take care of everything needed to properly build yaml-cpp. If you want to embed yaml-cpp and build it yourself without using cmake, then you will need to replicate all the logic yourself:
This was valid also before, and usually it is valid in any project for any build system different than one one(s) supported by the project. |
I'm not sure how your points are useful, all I know is it used to work, now it doesn't. |
Then simply... use cmake, which is the supported build system. If not, please describe what is the use case for not using the supported build system of the project. "it worked before, now it doesn't" is simply not enough to understand what is the actual situation. |
I'm sympathetic to @PhilipDeegan's issue here; even though the supported build system is CMake, I'd like for it to be easy to use in any build system someone wants. (E.g., there's a half-finished Bazel config for yaml-cpp that would be nice to get working, since I personally prefer Bazel to CMake.) yaml-cpp is so simple that you can usually just throw all of the sources into your build system and it'll just work. This conversation made me realize that this is the first example where that's no longer is true; although there are sometimes aspects of the CMake config that probably are useful, we've never straight-up generated a source file before. @pinotree, is there a way to solve the issue you were trying to solve (if I recall, reducing the non-public symbols that are exported) without generating the |
@pinotree, I would also like to hear the answer to this question.
It really is! In this I see a big plus of yaml-cpp. In my projects, I include yaml-cpp in the form of source codes, the assembly of the library takes place in conjunction with a specific project. CMake is not used at all. Therefore, after updating to commit 0733aeb, all my projects stopped building. Of course the situation will be corrected - I will return back the static file dll.h. In fact, now cmake has received a monopoly to build the yaml-cpp. Those who do not use cmake in their work will have to find solutions on their own. It seems to me that this is not very correct. @jbeder, did you manage to build the master-branch using Bazel? |
@hkarel @pinotree @PhilipDeegan If any of you has time and a Windows machine, could you please help getting PR #1063 passing the Windows shared lib test? |
Fixed by 2b65c65. Thanks @hkarel and @PhilipDeegan! And once more to @pinotree: I'm open to a PR fixing the original issue you were trying to fix! |
PR #984 broke the Windows use of the yaml-cpp branch. It was fixed when it was backed out on Sept 28th and it has broken the Windows usage again with the revert of the revert.
Here is what happens with the broken submission:
The text was updated successfully, but these errors were encountered: