Enabling MOZ_AUTOMATION_L10N_CHECK breaks MinGW Build

RESOLVED FIXED in Firefox 59

Status

()

Core
Localization
RESOLVED FIXED
8 months ago
4 months ago

People

(Reporter: tjr, Assigned: tjr)

Tracking

(Blocks: 1 bug)

unspecified
mozilla59
Points:
---

Firefox Tracking Flags

(firefox59 fixed)

Details

(Whiteboard: [tor])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

8 months ago
> [task 2017-08-25T14:52:55.412154Z] 14:52:55     INFO -  gmake[7]: *** No rule to make target '/home/worker/workspace/build/src/obj-firefox/dist/install/sea/target.installer.exe', needed by 'repackage-win32-installer'.  Stop.
> [task 2017-08-25T14:52:55.412320Z] 14:52:55     INFO -  Makefile:161: recipe for target 'repackage-win32-installer-x-test' failed
> [task 2017-08-25T14:52:55.412482Z] 14:52:55     INFO -  gmake[6]: *** [repackage-win32-installer-x-test] Error 2
> [task 2017-08-25T14:52:55.412637Z] 14:52:55     INFO -  Makefile:215: recipe for target 'l10n-check' failed
> [task 2017-08-25T14:52:55.412796Z] 14:52:55     INFO -  gmake[5]: *** [l10n-check] Error 2
> [task 2017-08-25T14:52:55.412992Z] 14:52:55     INFO -  /home/worker/workspace/build/src/browser/build.mk:39: recipe for target 'l10n-check' failed
Comment hidden (mozreview-request)
(Assignee)

Updated

6 months ago
Assignee: nobody → tom

Comment 3

5 months ago
mozreview-review
Comment on attachment 8923048 [details]
Bug 1393817 Fix L10N check for MinGW build

https://reviewboard.mozilla.org/r/194266/#review202558
Attachment #8923048 - Flags: review?(ted) → review+
(Assignee)

Updated

5 months ago
Keywords: checkin-needed

Comment 4

5 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/25f21f22a34b
Use buildconfig to get the path for 7z rather than hardcoding it in exe_7z_archive.py r=ted
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/25f21f22a34b
Status: NEW → RESOLVED
Last Resolved: 5 months ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58

Comment 6

5 months ago
Backed out 1 changesets (bug 1393817) for failing to repackage the nightly build on Windows a=backout

https://hg.mozilla.org/mozilla-central/rev/2203d23429f61f879753234b275ff141c50c11f0

https://treeherder.mozilla.org/logviewer.html#?job_id=143307017&repo=mozilla-central&lineNumber=450
Status: RESOLVED → REOPENED
status-firefox58: fixed → ---
Flags: needinfo?(tom)
Resolution: FIXED → ---
Target Milestone: mozilla58 → ---

Comment 7

5 months ago
:tjr, to test this on try you can do `./mach try fuzzy --full` and select for example `repackage-win32-nightly`
Comment hidden (mozreview-request)
(Assignee)

Comment 10

5 months ago
Okay completely new version for review.
Flags: needinfo?(ted)
Related: bug 1415965.
Flags: needinfo?(ted)

Comment 12

5 months ago
mozreview-review
Comment on attachment 8923048 [details]
Bug 1393817 Fix L10N check for MinGW build

https://reviewboard.mozilla.org/r/194266/#review210742

::: python/mozbuild/mozbuild/action/exe_7z_archive.py:24
(Diff revision 2)
>      try:
>          if pkg_dir:
>              shutil.move(pkg_dir, 'core')
>          subprocess.check_call(['upx', '--best', '-o', mozpath.join(tmpdir, '7zSD.sfx'), sfx_package])
>  
> -        subprocess.check_call(['7z', 'a', '-r', '-t7z', mozpath.join(tmpdir, 'app.7z'), '-mx', '-m0=BCJ2', '-m1=LZMA:d25', '-m2=LZMA:d19', '-m3=LZMA:d19', '-mb0:1', '-mb0s1:2', '-mb0s2:3'])
> +        subprocess.check_call([sevenz_command, 'a', '-r', '-t7z', mozpath.join(tmpdir, 'app.7z'), '-mx', '-m0=BCJ2', '-m1=LZMA:d25', '-m2=LZMA:d19', '-m3=LZMA:d19', '-mb0:1', '-mb0s1:2', '-mb0s2:3'])

