Closed Bug 579351 Opened 14 years ago Closed 14 years ago

Do not install svg-only tests when SVG is disabled

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED WONTFIX

People

(Reporter: mounir, Assigned: mounir)

Details

Attachments

(1 file, 2 obsolete files)

Patch v2.1
1.26 KB, patch
benjamin
: approval2.0+
Details | Diff | Splinter Review
Attached patch Patch v1 (obsolete) — — Splinter Review 
That is not going to fix all tests when svg is disabled but it's fixing 3 of 5 for content/html/content/test.
Attachment #457861 - Flags: review?(ted.mielczarek)
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Comment on attachment 457861 [details] [diff] [review]
Patch v1

You can't put ifdefs in the middle of line continuations. You'll need to move this out of this block to after this whole assignment and do something like
ifdef MOZ_SVG
_TEST_FILES += ...
endif
Attachment #457861 - Flags: review?(ted.mielczarek) → review-
(In reply to comment #1)
> Comment on attachment 457861 [details] [diff] [review]
> Patch v1
> 
> You can't put ifdefs in the middle of line continuations. You'll need to move
> this out of this block to after this whole assignment and do something like
> ifdef MOZ_SVG
> _TEST_FILES += ...
> endif

Sorry about that. My error is really stupid.
Attached patch Patch v2 (obsolete) — — Splinter Review 

        Attachment #457861 -
        Attachment is obsolete: true

        Attachment #461912 -
        Flags: review?(ted.mielczarek)

        Attachment #461912 -
        Flags: review?(ted.mielczarek) → review+

    
    

    
  
Comment on attachment 461912 [details] [diff] [review]
Patch v2

I'm not sure if that should be considered as 'not affecting build' or if that needs an approval...

        Attachment #461912 -
        Flags: approval2.0?

    
    

    
  
You need "+=", not "=". The latter overwrites the non-SVG tests, AIUI.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch v2.1
        — 
          — Splinter Review
      

    


  
Oups, typo :-/

        Attachment #462090 -
        Flags: approval2.0?

    
  

        Attachment #461912 -
        Attachment is obsolete: true

        Attachment #461912 -
        Flags: approval2.0?

    
    

    
  
I think we shouldn't do this.  We're saying SVG is an integral part of the Web platform; our test suites shouldn't pass if Gecko has been miscompiled to exclude it.

    
    

    
  
(In reply to comment #7)
> I think we shouldn't do this.  We're saying SVG is an integral part of the Web
> platform; our test suites shouldn't pass if Gecko has been miscompiled to
> exclude it.

In that case why not removing the option?

    
    

    
  
That seems like a sensible suggestion.

    
  

        Attachment #462090 -
        Flags: approval2.0? → approval2.0+

    
    

    
  
I've open bug 585020 for removing svg compilation option.

    
    

    
  
Wontfix, considering bug 585020
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → WONTFIX

    
  
Product: Core → Firefox Build System

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: