538699 - maemo repacks should use dpkg-deb

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[Aki Sasaki (not active)]

When bug 538174 landed, the deb filename is no longer fennec_VERSION_armel.deb.

Instead, it's fennec_NIGHTLYVERSION_armel.deb, where NIGHTLYVERSION strips any 'pre' and adds a ~DATETIME.  Because of this, we can't guess the filename after the fact; we need to figure it out.

Brad suggested a softlink (with a known name), an .htaccess file (I'm most iffy about this solution), or a text file with the latest deb name.

Softlink: we'd have to verify we can upload a softlink.  Then, make wget-deb would grab the softlink deb name; after extracting the contents we'd have to figure out what the real deb name should be (should be in the control files).

.htaccess: similar, but the wget would hopefully tell us what url we were redirected to, and what filename was downloaded.  However, you can only have one .htaccess file per directory, as opposed to softlinks, which could cause problems later.  I'd rather not go down this route.

A text file with a known name could also work.

I think all three of these will require makefile changes to work.

Comment 1

[Aki Sasaki (not active)]

(In reply to comment #0)
> When bug 538174 landed, the deb filename is no longer fennec_VERSION_armel.deb.

That should read "the *nightly* deb filename is ..."
The release deb filename can be fennec_VERSION_armel.deb.
However, it may, at some point, be fennec_VERSION+STRING_armel.deb, which will cause the same issues as the nightly.

Comment 2

[Aki Sasaki (not active)]

We could also potentially solve this with a sendchange and/or ssh poller and another buildbot property.

Comment 3

[Aki Sasaki (not active)]

Attachment: read the deb name from fennec_deb_name.txt

Tested; works.
Willing to rename the text file or alter conditionals if needed.

Dependent on creating+uploading a fennec_deb_name.txt in EN_US_BINARY_URL.

Comment 4

[Axel Hecht [:Pike]]

Comment on attachment 421179 [details] [diff] [review]
read the deb name from fennec_deb_name.txt

I think the define to test should probably EN_US_BINARY_URL. We likely need to change the build factories to keep that defined on both the wget and the unpack steps.

Is the non-fuzzled DEB_PKG_NAME good for anything? Or should we just @error in that case? 

How does the whole thing work for folks doing local builds?

(Ugh)

Comment 5

[Aki Sasaki (not active)]

I'd like to save this somehow since otherwise we may wget this file multiple times.

Maybe, without the ifdef,

  DEB_PKG_NAME ?= $(shell $(WGET) -q -O - $(EN_US_BINARY_URL)/deb_name.txt)

and an

  echo-deb-name:
      @echo $(DEB_PKG_NAME)

which we can save in a buildbot property?  Or a local file.

(In reply to comment #4)
> Is the non-fuzzled DEB_PKG_NAME good for anything? Or should we just @error in
> that case?

Probably.

> How does the whole thing work for folks doing local builds?

You mean local builds, not local repacks?
Local builds will give you the fennec-DEBVERSION~DATETIME_armel.deb ... and that info should be stored after the configure.  The problem here is we're not repacking on the build machine; we're pulling source and trying to figure out the non-predictable deb name to download.

If you mean local repacks, with the ?= we could define DEB_PKG_NAME if we know it. A simple = might work too; not sure.

Comment 6

[Axel Hecht [:Pike]]

My point about local builds was if I try to develop repacks, I build a package, and then I want to do a repack. That sounds like I'd be stuck there, without having a server to upload stuff to.

Maybe a extra target to pull the deb_pkg_name from a server might be cool, to then stuff it into a buildbot property. Not sure how that intermingles with the one given at configure time.

Just commenting without a good suggestion forward.

Comment 7

[Aki Sasaki (not active)]

Attachment: allow DEB_PKG_NAME to be set from shell; add echo-deb-name target

So one of the reasons why I'm using ?= outside of an ifdef is so that if you are doing local repacks, you can specify DEB_PKG_NAME=fennec_whatever_armel.deb

If this is good, I'll need to add the buildbot side + generate&upload this deb_name.txt file.

Comment 8

[Axel Hecht [:Pike]]

Comment on attachment 421381 [details] [diff] [review]
allow DEB_PKG_NAME to be set from shell; add echo-deb-name target

I'm still confused how all those things work together, but this looks right to me.

Comment 9

[Axel Hecht [:Pike]]

PS: Nit, if wget isn't found, that might leave a strange error message.

I wonder if we can do something like

WGET ?= $(error Wget not installed)

to catch all cases. No idea if that works, though.

Comment 10

[Aki Sasaki (not active)]

Hm, looks like make wget-en-US uses $(PACKAGE) instead of $(DEB_PKG_NAME).
Gotta figure out where that's set and where I can override it.

Comment 11

[Aki Sasaki (not active)]

(In reply to comment #10)
> Hm, looks like make wget-en-US uses $(PACKAGE) instead of $(DEB_PKG_NAME).
> Gotta figure out where that's set and where I can override it.

n/m, that's the tarball. carry on, me.

Comment 12

[Aki Sasaki (not active)]

Attachment: repack fix, version 3

I hope this is clearer.

I ran this on moz2-linux-slave04.  To get the deb repack to work:

  make wget-DEB_PKG_NAME EN_US_BINARY_URL=http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mobile-trunk

(save this deb name.  I suppose humans could |export DEB_PKG_NAME=`make wget-DEB_PKG_NAME ...`|)

  make wget-en-US EN_US_BINARY_URL=http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mobile-trunk DEB_PKG_NAME=fennec_1.1a1~20100113014056_armel.deb DEB_BUILD_ARCH=armel

  make wget-deb EN_US_BINARY_URL=http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mobile-trunk DEB_PKG_NAME=fennec_1.1a1~20100113014056_armel.deb DEB_BUILD_ARCH=armel

  make installers-fr deb-fr EN_US_BINARY_URL=http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mobile-trunk DEB_PKG_NAME=fennec_1.1a1~20100113014056_armel.deb DEB_BUILD_ARCH=armel

and ended up with an fr deb.  I bet I don't need all the variables on each of those... I'll just add the DEB_PKG_NAME=%(debName)s to existing buildbot steps.

I'll work on the buildbot side + creating the deb_name.txt file.

Comment 13

[Aki Sasaki (not active)]

Attachment: create mobile/dist/deb_name.txt

Comment 14

[Axel Hecht [:Pike]]

Comment on attachment 421683 [details] [diff] [review]
repack fix, version 3

I'd like to get rid of those ifndef's. That's really hard to read, and shouldn't be in the rules, in particular.

Can you try to replace them with ?=, and if that doesn't work, put them outside of the rules? That way we only have them once, and not all over the place.

Comment 15

[Aki Sasaki (not active)]

Attachment: move $(error)'s up

We can save the deb name via echo-variable-DEB_PKG_NAME

Comment 16

[Axel Hecht [:Pike]]

Comment on attachment 421716 [details] [diff] [review]
move $(error)'s up

-5/+4, like :-)

Comment 17

[Aki Sasaki (not active)]

Attachment: nightly deb rename fixes for buildbotcustom

a) upload deb_name.txt with the Maemo builds
b) store debname property
c) include EN_US_BINARY_URL in the various mobile repack-inherited classes' env (won't hurt others; will let Maemo builds run without re-defining those steps)

