port checksums work to l10n

RESOLVED FIXED in mozilla2.0b10

Status

()

Core
Build Config
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: bhearsum, Assigned: bhearsum)

Tracking

Trunk
mozilla2.0b10
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [l10n])

Attachments

(3 attachments, 2 obsolete attachments)

(Assignee)

Description

7 years ago
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.
(Assignee)

Updated

7 years ago
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
(Assignee)

Comment 3

7 years ago
(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?
(Assignee)

Comment 5

7 years ago
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?

Updated

7 years ago
Whiteboard: [l10n]
(Assignee)

Updated

7 years ago
Assignee: jhford → bhearsum
(Assignee)

Comment 9

7 years ago
(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.
(Assignee)

Comment 10

7 years ago
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.
(Assignee)

Updated

7 years ago
Attachment #486932 - Flags: review?(ted.mielczarek)
Attachment #486932 - Flags: review?(l10n)
(Assignee)

Comment 11

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

Comment 12

7 years ago
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-
(Assignee)

Comment 13

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

Comment 14

7 years ago
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?
(Assignee)

Comment 16

7 years ago
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)
(Assignee)

Comment 17

7 years ago
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)
(Assignee)

Comment 18

7 years ago
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)
(Assignee)

Updated

7 years ago
Attachment #502909 - Flags: review?(kairo)

Updated

7 years ago
Attachment #502909 - Flags: review?(aki) → review+

Comment 19

7 years ago
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+
(Assignee)

Comment 20

7 years ago
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)

Comment 21

7 years ago
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.
(Assignee)

Comment 22

7 years ago
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

7 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

7 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

7 years ago
CCing Callek for SeaMonkey RelEng, so he know this is happening.
(Assignee)

Comment 27

7 years ago
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)
(Assignee)

Comment 28

7 years ago
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)
(Assignee)

Comment 29

7 years ago
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
(Assignee)

Comment 30

7 years ago
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+
Blocks: 627271
No longer blocks: 478420
(Assignee)

Comment 32

7 years ago
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
(Assignee)

Updated

7 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Blocks: 630857
Blocks: 634238
Comment on attachment 502907 [details] [diff] [review]
[checked in] drop l10n upload target

http://hg.mozilla.org/mozilla-central/rev/a74b05ad7801
Flags: in-testsuite-
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: --- → mozilla2.0b10
Version: unspecified → Trunk
You need to log in before you can comment on or make changes to this bug.