-
Notifications
You must be signed in to change notification settings - Fork 1k
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 options to erizo build process #1028
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this approach to have both versions in the docker image, and we'll probably save disk anyway with this PR because we now clean object files. I just saw a couple of small things that wouldn't work, quick fixes anyway.
extras/basic_example/basicServer.js
Outdated
@@ -191,7 +191,7 @@ app.use(function(req, res, next) { | |||
cleanExampleRooms(function() { | |||
getOrCreateRoom(defaultRoomName, undefined, function (roomId) { | |||
defaultRoom = roomId; | |||
app.listen(3001); | |||
app.listen(3002); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you forgot to remove this change from the commit, right?
@@ -80,8 +80,9 @@ window.onload = function () { | |||
req.setRequestHeader('Content-Type', 'application/json'); | |||
req.send(JSON.stringify(roomData)); | |||
}; | |||
|
|||
var roomData = {username: 'user', role: 'presenter', room: roomName, type: roomType}; | |||
var names = ['pedro', 'juan', 'manolo', 'tito', 'pepe', 'alonso', 'guancho']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above?
extras/docker/initDockerLicode.sh
Outdated
@@ -115,7 +119,11 @@ run_erizoController() { | |||
run_erizoAgent() { | |||
echo "Starting erizoAgent" | |||
cd $ROOT/erizo_controller/erizoAgent | |||
node erizoAgent.js & | |||
if [ "$ERIZODEBUG" = "true" ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
==
or -eq
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're using =
throughout the whole file but ==
is more POSIX compliant so I'll switch all of them 👍
}, { # OS!="mac" | ||
'cflags!' : ['-fno-exceptions'], | ||
'cflags' : ['-D__STDC_CONSTANT_MACROS'], | ||
'cflags_cc' : ['-Wall', '-O3', '-g' , '-std=c++11', '-fexceptions'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -59,7 +59,8 @@ int ExternalInput::init() { | |||
|
|||
// VideoCodecInfo info; | |||
MediaInfo om; | |||
AVStream *st, *audio_st; | |||
AVStream *st = nullptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch!
scripts/installErizo.sh
Outdated
@@ -53,6 +55,9 @@ install_erizo(){ | |||
cd $ROOT/erizo | |||
./generateProject.sh | |||
./buildProject.sh $FAST_MAKE | |||
if [ $DELETE_OBJECT_FILES == 'true' ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in other places we do if [ "$DELETE_OBJECT_FILES" == "true" ];
, I didn't know it works like this too, great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if it works, I'll switch to "
for consistency 🙂
Description
This PR adds "Release" and "Debug" configurations for both erizo and erizoAPI. The main difference is that Erizo is now built with
-O3
. Both are built by default, while "Release" is the one that's normally used by ErizoJS.In order to run the debug version of both erizo and erizoAPI, you can start erizoAgent with
initErizoAgent.sh -d
or, if you are running the docker image, you can add--erizoDebug
.Also, we're now using
rpath
when building erizoAPI, meaning that we no longer needLD_LIBRARY_PATH
for Licode.Warning for Spine users: You do need
LD_LIBRARY_PATH
and note thatliberizo
is now undererizo/build/release/erizo
orerizo/build/debug/erizo
.Finally, in the Docker image we're keeping both builds but removing the object files in
CMakeFiles
to save some space.[] It needs and includes Unit Tests
Changes in Client or Server public APIs
[] It includes documentation for these changes in
/doc
.