port checksums work to l10n

RESOLVED FIXED in mozilla2.0b10

Status

RESOLVED FIXED
8 years ago
11 months ago

People

(Reporter: bhearsum, Assigned: bhearsum)

Tracking

Trunk
mozilla2.0b10
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [l10n])

Attachments

(3 attachments, 2 obsolete attachments)

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.
Blocks: 607399
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?
Comment on attachment 486230 [details] [diff] [review]
idea

forgot to cut a new patch before uploading.  Will do sometime tomorrow
Attachment #486230 - Attachment is obsolete: true
(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
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?
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.
Created attachment 486932 [details] [diff] [review]
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
Assignee: nobody → jhford
Status: NEW → ASSIGNED
Created attachment 486933 [details]
checksum file

sample output.  Notice that the layout of the upload directory matches the checksum file layout
(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?
Whiteboard: [l10n]
Assignee: jhford → 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.
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.
Attachment #486932 - Flags: review?(ted.mielczarek)
Attachment #486932 - Flags: review?(l10n)
jhford's patch worked on all platforms in my tests, fwiw
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.
Attachment #486932 - Flags: review?(l10n) → review-
You're talking about the first hunk of additions, right?
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.
Any response to Axel's comment?
Comment on attachment 486932 [details] [diff] [review]
upload checksums for l10n artifacts

Yeah, that's the direction I want to go.
Attachment #486932 - Attachment is obsolete: true
Attachment #486932 - Flags: review?(ted.mielczarek)
Created attachment 502907 [details] [diff] [review]
[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.
Attachment #502907 - Flags: review?(ted.mielczarek)
Attachment #502907 - Flags: review?(l10n)
Created attachment 502909 [details] [diff] [review]
[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?
Attachment #502909 - Flags: review?(aki)
Attachment #502909 - Flags: review?(kairo)

Updated

8 years ago
Attachment #502909 - Flags: review?(aki) → review+
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?
Attachment #502907 - Flags: review?(l10n) → feedback+
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?
Attachment #502907 - Flags: feedback?(kairo)
Attachment #502907 - Flags: feedback?(gozer)
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.
Yeah, the factory patch affects everyone, but I assume that if it works for one comm-central consumer, it works for all.
Attachment #502907 - Flags: review?(ted.mielczarek) → review+
(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

8 years ago
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.
Attachment #502907 - Flags: feedback?(kairo) → feedback+

Comment 25

8 years ago
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.
Attachment #502909 - Flags: review?(kairo) → review+

Comment 26

8 years ago
CCing Callek for SeaMonkey RelEng, so he know this is happening.
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.
Attachment #502909 - Flags: review?(philipp)
Comment on attachment 502907 [details] [diff] [review]
[checked in] drop l10n upload target

Removing gozer since this patch is Firefox-only.
Attachment #502907 - Flags: feedback?(gozer)
Comment on attachment 502909 [details] [diff] [review]
[checked in] use plain upload target in l10n factory

changeset:   1433:d8affe025056
Attachment #502909 - Attachment description: use plain upload target in l10n factory → [checked in] use plain upload target in l10n factory
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 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!
Attachment #502909 - Flags: review?(philipp) → review+
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.
Attachment #502907 - Attachment description: drop l10n upload target → [checked in] drop l10n upload target
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Blocks: 630857
Flags: in-testsuite-
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: --- → mozilla2.0b10
Version: unspecified → Trunk

Updated

11 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.