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

Set runtime and archive install destinations #31

Merged
merged 1 commit into from Oct 29, 2018

Conversation

2 participants
@donny-dont
Copy link
Contributor

donny-dont commented Oct 15, 2018

RUNTIME and ARCHIVE destinations were not properly set.

@aperezdc
Copy link
Contributor

aperezdc left a comment

Hi, thanks for the contribution! I have a doubt, though: Why are these changes needed? A comment in the commit log explaining the reason for setting those paths would be useful.

The wpe target is for a shared library, so the RUNTIME destination should not be needed, and I think we only support building it as a shared library (the target architecture must support shared libraries anyway due to usage of dlopen), so the ARCHIVE destination is not expected to be used either. Am I missing something else?

@donny-dont

This comment has been minimized.

Copy link
Contributor

donny-dont commented Oct 22, 2018

If you build for windows then there will be a dll and lib associated.

@aperezdc

This comment has been minimized.

Copy link
Contributor

aperezdc commented Oct 23, 2018

@donny-dont Thanks for the explanation. Could you please update the commit log adding a note that the change is needed for Windows? Maybe the need is clear for people used to develop for Windows, but it would be nice to have a note for those of us who are not used to develop for it. If you do that, I'll happily merge this. Thanks in advance!

Install runtime and artifacts when building for Windows
Windows outputs a dll and lib when building shared libraries.

@donny-dont donny-dont force-pushed the donny-dont:fix/target-install branch from df8432f to e5a43f3 Oct 26, 2018

@donny-dont

This comment has been minimized.

Copy link
Contributor

donny-dont commented Oct 26, 2018

@aperezdc hopefully that's better for you.

@aperezdc
Copy link
Contributor

aperezdc left a comment

Thanks for updating the commit log — this is now just perfect, so let's merge it.

@aperezdc aperezdc merged commit 2937620 into WebPlatformForEmbedded:master Oct 29, 2018

@aperezdc aperezdc added this to the Version 1.0.1 milestone Nov 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment