Implement DependencyChecker #900

amaitland · 2015-03-24T01:05:22Z

This is a rough first draft of DependencyChecker. Basically loops through list of all Cef/CefSharp dependencies.

Creating this PR before I go too far, some questions to be answered I think.

Should we provide the option to validate have some dependencies marked as optional?
What's the expected output if calling from user land? Is a list of strings containing which files are missing ok? Would people just prefer a formatted string?
Anyone have any thoughts on obtaining the path? I've started with the ExecutingAssembly and allowed an overload so that users can call themselves.

Will include an overload for Cef.Initialize() that enables/disables the dependency check when I'm happy the other questions have been answered.

Resolves #876

…CefSharp dependencies

amaitland · 2015-03-24T01:06:52Z

@jornh @jankurianski @rassilon Any thoughts/comments?

(Full disclaimer, first draft is a little rough).

jankurianski · 2015-03-25T00:08:04Z

Should we provide the option to validate have some dependencies marked as optional?

Yes, I would like that. I do not include pdf.dll or ffmpegsumo.dll with my app as they are unnecessary for me.

Is CefSharp.BrowserSubprocess.* required? Though I have not tried, I thought you could re-use your main .exe somehow.

What's the expected output if calling from user land? Is a list of strings containing which files are missing ok? Would people just prefer a formatted string?

An exception with a formatted string sounds good to me. It ensures that an informative message appears in Visual Studio if debugging, and when their app is deployed, it will ensure their exception handling code will log the relevant missing files (as most people will at a minimum log the exception type and message via e.ToString()).

Anyone have any thoughts on obtaining the path? I've started with the ExecutingAssembly and allowed an overload so that users can call themselves.

Default works for me, and overload seems flexible enough to handle anything weird.

Good job BTW 👍

amaitland · 2015-03-25T00:36:57Z

Yes, I would like that. I do not include pdf.dll or ffmpegsumo.dll with my app as they are unnecessary for me.

Any thoughts on handling this? There are so many dependencies that are only used in specific scenarios. Is it enough to check for Required or Required + Optional without going down into the meticulous detail of checking that d3dcompiler_43.dll is present for a windows xp machine?

Is CefSharp.BrowserSubprocess.* required? Though I have not tried, I thought you could re-use your main .exe somehow.

CEF allows for the scenario where you can use your main exe and launch the render/gpu processes via command line options. CefSharp explicitly implements render/gpu with BrowserSubprocess. Quite a common mistake to forget them, which means Cef will load without exception, just won't render any content.

…ckDependencies Plan is to use `CefSettings.Locale` and `CefSettings.LocalesDirPath` to infer the users local preferences

For now code is in `CefExample` will eventually be moved into `Cef.Initialize()`

…if dependencies missing Error message needs some work

Based on https://github.com/cefsharp/cef-binary/blob/master/README.txt

'The fully qualified path for the resources directory. If this value is empty the cef.pak and/or devtools_resources.pak files must be located in the module directory'

…er then implementing checks in `Cef.Initialize()`

amaitland · 2015-03-25T02:12:52Z

Think I've got something that I'm relatively happy with.

Further comments? It's not very well tested yet.

amaitland · 2015-03-25T02:14:03Z

There's likely to be a few refactoring to DRY the code out a bit. Keeping it simple until I'm happy we've got something that will work in 99% of cases.

amaitland · 2015-03-25T02:16:34Z

Should also note that disabling pack loading will cause the scrollbars to become red, known issue. #598

rassilon · 2015-03-25T03:21:52Z

CefSharp/DependencyChecker.cs

+        /// <param name="resourcesDirPath"></param>
+        /// <param name="localePackFile">The locale pack file e.g. <see cref="LocalesPackFile"/> </param>
+        /// <returns>List of missing dependencies, if all present an empty List will be returned</returns>
+        public static List<string> CheckDependencies(bool checkOptional, bool packLoadingDisabled, string path, string resourcesDirPath, string localePackFile = LocalesPackFile)


Whenever I see gobs of bool parameters I usually think: Are you really sure using a flag enum isn't the correct thing to do here?

Pack loading is a special case. checkOptional would be replaced with an enum if we went down that path.

rassilon · 2015-03-25T03:29:06Z

This is looking good. I think the optional dlls fall into these separate catagories:

GPUAcceleration
OSGPUAcceleration
AudioVideo
PDFPlugin

The flag enum idea would allow you to easily check which set to examine.

I don't really care at the moment whether or not the above suggestions are put in the PR before merging though.

Bill

…don't think it's worth spending too much time on though) Use Path.Combine instead of manual string concat

…led by default

amaitland · 2015-03-25T04:03:27Z

I've added a slightly crude IsWindowsXp static property, not great. Will do for now I guess. Also moved the code into Cef.Initialize(), it's not called by default as yet. Expanded the example to demo it.

rassilon · 2015-03-25T04:08:43Z

Still looking good.

rassilon · 2015-03-25T18:23:32Z

If we eventually do the flag thing might as well include:

WinForm
WPF
Generic OSR

Bill

rassilon · 2015-03-25T19:03:01Z

