Closed Bug 1298719 Opened 4 years ago Closed 4 years ago

Mozilla-central CSID has gone lost from the .txt file accompanying the installable archive

Categories

(MailNews Core :: Build Config, defect)

defect
Not set
normal

Tracking

(thunderbird50 fixed, thunderbird51 fixed, thunderbird52 fixed, seamonkey2.47 fixed, seamonkey2.48 fixed)

VERIFIED FIXED
Thunderbird 52.0
Tracking Status
thunderbird50 --- fixed
thunderbird51 --- fixed
thunderbird52 --- fixed
seamonkey2.47 --- fixed
seamonkey2.48 --- fixed

People

(Reporter: tonymec, Assigned: aleth)

References

(Blocks 1 open bug, )

Details

(Keywords: regression)

User Story

URL field = First known bad.
Last known good was on 10 August.
No builds found in the intervening gap.

Attachments

(3 files, 7 obsolete files)

My present seamonkey-2.48a1.en-US.linux-x86_64.txt (downloaded from the same ftp.m.o directory as the .tar.bz2) contains the following text:

20160828013444
http://hg.mozilla.org/comm-central/rev/60b332e98e134900539934e6f1cb41461857ddce
http://hg.mozilla.org/comm-central/rev/60b332e98e13

Notice that both URLs now point to comm-central. The first of them (line 2) used to point to mozilla-central. The change happened in the "gap" with no trunk nightly for any platform, between 2016-08-10 (Good) and 2016-08-15 (Bad)... Ah, I found a tinderbox-build of 2016-08-14 which is BAD. The immediately preceding one was on 2016-08-10 and is GOOD.

Windows builds have the same problem, I haven't checked L32 or Mac.

The mozilla-central URL in about:buildconfig has also been replaced by comm-central, this could be due to the same cause but AFAIK the about:buildconfig (and application.ini) problem is tracked in bug 456360. I am opening this bug specifically about the SeaMonkey .txt file mentioned above. I'm putting the URL of the "first known bad" in the URL field of this bug.

I suspect that this bug and bug 456360 are not duplicates of each other, but are partly due to a common cause.
I believe this is caused bug 1267270. I didn't follow this one closely if a followup bug is open.
Thank you Frank-Rainer. Lets ask Aleth who wrote the patch on bug 1267270.
Flags: needinfo?(aleth)
I don't know what generates that txt file, but it probably reads from platform.ini, so this is likely Bug 1297198.
Flags: needinfo?(aleth)
In the same ftp.m.o subdirectory, the .json (not .mozinfo.json which is about other stuff) has the same problem; for instance in the latest L64 tinderbox-build, which I'm about to install:

http://ftp.mozilla.org/pub/seamonkey/tinderbox-builds/comm-central-trunk-linux64/1472489313/seamonkey-2.48a1.en-US.linux-x86_64.txt
<quote>
20160829094833
http://hg.mozilla.org/comm-central/rev/bcb5f54ebc2ae554f923aab3f52925cc24b04d9a
http://hg.mozilla.org/comm-central/rev/bcb5f54ebc2a
</quote>

http://ftp.mozilla.org/pub/seamonkey/tinderbox-builds/comm-central-trunk-linux64/1472489313/seamonkey-2.48a1.en-US.linux-x86_64.json
<quote>
{
  "as": "$(CC)", 
  "buildid": "20160829094833", 
  "cc": "/usr/bin/ccache /builds/slave/c-cen-t-lnx64/build/gcc/bin/gcc -std=gnu99", 
  "cxx": "/usr/bin/ccache /builds/slave/c-cen-t-lnx64/build/gcc/bin/g++ -std=gnu++11", 
  "host_alias": "x86_64-pc-linux-gnu", 
  "host_cpu": "x86_64", 
  "host_os": "linux-gnu", 
  "host_vendor": "pc", 
  "ld": "ld", 
  "moz_app_id": "{92650c4d-4b8e-4d2a-b7eb-24ecf4f6b63a}", 
  "moz_app_maxversion": "2.48a1", 
  "moz_app_name": "seamonkey", 
  "moz_app_vendor": "Mozilla", 
  "moz_app_version": "2.48a1", 
  "moz_pkg_platform": "linux-x86_64", 
  "moz_source_repo": "MOZ_SOURCE_REPO=http://hg.mozilla.org/comm-central", 
  "moz_source_stamp": "bcb5f54ebc2ae554f923aab3f52925cc24b04d9a", 
  "moz_update_channel": "default", 
  "target_alias": "x86_64-pc-linux-gnu", 
  "target_cpu": "x86_64", 
  "target_os": "linux-gnu", 
  "target_vendor": "pc"
}
</quote>
the "moz-source-repo" and "moz-source-stamp" point to the same c-c CSID as in the .txt file. I suppose that both files take their CSIDs from the same source.

