race condition with nsinstall in toolkit/mozapps/plugins/tests Makefile

RESOLVED FIXED

Status

defect
RESOLVED FIXED
10 years ago
Last year

People

(Reporter: olrrm2020, Assigned: ted)

Tracking

Trunk
x86
Windows XP

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b4pre) Gecko/20090419 Shiretoko/3.5b4pre Firefox/3.0.8
Build Identifier: 

The Trunk Nightly Build failed/busted. See Mozilla-Central tinderbox. It needs to be determined why the building of the nightly failed to complete successfully.

Reproducible: Always
Status: UNCONFIRMED → NEW
Ever confirmed: true
The actual error was:

make[8]: Entering directory `/e/builds/moz2_slave/mozilla-central-win32-nightly/build/obj-firefox/toolkit/mozapps/plugins/tests'
d:/mozilla-build/moztools/bin/nsinstall.exe -m 755 GoodPlugin.exe BadPlugin.exe ../../../../dist/bin
d:/mozilla-build/moztools/bin/nsinstall.exe -D ../../../../_tests/testing/mochitest/browser/toolkit/mozapps/plugins/tests
d:/mozilla-build/moztools/bin/nsinstall.exe -D ../../../../_tests/testing/mochitest/browser/toolkit/mozapps/plugins/tests
d:/mozilla-build/moztools/bin/nsinstall.exe "/e/builds/moz2_slave/mozilla-central-win32-nightly/build/toolkit/mozapps/plugins/tests/browser_bug435788.js" "/e/builds/moz2_slave/mozilla-central-win32-nightly/build/toolkit/mozapps/plugins/tests/pfs_bug435788_1.rdf" "/e/builds/moz2_slave/mozilla-central-win32-nightly/build/toolkit/mozapps/plugins/tests/pfs_bug435788_2.rdf" "/e/builds/moz2_slave/mozilla-central-win32-nightly/build/toolkit/mozapps/plugins/tests/GoodExtension.xpi" "/e/builds/moz2_slave/mozilla-central-win32-nightly/build/toolkit/mozapps/plugins/tests/BadExtension.xpi" ../../../../_tests/testing/mochitest/browser/toolkit/mozapps/plugins/tests
cp BadPlugin.exe ../../../../_tests/testing/mochitest/browser/toolkit/mozapps/plugins/tests/BadPlugin
..\..\..\..\_tests\testing\mochitest\browser\toolkit\mozapps\plugins\tests: Could not create the directory: File exists
make[8]: *** [../../../../_tests/testing/mochitest/browser/toolkit/mozapps/plugins/tests/GoodPlugin] Error 3
Summary: Trunk (Minefield) Nightly Build For 20090419 Failed/Busted (Mozilla-Central Tinderbox) → Win32 Minefield nightly build for 20090419 failed
I think there's a subtle race condition in the Win32 version of nsinstall (bug 463411), tickled by the makefile rule here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/plugins/tests/Makefile.in#70

Mossop: do you really need 'cp' there, or will $(INSTALL) work? (nsinstall will create the target directory if it needs to.) Alternately, if you do, we can add a few lines to make this not race, something like:
$(PROGRAMS): $(TESTROOT)
$(TESTROOT):
    $(NSINSTALL) -D $(TESTROOT)
Last night's build went through ok.

If you need the build from 20090419, let me know.
(In reply to comment #2)
> I think there's a subtle race condition in the Win32 version of nsinstall (bug
> 463411), tickled by the makefile rule here:
> http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/plugins/tests/Makefile.in#70
> 
> Mossop: do you really need 'cp' there, or will $(INSTALL) work? (nsinstall will
> create the target directory if it needs to.) Alternately, if you do, we can add
> a few lines to make this not race, something like:
> $(PROGRAMS): $(TESTROOT)
> $(TESTROOT):
>     $(NSINSTALL) -D $(TESTROOT)

I seem to remember I couldn't get INSTALL to work right because it only copies/links a file to a directory, while what I need to do here is copy a file to a specific location, the difference being that the copy can have a different name.
Ok, then we should probably just do the Makefile fix for now. Don't care if we morph this bug or file a new one.
This same crash hit the M-C hourly build as well it seems, so not limited to just 'nightly' builds..

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1240239991.1240246479.25210.gz
Yeah, this is basically the perfect storm of Makefile logic to trigger bug 463411.
Assignee: nobody → ted.mielczarek
Status: NEW → ASSIGNED
Component: Release Engineering: Maintenance → Build Config
Product: mozilla.org → Core
QA Contact: release → build-config
Summary: Win32 Minefield nightly build for 20090419 failed → race condition with nsinstall in toolkit/mozapps/plugins/tests Makefile
Version: other → Trunk
bsmedberg is out on paternity leave, so r?Mossop since it's his Makefile. Let me explain what I did here for the non Makefile-inclined.

+$(PROGRAMS) $(_BROWSER_FILES): $(TESTROOT)
+$(TESTROOT):
+       $(NSINSTALL) -D $@

This is what I suggested above, it just makes sure that the rule to create $(TESTROOT) with nsinstall only gets executed once, and gets executed before the rules to copy $(PROGRAMS) and $(_BROWSER_FILES).

 $(PROGRAMS): $(TESTROOT)/% : %$(BIN_SUFFIX)
-       $(NSINSTALL) -D $(TESTROOT)
-       cp $^ $@
+       cp $< $@

$^ is "all prerequisites of this rule", $< is "just the first prerequisite of this rule". Unfortunately making $(TESTROOT) a prerequisite of $(PROGRAMS) gets it listed in $^, so this is a workaround.

-libs:: $(_BROWSER_FILES)
-       $(INSTALL) $(foreach f,$^,"$f") $(TESTROOT)
+libs::
+       $(INSTALL) $(foreach f,$(_BROWSER_FILES),"$(srcdir)/$f") $(TESTROOT)

Same thing here, $(TESTROOT) wound up in $^, so I just explicitly listed the files to install instead. (Having $(_BROWSER_FILES) as a prereq for libs doesn't get you much, since those aren't generated files.)
Attachment #373689 - Flags: review?(dtownsend)
Attachment #373689 - Flags: review?(dtownsend) → review+
Comment on attachment 373689 [details] [diff] [review]
tweak the makefile to avoid the race

Sounds reasonable to me
Pushed to m-c:
http://hg.mozilla.org/mozilla-central/rev/e795ecf4adbb
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.