456351 - use variables from packager-name.mk in prepare-upload-locale target

Status: RESOLVED WONTFIX

(Release Engineering :: General, defect)

Description

[Armen [:armenzg]]

According to comment
https://bugzilla.mozilla.org/show_bug.cgi?id=451461#c29

>+	mv $(DIST)/install/firefox-$(MOZ_APP_VERSION).$*.langpack.xpi \
>+	mv $(DIST)/firefox-$(MOZ_APP_VERSION).$*.* \
should be something like this:
>+	mv $(DIST)/install/$(MOZ_PKG_APPNAME)-$(MOZ_APP_VERSION).$*.langpack.xpi
>+	mv $(DIST)/$(MOZ_PKG_APPNAME)-$(MOZ_APP_VERSION).$*.$(MOZ_PKG_PLATFORM)

Is this right?

Comment 1

[Robert Kaiser]

bug 453840 is where I introduced those vars.

$(DIST)/install/firefox-$(MOZ_APP_VERSION).$*.langpack.xpi is supposed to be $(DIST)/$(PKG_LANGPACK_PATH)$(PKG_LANGPACK_BASENAME).xpi instead, you might need to do a AB_CD=$* or such before that though, see e.g. what we're doing in the langpack-% in that same locales/Makefile.

The installer is now at $(DIST)/$(PKG_INST_PATH)$(PKG_INST_BASENAME).exe and the normal installer is still at $(DIST)/$(PACKAGE) which is set by packager.mk to even include the right suffix (zip/dmg/tar.bz2) for the platform.

Using those vars finds the fails even with their different names and paths they land at when using so-called "pretty names" used for releases. And it makes us able to maybe one time move the default paths the packages get created and have all places in the buildsystem fixed automatically when we change package-name.mk (I particularly dislike having the installer in $(DIST)/install/sea/ and having fewer places where we hardcode that eases correcting that to something better one time).

Comment 2

[Robert Kaiser]

I did most of this already for SeaMonkey on trunk, see http://hg.mozilla.org/comm-central/file/tip/suite/locales/Makefile.in#l307 - I just didn't test it so there might be minor bugs in those changes.

Comment 3

[Armen [:armenzg]]

I haven't had time to work on this, putting back into the pool

Comment 4

[Robert Kaiser]

Attachment: patch: port work from SeaMonkey

This patch ports the work I did in the SeaMonkey Makefile, additionally (hopefully) making the mv commands for package and installer safe for "pretty" filenames.
It's wrong to have platform-specific directories for the language packs, as we care to make them cross-platform. Instead, we should set the PKG_*PATH variables throughout the build process where we want non-default paths and get what we need right away.

I agree that the current default paths we set in package-name.mk probably aren't ideal, but I didn't dare to correct those as a part of the prettyname patch as I feared to break upload setups of buildbots. Once we are using the PKG_*PATH variables everywhere, it'll be much easier to just set the defaults in toolkit to sane values.

Comment 5

[[e-mail] (:bhearsum)]

(In reply to comment #4)
> Created an attachment (id=349591) [details]
> patch: port work from SeaMonkey
> 
> This patch ports the work I did in the SeaMonkey Makefile, additionally
> (hopefully) making the mv commands for package and installer safe for "pretty"
> filenames.
> It's wrong to have platform-specific directories for the language packs, as we
> care to make them cross-platform.

In principle, I agree with you. However, for Firefox, we will continue to be offering XPIs in each platform directory until we consciously change that. I realize it's rather silly but we may have other processes which depend on that structure.

With that said, your patch looks fine in that respect - we can deal with putting them in the platform dirs at upload time.

Comment 6

[Armen [:armenzg]]

Comment on attachment 349591 [details] [diff] [review]
patch: port work from SeaMonkey

I have not run it but what bhearsum is right.
we need to follow this structure for langpacks
{linux,mac,windows}-xpi/locale.xpi

We can bring discussion regarding the folders for the xpi files but that should be aside of this bug.

We should be using XPI_DESTINATION for now, the rest of the patch looks good

Comment 7

[Axel Hecht [:Pike]]

Comment on attachment 349591 [details] [diff] [review]
patch: port work from SeaMonkey

Cancelling this, waiting on a new patch.

Comment 8

[Robert Kaiser]

Hrm, I dislike having XPI_DESTINATION at least by default enough that I didn't touch this again in 2 months, but repeatedly look at the comment, worry about it, and give up again. I'm also a bit lost as to how we should have both "the right way" of PKG_LANGPACK_PATH and "the currently needed/wanted way" of XPI_DESTINATION at the same time.
Any ideas how we can deal with that?

Comment 9

[Robert Kaiser]

I just read bug 464154 and every line it changed in package-name.mk sucks enough for me to unassign this and stop caring about those names, sorry. I was so glad we could get rid of that damn xpi/ subdir and the pretended platform-specificness of langpacks. It probably was too good to be true.

Comment 10

[Robert Kaiser]

So Ben tells me the prepare-upload-locale target is obsolete, which makes this a WONTFIX.

Comment 11

[Robert Kaiser]

I filed bug 474805 for removal of that target, along with some other improvements.

Comment 12

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

Moving closed Future bugs into Release Engineering in preparation for removing the Future component.