Closed Bug 763987 Opened 12 years ago Closed 12 years ago

Normalize autoconf.mk.in

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla16

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(2 files, 3 obsolete files)

There are various things in autoconf.mk.in that make it not very easily autogenerated:
- ifdefs
- variable definitions involving over variables (FOO = $(BAR) @FOO@)
- variable name mismatch (FOO = @BAR@)
Attached patch Normalize autoconf.mk.in (obsolete) — Splinter Review
This addresses most of comment 0. The remaining lines need to move to a separate file that autoconf.mk would include, because they need to be included first (config/config.mk is included too late). That will be done in a separate patch.

Testing on try:
https://tbpl.mozilla.org/?tree=Try&rev=41178e9555c6
Attachment #632286 - Flags: review?(ted.mielczarek)
Comment on attachment 632286 [details] [diff] [review]
Normalize autoconf.mk.in

Breaks b2g :(
Attachment #632286 - Flags: review?(ted.mielczarek)
Depends on: 764046
Attached patch Normalize autoconf.mk.in (obsolete) — Splinter Review
After all, I folded what I intended to be two patches. This is going through try:

https://tbpl.mozilla.org/?tree=Try&rev=abe247d8ba81

The previous B2G breakage was due to bug 764046, so this patch depends on that bug.
Attachment #632563 - Flags: review?(ted.mielczarek)
Attachment #632286 - Attachment is obsolete: true
Attached patch Normalize autoconf.mk.in (obsolete) — Splinter Review
Last one failed mac universal build.

https://tbpl.mozilla.org/?tree=Try&rev=ebb5a7ce747e
Attachment #632581 - Flags: review?(ted.mielczarek)
Attachment #632563 - Attachment is obsolete: true
Attachment #632563 - Flags: review?(ted.mielczarek)
Depends on: 764286
No longer depends on: 764286
Forgot to change a few SYSTEM_MAKEDEPEND.
Attachment #632601 - Flags: review?(ted.mielczarek)
Attachment #632581 - Attachment is obsolete: true
Attachment #632581 - Flags: review?(ted.mielczarek)
Assignee: nobody → mh+mozilla
Comment on attachment 632601 [details] [diff] [review]
Normalize autoconf.mk.in

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

::: build/macosx/js-icc-flight.mk
@@ -1,1 @@
>  # This Source Code Form is subject to the terms of the Mozilla Public

You could just hg rm this file honestly. It's never going to be used.

::: config/baseconfig.mk
@@ +2,5 @@
> +
> +includedir := $(includedir)/$(MOZ_APP_NAME)-$(MOZ_APP_VERSION)
> +idldir = $(datadir)/idl/$(MOZ_APP_NAME)-$(MOZ_APP_VERSION)
> +installdir = $(libdir)/$(MOZ_APP_NAME)-$(MOZ_APP_VERSION)
> +sdkdir = $(libdir)/$(MOZ_APP_NAME)-devel-$(MOZ_APP_VERSION)

Do you want to just format these variables in configure instead?

::: config/config.mk
@@ +55,5 @@
>  space :=$(nullstr) # EOL
>  
>  core_winabspath = $(firstword $(subst /, ,$(call core_abspath,$(1)))):$(subst $(space),,$(patsubst %,\\%,$(wordlist 2,$(words $(subst /, ,$(call core_abspath,$(1)))), $(strip $(subst /, ,$(call core_abspath,$(1)))))))
>  
> +RM = rm -f

I don't think this is actually necessary, since both gmake and pymake define this.

::: configure.in
@@ +4193,5 @@
>  else
> +    AC_CHECK_LIB(png, png_get_valid, [MOZ_NATIVE_PNG=1 MOZ_PNG_LIBS="-lpng"],
> +                 AC_MSG_ERROR([--with-system-png requested but no working libpng found]))
> +#    AC_CHECK_LIB(png, png_get_acTL, ,
> +#                 AC_MSG_ERROR([--with-system-png won't work because the system's libpng doesn't have APNG support]))

I would guess you don't actually want to leave this commented out. (Just for testing?)
Attachment #632601 - Flags: review?(ted.mielczarek) → review+
(In reply to Ted Mielczarek [:ted] from comment #6)
> ::: config/baseconfig.mk
> @@ +2,5 @@
> > +
> > +includedir := $(includedir)/$(MOZ_APP_NAME)-$(MOZ_APP_VERSION)
> > +idldir = $(datadir)/idl/$(MOZ_APP_NAME)-$(MOZ_APP_VERSION)
> > +installdir = $(libdir)/$(MOZ_APP_NAME)-$(MOZ_APP_VERSION)
> > +sdkdir = $(libdir)/$(MOZ_APP_NAME)-devel-$(MOZ_APP_VERSION)
> 
> Do you want to just format these variables in configure instead?

I'm not sure. Let's do that in a followup if that's worthwhile.
 
> ::: configure.in
> @@ +4193,5 @@
> >  else
> > +    AC_CHECK_LIB(png, png_get_valid, [MOZ_NATIVE_PNG=1 MOZ_PNG_LIBS="-lpng"],
> > +                 AC_MSG_ERROR([--with-system-png requested but no working libpng found]))
> > +#    AC_CHECK_LIB(png, png_get_acTL, ,
> > +#                 AC_MSG_ERROR([--with-system-png won't work because the system's libpng doesn't have APNG support]))
> 
> I would guess you don't actually want to leave this commented out. (Just for
> testing?)

Oops :)
Attached patch FixupSplinter Review
This should fix the jsreftest bustage I got when landing.
Attachment #634842 - Flags: review?(ted.mielczarek)
https://hg.mozilla.org/mozilla-central/rev/25b914405558
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Attachment #634842 - Flags: review?(ted.mielczarek) → review+
https://hg.mozilla.org/mozilla-central/rev/e96cc844629d
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/5409a2426f99
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Blocks: 767006
Depends on: 771870
Depends on: 773152
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.