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

Status: RESOLVED FIXED

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

Comment 1

[Tom Prince [:tomprince]]

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 2

[Tom Prince [:tomprince]]

Attachment: Bug 1405068: Look for xz.exe at the root of a checkout when making incremental updates.

Review commit: https://reviewboard.mozilla.org/r/185772/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/185772/

Comment 3

[Tom Prince [:tomprince]]

Attachment: Bug 1405068: Don't use a shell for running subcommands in unwrap_full_update.pl.

Review commit: https://reviewboard.mozilla.org/r/185774/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/185774/

Comment 4

[Tom Prince [:tomprince]]

Attachment: Bug 1405068: Don't use Cwd::abs_path in unwrap_full_update.pl.

It is broken on the version of perl on the legacy buildbots, which is the only
place that this branch of the code runs.

Review commit: https://reviewboard.mozilla.org/r/185776/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/185776/

Comment 5

[Robert Strong (they/them - no direct email)]

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.

Comment 6

[Robert Strong (they/them - no direct email)]

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

Comment 7

[Robert Strong (they/them - no direct email)]

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

Comment 8

[[e-mail] (: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 9

[Tom Prince [:tomprince]]

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 10

[[e-mail] (: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/#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.

Comment 11

[[e-mail] (:bhearsum)]

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

Comment 12

[[e-mail] (:bhearsum)]

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

Comment 13

[Pulsebot]

Pushed by [e-mail]:
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

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/ebb0ce96ac09
https://hg.mozilla.org/mozilla-central/rev/930a1be99d1b
https://hg.mozilla.org/mozilla-central/rev/40a199104c9a