Re: [opensource-dev] Review Request: OPEN-38: autobuild support for StandAlone

2011-02-27 Thread Boroondas Gupte

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/167/#review391
---

Ship it!


Ah, with that, configuration succeeds and GLH_INCLUDE_DIR gets correctly set to 
the dir with the symlinks. Could the need for that symlink dir be avoided by 
having the user directly set the GLH_INCLUDE_DIR CMake variable?


indra/cmake/Linking.cmake


I thought the removal of the NOT STANDALONE condition here would give 
standalone the same (repository-local) include directories that non-standalone 
has, so that it'd find autobuild-installed dependencies. But it seems those are 
just for the library binaries? Or is this just about the creation of those 
directories, not their addition to the search path?


Building still fails for me, with

Linking CXX shared library libllcommon.so
/usr/lib/gcc/x86_64-pc-linux-gnu/4.4.5/../../../../x86_64-pc-linux-gnu/bin/ld: 
skipping incompatible 
/usr/lib/gcc/x86_64-pc-linux-gnu/4.4.5/../../../libbreakpad_client.so when 
searching for -lbreakpad_client
/usr/lib/gcc/x86_64-pc-linux-gnu/4.4.5/../../../../x86_64-pc-linux-gnu/bin/ld: 
skipping incompatible 
/usr/lib/gcc/x86_64-pc-linux-gnu/4.4.5/../../../libbreakpad_client.a when 
searching for -lbreakpad_client
/usr/lib/gcc/x86_64-pc-linux-gnu/4.4.5/../../../../x86_64-pc-linux-gnu/bin/ld: 
skipping incompatible /usr/lib/libbreakpad_client.so when searching for 
-lbreakpad_client
/usr/lib/gcc/x86_64-pc-linux-gnu/4.4.5/../../../../x86_64-pc-linux-gnu/bin/ld: 
skipping incompatible /usr/lib/libbreakpad_client.a when searching for 
-lbreakpad_client
/usr/lib/gcc/x86_64-pc-linux-gnu/4.4.5/../../../../x86_64-pc-linux-gnu/bin/ld: 
cannot find -lbreakpad_client
collect2: ld returned 1 exit status
make[2]: *** [llcommon/libllcommon.so] Error 1
make[1]: *** [llcommon/CMakeFiles/llcommon.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs

and

In file included from /usr/include/curl/curl.h:36,
 from [...]/indra/llmessage/llcurl.h:39,
 from [...]/indra/llmessage/llurlrequest.h:42,
 from [...]/indra/llmessage/llhttpclient.h:38,
 from [...]/indra/llmessage/message.h:53,
 from [...]/indra/llinventory/llcategory.cpp:30:
/usr/include/curl/curlrules.h:144: error: size of array ‘__curl_rule_01__’ is 
negative
/usr/include/curl/curlrules.h:154: error: size of array ‘__curl_rule_02__’ is 
negative

but I guess those are both 32bit/64bit issues and not related to your changes 
here, so "Ship it!".

- Boroondas


On Feb. 26, 2011, 6:56 a.m., Aleric Inglewood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/167/
> ---
> 
> (Updated Feb. 26, 2011, 6:56 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Oz: please merge into your repository.
> 
> 
> This addresses bug OPEN-38.
> 
> 
> Diffs
> -
> 
>   indra/cmake/FindGLH.cmake PRE-CREATION 
>   indra/cmake/GLH.cmake PRE-CREATION 
>   indra/cmake/LLRender.cmake 5f0ab9443ece 
>   indra/cmake/LLSharedLibs.cmake 5f0ab9443ece 
>   indra/cmake/Linking.cmake 5f0ab9443ece 
>   indra/linux_crash_logger/CMakeLists.txt 5f0ab9443ece 
> 
> Diff: http://codereview.secondlife.com/r/167/diff
> 
> 
> Testing
> ---
> 
> configures, compiles, runs.
> 
> 
> Thanks,
> 
> Aleric
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: Nearby chat history is displaying both Display Names and user.names when the Display Name is not changed from default.

2011-02-27 Thread Boroondas Gupte

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/153/#review392
---

Ship it!



indra/llcommon/llavatarname.cpp


This could be shortened to

if (!mUsername.empty() && !mIsDisplayNameDefault)
{
name = mDisplayName + " (" + mUsername + ")";
}
else
{
// Display names are off, so legacy name is in mDisplayName,
// or Display Name is not changed from (user name based)
// default, so also showing the user name would be redundant.
name = mDisplayName;
}

but I guess the nested ifs make the intention more clear, so go for it.


- Boroondas


On Feb. 19, 2011, 9:32 a.m., ardy.lay wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/153/
> ---
> 
> (Updated Feb. 19, 2011, 9:32 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> https://jira.secondlife.com/browse/VWR-24917
> I have been finding the redundent display of functionally equivalent names in 
> nearby chat history and IM history quite tiresome.
> Simple proposal: If the resident's Display Name is at the default then do not 
> display their user.name.
> https://bitbucket.org/ArdyLay/viewer-development-vwr-24917
> 
> Change is to: LLAvatarName::getCompleteName
> 
> I find the following Callers:
> LLAvatarActions::requestFriendshipDialog
> LLAvatarActions::startIM
> LLAvatarActions::startCall
> LLIMModel::LLIMSession
> LLIMModel::logToFile
> LLPostponedNotification::onAvatarNameCache
> LLUrlEntryAgent::onAvatarNameCache
> LLUrlEntryAgent::getLabel
> LLUrlEntryAgentCompleteName::getName
> 
> // Callback for name resolution of a god/estate message
> llviewermessage.cpp(2149): args["NAME"] = av_name.getCompleteName();
> llviewermessage.cpp(2154): chat.mText = av_name.getCompleteName() + ": " + 
> message;
> 
> static void on_avatar_name_cache_toast ...
> llimview.cpp(108): args["FROM"] = av_name.getCompleteName();
> 
> Some of these make me wonder if this change will cause some defects and 
> should be implimented as a seperate function.
> 
> 
> This addresses bug VWR-24917.
> http://jira.secondlife.com/browse/VWR-24917
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt c10d5e37db1e 
>   indra/llcommon/llavatarname.cpp c10d5e37db1e 
> 
> Diff: http://codereview.secondlife.com/r/153/diff
> 
> 
> Testing
> ---
> 
> I have been using this trivial change and have shared it with a friend, via 
> bitbucket.  We have both built the viewer on Windows 7 and find the resulting 
> reduction in redundent text in chat and IM history on screen to be very 
> helpful.
> 
> 
> Thanks,
> 
> ardy.lay
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: Nearby chat history is displaying both Display Names and user.names when the Display Name is not changed from default.

2011-02-27 Thread Aleric Inglewood


> On Feb. 27, 2011, 5:57 a.m., Boroondas Gupte wrote:
> > indra/llcommon/llavatarname.cpp, lines 93-108
> > 
> >
> > This could be shortened to
> > 
> > if (!mUsername.empty() && !mIsDisplayNameDefault)
> > {
> > name = mDisplayName + " (" + mUsername + ")";
> > }
> > else
> > {
> > // Display names are off, so legacy name is in 
> > mDisplayName,
> > // or Display Name is not changed from (user name based)
> > // default, so also showing the user name would be 
> > redundant.
> > name = mDisplayName;
> > }
> > 
> > but I guess the nested ifs make the intention more clear, so go for it.

Or even more readable(?):

if (mIsDisplayNameDefault || mUsername.empty())
{
// Showing the user name when Display Names are off
// is redundant, since in that case it's the same string.
name = mDisplayName;
}
else
{
name = mDisplayName + " (" + mUsername + ")";
}


- Aleric


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/153/#review392
---


On Feb. 19, 2011, 9:32 a.m., ardy.lay wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/153/
> ---
> 
> (Updated Feb. 19, 2011, 9:32 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> https://jira.secondlife.com/browse/VWR-24917
> I have been finding the redundent display of functionally equivalent names in 
> nearby chat history and IM history quite tiresome.
> Simple proposal: If the resident's Display Name is at the default then do not 
> display their user.name.
> https://bitbucket.org/ArdyLay/viewer-development-vwr-24917
> 
> Change is to: LLAvatarName::getCompleteName
> 
> I find the following Callers:
> LLAvatarActions::requestFriendshipDialog
> LLAvatarActions::startIM
> LLAvatarActions::startCall
> LLIMModel::LLIMSession
> LLIMModel::logToFile
> LLPostponedNotification::onAvatarNameCache
> LLUrlEntryAgent::onAvatarNameCache
> LLUrlEntryAgent::getLabel
> LLUrlEntryAgentCompleteName::getName
> 
> // Callback for name resolution of a god/estate message
> llviewermessage.cpp(2149): args["NAME"] = av_name.getCompleteName();
> llviewermessage.cpp(2154): chat.mText = av_name.getCompleteName() + ": " + 
> message;
> 
> static void on_avatar_name_cache_toast ...
> llimview.cpp(108): args["FROM"] = av_name.getCompleteName();
> 
> Some of these make me wonder if this change will cause some defects and 
> should be implimented as a seperate function.
> 
> 
> This addresses bug VWR-24917.
> http://jira.secondlife.com/browse/VWR-24917
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt c10d5e37db1e 
>   indra/llcommon/llavatarname.cpp c10d5e37db1e 
> 
> Diff: http://codereview.secondlife.com/r/153/diff
> 
> 
> Testing
> ---
> 
> I have been using this trivial change and have shared it with a friend, via 
> bitbucket.  We have both built the viewer on Windows 7 and find the resulting 
> reduction in redundent text in chat and IM history on screen to be very 
> helpful.
> 
> 
> Thanks,
> 
> ardy.lay
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: OPEN-38: autobuild support for StandAlone

2011-02-27 Thread Aleric Inglewood


On Feb. 27, 2011, 3:47 a.m., Aleric Inglewood wrote:
> > Building still fails for me, with
> > 
> > Linking CXX shared library libllcommon.so
> > /usr/lib/gcc/x86_64-pc-linux-gnu/4.4.5/../../../../x86_64-pc-linux-gnu/bin/ld:
> >  skipping incompatible 
> > /usr/lib/gcc/x86_64-pc-linux-gnu/4.4.5/../../../libbreakpad_client.so when 
> > searching for -lbreakpad_client
> > /usr/lib/gcc/x86_64-pc-linux-gnu/4.4.5/../../../../x86_64-pc-linux-gnu/bin/ld:
> >  skipping incompatible 
> > /usr/lib/gcc/x86_64-pc-linux-gnu/4.4.5/../../../libbreakpad_client.a when 
> > searching for -lbreakpad_client
> > /usr/lib/gcc/x86_64-pc-linux-gnu/4.4.5/../../../../x86_64-pc-linux-gnu/bin/ld:
> >  skipping incompatible /usr/lib/libbreakpad_client.so when searching for 
> > -lbreakpad_client
> > /usr/lib/gcc/x86_64-pc-linux-gnu/4.4.5/../../../../x86_64-pc-linux-gnu/bin/ld:
> >  skipping incompatible /usr/lib/libbreakpad_client.a when searching for 
> > -lbreakpad_client
> > /usr/lib/gcc/x86_64-pc-linux-gnu/4.4.5/../../../../x86_64-pc-linux-gnu/bin/ld:
> >  cannot find -lbreakpad_client
> > collect2: ld returned 1 exit status
> > make[2]: *** [llcommon/libllcommon.so] Error 1
> > make[1]: *** [llcommon/CMakeFiles/llcommon.dir/all] Error 2
> > make[1]: *** Waiting for unfinished jobs
> > 
> > and
> > 
> > In file included from /usr/include/curl/curl.h:36,
> >  from [...]/indra/llmessage/llcurl.h:39,
> >  from [...]/indra/llmessage/llurlrequest.h:42,
> >  from [...]/indra/llmessage/llhttpclient.h:38,
> >  from [...]/indra/llmessage/message.h:53,
> >  from [...]/indra/llinventory/llcategory.cpp:30:
> > /usr/include/curl/curlrules.h:144: error: size of array ‘__curl_rule_01__’ 
> > is negative
> > /usr/include/curl/curlrules.h:154: error: size of array ‘__curl_rule_02__’ 
> > is negative
> > 
> > but I guess those are both 32bit/64bit issues and not related to your 
> > changes here, so "Ship it!".

The curl error means you have a wrongly configured curl; you seem to be 
including a 32-bit version while now compiling for 64-bit (or visa versa). This 
definitely doesn't have anything to do with my changes ;).

The fact that it can't find -lbreakpad_client seems to be the same problem: it 
is skipping /usr/lib/libbreakpad_client.a because that is incompatible (was 
compiled for 32-bit while now you are compiling 64-bit, or visa versa).


- Aleric


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/167/#review391
---


On Feb. 26, 2011, 6:56 a.m., Aleric Inglewood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/167/
> ---
> 
> (Updated Feb. 26, 2011, 6:56 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Oz: please merge into your repository.
> 
> 
> This addresses bug OPEN-38.
> 
> 
> Diffs
> -
> 
>   indra/cmake/FindGLH.cmake PRE-CREATION 
>   indra/cmake/GLH.cmake PRE-CREATION 
>   indra/cmake/LLRender.cmake 5f0ab9443ece 
>   indra/cmake/LLSharedLibs.cmake 5f0ab9443ece 
>   indra/cmake/Linking.cmake 5f0ab9443ece 
>   indra/linux_crash_logger/CMakeLists.txt 5f0ab9443ece 
> 
> Diff: http://codereview.secondlife.com/r/167/diff
> 
> 
> Testing
> ---
> 
> configures, compiles, runs.
> 
> 
> Thanks,
> 
> Aleric
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: OPEN-38: autobuild support for StandAlone

2011-02-27 Thread Aleric Inglewood


> On Feb. 27, 2011, 3:47 a.m., Boroondas Gupte wrote:
> > Ah, with that, configuration succeeds and GLH_INCLUDE_DIR gets correctly 
> > set to the dir with the symlinks. Could the need for that symlink dir be 
> > avoided by having the user directly set the GLH_INCLUDE_DIR CMake variable?

There is long standing issue (from the days of Rob Linden) that I wanted to 
address once things finally get running smoothly... I don't think that now is 
the time to address this. What needs to be done for tut and glh_linear is to 
install them in an architecture independent directory and then use them for 
standalone and non-standalone. The same goes for the slvoice stuff. Some 
packages ARE needed even on standalone, but the current setup doesn't support 
it well enough because LL never tests standalone is therefore unaware of the 
difference between packages that are only needed for non-standalone and 
packages that are needed for both.


- Aleric


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/167/#review391
---


On Feb. 26, 2011, 6:56 a.m., Aleric Inglewood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/167/
> ---
> 
> (Updated Feb. 26, 2011, 6:56 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Oz: please merge into your repository.
> 
> 
> This addresses bug OPEN-38.
> 
> 
> Diffs
> -
> 
>   indra/cmake/FindGLH.cmake PRE-CREATION 
>   indra/cmake/GLH.cmake PRE-CREATION 
>   indra/cmake/LLRender.cmake 5f0ab9443ece 
>   indra/cmake/LLSharedLibs.cmake 5f0ab9443ece 
>   indra/cmake/Linking.cmake 5f0ab9443ece 
>   indra/linux_crash_logger/CMakeLists.txt 5f0ab9443ece 
> 
> Diff: http://codereview.secondlife.com/r/167/diff
> 
> 
> Testing
> ---
> 
> configures, compiles, runs.
> 
> 
> Thanks,
> 
> Aleric
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: OPEN-38: autobuild support for StandAlone

2011-02-27 Thread Aleric Inglewood


> On Feb. 27, 2011, 3:47 a.m., Boroondas Gupte wrote:
> > indra/cmake/Linking.cmake, lines 6-20
> > 
> >
> > I thought the removal of the NOT STANDALONE condition here would give 
> > standalone the same (repository-local) include directories that 
> > non-standalone has, so that it'd find autobuild-installed dependencies. But 
> > it seems those are just for the library binaries? Or is this just about the 
> > creation of those directories, not their addition to the search path?

No, this just sets variables to certain (path) values. No test should ever test 
if they are set (that would be a wrong design), so they might as well be set. 
In fact, they ARE being used (at least some of them) on standalone (the staging 
directories), which is why I removed the if. Also, it's a Good Thing(tm) that 
the auto-build installed packages (and header files!) are completely invisible 
on standalone. This should not change. What we need, as I remarked in my other 
comment, is that we need to create a separate directory where packages can be 
installed that ARE needed on standalone and then use that, so that we ONLY get 
packages that we really want/need, and not left-overs from a non-standalone 
build as well).


- Aleric


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/167/#review391
---


On Feb. 26, 2011, 6:56 a.m., Aleric Inglewood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/167/
> ---
> 
> (Updated Feb. 26, 2011, 6:56 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Oz: please merge into your repository.
> 
> 
> This addresses bug OPEN-38.
> 
> 
> Diffs
> -
> 
>   indra/cmake/FindGLH.cmake PRE-CREATION 
>   indra/cmake/GLH.cmake PRE-CREATION 
>   indra/cmake/LLRender.cmake 5f0ab9443ece 
>   indra/cmake/LLSharedLibs.cmake 5f0ab9443ece 
>   indra/cmake/Linking.cmake 5f0ab9443ece 
>   indra/linux_crash_logger/CMakeLists.txt 5f0ab9443ece 
> 
> Diff: http://codereview.secondlife.com/r/167/diff
> 
> 
> Testing
> ---
> 
> configures, compiles, runs.
> 
> 
> Thanks,
> 
> Aleric
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges