Closed Bug 1228467 Opened 4 years ago Closed 4 years ago

Transform "no preprocessing directives" warnings into errors

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox45 fixed)

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(4 files)

No description provided.
FWIW, I checked the corresponding files in all locales, and they have no preprocessor directives either (which is unsurprising)
Attachment #8692926 - Flags: review?(gps)
There is preprocessing directive in the file. Also, the current support for
preprocessing doesn't take into account any DEFINES that would be set in the
corresponding moz.build, which would be quite surprising would someone want
to use preprocessing on that file in the future.
Attachment #8692927 - Flags: review?(nfroyd)
(In reply to Mike Hommey [:glandium] from comment #2)
> Created attachment 8692927 [details] [diff] [review]
> Don't preprocess dom/base/UseCounters.conf
> 
> There is preprocessing directive in the file. Also, the current support for
          ^ no
This will undoubtedly break c-c, but it's not really something hard to overcome, see the 3 other patches.
Attachment #8692929 - Flags: review?(gps)
Comment on attachment 8692927 [details] [diff] [review]
Don't preprocess dom/base/UseCounters.conf

Review of attachment 8692927 [details] [diff] [review]:
-----------------------------------------------------------------

I think this is OK, given that we usually use prefs rather than defines for DOM features, and we're unlikely to gate use counters (or features!) on particular platforms, which would be the other major reason for #ifdefs, judging from a skim through dom/
Attachment #8692927 - Flags: review?(nfroyd) → review+
Comment on attachment 8692926 [details] [diff] [review]
Don't preprocess chrome files without preprocessor directives

Review of attachment 8692926 [details] [diff] [review]:
-----------------------------------------------------------------

::: b2g/locales/jar.mn
@@ +8,5 @@
>  % locale b2g-l10n @AB_CD@ %locale/@AB_CD@/b2g-l10n/
>  
>  % override chrome://global/locale/aboutCertError.dtd     chrome://b2g-l10n/locale/aboutCertError.dtd
>  % override chrome://global/locale/appstrings.properties  chrome://b2g-l10n/locale/appstrings.properties
> +  locale/@AB_CD@/b2g-l10n/aboutCertError.dtd       (%chrome/overrides/aboutCertError.dtd)

What if a specific locale contains preprocessing directives? I have a very vague recollection of having to add a '*' somewhere because this occurs in the wild. I guess we won't know until we do repacks in a few weeks/months :/
Attachment #8692926 - Flags: review?(gps) → review+
Attachment #8692928 - Flags: review?(gps) → review+
Attachment #8692929 - Flags: review?(gps) → review+
(In reply to Gregory Szorc [:gps] from comment #7)
> ::: b2g/locales/jar.mn
> @@ +8,5 @@
> >  % locale b2g-l10n @AB_CD@ %locale/@AB_CD@/b2g-l10n/
> >  
> >  % override chrome://global/locale/aboutCertError.dtd     chrome://b2g-l10n/locale/aboutCertError.dtd
> >  % override chrome://global/locale/appstrings.properties  chrome://b2g-l10n/locale/appstrings.properties
> > +  locale/@AB_CD@/b2g-l10n/aboutCertError.dtd       (%chrome/overrides/aboutCertError.dtd)
> 
> What if a specific locale contains preprocessing directives? I have a very
> vague recollection of having to add a '*' somewhere because this occurs in
> the wild. I guess we won't know until we do repacks in a few weeks/months :/

As mentioned in comment 1, I checked all locales already.
Blocks: 1229810
Depends on: 1230502
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.