Successfully ran a repack on staging.  Woot!

... We do have to land the mobile-browser changes on the relbranch, or not land this patch until after 1.0 is out the door.  Otherwise the Maemo build will die on not being able to upload deb_name.txt.  I think the repacks will be fine though.

Comment 18

[Aki Sasaki (not active)]

Comment on attachment 421706 [details] [diff] [review]
create mobile/dist/deb_name.txt

http://hg.mozilla.org/mobile-browser/rev/e553f4bacb95

Comment 19

[Aki Sasaki (not active)]

Comment on attachment 421716 [details] [diff] [review]
move $(error)'s up

http://hg.mozilla.org/mobile-browser/rev/9ca8698660d5

Comment 20

[Aki Sasaki (not active)]

Comment on attachment 421751 [details] [diff] [review]
nightly deb rename fixes for buildbotcustom

http://hg.mozilla.org/build/buildbotcustom/rev/b07228f9f682

Comment 21

[Aki Sasaki (not active)]

Comment on attachment 421716 [details] [diff] [review]
move $(error)'s up

This breaks winmo builds unless I define EN_US_BINARY_URL -- that's broken.

That is a pretty strong argument to put these checks inside of rules.

Comment 22

[Aki Sasaki (not active)]

Comment on attachment 421683 [details] [diff] [review]
repack fix, version 3

This is looking a lot better. Un-obsoleting.

Comment 23

[Axel Hecht [:Pike]]