If we're detecting 7zip in configure then you should be able to `import buildconfig` and just use `buildconfig.substs['7Z']` to get the binary name instead of passing it down from make.
Attachment #8923048 - Flags: review+ → review-
(Assignee)

Comment 13

5 months ago
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #12)
> Comment on attachment 8923048 [details]
> Bug 1393817 Fix L10N check for MinGW build
> 
> https://reviewboard.mozilla.org/r/194266/#review210742
> 
> ::: python/mozbuild/mozbuild/action/exe_7z_archive.py:24
> (Diff revision 2)
> >      try:
> >          if pkg_dir:
> >              shutil.move(pkg_dir, 'core')
> >          subprocess.check_call(['upx', '--best', '-o', mozpath.join(tmpdir, '7zSD.sfx'), sfx_package])
> >  
> > -        subprocess.check_call(['7z', 'a', '-r', '-t7z', mozpath.join(tmpdir, 'app.7z'), '-mx', '-m0=BCJ2', '-m1=LZMA:d25', '-m2=LZMA:d19', '-m3=LZMA:d19', '-mb0:1', '-mb0s1:2', '-mb0s2:3'])
> > +        subprocess.check_call([sevenz_command, 'a', '-r', '-t7z', mozpath.join(tmpdir, 'app.7z'), '-mx', '-m0=BCJ2', '-m1=LZMA:d25', '-m2=LZMA:d19', '-m3=LZMA:d19', '-mb0:1', '-mb0s1:2', '-mb0s2:3'])
> 
> If we're detecting 7zip in configure then you should be able to `import
> buildconfig` and just use `buildconfig.substs['7Z']` to get the binary name
> instead of passing it down from make.

That was the first version :)  But it broke the repackage-win32-nightly build because configure isn't run and buildconfig.substs isn't populated. After talking to you about it, I attempted to get configure to run, but it didn't work. (I forget the exact error, as it was a few weeks ago, but I might be able to dig it up.)
(Assignee)

Comment 14

4 months ago
Forgot ni
Flags: needinfo?(ted)
OK, that sucks, but I think we can still do better by using that value when it's available, at least, and falling back to '7z' like the current code does when it's not. Something like:

```
import buildconfig
from mozbuild.base import BuildEnvironmentNotFoundException

try:
  sevenz = buildconfig.config.substs['7Z']
except BuildEnvironmentNotFoundException:
  # configure hasn't been run, just use the default
  sevenz = '7z'
```

You have to use `buildconfig.config.substs` instead of just `buildconfig.substs` for weird reasons related to recent changes to how things in the build system work, but that should reliably give you the value of `7Z` from configure when configure has been run.

You can test this by running `mach python` without a mozconfig available, or with a MOZ_OBJDIR set to some other place so that you don't have a configured objdir. That should be pretty close to the environment those repackage builds that don't run configure are using.
Flags: needinfo?(ted)
Comment hidden (mozreview-request)

Comment 18

4 months ago
mozreview-review
Comment on attachment 8923048 [details]
Bug 1393817 Fix L10N check for MinGW build

https://reviewboard.mozilla.org/r/194266/#review212026

Thanks, that feels much better!
Attachment #8923048 - Flags: review?(ted) → review+
(Assignee)

Comment 19

4 months ago
Thank you!
Keywords: checkin-needed

Comment 20

4 months ago
Pushed by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/988a2837b8dc
Fix L10N check for MinGW build r=ted
Keywords: checkin-needed

Comment 21

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/988a2837b8dc
Status: REOPENED → RESOLVED
Last Resolved: 5 months ago4 months ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.