Closed
Bug 1345781
Opened 8 years ago
Closed 7 years ago
bustage in wget-en-US due to PKG_PATH
Categories
(SeaMonkey :: Release Engineering, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: ewong, Assigned: ewong)
References
Details
Attachments
(2 files, 5 obsolete files)
1.30 KB,
patch
|
mshal
:
review+
|
Details | Diff | Splinter Review |
1.33 KB,
patch
|
Callek
:
review+
|
Details | Diff | Splinter Review |
Current repack bustage:
win32/en-US/seamonkey-2.48b1.zip
c:/builds/slave/rel-c-b-rpk/build/comm-beta/objdir-l10n/_virtualenv/Scripts/python.exe c:/builds/slave/rel-c-b-rpk/build/comm-beta/mozilla/config/nsinstall.py -D c:/builds/slave/rel-c-b-rpk/build/comm-beta/objdir-l10n/dist/win32/en-US/
(cd c:/builds/slave/rel-c-b-rpk/build/comm-beta/objdir-l10n/dist/win32/en-US/ && \
wget --no-cache -nv --no-iri -N -O win32/en-US/seamonkey-2.48b1.zip 'http://archive.mozilla.org/pub/seamonkey/candidates/2.48b1-candidates/build2/unsigned/win32/en-US/seamonkey-2.48b1.zip')
WARNING: timestamping does nothing in combination with -O. See the manual
for details.
win32/en-US/seamonkey-2.48b1.zip: No such file or directory
c:/builds/slave/rel-c-b-rpk/build/comm-beta/mozilla/toolkit/locales/l10n.mk:196: recipe for target 'wget-en-US' failed
pymake\..\..\mozmake.exe: *** [wget-en-US] Error 1
This stems from a change from bug 1301509.
The warning isn't the problem. The problem is that it's downloading the seamonkey-2.48b1.zip to win32/en-US when it's already in <objdir>/dist/win32/en-US
so my understanding is that PKG_PATH already includes win32/en-US.
so since Firefox 51 wasn't complaining about this(or not that I noticed),
there must be a change in the PKG_PATH that we need to set or something.
Assignee | ||
Updated•8 years ago
|
Summary: cannot wget package from archive.mo → bustage in wget-en-US due to PKG_PATH
Assignee | ||
Comment 1•8 years ago
|
||
$(PACKAGE) == win32/en-US/seamonkey-2.48b1.zip
$(PKG_PATH) == win32/en-US/
so given:
wget-en-US:
ifndef WGET
$(error Wget not installed)
endif
$(NSINSTALL) -D $(ABS_DIST)/$(PKG_PATH)
(cd $(ABS_DIST)/$(PKG_PATH) && \
$(WGET) --no-cache -nv --no-iri -N -O $(PACKAGE) '$(EN_US_BINARY_URL)/$(EN_US_PACKAGE_NAME)')
@echo 'Downloaded $(EN_US_BINARY_URL)/$(EN_US_PACKAGE_NAME) to $(ABS_DIST)/$(PKG_PATH)/$(PACKAGE)'
if it already goes into $(ABS_DIST)/$(PKG_PATH), i.e. dist/win32/en-US,
and this is an empty directory,
would "wget -O $(PACKAGE) ..." fail since it's trying to save the
ZIP file into win32/en-US/win32/en-US?
What change are we missing?
Comment 2•8 years ago
|
||
This was added in Bug 1293943 for Lightning. I think it needs to be backed out as it never worked right. TB doesn't have it.
Comment 3•8 years ago
|
||
Sorry forget the comment. wrong wget...
Assignee | ||
Comment 4•8 years ago
|
||
Both TB and Firefox don't have this problem because they seem to be
using mozharness or release_repack.sh which does the magic.
And with l10n dep repacks removed from Moco's buildbotcustom code
(bug 1268895), this part of the code |make wget-en-US| isn't used.
That said, there are probably other builders that use this rule.
Assignee | ||
Comment 5•8 years ago
|
||
This is a workaround. I'm not entirely sure changing the rule in toolkit/locales/l10n.mk
is a good idea since I don't know if anything else uses that rule.
That said, since it'll be pushed to a relbranch, I don't think it should
matter; *BUT*, eventually will hit this issue *again* for the next
release round so we need some sort of fix.
Attachment #8845706 -
Flags: feedback?(kairo)
Attachment #8845706 -
Flags: feedback?(bugspam.Callek)
Assignee | ||
Comment 6•8 years ago
|
||
Given:
wget-en-US:
ifndef WGET
$(error Wget not installed)
endif
$(NSINSTALL) -D $(ABS_DIST)/$(PKG_PATH)
(cd $(ABS_DIST)/$(PKG_PATH) && \
$(WGET) --no-cache -nv --no-iri -N -O $(PACKAGE) '$(EN_US_BINARY_URL)/$(EN_US_PACKAGE_NAME)')
@echo 'Downloaded $(EN_US_BINARY_URL)/$(EN_US_PACKAGE_NAME) to $(ABS_DIST)/$(PKG_PATH)/$(PACKAGE)'
ifdef RETRIEVE_WINDOWS_INSTALLER
ifeq ($(OS_ARCH), WINNT)
$(NSINSTALL) -D $(ABS_DIST)/$(PKG_INST_PATH)
(cd $(ABS_DIST)/$(PKG_INST_PATH) && \
$(WGET) --no-cache -nv --no-iri -N '$(EN_US_BINARY_URL)/$(PKG_PATH)$(PKG_INST_BASENAME).exe')
@echo 'Downloaded $(EN_US_BINARY_URL)/$(PKG_PATH)$(PKG_INST_BASENAME).exe to $(ABS_DIST)/$(PKG_INST_PATH)$(PKG_INST_BASENAME).exe'
endif
endif
if I set MOZ_PKG_PRETTYNAMES=1,
both the wget for the zip and the installer fails.
if I set MOZ_PKG_PRETTYNAMES= (ie. remove from env)
the wget for the zip file succeeds but the installer fails because
PKG_PATH is not set and thus $(EN_US_BINARY_URL)/$(PKG_PATH)$(PKG_INST_BASENAME).exe
becomes:
http://archive.mozilla.org/pub/seamonkey/candidates/2.48b1-candidates/build2/unsigned/seamonkey-2.48b1.en-US.win32.installer.exe
which isn't right.
The reason why Firefox and TB don't encounter this is because their
EN_US_BINARY_URL points to <some taskcluster url>/build/<installer>.exe
Since we use archive.mozilla.org directly AND our installer.exe
is stored in <archive.mozilla.org url>/win32/en-US, this won't work.
So would the workaround be something that we can consider?
[tagging n-i from IanN, frg and Callek]
Flags: needinfo?(iann_bugzilla)
Flags: needinfo?(frgrahl)
Flags: needinfo?(bugspam.Callek)
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(kairo)
Assignee | ||
Comment 7•8 years ago
|
||
Assignee | ||
Comment 8•8 years ago
|
||
Comment 9•8 years ago
|
||
ewong your patch is rather empty :)
Flags: needinfo?(frgrahl) → needinfo?(ewong)
Assignee | ||
Comment 10•8 years ago
|
||
(In reply to Frank-Rainer Grahl from comment #9)
> ewong your patch is rather empty :)
as usual.. I forgot to do the qrefresh before attaching...
Flags: needinfo?(ewong)
Assignee | ||
Updated•8 years ago
|
Attachment #8845947 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Attachment #8845948 -
Attachment is obsolete: true
Assignee | ||
Comment 11•8 years ago
|
||
On second thoughts, while PrettyNames will go away in Gecko 54, I don't
think it's a good idea to jump the gun with this change.
So back to the workaround.
Assignee | ||
Comment 12•8 years ago
|
||
Came up with this idea and tried it on the loaner and it works.
Assignee | ||
Updated•8 years ago
|
Attachment #8845706 -
Attachment description: proposed workaround → proposed workaround (option 1)
Flags: needinfo?(kairo)
Flags: needinfo?(iann_bugzilla)
Flags: needinfo?(bugspam.Callek)
Assignee | ||
Updated•8 years ago
|
Attachment #8846213 -
Flags: feedback?(kairo)
Attachment #8846213 -
Flags: feedback?(iann_bugzilla)
Attachment #8846213 -
Flags: feedback?(bugspam.Callek)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → ewong
Comment 13•8 years ago
|
||
Comment on attachment 8846213 [details] [diff] [review]
proposed patch (option 2)
Looks good. I would put a comment in with the bug reference or an explanation in there too and it might break something else later but if it works on the loaner hopefully not.
Attachment #8846213 -
Flags: feedback+
Assignee | ||
Comment 14•8 years ago
|
||
Well, it does break the final repack step, so going to go back to the drawing board.
Assignee | ||
Comment 15•8 years ago
|
||
(In reply to Edmund Wong (:ewong) from comment #14)
> Well, it does break the final repack step, so going to go back to the
> drawing board.
by changing the PACKAGE variable, I've changed the ZIP_IN variable which
is used in the make-installers-% stage.
https://hg.mozilla.org/releases/mozilla-beta/file/tip/toolkit/locales/l10n.mk#l42
Assignee | ||
Comment 16•8 years ago
|
||
The very *worst* case scenario is to backout
https://hg.mozilla.org/releases/mozilla-beta/35c4ed28dbac
because doing that on the loaner fixes the problem.
However, that said, backing out is just a temp-fix for our current
release. It needs repeating for other releases or fixed permanently.
Comment 17•8 years ago
|
||
(In reply to Edmund Wong (:ewong) from comment #16)
> The very *worst* case scenario is to backout
>
> https://hg.mozilla.org/releases/mozilla-beta/35c4ed28dbac
That URL returns an error, it's https://hg.mozilla.org/releases/mozilla-beta/rev/35c4ed28dbac ;-)
That said, I'm perplexed why that patch could even cause an error, are we overriding EN_US_PACKAGE_NAME?
Also, if backing that out works, and before that one a plain $(PACKAGE) was used, why do we suddenly need a basename on it?
Comment 18•8 years ago
|
||
In any case, I'm happy about any workaround that works, at least for this release.
Assignee | ||
Comment 19•8 years ago
|
||
(In reply to Robert Kaiser from comment #17)
> (In reply to Edmund Wong (:ewong) from comment #16)
> > The very *worst* case scenario is to backout
> >
> > https://hg.mozilla.org/releases/mozilla-beta/35c4ed28dbac
>
> That URL returns an error, it's
> https://hg.mozilla.org/releases/mozilla-beta/rev/35c4ed28dbac ;-)
>
> That said, I'm perplexed why that patch could even cause an error, are we
> overriding EN_US_PACKAGE_NAME?
>
We aren't overriding EN_US_PACKAGE_NAME.
> Also, if backing that out works, and before that one a plain $(PACKAGE) was
> used, why do we suddenly need a basename on it?
Given the new wget-en-US code as follows (first part):
wget-en-US:
ifndef WGET
$(error Wget not installed)
endif
$(NSINSTALL) -D $(ABS_DIST)/$(PKG_PATH)
(cd $(ABS_DIST)/$(PKG_PATH) && \
$(WGET) --no-cache -nv --no-iri -N -O $(PACKAGE) '$(EN_US_BINARY_URL)/$(EN_US_PACKAGE_NAME)')
@echo 'Downloaded $(EN_US_BINARY_URL)/$(EN_US_PACKAGE_NAME) to $(ABS_DIST)/$(PKG_PATH)/$(PACKAGE)'
Before, this patch, it would cd $(ABS_DIST)/$(PKG_PATH) and then
wget the file as specified in $(EN_US_BINARY_URL)/$(PACKAGE) (which
is win32/en-US/<pkgname>). However, now, it specifies -O $(PACKAGE).
So the above is essentially:
1) create <dist>/win32/en-US
2) cd <dist>/win32/en-US
3) wget --no-cache -nv --no-iri -N -O win32/en-US/<pkgname> \
'https://archive.mozilla.org/pub/seamonkey/candidates/2.48b1-candidates/build2/unsigned/win32/en-US/<pkgname>
but since we're already *in* <dist/win32/en-US> which was just created in
#1, nothing should be in it, but now we're expecting to save a
win32/en-US/<pkgname> to this folder (as specified by -O $(PACKAGE))
and thusly it fails because wget doesn't create folders for us.
Thus we need the BASEPACKAGE to get rid of the paths before <pkgname>
And with TB and Firefox both:
1) don't do l10n dep builds
2) base all their EN_US_BINARY_URL on a taskcluster build path,
they don't hit this.
1) cd $(ABS_DIST)/$(PKG_PATH) [ <dist>/win32/en-US ]
2) wget --no-cache -nv --no-iri -N -O $(PACKAGE) 'https://....'
since PACKAGE = win32/en-US/<pkgname>
#2 is essentially saving win32/en-US/pkgname in the <dist>/win32/en-US
path but when you're in <dist>/win32/en-US, win32/en-US doesn't exist
as a subdirectory of that.
So it fails.
Which is why if we don't touch
Assignee | ||
Comment 20•8 years ago
|
||
I forgot to delete the below ;/ Do ignore.
>
> 1) cd $(ABS_DIST)/$(PKG_PATH) [ <dist>/win32/en-US ]
> 2) wget --no-cache -nv --no-iri -N -O $(PACKAGE) 'https://....'
>
> since PACKAGE = win32/en-US/<pkgname>
>
> #2 is essentially saving win32/en-US/pkgname in the <dist>/win32/en-US
> path but when you're in <dist>/win32/en-US, win32/en-US doesn't exist
> as a subdirectory of that.
>
> So it fails.
>
> Which is why if we don't touch
Assignee | ||
Comment 21•8 years ago
|
||
It's my understanding that specifying -O $(PACKAGE) is superfluous
if EN_US_PACKAGE_NAME is PACKAGE. The problem is, $(PACKAGE) has paths
in it which will break the repackage part if where we do the 'wget' to
has *nothing* in it.
So removing the -O is still ok as it still downloads the EN_US_PACKAGE_NAME
to the (basename $(EN_US_PACKAGE_NAME)) to $(ABS_DIST)/$(PKG_PATH).
(if given a r+, this needs to be pushed to m-b's RELBRANCH *and*
m-i, *and* uplifted to m-a and m-r and m-b proper); but I suppose
I'm jumping the gun here.
Attachment #8845706 -
Attachment is obsolete: true
Attachment #8846213 -
Attachment is obsolete: true
Attachment #8845706 -
Flags: feedback?(kairo)
Attachment #8845706 -
Flags: feedback?(bugspam.Callek)
Attachment #8846213 -
Flags: feedback?(kairo)
Attachment #8846213 -
Flags: feedback?(iann_bugzilla)
Attachment #8846213 -
Flags: feedback?(bugspam.Callek)
Attachment #8846942 -
Flags: review?(mshal)
Assignee | ||
Comment 22•8 years ago
|
||
to note: this is the most simplest patch without getting so convoluted
and complicated.
Also, this was pushed to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f760bc60c6be0274e3f0b7ce4b0c4794d4cd5e0&selectedJob=83570457
Comment 23•8 years ago
|
||
Comment on attachment 8846942 [details] [diff] [review]
proposed patch
Review of attachment 8846942 [details] [diff] [review]:
-----------------------------------------------------------------
I understand the issue now.
For us, on the relbranch, I think is probably the shortest and least confusing way to do it. That said, I'm not sure if the original intent of the patch was that the file would have a different name and it would need to be "corrected" to the expected name by specifying -O here. In that case, your patch with basename is probably the right one.
Comment 24•8 years ago
|
||
Comment on attachment 8846942 [details] [diff] [review]
proposed patch
I don't think removing the -O $(PACKAGE) will work since I'm pretty sure it is needed from bug 1301509 in order to grab Taskcluster-named packages (eg: target.bz2 or whatever) and still use the regular packaging machinery with the firefox-foo names. Callek could confirm this if you think this is the only route.
Additionally, there are no more uses of PKG_PATH in mozilla-central, so we are planning to remove it in bug 1341291. I think any fix that relies on PKG_PATH still working is likely going to have to be fixed again in the future.
What is the value of EN_US_BINARY_URL for you? Does it work if you add win32/en-US/ to EN_US_BINARY_URL and remove it from PACKAGE and then delete PKG_PATH?
Attachment #8846942 -
Flags: review?(mshal) → review-
Assignee | ||
Comment 25•8 years ago
|
||
(In reply to Michael Shal [:mshal] from comment #24)
> Comment on attachment 8846942 [details] [diff] [review]
> proposed patch
>
> I don't think removing the -O $(PACKAGE) will work since I'm pretty sure it
> is needed from bug 1301509 in order to grab Taskcluster-named packages (eg:
> target.bz2 or whatever) and still use the regular packaging machinery with
> the firefox-foo names. Callek could confirm this if you think this is the
> only route.
Aren't Taskcluster named packages grabbed from the url and -O is where
it saves the package? so if you're in foo/bar, and you
"wget -O foo/bar/baz.tgz <url>", would you not get an error if foo/bar
doesn't exist already?
Since you're already in $(ABS_DIST)/$(PKG_PATH) (in m-b, it's still valid),
wget -O <pkgname only> <url> would suffice to get the package, no?
>
> Additionally, there are no more uses of PKG_PATH in mozilla-central, so we
> are planning to remove it in bug 1341291. I think any fix that relies on
> PKG_PATH still working is likely going to have to be fixed again in the
> future.
>
True.
> What is the value of EN_US_BINARY_URL for you? Does it work if you add
> win32/en-US/ to EN_US_BINARY_URL and remove it from PACKAGE and then delete
> PKG_PATH?
Unfortunately, that won't work because in:
https://hg.mozilla.org/releases/mozilla-beta/file/tip/toolkit/mozapps/installer/upload-files.mk#l132
it depends on saving to win32/<locale>/packagename. Without the PKG_PATH in
PACKAGE, it'll only save the package to the <dist> path and that throws
a wrench in the repackage-zip portion since it expects win32/<locale>/packagename
to exist.
Comment 26•8 years ago
|
||
(In reply to Edmund Wong (:ewong) from comment #25)
> (In reply to Michael Shal [:mshal] from comment #24)
> > I don't think removing the -O $(PACKAGE) will work since I'm pretty sure it
> > is needed from bug 1301509 in order to grab Taskcluster-named packages (eg:
> > target.bz2 or whatever) and still use the regular packaging machinery with
> > the firefox-foo names. Callek could confirm this if you think this is the
> > only route.
>
> Aren't Taskcluster named packages grabbed from the url and -O is where
> it saves the package? so if you're in foo/bar, and you
> "wget -O foo/bar/baz.tgz <url>", would you not get an error if foo/bar
> doesn't exist already?
The Taskcluster packages are named like "target.tar.bz2", but we can't use "target.tar.bz2" as the output for l10n because we build multiple locales at once (chunking), otherwise each locale that we build would just override the previous. Callek's solution to this in bug 1301509 was to not use the Taskcluster simple naming, but then he needed to fix the l10n repackaging code so that it could find the en-US build by essentially renaming it during the wget from "target.tar.bz2" to something like "firefox-53.0.en-US.linux-i686.tar.bz2".
It does look like the patch is technically incorrect since as you say we cd into PKG_PATH first, and then wget -O $(PACKAGE), which is defined as $(PKG_PATH)$(PKG_BASENAME)$(PKG_SUFFIX). However since we never set PKG_PATH anymore in firefox, we never hit this error.
> Since you're already in $(ABS_DIST)/$(PKG_PATH) (in m-b, it's still valid),
> wget -O <pkgname only> <url> would suffice to get the package, no?
Yeah, we should probably use '-O $(notdir $(PACKAGE))' instead of '-O $(PACKAGE)' if we need to support this in beta. However, even in beta, firefox still does not set PKG_PATH. There is still some code that appears to set it in package-name.mk, but that is only reached if MOZ_PKG_PRETTYNAMES is set, and we no longer use pretty names anywhere. We ended up removing that from the tree in bug 1329355, but it was already unused before that because of release promotion. Essentially, that logic exists in mozilla-beta but it is dead code (for firefox).
I would support replacing -O $(PACKAGE) with -O $(notdir $(PACKAGE)) in mozilla-beta+ if that helps unblock you, but for mozilla-central we're killing PKG_PATH anyway, so there's no reason to use $(notdir) there.
> > What is the value of EN_US_BINARY_URL for you? Does it work if you add
> > win32/en-US/ to EN_US_BINARY_URL and remove it from PACKAGE and then delete
> > PKG_PATH?
>
> Unfortunately, that won't work because in:
>
> https://hg.mozilla.org/releases/mozilla-beta/file/tip/toolkit/mozapps/
> installer/upload-files.mk#l132
>
> it depends on saving to win32/<locale>/packagename. Without the PKG_PATH in
> PACKAGE, it'll only save the package to the <dist> path and that throws
> a wrench in the repackage-zip portion since it expects
> win32/<locale>/packagename
> to exist.
Unfortunately I don't have a good solution here. It sounds like it will be difficult to support both l10n styles in the tree. Callek, do you have any suggestions here?
Flags: needinfo?(bugspam.Callek)
Assignee | ||
Comment 27•8 years ago
|
||
(In reply to Michael Shal [:mshal] from comment #26)
> (In reply to Edmund Wong (:ewong) from comment #25)
> > (In reply to Michael Shal [:mshal] from comment #24)
> > > I don't think removing the -O $(PACKAGE) will work since I'm pretty sure it
> > > is needed from bug 1301509 in order to grab Taskcluster-named packages (eg:
> > > target.bz2 or whatever) and still use the regular packaging machinery with
> > > the firefox-foo names. Callek could confirm this if you think this is the
> > > only route.
> >
> > Aren't Taskcluster named packages grabbed from the url and -O is where
> > it saves the package? so if you're in foo/bar, and you
> > "wget -O foo/bar/baz.tgz <url>", would you not get an error if foo/bar
> > doesn't exist already?
>
> The Taskcluster packages are named like "target.tar.bz2", but we can't use
> "target.tar.bz2" as the output for l10n because we build multiple locales at
> once (chunking), otherwise each locale that we build would just override the
> previous. Callek's solution to this in bug 1301509 was to not use the
> Taskcluster simple naming, but then he needed to fix the l10n repackaging
> code so that it could find the en-US build by essentially renaming it during
> the wget from "target.tar.bz2" to something like
> "firefox-53.0.en-US.linux-i686.tar.bz2".
>
> It does look like the patch is technically incorrect since as you say we cd
> into PKG_PATH first, and then wget -O $(PACKAGE), which is defined as
> $(PKG_PATH)$(PKG_BASENAME)$(PKG_SUFFIX). However since we never set PKG_PATH
> anymore in firefox, we never hit this error.
Fwiw, this is for Gecko 51 beta and not the current Gecko beta (which is
53, right?).
>
> > Since you're already in $(ABS_DIST)/$(PKG_PATH) (in m-b, it's still valid),
> > wget -O <pkgname only> <url> would suffice to get the package, no?
>
> Yeah, we should probably use '-O $(notdir $(PACKAGE))' instead of '-O
> $(PACKAGE)' if we need to support this in beta. However, even in beta,
> firefox still does not set PKG_PATH. There is still some code that appears
> to set it in package-name.mk, but that is only reached if
> MOZ_PKG_PRETTYNAMES is set, and we no longer use pretty names anywhere. We
> ended up removing that from the tree in bug 1329355, but it was already
> unused before that because of release promotion. Essentially, that logic
> exists in mozilla-beta but it is dead code (for firefox).
Ah ok. I'll keep that in mind.
>
> I would support replacing -O $(PACKAGE) with -O $(notdir $(PACKAGE)) in
> mozilla-beta+ if that helps unblock you, but for mozilla-central we're
> killing PKG_PATH anyway, so there's no reason to use $(notdir) there.
I know about the m-c removal and I think at that point, we'll need to
get rid of the MOZ_PKG_PRETTYNAMES as well.
Thanks!
Comment 28•8 years ago
|
||
(In reply to Edmund Wong (:ewong) from comment #27)
> (In reply to Michael Shal [:mshal] from comment #26)
> > It does look like the patch is technically incorrect since as you say we cd
> > into PKG_PATH first, and then wget -O $(PACKAGE), which is defined as
> > $(PKG_PATH)$(PKG_BASENAME)$(PKG_SUFFIX). However since we never set PKG_PATH
> > anymore in firefox, we never hit this error.
>
> Fwiw, this is for Gecko 51 beta and not the current Gecko beta (which is
> 53, right?).
Ahh, I don't remember the state of things in 51. Does the $(notdir) solution work for you there?
Assignee | ||
Comment 29•8 years ago
|
||
(In reply to Michael Shal [:mshal] from comment #28)
> (In reply to Edmund Wong (:ewong) from comment #27)
> > (In reply to Michael Shal [:mshal] from comment #26)
> > > It does look like the patch is technically incorrect since as you say we cd
> > > into PKG_PATH first, and then wget -O $(PACKAGE), which is defined as
> > > $(PKG_PATH)$(PKG_BASENAME)$(PKG_SUFFIX). However since we never set PKG_PATH
> > > anymore in firefox, we never hit this error.
> >
> > Fwiw, this is for Gecko 51 beta and not the current Gecko beta (which is
> > 53, right?).
>
> Ahh, I don't remember the state of things in 51. Does the $(notdir) solution
> work for you there?
Yes, the $(notdir) solution works perfectly!
Assignee | ||
Comment 30•8 years ago
|
||
ensure paths are not used in the PACKAGE variable.
Attachment #8846942 -
Attachment is obsolete: true
Attachment #8848389 -
Flags: review?(mshal)
Comment 31•8 years ago
|
||
Comment on attachment 8848389 [details] [diff] [review]
proposed patch (v2)
LGTM! Where all does this need to land?
Attachment #8848389 -
Flags: review?(mshal) → review+
Assignee | ||
Comment 32•8 years ago
|
||
(In reply to Michael Shal [:mshal] from comment #31)
> Comment on attachment 8848389 [details] [diff] [review]
> proposed patch (v2)
>
> LGTM! Where all does this need to land?
This needs to land first in m-b Relbranch(gecko 51) and Gecko 51's m-r.
Then the following:
1) Gecko 52's m-b, and m-esr52 (optional Gecko 52 m-r for good measure)
2) Gecko 53's m-b, and Gecko 54 m-a.
:mshal, is this even do-able?
Rationale (if someone can clarify if I have this right): This bug is
due to a change in Gecko 51. Which means it's in 52, 53, 54, and 55.
And I would need to have this patch pushed to all those trees listed
above to fix the repacks issue. The reason why I'll request all those
branches is because SeaMonkey (currently spinning 2.48b1) will do the
following releases (after 2.48b1):
1) 2.48 release [corresponds to Gecko 51 m-r]
2) 2.49b1 [ corresponds to Gecko 52 m-b]
3) 2.49esr release [corresponds to Gecko 52 m-esr]
4) 2.50b1 [ corresponds to Gecko 53 m-b]
Since we're that behind, we need some 'slack time' to catch up and
asking it to be pushed to Gecko 54 m-a will at least give us some
breathing space such that we won't need to worry about 2.51b1
busting us because of this.
As noted, I will push this patch to the SeaMonkey relbranch (which doesn't
require M-b approval as it's a relbranch). Then I'll attempt to request
the other branches (I'll even create relbranches on those old 52 m-b branch
if it's ok). [tbh, not even sure how I'll even accomplish the requests.]
Can someone loop in a Rel manager here for some insight?
Flags: needinfo?(mshal)
Comment 33•8 years ago
|
||
> Unfortunately I don't have a good solution here. It sounds like it will be
> difficult to support both l10n styles in the tree. Callek, do you have any
> suggestions here?
Looks like you guys found a solution
(In reply to Edmund Wong (:ewong) from comment #32)
> (In reply to Michael Shal [:mshal] from comment #31)
> > Comment on attachment 8848389 [details] [diff] [review]
> > proposed patch (v2)
> >
> > LGTM! Where all does this need to land?
>
...
> Can someone loop in a Rel manager here for some insight?
I'm not a manager, but you shouldn't need any explicit signoff for creating and landing on relbranches, please just (if possible) push them all at once, and let releng know in #releaseduty (just incase it trips some weird state in releasy stuff, but it shouldn't aiui)
Flags: needinfo?(bugspam.Callek)
Comment 34•8 years ago
|
||
I don't know about landing things on previously released versions (eg: beta 51). Looping in Sylvestre for guidance on #c32.
Flags: needinfo?(mshal) → needinfo?(sledru)
Assignee | ||
Comment 36•8 years ago
|
||
(In reply to Sylvestre Ledru [:sylvestre] from comment #35)
> You should use the relbranch for that
Thanks :sylvestre.. I'll push this to all the relbranches for those
old trees.
Assignee | ||
Comment 37•8 years ago
|
||
(In reply to Michael Shal [:mshal] from comment #34)
> I don't know about landing things on previously released versions (eg: beta
> 51). Looping in Sylvestre for guidance on #c32.
:mshal, I didn't realize that the mac package had space in it and so this patch
doesn't work for the mac platform.
I need to add quotes to the $(notdir $(PACKAGE)).
Assignee | ||
Comment 38•8 years ago
|
||
Attachment #8852285 -
Flags: review?(mshal)
Comment 39•8 years ago
|
||
Comment on attachment 8852285 [details] [diff] [review]
additional patch to fix mac package names
Review of attachment 8852285 [details] [diff] [review]:
-----------------------------------------------------------------
I see no reason this would break (so to speak)...
Attachment #8852285 -
Flags: review?(mshal) → review+
Assignee | ||
Comment 40•7 years ago
|
||
Fixed
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•