(In reply to comment #21)
> (From update of attachment 421716 [details] [diff] [review])
> This breaks winmo builds unless I define EN_US_BINARY_URL -- that's broken.

How so?

Comment 24

[Aki Sasaki (not active)]

Why should I have to define EN_US_BINARY_URL in an en-US winmo build?

Comment 25

[Axel Hecht [:Pike]]

How does it break the winmo build is more of my question. That shouldn't expand anything, really.

Comment 26

[Aki Sasaki (not active)]

http://tinderbox.mozilla.org/showlog.cgi?log=Mobile/1263588564.1263589602.23584.gz&fulltext=1

Looks like make[6] runs in mobile/locales.  I'm going to assume since EN_US_BINARY_URL ?= and DEB_PKG_NAME ?= are outside of the rules, they get set.

I suppose I could ifneq $(OS_ARCH,WINCE) or whatever, but will that break winmo repacks?

Comment 27

[Aki Sasaki (not active)]

Looks like this broke linux fennec desktop builds as well.

Comment 28

[Aki Sasaki (not active)]

Attachment: move declarations down

This still breaks winmo builds, after moving the $(error) below *.mk

Comment 29

[Aki Sasaki (not active)]

Comment on attachment 421683 [details] [diff] [review]
repack fix, version 3

Verified that this patch does not break winmo builds.

Comment 30

[Aki Sasaki (not active)]

Comment on attachment 421683 [details] [diff] [review]
repack fix, version 3

Can I assume r+ from the phone call this morning?

Comment 31

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 421683 [details] [diff] [review]
repack fix, version 3

Looks good for now. We can file a new bug if we want to clean this up.

Comment 32

[Axel Hecht [:Pike]]

Comment on attachment 421683 [details] [diff] [review]
repack fix, version 3

Sure, follow up bug would be nice.

Comment 33

[Aki Sasaki (not active)]

Comment on attachment 421683 [details] [diff] [review]
repack fix, version 3

http://hg.mozilla.org/mobile-browser/rev/9fd972c718fc

New buildbotcustom patch incoming.

Comment 34

[Aki Sasaki (not active)]

Attachment: new buildbotcustom patch

Same as the previous patch, except make echo-variable-DEB_PKG_NAME -> make wget-DEB_PKG_NAME

Comment 35

[Aki Sasaki (not active)]

Comment on attachment 423444 [details] [diff] [review]
new buildbotcustom patch

Carrying over the r+ since only the target name changed.

Also, I have the sneaking suspicion that since rc3 will have a special deb version, this will be needed.

http://hg.mozilla.org/build/buildbotcustom/rev/c0cc157024ea

Comment 36

[Aki Sasaki (not active)]

Attachment: prepend wget with @ to avoid setting DEB_PKG_NAME to the echoed command

sigh.
1 char patch!

Comment 37

[Aki Sasaki (not active)]

Comment on attachment 423466 [details] [diff] [review]
prepend wget with @ to avoid setting DEB_PKG_NAME to the echoed command

http://hg.mozilla.org/mobile-browser/rev/f6dbf652e14a

fr deb repack just finished successfully.
phew.

Comment 38

[Aki Sasaki (not active)]

Reopening.

a) deb_name.txt wasn't being uploaded.  partially fixed by bug 542295, but it errored on uploading the multi deb_name.txt (can probably just drop that line)
b) deb_name.txt has the wrong deb name.  Thought this was a multilocale thing, https://bugzilla.mozilla.org/show_bug.cgi?id=538699but:

http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mobile-trunk/1264520200/

deb_name.txt says fennec_1.0~20100126083429_armel.deb
fennec filename is fennec_1.0~20100126083417_armel.deb
xulrunner filename is xulrunner_1.9.3a1pre-20100126074655_armel.deb

MOZ_APP_BUILDID is changing per step.

Comment 39

[Aki Sasaki (not active)]

My vote *so* goes for not having a buildid in the filename, ever.

Comment 40

[Aki Sasaki (not active)]

Giving up til nightly deb versions are fixed. bug 542349.

Comment 41

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Per irc after today's mobile meeting, mfinkle's work in depbugs is expected to land soon, so its ok (not ideal) to have broken maemo nightlies until his stuff lands. 

We can revisit if his work changes scope or slips dates. Until then, not much we can do, hence in Future

Comment 42

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

Mass move of bugs from Release Engineering:Future -> Release Engineering. See
http://coop.deadsquid.com/2010/02/kiss-the-future-goodbye/ for more details.

Comment 43

[Aki Sasaki (not active)]

Should be unblocked. Grabbing again.

Comment 45

[Aki Sasaki (not active)]

Attachment: fix maemo deb repacks, makefile version

This is the right bug; here's my misfired comment from bug 538455 :

Ok, this works. I was able to manually repack an en-US deb with fy-NL and
install that deb and verify the localization.

Caveats:

 - I've got a dup DEB_PKG_VERSION define here (also in installer/Makefile.in),
which isn't ideal. Not sure what the best way to centralize that is.
 - We were getting by without using scratchbox to repack. No longer... now a
