Skip to content
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

Add BrowserSubProcess dependency checking. #959

Merged
merged 2 commits into from
Apr 21, 2015
Merged

Add BrowserSubProcess dependency checking. #959

merged 2 commits into from
Apr 21, 2015

Conversation

pushplay
Copy link
Contributor

No description provided.

var browserSubprocessDir = Path.GetDirectoryName(browserSubProcessPath);
if (browserSubprocessDir == null)
{
missingDependencies.AddRange(BrowserSubprocessDependencies);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be a CheckDependencyList call here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No at this point browserSubProcessPath is something that doesn't exist, so everything that should be in there is by default missing. I considered adding browserSubProcessPath itself to the collection instead, but that didn't seem consistent with the rest of the code. Also browserSubProcessPath could be null and then there's not enough explanation of what's wrong.

@amaitland amaitland added this to the 39.0.1 milestone Apr 21, 2015
@amaitland amaitland merged commit 2abd9a2 into cefsharp:master Apr 21, 2015
@amaitland
Copy link
Member

I made one small tweak, more from a style/readability point of view
969b694

Other than that, looks solid 👍 Thanks!

@amaitland
Copy link
Member

When I tried in Debug mode the extra subprocess checking throws an exception.

So temporarily I've disabled if the debugger is attached see 3e3ea4f

Previously the BrowserSubProcess worked fine being isolated on it's own without libcef.dll etc, so I'm not sure we actually need those checks or not?

@pushplay
Copy link
Contributor Author

In my dev environment BrowserSubProcess would not launch unless libcef.dll is there. I think the difference might come down to vhost.exe versions. Which version of VS are you running?

@amaitland
Copy link
Member

I use VS2012 at work and VS2013 at home. I've tested in VS2013, can try VS2012 later.

I've never had a problem with either previously.

@pushplay
Copy link
Contributor Author

Yeah I have VS 2013 at work. So much for that theory.

@amaitland
Copy link
Member

VS 2013 Update 4?

@pushplay
Copy link
Contributor Author

Yup.

@amaitland
Copy link
Member

It's easy enough to add a .props file to copy the dependencies. I'm just curious as to what's different between environments?

I'll try on my work machine later, it has both VS2012 and VS2013 (I use VS2012 for the most part as the Resharper license I have is old and doesn't support VS2013). It's Windows 7 where I'm running Windows 8.1 at home

@pushplay
Copy link
Contributor Author

I'm also curious but I'm out of ideas. I don't know too much about the rules for loading and sharing libraries in Windows. OS is Windows 8.1 Enterprise here. Maybe there's some obscure security policy that's relevant?

Programming in VS without ReSharper is sadness.

@rassilon
Copy link
Contributor

@amaitland , maybe we could ask for ReSharper/NDepend OpenSource licenses for CefSharp?

That'd be very cool.

@amaitland
Copy link
Member

I believe at one point in time @perlun had organized Resharper licenses for CefSharp. I think it's worth following up on.

I'd like to try out Resharper for C++, that could be very handy 👍

@perlun
Copy link
Member

perlun commented Apr 23, 2015

I have that, yes. Give me your email (perlun in Google Mail) and I'll see what I can do. Didn't even know about ReSharper for C++, that sounds incredibly nice, since C++ is so horrible to work with otherwise. 😛

@amaitland
Copy link
Member

Didn't even know about ReSharper for C++

It was only just released a couple of weeks ago. See release announcement for some pretty detailed info http://blog.jetbrains.com/dotnet/2015/04/10/introducing-resharper-cpp/

@amaitland
Copy link
Member

Give me your email (perlun in Google Mail)

Email sent, thanks 👍

@rassilon
Copy link
Contributor

Ditto, thanks 👍

@rassilon
Copy link
Contributor

Hey @perlun, @amaitland , the NDepend folks got back to me, they are still willing to give out licenses for OSS projects, however they require additional details in order to issue the licenses.

I emailed @perlun about it in hopes he'd reply-all to all of us via email about it, but I haven't heard back yet. I'm <firstname>.<lastname> at Google Mail. You can find that info in my github profile now.

Fyi,
Bill

@amaitland
Copy link
Member

the NDepend folks got back to me, they are still willing to give out licenses for OSS projects

Nice one 👍

however they require additional details in order to issue the licenses.

Out of curiosity? What sort of information are they asking for?

@rassilon
Copy link
Contributor

Postal address & phone number.

@rassilon
Copy link
Contributor

@amaitland , would you like to be included on the NDepend thing?

@amaitland
Copy link
Member

Sure. Best to email?

@rassilon
Copy link
Contributor

Yeah. See previous comment about my email address. I've edited the Markdown so it renders properly now.

@amaitland
Copy link
Member

Email sent.

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