Closed Bug 1405068 Opened 2 years ago Closed 2 years ago

Detection of tooltool provided xz.exe doesn't work on thunderbird's buildbots.

Categories

(Release Engineering :: Release Automation: Other, enhancement)

enhancement
Not set

Tracking

(firefox58 fixed)

RESOLVED FIXED
Tracking Status
firefox58 --- fixed

People

(Reporter: tomprince, Assigned: tomprince)

References

Details

Attachments

(3 files)

No description provided.
There are a couple of issues we ran into when building Thunderbird 56.0b4 (the first release with new format).

- tools/update-packaging/make_incremental_update.sh doesn't look for a tooltool installed xz at all. It is hard to work around this by passing in a path in $XZ because it is called directly by buildbot.
- tools/update-packaging/unwrap_full_update.pl does look for a tooltool installed xz but:
  - only looks in m-c root, not c-c root (which is one level higher)
  - Thunderbird's windows builders have a 1.x MozillaBuild installation which has a very old perl, which has Cwd::abs_path which doesn't work on files on windows.
Comment on attachment 8914465 [details]
Bug 1405068: Look for xz.exe at the root of a checkout when making incremental updates.

https://reviewboard.mozilla.org/r/185772/#review190714

Tom, I don't own these scripts and when I make changes to them I request review from someone in releng which is what you should do for these changes.
Attachment #8914465 - Flags: review?(robert.strong.bugs)
Comment on attachment 8914466 [details]
Bug 1405068: Don't use a shell for running subcommands in unwrap_full_update.pl.

https://reviewboard.mozilla.org/r/185774/#review190716
Attachment #8914466 - Flags: review?(robert.strong.bugs)
Comment on attachment 8914467 [details]
Bug 1405068: Don't use Cwd::abs_path in unwrap_full_update.pl.

https://reviewboard.mozilla.org/r/185776/#review190718
Attachment #8914467 - Flags: review?(robert.strong.bugs)
Attachment #8914465 - Flags: review?(bhearsum)
Attachment #8914466 - Flags: review?(bhearsum)
Attachment #8914467 - Flags: review?(bhearsum)
Comment on attachment 8914465 [details]
Bug 1405068: Look for xz.exe at the root of a checkout when making incremental updates.

https://reviewboard.mozilla.org/r/185772/#review191164

::: tools/update-packaging/common.sh:26
(Diff revision 1)
> +    # xz.exe in topsrcdir/xz/. Look in the places this would be in both a
> +    # mozilla-central and comm-central build.
> +    XZ="$(dirname "$(dirname "$(dirname "$0")")")/xz/xz.exe"
> +    $XZ --version > /dev/null 2>&1
> +    if [ $? -ne 0 ]; then
> +      XZ="$(dirname "$(dirname "$(dirname "$(dirname "$0")")")")/xz/xz.exe"

This seems....very hacky. Is there a reason we're not able to get xz in $PATH on these systems, or otherwise set $XZ accurately?
Comment on attachment 8914465 [details]
Bug 1405068: Look for xz.exe at the root of a checkout when making incremental updates.

https://reviewboard.mozilla.org/r/185772/#review191164

> This seems....very hacky. Is there a reason we're not able to get xz in $PATH on these systems, or otherwise set $XZ accurately?

I agree that it is very hacky. Both make_incremental_update.sh and unwrap_full_update.pl get called directly by buildbot. I'd rather avoid changes to the release steps in buildbot, as it may impact esr52 releases as well.
Comment on attachment 8914465 [details]
Bug 1405068: Look for xz.exe at the root of a checkout when making incremental updates.

https://reviewboard.mozilla.org/r/185772/#review192262

I'm not in a great position to know how costly fixing $PATH is, so this is more or less a rubberstamp. AFAICT this patch shouldn't risk any bustage to anything that already has XZ, because it's only adding some fallback behaviour if xz isn't found in the first place.
Attachment #8914465 - Flags: review?(bhearsum) → review+
Comment on attachment 8914466 [details]
Bug 1405068: Don't use a shell for running subcommands in unwrap_full_update.pl.

https://reviewboard.mozilla.org/r/185774/#review192296
Attachment #8914466 - Flags: review?(bhearsum) → review+
Comment on attachment 8914467 [details]
Bug 1405068: Don't use Cwd::abs_path in unwrap_full_update.pl.

https://reviewboard.mozilla.org/r/185776/#review192298
Attachment #8914467 - Flags: review?(bhearsum) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ebb0ce96ac09
Look for xz.exe at the root of a checkout when making incremental updates. r=bhearsum
https://hg.mozilla.org/integration/autoland/rev/930a1be99d1b
Don't use a shell for running subcommands in unwrap_full_update.pl. r=bhearsum
https://hg.mozilla.org/integration/autoland/rev/40a199104c9a
Don't use Cwd::abs_path in unwrap_full_update.pl. r=bhearsum
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.