Add IceBox CMake files #266
Conversation
| cerr << "invalid proxy" << endl; | ||
| return 1; | ||
| } | ||
| HelloPrx twoway{communicator, "hello:tcp -h localhost -p 10000:udp -h localhost -p 10000"}; |
There was a problem hiding this comment.
We should further simplify in a follow up PR and have IceBox/greeter, inline with Ice/greeter.
There was a problem hiding this comment.
We could though we'll have to figure out what the behavior will be.
| #include "HelloI.h" | ||
|
|
||
| #include <Ice/Ice.h> | ||
| #include <iostream> |
There was a problem hiding this comment.
I suggest adding a blank like between Ice/IceStorm etc. headers and other system headers.
cpp/IceBox/hello/README.md
Outdated
| In the second window, run the client: | ||
| To run this demo, open two terminal windows. In the first window: | ||
|
|
||
| **Linux/macOS:** |
There was a problem hiding this comment.
I don't like this relative path to "library", where we don't even provide the actual library name.
That's not how we should recommend using/configuring/starting icebox.
I'd rather show including the directory in LD_LIBRARY_PATH/DYLD_LIBRARY_PATH/PATH before running IceBox, as in
On Linux:
LD_LIBRARY_PATH=./build icebox --Ice.Config=config.icebox| # | ||
| # The hello service | ||
| # | ||
| IceBox.Service.Hello=./HelloService:create --Ice.Config=config.service |
There was a problem hiding this comment.
The downside of removing config.icebox + config.service is it's now unclear how to pass command-line options (such as --Ice.Config=...) to your IceBox service.
If we get rid of all config files, we should at least show an option such as:
icebox --IceBox.Service.Hello="HelloService:create --Ice.Trace.Dispatch"
| **Windows:** | ||
|
|
||
| ```shell | ||
| set PATH=%PATH%;./build/Release |
There was a problem hiding this comment.
I would not use '/' in a Windows PATH. Does it actually work?
3 participants
This PR adds CMake support to the IceBox/hello demo.
Due to the nature of CMake's support of multi-config generators there are two places the HelloService library may end up by default:
./build/and./build/Release. This causes a bit of havoc with the config file locations.Having 4 config files (most just full of superfluous config options) for this "simple" demo seems like a lot.
XXXadmintool to shutdown a service. We don't do this for the IceGrid demos, it's unnecessary here.IMO these changes make the demo much easier to understand and digest.