requirement due to libhildon + dpkg-deb.

I'll add Axel to the r? since I think this was originally his stuff.

I'm going to dup bug 544031 to this one since I'll fix both with this patch (or
subsequent ones). Built this patch from Stuart's original fix in
https://bugzilla.mozilla.org/attachment.cgi?id=425020 .

Comment 46

[Aki Sasaki (not active)]

Comment on attachment 427513 [details] [diff] [review]
fix maemo deb repacks, makefile version

Maybe Armen or Coop want to join in the review fun ?

Comment 47

[Armen [:armenzg]]

Comment on attachment 427513 [details] [diff] [review]
fix maemo deb repacks, makefile version

You might want to be consistent with $(TAR) by changing "tar -xf -" as well.

Besides that r+

Comment 48

[Aki Sasaki (not active)]

Mark, Axel: should I land this patch or do you want me to wait?

Comment 49

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 427513 [details] [diff] [review]
fix maemo deb repacks, makefile version

I suppose it can't hurt to land this for now.

As we discussed, we'll need to find a way to support using datetime suffixes in the version (and filename) eventually.

Comment 50

[Aki Sasaki (not active)]

Comment on attachment 427513 [details] [diff] [review]
fix maemo deb repacks, makefile version

obsoleting due to bug 547903.

Comment 51

[Axel Hecht [:Pike]]

General comment, as there's apparently a next revision coming up.

datastage doesn't necessarily need to be just data. When creating the stage, we should extract as much data as we need to repack, so probably control.tar.gz as well, and then only fiddle with the things that actually do change from locale to locale in the libs-% target.

If nothing more, that should speed things up, or, rather, in this case, make them less slower.

Comment 52

[Aki Sasaki (not active)]

I'm not sure what the difference will be since we only run one locale per repack these days, but since I'm rewriting this patch I can move that logic to datastage.

Comment 53

[Axel Hecht [:Pike]]

The repacks are depend builds, in a sense. So if the stage from the previous locale on that slave is still up-to-date, we don't unpack again.

Comment 54

[Aki Sasaki (not active)]

Morphing the bug since armenzg has bug 544035.

Comment 55

[Aki Sasaki (not active)]

Attachment: change repackage-deb to use dpkg-deb command

- extraction of control.tar.gz moved to $(DATASTAGE) target
- removed DEB_PKG_NAME setting; we'll set this by calling wget-DEB_PKG_NAME and then specifying DEB_PKG_NAME=_____ in future make steps
- use dpkg-deb on a directory $(AB_CD)/tmp rather than replacing data.tar.gz in the deb (see http://wilmer.gaast.net/blog/archives/3-.deb-files-are-ar-archives,-but-....html )

We check for DEB_PKG_NAME in deb-% and wget-deb which should handle errors from forgetting that.

Comment 56

[Aki Sasaki (not active)]

(tested manually on local vm)

Comment 57

[Armen [:armenzg]]

Comment on attachment 429858 [details] [diff] [review]
change repackage-deb to use dpkg-deb command

It looks fine to me.

Comment 58

[Axel Hecht [:Pike]]

Comment on attachment 429858 [details] [diff] [review]
change repackage-deb to use dpkg-deb command

r=me with a nit, 

-	cd $(DATASTAGE) && $(TAR) $(TAR_CREATE_FLAGS) - * | (cd $(CURDIR)/$(AB_CD)/tmp && tar -xf - )

+	cd $(DATASTAGE) && $(TAR) $(TAR_CREATE_FLAGS) - * | (cd $(CURDIR)/$(AB_CD)/tmp && $(TAR) -xf - )

in both places, so that we don't use plain tar anymore at all.

Comment 59

[Aki Sasaki (not active)]

Comment on attachment 429858 [details] [diff] [review]
change repackage-deb to use dpkg-deb command

http://hg.mozilla.org/mobile-browser/rev/1cd7f43892c4

checked in with tar->$(TAR)

Comment 60

[Aki Sasaki (not active)]

We probably need to verify this, and land in the relbranch if we want any working l10n repacks for Fennec 1.0.1.

Comment 61

[Aki Sasaki (not active)]

Ok, I was able to install 1.9.2_de nightly on an n800 after this landed.
I did have some issues doing this, but this was caused by having the gpg key imported.  If that's an issue, then that should be part of bug 546835 rather than this.

Comment 62

[Aki Sasaki (not active)]

Comment on attachment 429858 [details] [diff] [review]
change repackage-deb to use dpkg-deb command

Landed in GECKO1921_20100126_RELBRANCH (verbal a=stuart)

http://hg.mozilla.org/mobile-browser/rev/8c76c50845b6