CefSharp/DependencyChecker.cs

+                }
+
+                builder.AppendLine("Executing Assembly Path:" + path);
+


We could also add an informational reminder in this string about locales\xxxx.pak that explains what they do if we know even though I would never suggest checking for the dependency to exist.

@rassilon Not sure I understand, can you clarify?

amaitland · 2015-03-30T04:39:21Z

Anyone up for taking this for a spin and reporting back?

At some point I'd still like to see the enum option implemented, though I'm thinking this might be enough for 39 and we improve for 41?

amaitland · 2015-04-02T00:08:44Z

Merging this one now, if anyone has improvements they wish to make just shout out so we can work out if they are done before 39 release.

Implement DependencyChecker

amaitland added 2 commits March 24, 2015 10:58

Add rough draft of DependencyChecker - loops through list of all Cef/…

0eda968

…CefSharp dependencies

Untabify

8a7b9e2

amaitland added this to the 39.0.0 milestone Mar 24, 2015

amaitland added the enhancement label Mar 24, 2015

amaitland added 12 commits March 25, 2015 11:08

Move localePackFile param with default value to first overload of Che…

591b477

…ckDependencies Plan is to use `CefSettings.Locale` and `CefSettings.LocalesDirPath` to infer the users local preferences

Allow for full locales path

2b89238

Expand Locale support to check CefSettings for user specified values

a17b145

For now code is in `CefExample` will eventually be moved into `Cef.Initialize()`

Fix typo in CefExample

ce32740

Refactor code so now AssetAllDependenciesPresent throws an exception …

a188b86

…if dependencies missing Error message needs some work

First attempt at splitting out into required/optional dependencies

36ea815

Based on https://github.com/cefsharp/cef-binary/blob/master/README.txt

Add optional bool for checking dependencies

a9762bb

Allow setting cefSettings->resources_dir_path

fea3d8f

'The fully qualified path for the resources directory. If this value is empty the cef.pak and/or devtools_resources.pak files must be located in the module directory'

Refactor to pass params directly in from CefSettings - make life easi…

be41287

…er then implementing checks in `Cef.Initialize()`

Add packLoadingDisabled check

76ea449

Re-add LocalesPackFile default param value (moved to end of function)

b12f131

Reorder method params (move packLoadingDisabled to second param)

475067b

rassilon reviewed Mar 25, 2015
View reviewed changes

amaitland added 2 commits March 25, 2015 13:53

Added IsWindowsXp special case (It's not the best code in the world, …

3a47bbb

…don't think it's worth spending too much time on though) Use Path.Combine instead of manual string concat

Move dependency check call into Cef.Initialize() - currently not enab…

4ad11e7

…led by default

amaitland added 2 commits March 25, 2015 14:27

Assert not Asset - fix typo

ea2ad10

Update AssertAllDependenciesPresent reference

ccbaebd

rassilon reviewed Mar 25, 2015
View reviewed changes

amaitland mentioned this pull request Mar 26, 2015

CefSharp silently ignores missing subprocess.exe #905

Closed

amaitland added a commit that referenced this pull request Apr 2, 2015

Merge pull request #900 from amaitland/feature/dependencychecker

a434d3c

Implement DependencyChecker

amaitland merged commit a434d3c into cefsharp:master Apr 2, 2015

amaitland mentioned this pull request Apr 8, 2015

cefSharp causes the host application to crash if it cannot load its dependencies #605

Closed

amaitland deleted the feature/dependencychecker branch July 13, 2015 09:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement DependencyChecker #900

Implement DependencyChecker #900

amaitland commented Mar 24, 2015

amaitland commented Mar 24, 2015

jankurianski commented Mar 25, 2015

amaitland commented Mar 25, 2015

amaitland commented Mar 25, 2015

amaitland commented Mar 25, 2015

amaitland commented Mar 25, 2015

rassilon Mar 25, 2015

amaitland Mar 25, 2015

rassilon commented Mar 25, 2015

amaitland commented Mar 25, 2015

rassilon commented Mar 25, 2015

rassilon commented Mar 25, 2015

rassilon Mar 25, 2015

amaitland Mar 27, 2015

amaitland commented Mar 30, 2015

amaitland commented Apr 2, 2015

Implement DependencyChecker #900

Implement DependencyChecker #900

Conversation

amaitland commented Mar 24, 2015

amaitland commented Mar 24, 2015

jankurianski commented Mar 25, 2015

amaitland commented Mar 25, 2015

amaitland commented Mar 25, 2015

amaitland commented Mar 25, 2015

amaitland commented Mar 25, 2015

rassilon Mar 25, 2015

Choose a reason for hiding this comment

amaitland Mar 25, 2015

Choose a reason for hiding this comment

rassilon commented Mar 25, 2015

amaitland commented Mar 25, 2015

rassilon commented Mar 25, 2015

rassilon commented Mar 25, 2015

rassilon Mar 25, 2015

Choose a reason for hiding this comment

amaitland Mar 27, 2015

Choose a reason for hiding this comment

amaitland commented Mar 30, 2015

amaitland commented Apr 2, 2015