1136750 - Partial mar is generated but not uploaded

Status: RESOLVED WONTFIX

(Release Engineering :: Applications: MozharnessCore, defect)

Description

[Massimo Gervasini [:massimo]]

Last run of l10n repacks failed with this error:

07:16:01     INFO - Setting buildbot property completeMarUrl to http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015/02/2015-02-25-03-02-26-ash-l10n/firefox-39.0a1.he.linux-i686.complete.mar
07:16:01     INFO - Setting buildbot property locale to he
...
07:16:01    FATAL -   File "scripts/scripts/desktop_l10n.py", line 1022, in _get_partialInfo
07:16:01    FATAL -     partial_url = self.package_urls[locale]["partialMarUrl"]
07:16:01    FATAL - KeyError: 'partialMarUrl'
07:16:01    FATAL - Running post_fatal callback...
07:16:01    FATAL - Exiting -1
program finished with exit code 255

The partial mar file is generated but not uploaded, so self.package_urls[locale] has not "partialMarUrl" key and the job fails.

Comment 1

[Massimo Gervasini [:massimo]]

Our make file is deleting the partial [1]

We should try to run "make upload", before generating the next locale installer.

[1] https://hg.mozilla.org/mozilla-central/file/6608e0605dfc/tools/update-packaging/Makefile.in#l93

Comment 2

[Massimo Gervasini [:massimo]]

Attachment: [mozharness] Bug 1136750 - make upload is part of repack_locale.patch

Changes in this patch:

* removed 'upload' action
* moved the make_upload(locale) call, just after the partial generation

Comment 3

[Pete Moore [:pmoore][:pete]]

Comment on attachment 8569835 [details] [diff] [review]
[mozharness] Bug 1136750 - make upload is part of repack_locale.patch

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

Thanks for going through this with me on vidyo. Looks good to me!

Comment 4

[Massimo Gervasini [:massimo]]

Comment on attachment 8569835 [details] [diff] [review]
[mozharness] Bug 1136750 - make upload is part of repack_locale.patch

Thanks Pete!

Comment 5

[Massimo Gervasini [:massimo]]

Attachment: [mozharness] Bug 1136750 - partial file should be created in dist/upgrade

The partial file is created in dist/previous, it should be created in dist/current

Comment 6

[Jordan Lund (:jlund)]

Comment on attachment 8571526 [details] [diff] [review]
[mozharness] Bug 1136750 - partial file should be created in dist/upgrade

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

sanity check, your patch name says it should be created in 'dist/upgrade' and the comment[1] for the patch says 'dist/current'.

for extra fun, since I am not too familiar, I dug a bit and to me it seems like this will put things in 'dist/update':
http://mxr.mozilla.org/build/source/mozharness/scripts/desktop_l10n.py#1060
http://mxr.mozilla.org/build/source/mozharness/scripts/desktop_l10n.py#1156
http://mxr.mozilla.org/build/search?string=update_mar_dir&find=configs%2Fsingle_locale&findi=&filter=%5E%5B%5E%5C0%5D*%24&hitlimit=&tree=build

please bear ignorant me here, which of the three do we want?


[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1136750#c5

Comment 7

[Massimo Gervasini [:massimo]]

Thanks Jordan,

(In reply to Jordan Lund (:jlund) from comment #6)
> Comment on attachment 8571526 [details] [diff] [review]
> [mozharness] Bug 1136750 - partial file should be created in dist/upgrade
> 
> Review of attachment 8571526 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> sanity check, your patch name says it should be created in 'dist/upgrade'
> and the comment[1] for the patch says 'dist/current'.

Yes, you're right it must be in dist/upgrade, the comment is wrong!

> for extra fun, since I am not too familiar, I dug a bit and to me it seems
> like this will put things in 'dist/update':
> http://mxr.mozilla.org/build/source/mozharness/scripts/desktop_l10n.py#1060
1) absolute path of the partial mar (depends on the locale, current and previous buildid)

> http://mxr.mozilla.org/build/source/mozharness/scripts/desktop_l10n.py#1156
2) absoulte path of the update directory

> http://mxr.mozilla.org/build/
> search?string=update_mar_dir&find=configs%2Fsingle_locale&findi=&filter=%5E%5
> B%5E%5C0%5D*%24&hitlimit=&tree=build
3) here's the "update" directory name
 
> please bear ignorant me here, which of the three do we want?
Yes, it's a little bit complex/confusing. Here we want 1, we are about to generate the partial mar. 
I have already planned a minor refactoring of this script, to make it more clear

Comment 8

[Jordan Lund (:jlund)]

