1485057 - uninstall/helper.exe is never rebuilt

Status: RESOLVED FIXED

(Firefox Build System :: General, defect, P1)

Description

[Molly Howell (she/her) (no longer active)]

helper.exe (the Windows uninstaller) doesn't seem to ever be recompiled in an incremental build when its sources change. It only seems to get built during clobber builds.

I can't tell whether this is fallout from bug 1385227 or from bug 1436662, but one of those seems likely since there haven't been any other changes that look relevant.

Comment 1

[(not currently active) Ted Mielczarek]

helper.exe is currently still expressed in a Makefile.in:
https://dxr.mozilla.org/mozilla-central/rev/4e56a2f51ad739ca52046723448f3129a58f1666/toolkit/mozapps/installer/windows/nsis/makensis.mk#83

...and the make rule doesn't specify any dependencies, which is unfortunate. My H2 goals include starting to rewrite a bunch of l10n repack bits. We identified three steps for the work, and fixing these makefile rules is #2:
https://docs.google.com/document/d/1_ooGkzRx5kKNGj6FPETyNKMBaIuYeW4a65ePVmCEk2U/edit#heading=h.o7o5mt5rbjoc

...so I'm hoping to make this better in the near future.

Comment 2

[Molly Howell (she/her) (no longer active)]

I don't like asking this sort of thing, but would there be any way to get this bumped up the queue a bit? I just caused bug 1495212 because I forgot that this bug was happening and thought I was seeing my changes for bug 1494900 working fine when said changes were not actually getting built.

Comment 3

[Kim Moir [:kmoir] ET]

Ted,

What timeframe are you looking at for your l10n work that would help mhowell out?

Comment 4

[Robert Strong (they/them - no direct email)]

I suspect that it does get rebuilt but not all files are updated. I just did an incremental with a change to uninstaller.nsi and it did pick up that change. Also, the console showed that it was rebuilding helper.exe even without any changes. I also recall at one point that the branding images weren't being updated since the installer doesn't run make on the branding dir so branding.nsi is probably not updated as well.

Comment 5

[(No longer employed by Mozilla) Aaron Klotz]

I took a crack at a patch that seems to work for me locally, though I'm not sure how correct it is. I'll put it up for review and we'll see.

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Bug 1485057: Fix makensis.mk to allow helper.exe to build incrementally; r=mshal

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Kim, sorry but could you please find a new person for the needinfo?

Comment 8

[Kim Moir [:kmoir] ET]

redirecting ni to mshal

Comment 9

[Michael Shal [:mshal]]

aklotz already provided a patch, which I believe just needs to have the review feedback addressed in order to land. :aklotz, is there anything problematic in the review?

Comment 10

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Michael Shal [:mshal] from comment #9)

aklotz already provided a patch, which I believe just needs to have the review feedback addressed in order to land. :aklotz, is there anything problematic in the review?

This has just fallen off my radar, is all. I'll leave myself on ni? so that I address the feedback.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/14f130c1a3cf
Fix makensis.mk to allow helper.exe to build incrementally; r=mshal

Comment 12

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/14f130c1a3cf