444992 - NATIVE_DIST definition used in packager.mk for SIGN_CMD is crap

Status: RESOLVED FIXED

(Toolkit Graveyard :: Build Config, defect)

Description

[Robert Kaiser]

In http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/packager.mk#193 a NATIVE_DIST var is being defined for being used only in the following line for assembling SIGN_CMD.
Its intention is to add a "../$(NATIVE_ARCH)/" in the DIST definition between DEPTH and the rest, what what it does is more or less cutting off the "/dist" at the end with "..", then adding that intended part and then adding "/dist" again, which achieves the same as if it just was
NATIVE_DIST = $(DEPTH)/../$(NATIVE_ARCH)/dist
but does this in a much less clean way.

Needless to say that this breaks with the comm-central build system the same way as the simpler line would.

There are three possible ways for solving this so that comm-central can also use it:

1) Clean the definition up to the line I stated above, but wrap it in a |ifndef NATIVE_DIST| so comm-central files can override it, or
2) Implement it correctly as intended and do something like a s|^$(DEPTH)|$(DEPTH)/../$(NATIVE_ARCH)| on DIST for assigning it to NATIVE_DIST, or even
3) Use just DIST here and disallow making packages from any other than the native platform for universal builds.

Comment 1

[Robert Kaiser]

Attachment: patch v1: correct approach with substitution reference

I decided to try the correct approach (number 2 in comment #0) and use a substitution reference <http://www.gnu.org/software/automake/manual/make/Substitution-Refs.html#Substitution-Refs> to do the trick and really inject the part we want after the DEPTH.
This has been tested to work on the Mac buildbot machine I'm preparing for SeaMonkey going comm-central (and this is Mac-only code of course, living inside |ifdef UNIVERSAL_BINARY|).

Comment 2

[Robert Kaiser]

checked in back on 2008-07-21 as http://hg.mozilla.org/mozilla-central/rev/b77757245257