(In reply to Massimo Gervasini [:mgerva] from comment #7)
> Thanks Jordan,
> 
> (In reply to Jordan Lund (:jlund) from comment #6)
> > Comment on attachment 8571526 [details] [diff] [review]
> > [mozharness] Bug 1136750 - partial file should be created in dist/upgrade
> > 
> > Review of attachment 8571526 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > sanity check, your patch name says it should be created in 'dist/upgrade'
> > and the comment[1] for the patch says 'dist/current'.
> 
> Yes, you're right it must be in dist/upgrade, the comment is wrong!

hmm, you said 'upgrade' again. I'm assuming you mean 'update' because you mention that below

> 3) here's the "update" directory name

Comment 9

[Massimo Gervasini [:massimo]]

Comment on attachment 8571526 [details] [diff] [review]
[mozharness] Bug 1136750 - partial file should be created in dist/upgrade

https://hg.mozilla.org/build/mozharness/rev/2e86a723f19d

Comment 10

[Chris Cooper [:coop] (he/him)]

In production: https://hg.mozilla.org/build/mozharness/rev/2e86a723f19d

Comment 11

[Massimo Gervasini [:massimo]]

Attachment: [mozharness] Bug 1136750 - Partial mar is generated but not uploaded - wrong filename.patch

There is an error in the branch of code that decides is we need to generate partial updates or not.

Changes in this patch:
* To decide if a partial update needs to be generated, we check if the partial file already exists but at the moment, we are checking the existence of _previous_mar_filename() (wrong) instead of _query_partial_mar_filename() (correct). 
* In addition, to determine the name of the partial update file, we must be able to get the previous buildid, (unpacking the previous mar file). The download/unpack of the previous mar file is now part of _partial_filename().

I have tested this patch in staging and it just works fine. I'd like to land this in our mozharness even if the use case where the unpack_mar returns an error is not properly managed but I'd like to fix this in a separate bug, if you agree, of course.

Comment 12

[Jordan Lund (:jlund)]

Comment on attachment 8574712 [details] [diff] [review]
[mozharness] Bug 1136750 - Partial mar is generated but not uploaded - wrong filename.patch

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

ship it! :)

Comment 13

[Massimo Gervasini [:massimo]]

Comment on attachment 8574712 [details] [diff] [review]
[mozharness] Bug 1136750 - Partial mar is generated but not uploaded - wrong filename.patch

Thanks Jordan!

https://hg.mozilla.org/build/mozharness/rev/40a974d1324f

Comment 14

[Chris Cooper [:coop] (he/him)]

mozharness production tag moved to: https://hg.mozilla.org/build/mozharness/rev/40a974d1324f

Comment 15

[Massimo Gervasini [:massimo]]

Attachment: [mozharness] Bug 1136750 - Partial mar is generated but not uploaded.patch

Hi Chris,

I'd like to test a complete cycle of l10n builds using only the in tree mechanism. If this works, we can just delete tons of code

Comment 16

[Massimo Gervasini [:massimo]]

Comment on attachment 8577013 [details] [diff] [review]
[mozharness] Bug 1136750 - Partial mar is generated but not uploaded.patch

Thanks!

landed:

https://hg.mozilla.org/build/mozharness/rev/5f1fbf9f44a9
https://hg.mozilla.org/build/mozharness/rev/5e18ca331ad5

Comment 17

[Massimo Gervasini [:massimo]]

\o/ it looks it's working:

locales:{"en-GB": "Success", "eo": "Success", "ast": "Success", "es-AR": "Success", "de": "Success", "fr": "Success", "es-MX": "Success", "es-CL": "Success", "fa": "Success", "ar": "Success", "fy-NL": "Success", "cs": "Success", "gl": "Success", "es-ES": "Success"}


full log here: http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/ash-l10n/Linux-ash-l10n-1_3-unknown-bm72-build1-build2.txt.gz

Comment 18

[[e-mail] (:bhearsum)]

In production: https://hg.mozilla.org/build/mozharness/rev/5f1fbf9f44a9

Comment 19

[Massimo Gervasini [:massimo]]

and mac and linux64 repacks are working too,

on Windows 32 make installers-<locale> fails with this message:

INFO -  03:53:19 (57.21 MB/s) - `c:/builds/moz2_slave/WINNT-5.2-ash-l10n-1_3-0000000/build/ash/obj-l10n/dist/update/previous.mar' saved [49599499/49599499]
INFO -  mozmake[4]: Entering directory 'c:/builds/moz2_slave/WINNT-5.2-ash-l10n-1_3-0000000/build/ash/obj-l10n/tools/update-packaging'
ERROR -  c:/builds/moz2_slave/WINNT-5.2-ash-l10n-1_3-0000000/build/ash/config/rules.mk:1235: *** target pattern contains no '%'.  Stop.
INFO -  mozmake[4]: Leaving directory 'c:/builds/moz2_slave/WINNT-5.2-ash-l10n-1_3-0000000/build/ash/obj-l10n/tools/update-packaging'
INFO -  Makefile:89: recipe for target 'automation-partial-patch' failed

full log here: http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/ash-l10n/WINNT-5.2-ash-l10n-3_3-unknown-bm84-build1-build5.txt.gz

Comment 20

[Massimo Gervasini [:massimo]]

Switching to the intree mechanism (comment #15), solved a lot issues and now, thanks to mshal we have a patch to fix the windows repacks problems too.

I'll file separate bugs for the windows fix and the code clean up (we can delete tons of code from the  desktop_l10n.py script)