755339 - Include testing-only modules in xpcshell archive

Status: RESOLVED FIXED

(Testing :: XPCShell Harness, defect)

Description

[Gregory Szorc [:gps]]

Testing-only modules (bug 748490) works... locally. However, I neglected to add the new directory to the *-tests.zip archive. So, if you try to reference one of this modules in a test, the build infrastructure fails because the file is not found.

Comment 1

[Gregory Szorc [:gps]]

Attachment: Include testing modules in test package

Try at https://tbpl.mozilla.org/?tree=Try&rev=4c152e01ca1b. Also trying bug 744323 at the same time. I'm feeling lucky.

Comment 2

[Gregory Szorc [:gps]]

My in-head Try build failed before the infra. New Try at https://tbpl.mozilla.org/?tree=Try&rev=eddda3c66e2d

Comment 3

[Gregory Szorc [:gps]]

Attachment: Include testing modules in test package, v2

Try was busted.

Added new nsinstall -D to makefile. While I was there, I formatted it nicely.

New try at http://tbpl.mozilla.org/?tree=Try&rev=82d88a6458f0

This patch will fail without bug 755196 also applied since _tests/modules does not exist and cp will fail. I suppose I could work around that temporarily by having make ignore command failures temporarily. I'd rather just land as one push.

Comment 4

[Gregory Szorc [:gps]]

Gah. I think this will require buildbot changes:

========= Started unpack tests (results: 0, elapsed: 29 secs) (at 2012-05-22 06:01:04.105925) =========
unzip -o firefox-15.0a1.en-US.linux-i686.tests.zip 'bin*' 'certs*' 'xpcshell*'
 in dir /home/cltbld/talos-slave/test/build (timeout 1200 secs)
 watching logfiles {}
 argv: ['unzip', '-o', u'firefox-15.0a1.en-US.linux-i686.tests.zip', 'bin*', 'certs*', 'xpcshell*']

That command comes from some config file not in the tree, correct?

Comment 5

[Gregory Szorc [:gps]]

Answered my own question:

https://mxr.mozilla.org/build/source/buildbotcustom/steps/misc.py#568

How do we handle bugs like this?

Comment 6

[(not currently active) Ted Mielczarek]

It's a pain. You'll have to change the buildbot configs, but then unzip will probably fail unless your changes have already landed... (also this buildbot code gets shared across branches)

Comment 7

[Gregory Szorc [:gps]]

Attachment: Part 1: Create modules directory in tests archive

Per input from Ted, because we need to make a change to buildbot and said change will cause buildbot to fail if the "modules" directory isn't present in the tests.zip archive, we'll need to roll out the creation of this directory in all trees. Then, we can upgrade buildbot. Once that is done, we can land code that actually uses this directory.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Build system voodoo. I wish we didn't have to do it too.
User impact if declined: Delays landing of other patches, including Apps in the Cloud.
Fix Landed on Version: Not a bug. New feature.
Risk to taking this patch (and alternatives if risky): Shouldn't be risky at all. Very basic build system change to add a new empty directory to one archive.
String or UUID changes made by this patch: None

Comment 8

[(not currently active) Ted Mielczarek]

Comment on attachment 625975 [details] [diff] [review]
Include testing modules in test package, v2

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

::: testing/testsuite-targets.mk
@@ +301,5 @@
> +	  $(NSINSTALL) -D $(PKG_STAGE)/jetpack && \
> +	  $(NSINSTALL) -D $(PKG_STAGE)/firebug && \
> +	  $(NSINSTALL) -D $(PKG_STAGE)/peptest && \
> +	  $(NSINSTALL) -D $(PKG_STAGE)/mozbase && \
> +	  $(NSINSTALL) -D $(PKG_STAGE)/modules

I'd just give up on the && \ and put these each on their own lines at this point.

Comment 9

[Gregory Szorc [:gps]]

Part 1: https://hg.mozilla.org/integration/mozilla-inbound/rev/71075a6e5d4d

Part 2 will land after bug 757460 and in the same push as bug 755196 so the logic doesn't have to conditionally copy files based on the presence of objdir/_tests/modules. Bug 757460 can't land until part 1 lands in all the trees and gets merged everywhere.

Yeah, I'm confused too.

Comment 10

[Alex Keybl [:akeybl]]

Comment on attachment 626496 [details] [diff] [review]
Part 1: Create modules directory in tests archive

[Triage Comment]
Very low risk change in support of testing - approved for all branches. We'd expect to find issues with this in testing if there are any.

Comment 11

