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

Status: VERIFIED FIXED

(MailNews Core :: Build Config, defect)

Description

[Tony Mechelynck [:tonymec]]

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.

Comment 1

[Frank-Rainer Grahl (:frg)]

I believe this is caused bug 1267270. I didn't follow this one closely if a followup bug is open.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thank you Frank-Rainer. Lets ask Aleth who wrote the patch on bug 1267270.

Comment 3

[aleth [:aleth]]

I don't know what generates that txt file, but it probably reads from platform.ini, so this is likely Bug 1297198.

Comment 4

[Tony Mechelynck [:tonymec]]

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.

Comment 5

[aleth [:aleth]]

(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.

Comment 6

[aleth [:aleth]]

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

sorry, I meant Bug 1267270 comment 27.

Comment 7

[Tony Mechelynck [:tonymec]]

(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".

Comment 9

[Edmund Wong (:ewong)]

Attachment: proposed patch

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?

Comment 10

[aleth [:aleth]]

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)

Comment 11

[Edmund Wong (:ewong)]

(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.

Comment 12

[Edmund Wong (:ewong)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b3bd6d190dbb

Comment 13

[Edmund Wong (:ewong)]

Attachment: proposed patch (v2)

Comment 14

[Edmund Wong (:ewong)]

Attachment: proposed patch (v3) (v2 + comments)

Comment 15

[Edmund Wong (:ewong)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2e60bde50cda

Comment 17

[aleth [:aleth]]

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:/")

Comment 18

[aleth [:aleth]]

(In reply to Edmund Wong (:ewong) from comment #11)
> MOZILLA_SRCDIR isn't defined there.

That's slightly surprising as configure exports it.

Comment 19

[aleth [:aleth]]

(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.

Comment 20

[aleth [:aleth]]

(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.

Comment 21

[Edmund Wong (:ewong)]

(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)

Comment 22

[aleth [:aleth]]

(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.

Comment 23

[Edmund Wong (:ewong)]

Attachment: proposed patch (v4)

Comment 24

[aleth [:aleth]]

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

Comment 25

[aleth [:aleth]]

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.

Comment 26

[aleth [:aleth]]

Attachment: Alternative approach from the Makefile

Comment 27

[aleth [: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 28

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

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.

Comment 29

[aleth [:aleth]]

Attachment: Ensure platform.ini contains the correct m-* source revision

Comment 30

[aleth [:aleth]]

Updated try: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=d5bec1e30a664740407da350dc35330cc56be8fc

Comment 31

[aleth [:aleth]]

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.

Comment 32

[aleth [:aleth]]

Attachment: Fix the m-* revision in the sourcestamp file

Comment 33

[aleth [:aleth]]

Worked on try: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=441934ffa48dd526b957fa0fcf296f8375dcede7

Comment 34

[aleth [:aleth]]

Attachment: Ensure platform.ini contains the correct m-* source revision

Comment 35

[aleth [:aleth]]

Attachment: Ensure platform.ini contains the correct m-* source revision

Comment 36

[aleth [:aleth]]

Attachment: Fix the m-* revision in the sourcestamp file

Rebased after bitrot

Comment 37

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

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.

Comment 38

[Edmund Wong (:ewong)]

Comment on attachment 8799481 [details] [diff] [review]
Fix the m-* revision in the sourcestamp file

looks good

Comment 39

[aleth [:aleth]]

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

Comment 40

[aleth [:aleth]]

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

Comment 41

[Takanori MATSUURA]

(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:/".

Comment 42

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/d79282cd0a5db6528eca6f98a41e72008c383c57
Bug 1298719 - Followup typo fix to use https. rs,a=bustage-fix

Comment 43

[Jorg K (CEST = GMT+2)]

Aurora (TB 51, SM 2.48):
https://hg.mozilla.org/releases/comm-aurora/rev/88f589017731a194eb5f890ff2c15617da5ed268
https://hg.mozilla.org/releases/comm-aurora/rev/83942b1cb0724718b52517854635156df038b43c
https://hg.mozilla.org/releases/comm-aurora/rev/6e194c5bf84dd7bf96fbdeda2efd5aea8d5c5a94

Comment 44

[Jorg K (CEST = GMT+2)]

Beta (TB 50, SM 2.47)
https://hg.mozilla.org/releases/comm-beta/rev/6fad5ffd4d96c7ea0a5591431674ab5881f63f64
https://hg.mozilla.org/releases/comm-beta/rev/195a354299cae5bfefa740b64262d783712082a9
https://hg.mozilla.org/releases/comm-beta/rev/cc8bac76b04d5b18669cc1f6048eea2d7809637c

Can someone explain to me why SM 2.47 as marked as "unaffected", yet there was an uplift request for it.

Comment 45

[Frank-Rainer Grahl (:frg)]

>> 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.

Comment 46

[Tony Mechelynck [:tonymec]]

The latest SeaMonkey nightly has the following .txt file, which shows that it doesn't have the bug:

20161025003001
http://hg.mozilla.org/comm-central/rev/2294e347fd430898dfb226cf61b1496cd031d475
http://hg.mozilla.org/mozilla-central/rev/c845bfd0accb

Similarly for Thunderbird:

20161025110801
https://hg.mozilla.org/comm-central/rev/edfe22f3146694cab42ca04046cf8c6dc52e53d5
https://hg.mozilla.org/mozilla-central/rev/c6ccd71126ff514bfc44b53e2217562e29a0cc38

=> VERIFIED