812299 - Allow extending l10n.mk and packager.mk targets

Status: RESOLVED WONTFIX

(Release Engineering :: General, defect)

Description

[Philipp Kewisch [:Fallen][📱📆]]

I am currently in the process of making use of the Thunderbird infrastructure to do Lightning builds. This requires us to hook up the Thunderbird targets to Lightning, as you can see here:

http://hg.mozilla.org/users/mozilla_kewis.ch/tb-build-integration-aurora/file/6edbebceec73/mail/locales/Makefile.in#l206

I initially thought I could just extend the existing targets, but this would require them to be :: rules, which in turn doesn't work with pattern rules like langpack-%. Instead, I've added empty rules like app-langpack-% that can be overriden. This will produce a warning like:

Makefile:211: warning: overriding commands for target `app-unpack'

but since the targets are empty I think this is ok.

If you know a more elegant way to do this, please let me know and I will adjust the patch or my comm-central patches.

Comment 1

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Fix - v1

Comment 2

[Philipp Kewisch [:Fallen][📱📆]]

Alternatively, if you want less changes, I could probably do it with the existing targets like this:

clobber-zip: Use this to unpack the Lightning en-US xpi AND remove en-US files
libs-%: Use this target to put together the localized lightning xpi
installer-%: Add a Lightning target that does the repackage

I would still need something to extend these targets though:

wget-en-US
upload

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 682132 [details] [diff] [review]
Fix - v1

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

This is mostly okay, but I have a few issues. Sorry for the review delay.

::: toolkit/locales/l10n.mk
@@ +31,5 @@
> +# piggyback actions to the individual targets. Before using these targets,
> +# ensure that what you are doing isn't something worth adding to this file.
> +# app-clobber-%
> +#   This target is called before the clobber-% target is called.
> +# app-unpack-%

You say app-unpack-% here, but the rule in the patch is just "app-unpack".

@@ +104,5 @@
>  	  $(UNMAKE_PACKAGE)
>  	$(MAKE) clobber-zip AB_CD=en-US
>  
> +app-unpack: ;
> +unpack: $(STAGEDIST) app-unpack

We shouldn't provide non-pattern rules like this. Implementing app-unpack: in another Makefile will give two warnings: "overriding commands for target `app-unpack' and "ignoring old commands for target `app-unpack'".

You can simply write:
unpack: app-unpack
app-unpack:
   ...

in the other Makefile.

Comment 4

[Philipp Kewisch [:Fallen][📱📆]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #3)

> > +# app-unpack-%
> 
> You say app-unpack-% here, but the rule in the patch is just "app-unpack".
Ok, will fix.

> You can simply write:
> unpack: app-unpack
> app-unpack:
>    ...
Wouldn't this cause the same warning, overriding commands for target `unpack' ?

Comment 5

[(not currently active) Ted Mielczarek]

(In reply to Philipp Kewisch [:Fallen] from comment #4)
> > You can simply write:
> > unpack: app-unpack
> > app-unpack:
> >    ...
> Wouldn't this cause the same warning, overriding commands for target
> `unpack' ?

No, because you're not overriding the commands, you're simply providing a dependency. If you stick the ';' at the end of the line, you give it an empty recipe which you then have to override.

Comment 6

[Philipp Kewisch [:Fallen][📱📆]]

    I've found a good way to do this without changing toolkit. Thanks for the hints, they definitely helped!