[Gregory Szorc [:gps]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5b7d64ec3bac

Comment 12

[Gregory Szorc [:gps]]

https://hg.mozilla.org/releases/mozilla-beta/rev/b20dd59c39df

rnewman, et al: any chance I could have someone else do the commit to esr and release? I don't have the trees checked out and it is my birthday and I'm supposed to be on PTO...

Comment 13

[Gregory Szorc [:gps]]

https://hg.mozilla.org/releases/mozilla-esr10/rev/9e6c975695ed
https://hg.mozilla.org/releases/mozilla-release/rev/a8f6432c2b1a

Comment 14

[Ed Morley [:emorley]]

(In reply to Gregory Szorc [:gps] from comment #9)
> Part 1: https://hg.mozilla.org/integration/mozilla-inbound/rev/71075a6e5d4d

https://hg.mozilla.org/mozilla-central/rev/71075a6e5d4d

Comment 15

[Chris Cooper [:coop] (he/him)]

Build masters were reconfigured with your change just now:

https://hg.mozilla.org/build/buildbotcustom/rev/7d0cd546fb77

Comment 16

[Gregory Szorc [:gps]]

This broke the infrastructure b/c modules/ is empty. I thought I had tested everything locally and got confirmation from aki that all was well. Well, in my local directory, there was at least a modules/.mkdir.done that wasn't showing up in a regular directory listing, so the directory wasn't truly empty. This file was left over from a locally-applied patch.

Anyway, we'll likely need to seed a follow-up patch which adds a file to modules/ so unzip won't complain on unzip. Ugh.

Comment 17

[Gregory Szorc [:gps]]

Attachment: Part 1b: Create modules directory with empty file

Last patch didn't actually work. zip/unzip didn't like empty directories. It turned the trees red. This patch creates an empty file in the modules/ directory so zip/unzip are happy. I've verified it works in a local build.

[Approval Request Comment]
See last approval. Build system foo.

Comment 18

[(not currently active) Ted Mielczarek]

Comment on attachment 628857 [details] [diff] [review]
Part 1b: Create modules directory with empty file

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

::: testing/testsuite-targets.mk
@@ +338,5 @@
>  	(cd $(topsrcdir)/services/sync/tests/tps && tar $(TAR_CREATE_FLAGS_QUIET) - *) | (cd $(PKG_STAGE)/tps/tests && tar -xf -)
>  
> +# This will get replaced by actual logic in a subsequent patch.
> +stage-modules: make-stage-dir
> +	touch $(PKG_STAGE)/modules/.dummy

$(TOUCH) (it's a native command in pymake)

Comment 19

[Gregory Szorc [:gps]]

Part 1b: https://hg.mozilla.org/mozilla-central/rev/4411b40ef38e

Comment 20

[Gregory Szorc [:gps]]

I pulled a zip file produced from an official build with this patch applied at verified unzip 'modules*' works. Once this goes out to all the trees, we should be clear to attempt bug 757460 again.

Comment 21

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 628857 [details] [diff] [review]
Part 1b: Create modules directory with empty file

Approving this build-system/test-only change for all branches.  Please land before Monday June 4th, which is merge day.

Comment 22

[Gregory Szorc [:gps]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b0e2daf63f06
https://hg.mozilla.org/releases/mozilla-beta/rev/8274534dbbe2
https://hg.mozilla.org/releases/mozilla-release/rev/ebf85d517554
https://hg.mozilla.org/releases/mozilla-esr10/rev/6627ef29f44d

The patch only applied cleanly against aurora. So, manual intervention was required on the other trees. Hopefully things don't break.

Comment 23

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/releases/mozilla-esr10/rev/a5afc55719f8 and https://hg.mozilla.org/releases/mozilla-release/rev/d67e20a0ae17

There aren't enough capslock keys in the world for me to adequately explain to you how wrong your behavior with those pushes was.

Comment 24

[Gregory Szorc [:gps]]

(In reply to Phil Ringnalda (:philor) from comment #23)
> Backed out in https://hg.mozilla.org/releases/mozilla-esr10/rev/a5afc55719f8
> and https://hg.mozilla.org/releases/mozilla-release/rev/d67e20a0ae17
> 
> There aren't enough capslock keys in the world for me to adequately explain
> to you how wrong your behavior with those pushes was.

Ugh. I feel really bad for doing this. I really do. I made a vim keystroke typo applying/saving the beta patch and didn't catch it when looking at the final qdiff. And, since I cherry-picked the esr and release patches from the beta commit, they got the bad patch. The beta tree turned red pretty quickly, so I fixed it with lsblakk's guidance. For whatever reason, I didn't make the connection to also check the diff on the esr and release trees. To top it off, I've been fighting some flu-like symptoms and passed out before the trees fully completed. I checked the trees before I went to bed and they were all doing well. I guess I narrowly missed seeing breakage.

In hindsight, I probably shouldn't have pushed to these trees in the mental/physical condition I was in. I was bound to make a stupid mistake. And, I did. At the time, I thought the patch was trivial enough to think there was no way I could mess it up. Again, my sickly brain thinking.

I know what the proper patch needs to be. However, I'll wait to apply it to esr and release until someone tells me it is OK.

Comment 25

[Gregory Szorc [:gps]]

https://hg.mozilla.org/releases/mozilla-esr10/rev/d9b2f3585986

Release should inherit the fix from beta. So, we can consider the seeding of this patch to all necessary trees complete. I'll work with RelEng in bug 757460 to get the build infra changes rolled out. Might wait until the 2nd build config push of the week, however.

Comment 26

[Gregory Szorc [:gps]]

philor was kind enough to point out that release did not inherit the fix from beta (because it was cut over before it landed). So,

https://hg.mozilla.org/releases/mozilla-release/rev/55ba50a35fd9

Comment 27

[Gregory Szorc [:gps]]

https://hg.mozilla.org/services/services-central/rev/6519f404443d

Comment 28

[Gregory Szorc [:gps]]

https://hg.mozilla.org/mozilla-central/rev/6519f404443d

Finally.