Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Include testing-only modules in xpcshell archive

RESOLVED FIXED in Firefox -esr10

Status

Testing
XPCShell Harness
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gps, Assigned: gps)

Tracking

unspecified
mozilla16
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr10 fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Created attachment 625961 [details] [diff] [review]
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.
Assignee: nobody → gps
Status: NEW → ASSIGNED
Attachment #625961 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 2

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

Comment 3

5 years ago
Created attachment 625975 [details] [diff] [review]
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.
Attachment #625961 - Attachment is obsolete: true
Attachment #625961 - Flags: review?(ted.mielczarek)
Attachment #625975 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 4

5 years ago
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?
(Assignee)

Comment 5

5 years ago
Answered my own question:

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

How do we handle bugs like this?
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)
(Assignee)

Updated

5 years ago
Depends on: 757460
(Assignee)

Updated

5 years ago
Blocks: 757860
(Assignee)

Comment 7

5 years ago
Created attachment 626496 [details] [diff] [review]
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
Attachment #626496 - Flags: review?(ted.mielczarek)
Attachment #626496 - Flags: approval-mozilla-release?
Attachment #626496 - Flags: approval-mozilla-esr10?
Attachment #626496 - Flags: approval-mozilla-beta?
Attachment #626496 - Flags: approval-mozilla-aurora?
Attachment #626496 - Flags: review?(ted.mielczarek) → review+
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.
Attachment #625975 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 9

5 years ago
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.
Whiteboard: [do not resolve bug on inbound merge]
(Assignee)

Updated

5 years ago
Blocks: 749336
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.
Attachment #626496 - Flags: approval-mozilla-release?
Attachment #626496 - Flags: approval-mozilla-release+
Attachment #626496 - Flags: approval-mozilla-esr10?
Attachment #626496 - Flags: approval-mozilla-esr10+
Attachment #626496 - Flags: approval-mozilla-beta?
Attachment #626496 - Flags: approval-mozilla-beta+
Attachment #626496 - Flags: approval-mozilla-aurora?
Attachment #626496 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 11

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/5b7d64ec3bac
(Assignee)

Comment 12

5 years ago
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...
(Assignee)

Comment 13

5 years ago
https://hg.mozilla.org/releases/mozilla-esr10/rev/9e6c975695ed
https://hg.mozilla.org/releases/mozilla-release/rev/a8f6432c2b1a
(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
Whiteboard: [do not resolve bug on inbound merge] → [leave open]
Build masters were reconfigured with your change just now:

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

Comment 16

5 years ago
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.
(Assignee)

Comment 17

5 years ago
Created attachment 628857 [details] [diff] [review]
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.
Attachment #628857 - Flags: review?(ted.mielczarek)
Attachment #628857 - Flags: approval-mozilla-release?
Attachment #628857 - Flags: approval-mozilla-esr10?
Attachment #628857 - Flags: approval-mozilla-beta?
Attachment #628857 - Flags: approval-mozilla-aurora?
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)
Attachment #628857 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 19

5 years ago
Part 1b: https://hg.mozilla.org/mozilla-central/rev/4411b40ef38e
(Assignee)

Comment 20

5 years ago
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.
(Assignee)

Updated

5 years ago
No longer blocks: 749336
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.
Attachment #628857 - Flags: approval-mozilla-release?
Attachment #628857 - Flags: approval-mozilla-release+
Attachment #628857 - Flags: approval-mozilla-esr10?
Attachment #628857 - Flags: approval-mozilla-esr10+
Attachment #628857 - Flags: approval-mozilla-beta?
Attachment #628857 - Flags: approval-mozilla-beta+
Attachment #628857 - Flags: approval-mozilla-aurora?
Attachment #628857 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 22

5 years ago
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.
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.
(Assignee)

Comment 24

5 years ago
(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.
(Assignee)

Comment 25

5 years ago
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.
(Assignee)

Comment 26

5 years ago
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
(Assignee)

Updated

5 years ago
Depends on: 762837
(Assignee)

Comment 27

5 years ago
https://hg.mozilla.org/services/services-central/rev/6519f404443d
Whiteboard: [leave open] → [fixed in services]
Target Milestone: --- → mozilla16
(Assignee)

Comment 28

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

Finally.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in services]
status-firefox-esr10: --- → fixed
You need to log in before you can comment on or make changes to this bug.