In the "good" tinderbox-build from 10 August, in http://ftp.mozilla.org/pub/seamonkey/tinderbox-builds/comm-central-trunk-linux64/1470814202/seamonkey-2.48a1.en-US.linux-x86_64.json the same fields point to mozilla-central.
(In reply to Tony Mechelynck [:tonymec] from comment #4)
> the "moz-source-repo" and "moz-source-stamp" point to the same c-c CSID as
> in the .txt file. I suppose that both files take their CSIDs from the same
> source.
> 
> In the "good" tinderbox-build from 10 August, in
> http://ftp.mozilla.org/pub/seamonkey/tinderbox-builds/comm-central-trunk-
> linux64/1470814202/seamonkey-2.48a1.en-US.linux-x86_64.json the same fields
> point to mozilla-central.

Right, see comment 3 and Bug 1267270 comment 24.
(In reply to aleth [:aleth] from comment #5)
> Right, see comment 3 and Bug 1267270 comment 24.

sorry, I meant Bug 1267270 comment 27.
(In reply to aleth [:aleth] from comment #3)
> I don't know what generates that txt file, but it probably reads from
> platform.ini, so this is likely Bug 1297198.

Bug 1297198 is in Thunderbird :: Build Config, this one is in SeaMonkey :: Build Config. My guess (from past experience) is that rather than duplicates, these two bugs are siblings, i.e., I guess that they may be partly in common source, but there will be some Thunderbird-only source and some SeaMonkey-only source which will both need fixing at homologous points in the tree.

Therefore I'm linking them by "See also" rather than "Depends on" or "Resolved Duplicate".
See Also: → 1297198
Duplicate of this bug: 1304285
Blocks: 1304284
Attached patch proposed patch (obsolete) — Splinter Review
basically during postflight, it fixes the platform.ini.  so that anything
after this should have a proper platform.ini.

trying to fix this within m-c would be 'problematic' since we'd probably
need to add a whole bunch of checks for c-c builds during the platform.ini 
creation stage.

:jcranmer, is there a better way of doing this?
Attachment #8794245 - Flags: review?(Pidgeot18)
Comment on attachment 8794245 [details] [diff] [review]
proposed patch

Review of attachment 8794245 [details] [diff] [review]:
-----------------------------------------------------------------

Interesting hack!

::: client.mk
@@ +402,5 @@
>  	+$(MOZ_MAKE) $@
>  
>  ####################################
>  # Postflight
> +MOZ_REV=$(shell cd $(TOPSRCDIR)/mozilla && hg ident --debug | tail -1 | cut -d ' ' -f 1)

You could use MOZILLA_SRCDIR instead of $(TOPSRCDIR)/mozilla, not that it makes any difference.

Note for the MOZ_SOURCESTAMP we use (instead of hg ident)
MOZ_SOURCE_CHANGESET=$(hg -R "$TOPSRCDIR" parent --template="{node}" 2>/dev/null)
(In reply to aleth [:aleth] from comment #10)
> Comment on attachment 8794245 [details] [diff] [review]
> proposed patch
> 
> Review of attachment 8794245 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Interesting hack!
> 
> ::: client.mk
> @@ +402,5 @@
> >  	+$(MOZ_MAKE) $@
> >  
> >  ####################################
> >  # Postflight
> > +MOZ_REV=$(shell cd $(TOPSRCDIR)/mozilla && hg ident --debug | tail -1 | cut -d ' ' -f 1)
> 
> You could use MOZILLA_SRCDIR instead of $(TOPSRCDIR)/mozilla, not that it
> makes any difference.

MOZILLA_SRCDIR isn't defined there.
> 
> Note for the MOZ_SOURCESTAMP we use (instead of hg ident)
> MOZ_SOURCE_CHANGESET=$(hg -R "$TOPSRCDIR" parent --template="{node}"
> 2>/dev/null)

changed the patch to use this.
Attached patch proposed patch (v2) (obsolete) — Splinter Review
Attachment #8794245 - Attachment is obsolete: true
Attachment #8794245 - Flags: review?(Pidgeot18)
Attachment #8794450 - Flags: review?(Pidgeot18)
Attachment #8794450 - Attachment is obsolete: true
Attachment #8794450 - Flags: review?(Pidgeot18)
Attachment #8794460 - Flags: review?(Pidgeot18)
Duplicate of this bug: 1297198
Comment on attachment 8794460 [details] [diff] [review]
proposed patch (v3) (v2 + comments)

Review of attachment 8794460 [details] [diff] [review]:
-----------------------------------------------------------------

::: client.mk
@@ +408,5 @@
> +# platform.ini are the same, which isn't a problem for Firefox; but
> +# it's not right for anything else, particularly comm-central(et.al).
> +# This portion of Postflight recovers the mozilla's cset and stores it
> +# back into platform.ini.
> +MOZ_REV=$(shell hg -R $(TOPSRCDIR)/mozilla parent --template="{node}" 2>/dev/null)

Doesn't this need quotes around the path to be safe?

@@ +410,5 @@
> +# This portion of Postflight recovers the mozilla's cset and stores it
> +# back into platform.ini.
> +MOZ_REV=$(shell hg -R $(TOPSRCDIR)/mozilla parent --template="{node}" 2>/dev/null)
> +recover-ini:
> +	cat $(OBJDIR)/dist/bin/platform.ini | sed -e "s/^\(SourceStamp=\).*/\1$(MOZ_REV)/" | tee $(OBJDIR)/dist/bin/platform.ini

Will $(DIST) work for $(OBJDIR)/dist or is that also not defined here?

Might be a good idea to replace the SourceRepository in platform.ini as well while you are at it. You can get it with
$(hg -R "$TOPSRCDIR/mozilla" showconfig paths.default 2>/dev/null | sed -e "s/^ssh:/https:/")
(In reply to Edmund Wong (:ewong) from comment #11)
> MOZILLA_SRCDIR isn't defined there.

That's slightly surprising as configure exports it.
(In reply to aleth [:aleth] from comment #18)
> (In reply to Edmund Wong (:ewong) from comment #11)
> > MOZILLA_SRCDIR isn't defined there.
> 
> That's slightly surprising as configure exports it.

Oh wait, it doesn't. Sorry, I misremembered.
(In reply to aleth [:aleth] from comment #17)
> Will $(DIST) work for $(OBJDIR)/dist or is that also not defined here?

The reason I ask is because I don't think it is correct otherwise for OS X universal builds.
(In reply to aleth [:aleth] from comment #20)
> (In reply to aleth [:aleth] from comment #17)
> > Will $(DIST) work for $(OBJDIR)/dist or is that also not defined here?
> 
> The reason I ask is because I don't think it is correct otherwise for OS X
> universal builds.

True..  But $(DIST) is also not defined at that point (from my builds,
at least)
(In reply to aleth [:aleth] from comment #20)
> (In reply to aleth [:aleth] from comment #17)
> > Will $(DIST) work for $(OBJDIR)/dist or is that also not defined here?
> 
> The reason I ask is because I don't think it is correct otherwise for OS X
> universal builds.

Actually it may be OK as you are hooking into postflight and not postflight_all. Probably best to just try it ;)

If you can't reach jcranmer I could rs+ this so we can keep moving. I've looked a bit at what would be needed to get m-c to support platform.ini properly again and I am not sure it will be easy to get r+ for the added complexity, so this seems a workable alternative.

Please check about the quotes because users might have spaces in TOPSRCDIR.
Product: SeaMonkey → MailNews Core
Assignee: nobody → ewong
Attachment #8794460 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8794460 - Flags: review?(Pidgeot18)
Attachment #8795625 - Flags: review?(Pidgeot18)
Comment on attachment 8795625 [details] [diff] [review]
proposed patch (v4)

Review of attachment 8795625 [details] [diff] [review]:
-----------------------------------------------------------------

::: client.mk
@@ +410,5 @@
> +# This portion of Postflight recovers the mozilla's cset and stores it
> +# back into platform.ini.
> +MOZ_REV=$(shell hg -R "$(TOPSRCDIR)/mozilla" parent --template="{node}" 2>/dev/null)
> +recover-ini:
> +	echo "Dist = $(DIST)"

nit: leftover debugging echo
Blocks: 1267523
Comment on attachment 8795625 [details] [diff] [review]
proposed patch (v4)

Review of attachment 8795625 [details] [diff] [review]:
-----------------------------------------------------------------

::: client.mk
@@ +411,5 @@
> +# back into platform.ini.
> +MOZ_REV=$(shell hg -R "$(TOPSRCDIR)/mozilla" parent --template="{node}" 2>/dev/null)
> +recover-ini:
> +	echo "Dist = $(DIST)"
> +	cat $(OBJDIR)/dist/bin/platform.ini | sed -e "s/^\(SourceStamp=\).*/\1$(MOZ_REV)/" | tee $(OBJDIR)/dist/bin/platform.ini

Fallen suggested using $(SED) here and (schematically) sed < platform.ini > platform.ini to avoid tee printing the file.
Attachment #8799262 - Flags: feedback?(philipp)
Attachment #8799262 - Flags: feedback?(ewong)
Assignee: ewong → aleth
This way would avoid hooking into postflight.

(Note $(SED) isn't defined here, and I retained tee as redirecting stdin and stdout to the same file fails.)

Try: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central
Comment on attachment 8799262 [details] [diff] [review]
Alternative approach from the Makefile

Review of attachment 8799262 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/Makefile.in
@@ +35,5 @@
> +
> +MOZ_REV=$(shell hg -R "$(MOZSRCDIR)" parent --template="{node}" 2>/dev/null)
> +
> +libs:: $(DIST)/bin/platform.ini
> +	sed -e "s/^\(SourceStamp=\).*/\1$(MOZ_REV)/" < $(DIST)/bin/platform.ini | tee $(DIST)/bin/platform.ini > /dev/null

I'd actually go with something more obvious here:

sed -e "s/^\(SourceStamp=\).*/\1$(MOZ_REV)/" $(DIST)/bin/platform.ini > $(DIST)/bin/platform.ini~ && mv $(DIST)/bin/platform.ini~ $(DIST)/bin/platform.ini

too bad the -i option is not standardized across platforms and we can't trust to be using gnu sed.
Attachment #8799262 - Flags: feedback?(philipp) → feedback+
Blocks: 1247162
Keywords: regression
Attachment #8799263 - Flags: review?(ewong)
Attachment #8799262 - Attachment is obsolete: true
Attachment #8799262 - Flags: feedback?(ewong)
NB I don't think either of these patches fix the txt file from the bug title, as that is generated independently. We'll need something similar for that.
Attachment #8799359 - Flags: review?(madperson)
Attachment #8799359 - Flags: review?(ewong)
Attachment #8799359 - Flags: review?(madperson) → review?(philipp)
Attachment #8799263 - Flags: review?(philipp)
Attachment #8799384 - Flags: review?(philipp)
Attachment #8799384 - Flags: review?(ewong)
Attachment #8799263 - Attachment is obsolete: true
Attachment #8799263 - Flags: review?(philipp)
Attachment #8799263 - Flags: review?(ewong)
Attachment #8799385 - Flags: review?(philipp)
Attachment #8799385 - Flags: review?(ewong)
Attachment #8799384 - Attachment is obsolete: true
Attachment #8799384 - Flags: review?(philipp)
Attachment #8799384 - Flags: review?(ewong)
Rebased after bitrot
Attachment #8799481 - Flags: review?(philipp)
Attachment #8799481 - Flags: review?(ewong)
Attachment #8799359 - Attachment is obsolete: true
Attachment #8799359 - Flags: review?(philipp)
Attachment #8799359 - Flags: review?(ewong)
Attachment #8799385 - Flags: review?(philipp)
Attachment #8799385 - Flags: review?(ewong)
Attachment #8799385 - Flags: review+
Attachment #8799481 - Flags: review?(philipp) → review+
Comment on attachment 8799385 [details] [diff] [review]
Ensure platform.ini contains the correct m-* source revision

Not sure if you wanted a separate review for suite, removing the r? was an accident. In any case, r=philipp for both patches.
Attachment #8799385 - Flags: review?(ewong)
Comment on attachment 8799481 [details] [diff] [review]
Fix the m-* revision in the sourcestamp file

looks good
Attachment #8799481 - Flags: review?(ewong) → review+
Attachment #8799385 - Flags: review?(ewong) → review+
https://hg.mozilla.org/comm-central/rev/34dd874bb704b51d666dc65223bdeb98fc12d2df
Bug 1298719 - Ensure platform.ini contains the correct m-* source revision. r=ewong,philipp

https://hg.mozilla.org/comm-central/rev/bcc02e1109fa1dceac81d48951adef3a5427b99c
Bug 1298719 - Fix the m-* revision in the sourcestamp file. r=ewong,philipp a=ewong
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 52.0
Attachment #8795625 - Flags: review?(Pidgeot18)
Comment on attachment 8799385 [details] [diff] [review]
Ensure platform.ini contains the correct m-* source revision

[Approval Request Comment]
Regression caused by (bug #): 1247162
Testing completed (on c-c, etc.): see try pushes
Risk to taking this patch (and alternatives if risky): it's all broken at the moment, so things can only get better
Attachment #8799385 - Flags: approval-comm-beta?
Attachment #8799385 - Flags: approval-comm-aurora?
Attachment #8799481 - Flags: approval-comm-beta?
Attachment #8799481 - Flags: approval-comm-aurora?
(In reply to aleth [:aleth] from comment #39)
> https://hg.mozilla.org/comm-central/rev/
> bcc02e1109fa1dceac81d48951adef3a5427b99c
> Bug 1298719 - Fix the m-* revision in the sourcestamp file. r=ewong,philipp
> a=ewong

I suppose "s/^ssh:/http:/" should be "s/^ssh:/https:/".
https://hg.mozilla.org/comm-central/rev/d79282cd0a5db6528eca6f98a41e72008c383c57
Bug 1298719 - Followup typo fix to use https. rs,a=bustage-fix
Attachment #8799385 - Flags: approval-comm-beta?
Attachment #8799385 - Flags: approval-comm-beta+
Attachment #8799385 - Flags: approval-comm-aurora?
Attachment #8799385 - Flags: approval-comm-aurora+
Attachment #8799481 - Flags: approval-comm-beta?
Attachment #8799481 - Flags: approval-comm-beta+
Attachment #8799481 - Flags: approval-comm-aurora?
Attachment #8799481 - Flags: approval-comm-aurora+
>> Can someone explain to me why SM 2.47 as marked as "unaffected", yet there was an uplift request for it.

Sun spots or erroneous mark. My money would be on the later :) Thanks for the correction.
You need to log in before you can comment on or make changes to this bug.