Closed Bug 753501 Opened 8 years ago Closed 8 years ago

Add empty tooltool manifests to some platforms

Categories

(Release Engineering :: General, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: rail, Assigned: rail)

Details

(Whiteboard: [tooltool])

Attachments

(5 files, 3 obsolete files)

... so devs can easier test new toolkits.
Platforms: linux{,64}{,-debug}, macosx64{,-debug}, android{,-xul}
Attached patch empty manifestsSplinter Review
Attached patch configs (obsolete) — Splinter Review
I moved tooltool_url_list to localcofig's GLOBAL_VARS, so it's not platform specific anymore (no need to copy paste it around) and you can easily redefine it for staging/pp.
Attachment #622718 - Flags: review?(jhford)
Attached patch buildbotcustomSplinter Review
Inline comments incoming
Attachment #622719 - Flags: review?(jhford)
Comment on attachment 622719 [details] [diff] [review]
buildbotcustom

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

::: process/factory.py
@@ +768,5 @@
>                   mozharnessMultiOptions=None,
>                   mock_packages=[],
>                   tooltool_manifest_src=None,
>                   tooltool_bootstrap="setup.sh",
> +                 tooltool_url_list=None,

Using None is safer

@@ +1202,5 @@
>          ))
>          if self.tooltool_manifest_src:
>              self.addStep(RetryingShellCommand(
>                  name='fetch_tooltool_resources',
> +                command=[self.tooltool_script, '--url', self.tooltool_url_list[0],

"python /tools/tooltool.py" ignores the shebang "/tools/tooltool.py" doesn't.

@@ +1209,5 @@
>                  name='tooltool_bootstrap',
> +                command=['bash', '-c',
> +                         'if [ -e "%s" ]; then bash "%s"; fi' % \
> +                         (self.tooltool_bootstrap, self.tooltool_bootstrap)]
> +            ))

I added this check to allow empty manifests.

The patches can't be landed before we deploy tooltool.py to builders.
Attachment #622718 - Flags: review?(jhford) → review+
Attachment #622719 - Flags: review?(jhford) → review+
Attached patch tooltool (obsolete) — Splinter Review
Some enhancements and fixes. 
Still need to see how pretty is the output of Manifest.__eq__() and add download progress (optional) + some testing.
Attachment #622737 - Flags: feedback?(jhford)
Comment on attachment 622718 [details] [diff] [review]
configs

Probably I should define tooltool_manifest_src per branch and disable it by default in PLATFORM_VARS (except b2g).
Comment on attachment 622737 [details] [diff] [review]
tooltool

Please open a pull request on https://github.com/jhford/tooltool for this.  The copy checked in here is a snapshot of that repository's tooltool.  Also, there are tests that should be updated if behaviour is changed.
(In reply to John Ford [:jhford] from comment #7)
> Comment on attachment 622737 [details] [diff] [review]
> tooltool
> 
> Please open a pull request on https://github.com/jhford/tooltool for this. 
> The copy checked in here is a snapshot of that repository's tooltool.  Also,
> there are tests that should be updated if behaviour is changed.

Woooo! Much better! Will do.
Attachment #622737 - Attachment is obsolete: true
Attachment #622737 - Flags: feedback?(jhford)
Attached patch configsSplinter Review
Sorry for rush with the first patch, this one is more conservative. Let's start using this on try only.
Attachment #622718 - Attachment is obsolete: true
Attachment #622827 - Flags: review?(jhford)
Attached patch puppet-manifests (obsolete) — Splinter Review
Almost copy/paste from /build/puppet
Attachment #623121 - Flags: review?(jhford)
Attachment #622827 - Flags: review?(jhford) → review+
Comment on attachment 623121 [details] [diff] [review]
puppet-manifests

my assumption is that this is using a tooltool.py that is mostly identical to the HEAD of github.com/jhford/tooltool.  If that's correct, r+
Attachment #623121 - Flags: review?(jhford) → review+
Comment on attachment 622717 [details] [diff] [review]
empty manifests

And the last piece! The hardest one. :)
Attachment #622717 - Flags: review?(jhford)
One todo left: lion has python 2.7 installed, probably the patch for puppet-manifests will be resubmitted. :/
Attached patch puppet-manifestsSplinter Review
* Use "wheel" group, which is common for linux/mac
Attachment #623121 - Attachment is obsolete: true
Attachment #624129 - Flags: review?(bhearsum)
Attachment #624129 - Flags: review?(bhearsum) → review+
Comment on attachment 622717 [details] [diff] [review]
empty manifests

Need ted's rubber stamp here. Empty manifests passing staging tests.
Attachment #622717 - Flags: review?(jhford) → review?(ted.mielczarek)
Comment on attachment 622717 [details] [diff] [review]
empty manifests

I have no idea what a tooltool manifest is, but if this makes your life easier then go ahead.
Attachment #622717 - Flags: review?(ted.mielczarek) → review+
Whiteboard: [tooltool] → [Leave open after merge][tooltool]
Comment on attachment 622717 [details] [diff] [review]
empty manifests

It was backed out by khuey's "big hammer" (multiple backouts due to the tree's red status)
https://hg.mozilla.org/integration/mozilla-inbound/rev/dad0e473e803
Attachment #622717 - Flags: checked-in+ → checked-in-
(In reply to Rail Aliiev [:rail] from comment #21)
> Comment on attachment 622717 [details] [diff] [review]
> empty manifests
> 
> http://hg.mozilla.org/integration/mozilla-inbound/rev/ff731bcba745

https://hg.mozilla.org/mozilla-central/rev/ff731bcba745
Whiteboard: [Leave open after merge][tooltool] → [tooltool]
in production 2012-05-17 0750 PDT
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Comment on attachment 622827 [details] [diff] [review]
configs

>+BRANCHES['try']['platforms']['android-xul']['tooltool_manifest_src'] = 'mobile/android-xul/config/tooltool-manifests/android

It's a good thing that nobody cares about their Android results on try, much less their Android XUL results on try, because the manifest is in mobile/xul/ rather than mobile/android-xul/, so every single build since this landed has burned.
Attached patch try xul configSplinter Review
Attachment #625855 - Flags: review?(rail)
Comment on attachment 625855 [details] [diff] [review]
try xul config

Bah! My bad...
Attachment #625855 - Flags: review?(rail) → review+
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Caught a reconfig yesterday, and I see green XUL builds.
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.