All submissions to this site are governed by the Second Life Viewer Contribution Agreement. By submitting patches and other information using this site, you acknowledge that you have read, understood, and agreed to those terms.

Review Board 1.6.11

Welcome to the Second Life Viewer Code Review tool.
See the documentation on our wiki for how to use this site.

STORM-1215: restore proprietary components in Release builds

Review Request #284 - Created May 2, 2011 and submitted

Oz Linden Reviewers
STORM-1215 brad.linden
None viewer-development 
The INSTALL_PROPRIETARY switch was incorrectly removed.   This caused both KDU and FMOD to be omitted: the lack of FMOD caused the problem with sound.

 

 










  

 
















 

  




 

 

  
  

  
Wolfpup Lowenhar

  
Posted  (May 2, 2011, 7:25 a.m.)

 

 

   

   

  

   

   
    
    
   


  






   







    

     
     

      

       
        

         
          autobuild.xml
          

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    

     
This is needed so that Windows Developers can get the 3-p Quicktime libs from LL so mabey move this to the windows settings instead of deleting it.


     

  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    I'm not sure I understand this comment.  LL_RELEASE_FOR_DOWNLOAD is a preprocessor symbol and has nothing to do with cmake configuration for any third party library, including quicktime.  Setting this flag for cmake does nothing, and its introduction was certainly an error.
    
    
    
   
    2. 


   
  2. 
    
    
     
    
      
     
    
     
    Actually if this is not active for Windows OS Developers Quicktime will not be included in media_plugin_quicktime.dll and there will be no Quicktime media functionality in the resultant viewer. media_plugin_quuicktime.dll will only be 85K instead of the 58M it should be. This may be another issue but this is the current workaround to get it to build propperly.
    
    
    
   
    3. 

   


  





    

    

   


  
 




  

 

















 

  




 

 

  
Ship it!

  

  
Brad Kittenbrink

  
Posted  (May 2, 2011, 3:10 p.m.)

 

 

   

   

  

   

   
    
    
   


  






   







    

     
     

      

       
        

         
          autobuild.xml
          

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    

     
Looks good. Additionally to avoid unnecessary confusion, I'd recommend removing the explicit setting of ROOT_PROJECT_NAME as well, and rely on the correct defaults in indra/CMakeLists.txt.


     

  

   

   
    
    
   


  





    

    

   


  
 




  

 



















 

  




 

 

  
  

  
Ima Mechanique

  
Posted  (May 3, 2011, 11:16 a.m.)

 

 

   

   

  

   

   
    
    
   


  






   







    

     
     

      

       
        

         
          autobuild.xml
          

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    

     
If this line needs to be removed, which I'm not convinced of, shouldn't it be done as a separate commit rather than as one purporting to fix 'Release' builds not 'ReleaseOS' where this line occurs?


     

  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    The goal of this commit as far as I understand it is to make Release and ReleaseOS more consistent with each other, and to correct errors that were made in previous work towards that goal.  It's 2 lines of code, I really don't care what color bikeshed we make this but that line needs to be removed.  Whatever bug you folks are claiming that line fixes, I guarantee you it doesn't do what you think it does. It's an error.  Setting a cmake varaible that never gets read anywhere cannot possibly fix the problem that you are claiming it fixes.
    
    
    
   
    2.