Closed Bug 538699 Opened 10 years ago Closed 10 years ago

maemo repacks should use dpkg-deb

Categories

(Release Engineering :: General, defect, P2)

ARM
Maemo
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aki, Assigned: aki)

References

Details

(Whiteboard: [fennec l10n][l10n][fixed-fennec-1.0.1])

Attachments

(1 file, 10 obsolete files)

1.78 KB, patch
Pike
: review+
armenzg
: review+
Details | Diff | Splinter Review
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.
(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.
We could also potentially solve this with a sendchange and/or ssh poller and another buildbot property.
Assignee: nobody → aki
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.
Attachment #421179 - Flags: review?(l10n)
Attachment #421179 - Flags: review?(mark.finkle)
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)
Attachment #421179 - Flags: review?(l10n) → review-
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.
Attachment #421179 - Flags: review?(mark.finkle)
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.
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.
Attachment #421179 - Attachment is obsolete: true
Attachment #421381 - Flags: review?(l10n)
Attachment #421381 - Flags: review?(l10n) → review+
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.
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.
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.
(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.
Attached patch repack fix, version 3 (obsolete) — Splinter Review
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.
Attachment #421381 - Attachment is obsolete: true
Attachment #421683 - Flags: review?(l10n)
Attached patch create mobile/dist/deb_name.txt (obsolete) — Splinter Review
Attachment #421706 - Flags: review?(mark.finkle)
Attachment #421706 - Flags: review?(mark.finkle) → review+
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.
Attachment #421683 - Flags: review?(l10n) → review-
Attached patch move $(error)'s up (obsolete) — Splinter Review
We can save the deb name via echo-variable-DEB_PKG_NAME
Attachment #421683 - Attachment is obsolete: true
Attachment #421716 - Flags: review?(l10n)
Comment on attachment 421716 [details] [diff] [review]
move $(error)'s up

-5/+4, like :-)
Attachment #421716 - Flags: review?(l10n) → review+
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.
Attachment #421751 - Flags: review?(ccooper)
Attachment #421751 - Flags: review?(ccooper) → review+
Comment on attachment 421751 [details] [diff] [review]
nightly deb rename fixes for buildbotcustom

http://hg.mozilla.org/build/buildbotcustom/rev/b07228f9f682
Attachment #421751 - Flags: checked-in+
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.
Attachment #421716 - Flags: checked-in+ → checked-in-
Comment on attachment 421683 [details] [diff] [review]
repack fix, version 3

This is looking a lot better. Un-obsoleting.
Attachment #421683 - Attachment is obsolete: false
(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?
Why should I have to define EN_US_BINARY_URL in an en-US winmo build?
How does it break the winmo build is more of my question. That shouldn't expand anything, really.
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?
Looks like this broke linux fennec desktop builds as well.
Attachment #421751 - Flags: checked-in+ → checked-in-
Attached patch move declarations down (obsolete) — Splinter Review
This still breaks winmo builds, after moving the $(error) below *.mk
Attachment #421716 - Attachment is obsolete: true
Comment on attachment 421683 [details] [diff] [review]
repack fix, version 3

Verified that this patch does not break winmo builds.
Attachment #421683 - Flags: review?(mark.finkle)
Comment on attachment 421683 [details] [diff] [review]
repack fix, version 3

Can I assume r+ from the phone call this morning?
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.
Attachment #421683 - Flags: review?(mark.finkle) → review+
Comment on attachment 421683 [details] [diff] [review]
repack fix, version 3

Sure, follow up bug would be nice.
Attachment #421683 - Flags: review- → review+
Comment on attachment 421683 [details] [diff] [review]
repack fix, version 3

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

New buildbotcustom patch incoming.
Attachment #421683 - Flags: checked-in+
Attachment #422476 - Attachment is obsolete: true
Attachment #421751 - Attachment is obsolete: true
Attached patch new buildbotcustom patch (obsolete) — Splinter Review
Same as the previous patch, except make echo-variable-DEB_PKG_NAME -> make wget-DEB_PKG_NAME
Attachment #423444 - Flags: review?(ccooper)
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
Attachment #423444 - Flags: review?(ccooper)
Attachment #423444 - Flags: review+
Attachment #423444 - Flags: checked-in+
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
sigh.
1 char patch!
Attachment #423466 - Flags: review?(mark.finkle)
Attachment #423466 - Flags: review?(mark.finkle) → review+
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.
Attachment #423466 - Flags: checked-in+
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → REOPENED
Depends on: 542295
Resolution: FIXED → ---
My vote *so* goes for not having a buildid in the filename, ever.
Depends on: 542349
Giving up til nightly deb versions are fixed. bug 542349.
Assignee: aki → nobody
Component: Release Engineering → Release Engineering: Future
Blocks: 542413
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
Blocks: 543489
Mass move of bugs from Release Engineering:Future -> Release Engineering. See
http://coop.deadsquid.com/2010/02/kiss-the-future-goodbye/ for more details.
Component: Release Engineering: Future → Release Engineering
Priority: -- → P3
Should be unblocked. Grabbing again.
Assignee: nobody → aki
Priority: P3 → P2
Duplicate of this bug: 544031
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 .
Attachment #421683 - Attachment is obsolete: true
Attachment #421706 - Attachment is obsolete: true
Attachment #423444 - Attachment is obsolete: true
Attachment #423466 - Attachment is obsolete: true
Attachment #427513 - Flags: review?(mark.finkle)
Attachment #427513 - Flags: review?(l10n)
Blocks: 506989
Comment on attachment 427513 [details] [diff] [review]
fix maemo deb repacks, makefile version

Maybe Armen or Coop want to join in the review fun ?
Attachment #427513 - Flags: review?(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+
Attachment #427513 - Flags: review?(armenzg) → review+
Blocks: 544035
Mark, Axel: should I land this patch or do you want me to wait?
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.
Attachment #427513 - Flags: review?(mark.finkle) → review+
Comment on attachment 427513 [details] [diff] [review]
fix maemo deb repacks, makefile version

obsoleting due to bug 547903.
Attachment #427513 - Attachment is obsolete: true
Attachment #427513 - Flags: review?(l10n)
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.
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.
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.
Morphing the bug since armenzg has bug 544035.
Summary: maemo nightly repacks broken due to deb rename → maemo repacks should use dpkg-deb
- 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.
Attachment #429858 - Flags: review?(l10n)
Attachment #429858 - Flags: review?(armenzg)
(tested manually on local vm)
Comment on attachment 429858 [details] [diff] [review]
change repackage-deb to use dpkg-deb command

It looks fine to me.
Attachment #429858 - Flags: review?(armenzg) → review+
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.
Attachment #429858 - Flags: review?(l10n) → review+
Blocks: 550023
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)
Attachment #429858 - Flags: checked-in+
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
We probably need to verify this, and land in the relbranch if we want any working l10n repacks for Fennec 1.0.1.
Whiteboard: [fennec l10n][l10n] → [fennec l10n][l10n][blocking-fennec-1.0.1?]
Blocks: 552072
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 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
Whiteboard: [fennec l10n][l10n][blocking-fennec-1.0.1?] → [fennec l10n][l10n][fixed-fennec-1.0.1]
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.