607396 - port checksums work to l10n

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[[e-mail] (:bhearsum)]

bug 578393 added a checksums file that dumped sha512 hashes for en-US files into a single file, and uploaded it. We should do the same for l10n to make update snippet generation easier and possibly other things.

Comment 1

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: idea

here is an idea.  Tested with a simple non-mar repack without pretty names.

What code paths would need to be tested?

Comment 2

[John Ford [:jhford] CET/CEST Berlin Time]

Comment on attachment 486230 [details] [diff] [review]
idea

forgot to cut a new patch before uploading.  Will do sometime tomorrow

Comment 3

[[e-mail] (:bhearsum)]

(In reply to comment #1)
> Created attachment 486230 [details] [diff] [review]
> idea
> 
> here is an idea.  Tested with a simple non-mar repack without pretty names.
> 
> What code paths would need to be tested?

At the very least, uploading in the nightly and release (MOZ_PKG_PRETTYNAMES) scenarios

Comment 4

[John Ford [:jhford] CET/CEST Berlin Time]

is it possible to generate the complete and partial mars without doing a build?  Can i use a build on ftp to generate these?  

For prettynames, do i have to use a build that has pretty filenames in the en-US-binary?

Comment 5

[[e-mail] (:bhearsum)]

You can't generate the MARs without doing a build, but can you download the builds/MARs to the right places in dist/ and just test 'make upload'. Doesn't matter what the builds were generated with.

Comment 6

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: upload checksums for l10n artifacts

Tested using the nightly codepath.  I tested using
 MOZ_PKG_USEPRETTYNAMES=1 MOZ_PKG_VERSION=4.0b8pre make l10n-upload-en-GB
and that worked.  Not sure if that exercises the code well enough though.

I haven't tested on windows or osx yet. 

[jhford@mobile-image01 locales]$ make l10n-upload-en-GBecho firefox-4.0b8pre.en-GB.linux-x86_64
firefox-4.0b8pre.en-GB.linux-x86_64
/usr/bin/python2.6 ../../build/checksums.py \
		-o "../../dist//firefox-4.0b8pre.en-GB.linux-x86_64.checksums" \
		-d 'sha512' \
		-s ../../dist \
		"../../dist/firefox-4.0b8pre.en-GB.linux-x86_64.tar.bz2" ../../dist/install/firefox-4.0b8pre.en-GB.langpack.xpi ../../dist/update/firefox-4.0b8pre.en-GB.linux-x86_64.complete.mar "../../dist/update/firefox-4.0b8pre.en-GB.linux-x86_64.partial.20101029031004-20101029030658.mar" 
/usr/bin/python2.6 ../../build/upload.py \
		--base-path ../../dist \
		"../../dist/firefox-4.0b8pre.en-GB.linux-x86_64.tar.bz2" ../../dist/install/firefox-4.0b8pre.en-GB.langpack.xpi ../../dist/update/firefox-4.0b8pre.en-GB.linux-x86_64.complete.mar "../../dist/update/firefox-4.0b8pre.en-GB.linux-x86_64.partial.20101029031004-20101029030658.mar" \
		"../../dist//firefox-4.0b8pre.en-GB.linux-x86_64.checksums"
Uploading /home/jhford/mozilla/make-hashses/testing-l10n/mozilla-central/dist/firefox-4.0b8pre.en-GB.linux-x86_64.tar.bz2
firefox-4.0b8pre.en-GB.linux-x86_64.tar.bz2   100%   14MB  14.2MB/s   00:00    
Uploading /home/jhford/mozilla/make-hashses/testing-l10n/mozilla-central/dist/install/firefox-4.0b8pre.en-GB.langpack.xpi
firefox-4.0b8pre.en-GB.langpack.xpi           100%  210KB 210.0KB/s   00:00    
Uploading /home/jhford/mozilla/make-hashses/testing-l10n/mozilla-central/dist/update/firefox-4.0b8pre.en-GB.linux-x86_64.complete.mar
firefox-4.0b8pre.en-GB.linux-x86_64.complete. 100%   14MB  14.2MB/s   00:00    
Uploading /home/jhford/mozilla/make-hashses/testing-l10n/mozilla-central/dist/update/firefox-4.0b8pre.en-GB.linux-x86_64.partial.20101029031004-20101029030658.mar
firefox-4.0b8pre.en-GB.linux-x86_64.partial.2 100%   12MB  11.7MB/s   00:00    
Uploading /home/jhford/mozilla/make-hashses/testing-l10n/mozilla-central/dist/firefox-4.0b8pre.en-GB.linux-x86_64.checksums
firefox-4.0b8pre.en-GB.linux-x86_64.checksums 100%  807     0.8KB/s   00:00    
Upload complete
[jhford@mobile-image01 locales]$ ls ~/mozilla/fakestage/
firefox-4.0b8pre.en-GB.linux-x86_64.checksums  install
firefox-4.0b8pre.en-GB.linux-x86_64.tar.bz2    update

Comment 7

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: checksum file

sample output.  Notice that the layout of the upload directory matches the checksum file layout

Comment 8

[John Ford [:jhford] CET/CEST Berlin Time]

(In reply to comment #7)
> Created attachment 486933 [details]
> checksum file
> 
> sample output.  Notice that the layout of the upload directory matches the
> checksum file layout

but oddly, doesn't on stage.  post_upload.py?

Comment 9

[[e-mail] (:bhearsum)]

(In reply to comment #8)
> (In reply to comment #7)
> > Created attachment 486933 [details] [details]
> > checksum file
> > 
> > sample output.  Notice that the layout of the upload directory matches the
> > checksum file layout
> 
> but oddly, doesn't on stage.  post_upload.py?

Yeah, probably. It might be best to build some checksum smarts into post_upload.py -- maybe have it adjust the paths to be correct.

Comment 10

[[e-mail] (:bhearsum)]

If we started putting langpack and MARs in the root of dist/ rather than pointless subdirectories we'd be 95% of the way there, I think. The paths won't be an issue in releases because we use UPLOAD_BASE_PATH when we upload.

Comment 11

[[e-mail] (:bhearsum)]

jhford's patch worked on all platforms in my tests, fwiw

Comment 12

[Axel Hecht [:Pike]]

Comment on attachment 486932 [details] [diff] [review]
upload checksums for l10n artifacts

Given that browser/locales/Makefile.in already includes packager.mk via l10n.mk, we should use the code there as much as possible.

Hopefully, just making UPLOAD_FILES be the right list should work, or be made to work, IMHO. And then just trigger it from l10n-upload-% like it is from upload in packager.mk.

Comment 13

[[e-mail] (:bhearsum)]

You're talking about the first hunk of additions, right?

Comment 14

[Axel Hecht [:Pike]]

I'm talking about leaving UPLOAD_FILES as is, and to make

l10n-upload-%: AB_CD=$*
l10n-upload-%: checksum
	$(PYTHON) $(MOZILLA_DIR)/build/upload.py --base-path $(DIST) \
		$(UPLOAD_FILES) \
		$(CHECKSUM_FILE)

and that'd hopefully be it. maybe l10n-upload should really just be a

	@$(MAKE) upload AB_CD=$(AB_CD) UPLOAD_FILES=$(UPLOAD_FILES)

? Not sure if that works with QUOTED_WILDCARD.

Comment 15

[(not currently active) Ted Mielczarek]

Any response to Axel's comment?

Comment 16

[[e-mail] (:bhearsum)]

Comment on attachment 486932 [details] [diff] [review]
upload checksums for l10n artifacts

Yeah, that's the direction I want to go.

Comment 17

[[e-mail] (:bhearsum)]

Attachment: [checked in] drop l10n upload target

Actually, I want to go a bit further and just drop it altogether. We (firefox) don't actually use it for release builds anymore, and switching nightlies over is trivial. Re-using the packager.mk version means we don't have to redefine anything, which is nice. If l10n ever needs things that aren't appropriate for packager.mk, I believe we can define UPLOAD_EXTRA_FILES prior to including l10n.mk (or whichever makefile ends up pulling in packager.mk).

We'd have to make sure no one else is using l10n-upload-% before this could land, too.

Comment 18

[[e-mail] (:bhearsum)]

Attachment: [checked in] use plain upload target in l10n factory

Looks like this is the only remaining referencing to l10n-upload-%. Kairo, I'm not going to be able to test this against comm-central, any chance you can before this lands?

Comment 19

[Axel Hecht [:Pike]]

Comment on attachment 502907 [details] [diff] [review]
[checked in] drop l10n upload target

Nothing in my realm uses l10n-upload, nor anything outside of core infrastucture that I know of.

Maybe check in with gozer for tb, too?

Comment 20

[[e-mail] (:bhearsum)]

Comment on attachment 502907 [details] [diff] [review]
[checked in] drop l10n upload target

I'd be surprised if anyone else is using the l10n-upload target from browser/locales, but regardless, feedback?

Comment 21

[Axel Hecht [:Pike]]

True, I guess this is more about the factory patch, which, I assume, doesn't only roll down to SM but also to TB?

The browser patch, yeah, that should be just ted and I.

Comment 22

[[e-mail] (:bhearsum)]

Yeah, the factory patch affects everyone, but I assume that if it works for one comm-central consumer, it works for all.

Comment 23

[Philippe M. Chiasson (:gozer)]

(In reply to comment #19)
> Comment on attachment 502907 [details] [diff] [review]
> drop l10n upload target
> 
> Nothing in my realm uses l10n-upload, nor anything outside of core
> infrastucture that I know of.
> 
> Maybe check in with gozer for tb, too?

We don't use that target anymore, but I believe the calendar folks might be using it.

Comment 24

[Robert Kaiser]

Comment on attachment 502907 [details] [diff] [review]
[checked in] drop l10n upload target

We probably should do the same in comm-central for our apps, but as this is strictly removal, I guess nothing should break us.

Comment 25

[Robert Kaiser]

Comment on attachment 502909 [details] [diff] [review]
[checked in] use plain upload target in l10n factory

I haven't tested this (no staging env available), but I'd guess that it should work fine for us.

Comment 26

[Robert Kaiser]

CCing Callek for SeaMonkey RelEng, so he know this is happening.

Comment 27

[[e-mail] (:bhearsum)]

Comment on attachment 502909 [details] [diff] [review]
[checked in] use plain upload target in l10n factory

Philipp, gozer tells me that you're the one to ask about whether this code is used by/will break Calendar.

Comment 28

[[e-mail] (:bhearsum)]

Comment on attachment 502907 [details] [diff] [review]
[checked in] drop l10n upload target

Removing gozer since this patch is Firefox-only.

Comment 29

[[e-mail] (:bhearsum)]

Comment on attachment 502909 [details] [diff] [review]
[checked in] use plain upload target in l10n factory

changeset:   1433:d8affe025056

Comment 30

[[e-mail] (:bhearsum)]

Once the buildbotcustom patch gets merged to production I'll be landing the mozilla-central one and closing this out. Not going to bother with it on 1.9.1/1.9.2.

Comment 31

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 502909 [details] [diff] [review]
[checked in] use plain upload target in l10n factory

Since Lightning needs DIY work anyway to create l10n builds and we don't have that in place yet, this will not break anything. Thanks for asking though!

Comment 32

[[e-mail] (:bhearsum)]

Comment on attachment 502907 [details] [diff] [review]
[checked in] drop l10n upload target

The buildbot patch made it to production today, so I landed the mozilla-central patch.

Comment 33

[Serge Gautherie (:sgautherie)]

Comment on attachment 502907 [details] [diff] [review]
[checked in] drop l10n upload target

http://hg.mozilla.org/mozilla-central/rev/a74b05ad7801