Re: [opensource-dev] Review Request: OPEN-77 Adding ability for Prebuilt.cmake to pass necessary additional options to 'autobuild install'

[Oz Linden]

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


This should not be needed, and is not a good general way to solve the problem.

See comment in the jira:  
https://jira.secondlife.com/browse/OPEN-76?focusedCommentId=264587&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-264587

- Oz


On June 7, 2011, 3:17 p.m., Ima Mechanique wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/327/
> ---
> 
> (Updated June 7, 2011, 3:17 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> This is primarily to allow the passing of '--config-file file-name' through 
> cmake, so that 'autobuild configure' can work correctly.
> The options are passed by adding -- 
> -DAUTOBUILD_EXTRA_ARGS:STRING=options-to-add to the end of the configure 
> command.
> e.g. autobuild configure --config-file ab-custom.xml -c ReleaseOS -- 
> -DAUTOBUILD_EXTRA_ARGS:STRING=--config-file=ab-custom.xml
> Thanks to Brad Linden, for most of the work.
> 
> 
> This addresses bugs OPEN-76 and OPEN-77.
> http://jira.secondlife.com/browse/OPEN-76
> http://jira.secondlife.com/browse/OPEN-77
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 1d4d568986e3 
>   indra/cmake/Prebuilt.cmake 1d4d568986e3 
> 
> Diff: http://codereview.secondlife.com/r/327/diff
> 
> 
> Testing
> ---
> 
> Successfully configured and built the viewer with a custom configuration and 
> no autobuild.xml present.
> 
> 
> Thanks,
> 
> Ima
> 
>

___
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-78 Automate the process of passing additional arguments to prebuilt.cmake

[Boroondas Gupte]

> On June 7, 2011, 5:36 p.m., Boroondas Gupte wrote:
> > autobuild/autobuild_tool_configure.py, line 69
> > 
> >
> > Don't forget that file UNIX filesystems are case sensitive. Even worse, 
> > the fact that GNU make looks first for a file named 'makefile' then for one 
> > named 'Makefile' makes it common that projects ship with a 'makefile' file, 
> > and the user would then create a 'Makefile' file to override that, if 
> > wanted. That might inspire Mac/Linux users to use Autobuild.xml or some 
> > other capitalization variation for their own autobuild config file.
> > 
> > Luckily, there's a 
> > http://docs.python.org/library/os.path.html#os.path.normcase , which does 
> > just what we want here.

meh, got GNU make's search order mixed up, but I guess you know what I mean.


- Boroondas


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


On June 7, 2011, 3:03 p.m., Ima Mechanique wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/331/
> ---
> 
> (Updated June 7, 2011, 3:03 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> The fix in OPEN-77 allows configure to run correctly, but is cumbersome and 
> not exactly pretty on the command line ;-)
> This fix allows autobuild itself, to automatically add the --config-file 
> option when it determines the default (autobuild.xml) is not being used.
> 
> 
> This addresses bugs OPEN-76 and OPEN-78.
> http://jira.secondlife.com/browse/OPEN-76
> http://jira.secondlife.com/browse/OPEN-78
> 
> 
> Diffs
> -
> 
>   autobuild/autobuild_tool_configure.py 2a560b1d8f95 
> 
> Diff: http://codereview.secondlife.com/r/331/diff
> 
> 
> Testing
> ---
> 
> Configure and built viewer-development with custom configuration and no 
> default file present.
> 
> 
> Thanks,
> 
> Ima
> 
>

___
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-78 Automate the process of passing additional arguments to prebuilt.cmake

[Boroondas Gupte]

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


It should be noted that this fix depends on your OPEN-77 implementation (being 
reviewed at http://codereview.secondlife.com/r/327/ ) and should not be pulled 
without that.


autobuild/autobuild_tool_configure.py


Don't forget that file UNIX filesystems are case sensitive. Even worse, the 
fact that GNU make looks first for a file named 'makefile' then for one named 
'Makefile' makes it common that projects ship with a 'makefile' file, and the 
user would then create a 'Makefile' file to override that, if wanted. That 
might inspire Mac/Linux users to use Autobuild.xml or some other capitalization 
variation for their own autobuild config file.

Luckily, there's a 
http://docs.python.org/library/os.path.html#os.path.normcase , which does just 
what we want here.



autobuild/autobuild_tool_configure.py


somelist[0:0] = [something]
might be more readable rewritten as
somelist.insert(0, something)
(Though maybe that's just a matter of different taste. I dunno whether 
somelist[0:0] is a common idiom in python ... but it had me think twice before 
I realized it was simply used for prepending)

Because args.config_file is appended at the end, you might want to use the 
string concatenation operator (+) rather than substitution:
'foo%s' % bar
is the same as
'foo' + bar

So this line could become
args.additional_options.insert(0, 
'-DAUTOBUILD_EXTRA_ARGS:STRING=--config-file=' + args.config_file)

(Maybe this should be broken into two lines.)


- Boroondas


On June 7, 2011, 3:03 p.m., Ima Mechanique wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/331/
> ---
> 
> (Updated June 7, 2011, 3:03 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> The fix in OPEN-77 allows configure to run correctly, but is cumbersome and 
> not exactly pretty on the command line ;-)
> This fix allows autobuild itself, to automatically add the --config-file 
> option when it determines the default (autobuild.xml) is not being used.
> 
> 
> This addresses bugs OPEN-76 and OPEN-78.
> http://jira.secondlife.com/browse/OPEN-76
> http://jira.secondlife.com/browse/OPEN-78
> 
> 
> Diffs
> -
> 
>   autobuild/autobuild_tool_configure.py 2a560b1d8f95 
> 
> Diff: http://codereview.secondlife.com/r/331/diff
> 
> 
> Testing
> ---
> 
> Configure and built viewer-development with custom configuration and no 
> default file present.
> 
> 
> Thanks,
> 
> Ima
> 
>

___
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-77 Adding ability for Prebuilt.cmake to pass necessary additional options to 'autobuild install'

[Boroondas Gupte]

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

Ship it!


Looks good.


indra/cmake/Prebuilt.cmake


It looks like currently, the call to autobuild install is the only call 
from CMake back to autobuild. If we plan to ever add more back-calls (I don't 
think we do nor should), we should think about whether all back-calls will take 
the same extra arguments. If not, better give a specific name here, like 
AUTOBUILD_INSTALL_EXTRA_ARGS, so they can be distinguished.


- Boroondas


On June 7, 2011, 3:17 p.m., Ima Mechanique wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/327/
> ---
> 
> (Updated June 7, 2011, 3:17 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> This is primarily to allow the passing of '--config-file file-name' through 
> cmake, so that 'autobuild configure' can work correctly.
> The options are passed by adding -- 
> -DAUTOBUILD_EXTRA_ARGS:STRING=options-to-add to the end of the configure 
> command.
> e.g. autobuild configure --config-file ab-custom.xml -c ReleaseOS -- 
> -DAUTOBUILD_EXTRA_ARGS:STRING=--config-file=ab-custom.xml
> Thanks to Brad Linden, for most of the work.
> 
> 
> This addresses bugs OPEN-76 and OPEN-77.
> http://jira.secondlife.com/browse/OPEN-76
> http://jira.secondlife.com/browse/OPEN-77
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 1d4d568986e3 
>   indra/cmake/Prebuilt.cmake 1d4d568986e3 
> 
> Diff: http://codereview.secondlife.com/r/327/diff
> 
> 
> Testing
> ---
> 
> Successfully configured and built the viewer with a custom configuration and 
> no autobuild.xml present.
> 
> 
> Thanks,
> 
> Ima
> 
>

___
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-77 Adding ability for Prebuilt.cmake to pass necessary additional options to 'autobuild install'

[Ima Mechanique]

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

(Updated June 7, 2011, 3:17 p.m.)


Review request for Viewer.


Changes
---

Updated diff (includes contributions.txt change now) and summary (to include 
jira number).


Summary (updated)
---

This is primarily to allow the passing of '--config-file file-name' through 
cmake, so that 'autobuild configure' can work correctly.
The options are passed by adding -- 
-DAUTOBUILD_EXTRA_ARGS:STRING=options-to-add to the end of the configure 
command.
e.g. autobuild configure --config-file ab-custom.xml -c ReleaseOS -- 
-DAUTOBUILD_EXTRA_ARGS:STRING=--config-file=ab-custom.xml
Thanks to Brad Linden, for most of the work.


This addresses bugs OPEN-76 and OPEN-77.
http://jira.secondlife.com/browse/OPEN-76
http://jira.secondlife.com/browse/OPEN-77


Diffs (updated)
-

  doc/contributions.txt 1d4d568986e3 
  indra/cmake/Prebuilt.cmake 1d4d568986e3 

Diff: http://codereview.secondlife.com/r/327/diff


Testing
---

Successfully configured and built the viewer with a custom configuration and no 
autobuild.xml present.


Thanks,

Ima

___
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

[opensource-dev] Review Request: OPEN-78 Automate the process of passing additional arguments to prebuilt.cmake

[Ima Mechanique]

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

Review request for Viewer.


Summary
---

The fix in OPEN-77 allows configure to run correctly, but is cumbersome and not 
exactly pretty on the command line ;-)
This fix allows autobuild itself, to automatically add the --config-file option 
when it determines the default (autobuild.xml) is not being used.


This addresses bugs OPEN-76 and OPEN-78.
http://jira.secondlife.com/browse/OPEN-76
http://jira.secondlife.com/browse/OPEN-78


Diffs
-

  autobuild/autobuild_tool_configure.py 2a560b1d8f95 

Diff: http://codereview.secondlife.com/r/331/diff


Testing
---

Configure and built viewer-development with custom configuration and no default 
file present.


Thanks,

Ima

___
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

[opensource-dev] Review Request: Adding ability for Prebuilt.cmake to pass necessary additional options to 'autobuild install'

[Ima Mechanique]

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

Review request for Viewer.


Summary
---

This is primarily to allow the passing of '--config-file file-name' through 
cmake, so that 'autobuild configure' can work correctly.
The options are passed by adding -- 
-DAUTOBUILD_EXTRA_ARGS:STRING=options-to-add to the end of the configure 
command.
e.g. autobuild configure --config-file ab-custom.xml -c ReleaseOS -- 
-DAUTOBUILD_EXTRA_ARGS:STRING=--config-file=ab-custom.xml
Thanks to Brad Linden, for most of the work.


This addresses bugs OPEN-76 and OPEN-77.
http://jira.secondlife.com/browse/OPEN-76
http://jira.secondlife.com/browse/OPEN-77


Diffs
-

  indra/cmake/Prebuilt.cmake 1d4d568986e3 

Diff: http://codereview.secondlife.com/r/327/diff


Testing
---

Successfully configured and built the viewer with a custom configuration and no 
autobuild.xml present.


Thanks,

Ima

___
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

[opensource-dev] Review Request: STORM-899 'No attachments worn' text on blank 'Attachments' accordion remains in English for all locales

[Jonathan Yap]

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

Review request for Viewer.


Summary
---

1. Launch viewer, change language to German for example.
2. Re-login, open COF for edit.
3. Open 'Attachments' accordion, delete all objects.
===>
Actual: 'No attachments worn' text appears in English.


This addresses bug STORM-899.
http://jira.secondlife.com/browse/STORM-899


Diffs
-

  doc/contributions.txt c0c940514b74 
  indra/newview/llcofwearables.cpp c0c940514b74 
  indra/newview/skins/default/xui/en/panel_cof_wearables.xml c0c940514b74 
  indra/newview/skins/default/xui/en/strings.xml c0c940514b74 

Diff: http://codereview.secondlife.com/r/326/diff


Testing
---

Opened this tab with the viewer in English and also in French.  I saw the same 
message, but that is because there is no translation for French, but it shows 
the call for a translation is working.

What I am not sure of is if I made fix in the right way.


Thanks,

Jonathan

___
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: VWR-25965 LLDirIterator fixes in the viewer cache migration and VFS fallback.

[Altair Memo]

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

Ship it!


work fine on linux, all test passed (no problem if cache is stored on ramdisk 
too, erased every reboot start fine each run)

- Altair


On June 7, 2011, 11:42 a.m., Log Linden wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/324/
> ---
> 
> (Updated June 7, 2011, 11:42 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> This removes additional leading delimiters in llappviewer needed due to the 
> STORM-477 changes. This change should fix the fallback used when the viewer 
> cannot find a VFS with the salt it expects to find and instead looks for any 
> VFS data file in the cache directory to use.  It also fixes cache directory 
> migration code that runs on Mac and Windows every time the viewer is updated. 
> 
> 
> This addresses bug VWR-25964.
> http://jira.secondlife.com/browse/VWR-25964
> 
> 
> Diffs
> -
> 
>   indra/newview/llappviewer.cpp 1d4d568986e3 
> 
> Diff: http://codereview.secondlife.com/r/324/diff
> 
> 
> Testing
> ---
> 
> Test Plan:
> https://jira.secondlife.com/browse/VWR-25965?focusedCommentId=264386
> 
> I ran Case 1 on Linux and Case 2 on Mac and Windows. Everything worked as 
> expected.
> 
> 
> Thanks,
> 
> Log
> 
>

___
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

[opensource-dev] Review Request: VWR-25965 LLDirIterator fixes in the viewer cache migration and VFS fallback.

[Log Linden]

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

Review request for Viewer.


Summary
---

This removes additional leading delimiters in llappviewer needed due to the 
STORM-477 changes. This change should fix the fallback used when the viewer 
cannot find a VFS with the salt it expects to find and instead looks for any 
VFS data file in the cache directory to use.  It also fixes cache directory 
migration code that runs on Mac and Windows every time the viewer is updated. 


This addresses bug VWR-25964.
http://jira.secondlife.com/browse/VWR-25964


Diffs
-

  indra/newview/llappviewer.cpp 1d4d568986e3 

Diff: http://codereview.secondlife.com/r/324/diff


Testing
---

Test Plan:
https://jira.secondlife.com/browse/VWR-25965?focusedCommentId=264386

I ran Case 1 on Linux and Case 2 on Mac and Windows. Everything worked as 
expected.


Thanks,

Log

___
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] Minimum draw distance

[Hitomi Tiponi]

On 6/6/2011 12:37 PM, Jonathan Welch wrote:
> I have been working on a draw distance slider and realized this would
> be a good time to have a discussion about what the lowest value you
> can set your draw distance to should be.
>
> If you have an opinion of why it should be lowered from what it is
> now, 64, please reply to this message with your reasoning.
>
> Thank you,
>
> -Jonathan

Yay - good to see that you guys are finally working on something that has been 
in the StarLight skin for over a year - please have a look at my xml code if 
you 
want.

I did some experimentation with it a while back and found that 32 was a good 
minimum distance - if you go much lower you find that the camera behaves oddly 
when editing appearance.  Also you have to think what you want as a maximum 
distance - I have chosen 992 because some people want to see a long way when 
sailing etc.  (and it was highest multiple of 32 under 1000).

Hitomi
___
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