Closed Bug 1117637 Opened 7 years ago Closed 7 years ago

plugins and extensions moved to wrong directory by buildbotcustom

Categories

(Release Engineering :: General, defect)

x86_64
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: hiro, Assigned: Fallen)

References

Details

Attachments

(2 files, 3 obsolete files)

Mozmill tests on comm-central for MacOSX has still failed.

From a failure log:
sh -c 'if [ ! -d ./Daily.app/Contents/MacOS/plugins ]; then mkdir ./Daily.app/Contents/MacOS/plugins; fi && if [ ! -d ./Daily.app/Contents/MacOS/extensions ]; then mkdir ./Daily.app/Contents/MacOS/extensions; fi && cp -R bin/plugins/* ./Daily.app/Contents/MacOS/plugins/ && if [ -d extensions ]; then cp -R extensions/* ./Daily.app/Contents/MacOS/extensions/; fi'

The plugins path should be Contents/Resources.
MozMill doesn't currently use mozharness (which the other bug changed), so this needs this section of code changing:

http://hg.mozilla.org/build/buildbotcustom/annotate/c5fc38e6fc3e/process/factory.py#l5009
Attached patch Fix - v1 (obsolete) β€” β€” Splinter Review
This should take care. Not sure to pick as a reviewer, so I picked somebody random that has reviews in this component. Please forward if someone else is a better pick.
Assignee: nobody → philipp
Status: NEW → ASSIGNED
Attachment #8550755 - Flags: review?(pmoore)
Attached patch buildbotcustom changes - v2 (obsolete) β€” β€” Splinter Review
I realized we might need to differentiate per gecko version like in the patch for mozharness xpcshell. This patch does that using platform config. I will attach another patch in a moment with the buildbot-configs changes.
Attachment #8550755 - Attachment is obsolete: true
Attachment #8550755 - Flags: review?(pmoore)
Attachment #8550784 - Flags: review?(pmoore)
Attached patch buildbot-configs changes - v1 (obsolete) β€” β€” Splinter Review
Attachment #8550785 - Flags: review?(pmoore)
Attachment #8550784 - Attachment description: Fix - v2 → buildbotcustom changes - v2
Comment on attachment 8550784 [details] [diff] [review]
buildbotcustom changes - v2

(In reply to Philipp Kewisch [:Fallen] from comment #2)
> Created attachment 8550755 [details] [diff] [review]
> Fix - v1
> 
> This should take care. Not sure to pick as a reviewer, so I picked somebody
> random that has reviews in this component. Please forward if someone else is
> a better pick.

Thanks Fallen. Let's see if catlee can help, otherwise maybe armenzg, bhearsum, kmoir, jlund might be good reviewers for buildbot internals. I'm asking catlee first as a) he landed me three reviews on Friday :p, and b) he also has an awesome standing on https://github.com/mozilla/build-buildbotcustom/graphs/contributors. I'm not so hot in this area.

Catlee if you're busy etc maybe you can pass it onto one of the others?

Thanks!
Pete
Attachment #8550784 - Flags: review?(pmoore) → review?(catlee)
Attachment #8550785 - Flags: review?(pmoore) → review?(catlee)
Comment on attachment 8550784 [details] [diff] [review]
buildbotcustom changes - v2

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

I think this looks ok.

::: process/factory.py
@@ +4753,5 @@
> +        # OSX 10.9.5+ requires putting extensions and plugins into
> +        # Contents/Resources, this property sets up that directory.
> +        def get_xredir(build):
> +            if self.platform.startswith("macosx"):
> +                exedir = os.path.dirname(build.getProperty('exepath'))

can you use get_exedir(build) here too?
Attachment #8550784 - Flags: review?(catlee) → review+
Comment on attachment 8550785 [details] [diff] [review]
buildbot-configs changes - v1

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

::: mozilla-tests/config.py
@@ +2239,5 @@
> +    if platform != 'macosx64':
> +        continue
> +    for name, branch in items_before(BRANCHES, 'gecko_version', 34):
> +      if 'macosx64' in BRANCHES[name]['platforms']:
> +        BRANCHES[name]['platforms']['macosx64']['mac_res_subdir'] = 'MacOS'

I don't think your outer loop is doing anything. Can this be better written as:

for name, branch in items_before(BRANCHES, 'gecko_version', 34):
  if 'macosx64' in BRANCHES[name]['platforms']:
    BRANCHES[name]['platforms']['macosx64']['mac_res_subdir'] = 'MacOS'
Attachment #8550785 - Flags: review?(catlee) → review-
You are right, it should be the same thing. Here is an updated patch.
Attachment #8550785 - Attachment is obsolete: true
Attachment #8550784 - Attachment is obsolete: true
Attachment #8551376 - Flags: review?(catlee)
Attachment #8551376 - Flags: review?(catlee) → review+
Merged to production and is live. Thanks all!
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Component: Tools → General
You need to log in before you can comment on or make changes to this bug.