202315 - remove debug UI from final release branches

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect, P1)

Description

[Daniel (Leaf) Nunes]

Open perma-bug to track removing UI that deals with debug functionality in final
release builds.

Comment 1

[Jon Granrose]

lack of final branch milestone targets is lame.

Comment 2

[Asa Dotzler [:asa]]

*poke*

Comment 3

[Michael Lefevre]

This seems to have been done for 1.6 branch now. not resolved because it's
supposed to be a "perma-bug", but I guess fixed1.6 is appropriate.

Comment 4

[Robert Accettura [:raccettura]]

I see the same as Comment 3.  

Also nice to see W32 builds back for /latest-1.6

Comment 5

[chris hofmann]

this has been fixed now so taking off the 1.6+ and moving to 1.7+

Comment 6

[Frédéric Buclin]

Should not Debug and QA menus be deleted in RCs? It is not the case in 1.7 RC1.

Comment 7

[Michael Lefevre]

well RC1 isn't a true candidate for release (there are plans for another 2
release candidates), so it doesn't matter too much.  However, the debug UI
should be removed from the 1.7 branch before the last release candidate.

Comment 8

[Frédéric Buclin]

I understand. In this case, bug 241306 depends (more or less) on this one, right?

Comment 9

[Michael Lefevre]

Not really, no. The release note link and versions are handled separately from
this UI stuff. They both need to happen, but there's no dependency - either can
be fixed without the other.

Comment 10

[Daniel (Leaf) Nunes]

Attachment: remove debug UI for composer/browser, buildid from titlebar, debug prefs

Comment 11

[Daniel (Leaf) Nunes]

Comment on attachment 147047 [details] [diff] [review]
remove debug UI for composer/browser, buildid from titlebar, debug prefs

not sure who i should ask for review, but blizzard's sr should be sufficient.

Comment 12

[chris hofmann]

Comment on attachment 147047 [details] [diff] [review]
remove debug UI for composer/browser, buildid from titlebar, debug prefs

a=chofmann for 1.7, just do it ( since it has been done so many times before
;-)

Comment 13

[Daniel (Leaf) Nunes]

changes committed to 1.7 branch; i will run this morning's builds when they get
done and make sure no additional debug items have cropped up.

Comment 14

[Olivier Vit (just a reporter)]

status need to be updated ?

Comment 15

[Michael Lefevre]

No - it's already marked fixed for 1.7. It will need to be done again on the 1.8
branch.

Comment 16

[Jacek Piskozub]

So the blocking1.7+ flag can go down, cannot it?

Comment 17

[Michael Lefevre]

If someone is offended by it being there, I guess it could.  Usually it's left
as a record of the fact that it was a blocker - the fixed1.7 keyword indicates
that it's been fixed for 1.7.

Comment 18

[Jacek Piskozub]

I believe the whole point of the blocking flags is to find the bugs that still
block a release.

Otherwise, why at present are there only 5 blocking1.7+ bugs when 10 days ago
there were 17 (or something like that)?  

Comment 19

[Michael Lefevre]

There aren't 5 blockers - there are none now.

You're looking at the resolved status of the bug, which relates to whether
the bug is fixed on the trunk or not.  The status of the bug on the branch
is indicated by the "fixed1.7" keyword. The correct queries to use are linked from
http://www.mozilla.org/roadmap/release-status.html

This discussion doesn't belong in this bug anyway - you're welcome to email me
if you want to reply.

Comment 20

[Jacek Piskozub]

I never said there are 5 blockers. I said the drivers actually _do_ remove the
flag after fixing a blocker. The proof is that there are now only 5 blocking1.7+
bugs when recently there were about 20. It seems you read my comment in a very
cursory manner, indeed.

No. I actually do not need your reply. Thanks for the offer, anyway. And sorry
for the spam the above mentioned misunderstanding has created.

Comment 21

[David Epstein]

Verified in Mozilla 1.7 branch. Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.7) Gecko/20040617. Debug UI removed.

Comment 22

[Christopher Aillon (sabbatical, not receiving bugmail)]

Out of curiosity, instead of doing this for every release cycle, why not just
put the debug UI inside #ifndef MOZ_OFFICIAL or whatnot?

Comment 23

[Stefan [:stefanh]]

Fwiw, attachment #147047 [details] [diff] [review] still applies.

Comment 24

[Robert Kaiser]

We should look into that after Beta, maybe we can even wait for a point when the 1.8.1 branch was already cut (dunno when that will be)

Comment 25

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Comment on attachment 147047 [details] [diff] [review]
remove debug UI for composer/browser, buildid from titlebar, debug prefs

first a=me for 1.0, need one more

(tested and it seems to work)

Comment 26

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Checked in on 1.8.0

Comment 27

[Chase Phillips]

Mass reassign of open bugs for [e-mail] to [e-mail].

Comment 28

[Mike Schroepfer]

Chris - sending this bug your way - just to confirm if we need this on the trunk /1.8 branches or is this issue closed?

Comment 29

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

(In reply to comment #28)
> Chris - sending this bug your way - just to confirm if we need this on the
> trunk /1.8 branches or is this issue closed?
> 

As I understand it, we will need this for every release on every branch except 1.8.0.x-based ones.  I was under the impression this is a bug that won't ever get resolved.

Comment 30

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

bienvenu, this is going to affect Thunderbird again.  Is that ok with you?

Comment 31

[David :Bienvenu]

CTho - how does it affect TB? The editor.xul change?

Comment 32

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

(In reply to comment #31)
> CTho - how does it affect TB? The editor.xul change?
> 

I guess so.  Last time I landed it, someone said I affected TB.  I don't remember how exactly.

Comment 33

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Fixed for seamonkey 1.1 on mozilla1.8 branch.

Comment 34

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

standard8, you're bitrotting these patches, aren't you?

Comment 35

[Robert Kaiser]

CTho, his work in bug 381343 is surely bitrotting this as it's trying to finally fix this in a way that we only need to disable an extension and achieve the same :)

Comment 36

[Mark Banner (:standard8)]

(In reply to comment #34)
> standard8, you're bitrotting these patches, aren't you?
> 
Yep, like Robert said, bug 381343 is moving the debug/QA UI and functions to an extension so that all we'll have to do is disable building of an extension for releases, if not automatically.

It'll also mean that our release builds are slightly less bloated by debug functionality (though that is probably small in comparison with the rest of the build - but still worth doing it). The other advantage is that we could publish the new extension on amo so that testers can get debug & qa functions for release builds.

If we're coming to a release and we need to do some commenting out and I haven't finished the extension, then I'll be happy to supply updated patches, but I'm hoping to have the extension "finished" by then.

Comment 37

[Mark Banner (:standard8)]

I believe SeaMonkey now has all the debug and QA code that used to be disabled for releases (by the patch on this bug) in one extension under suite/debugQA.

It can be excluded from the build by commenting out its entry in suite/Makefile.in.

Do we wish to do that normally, or do we want to make building it depend on version.txt containing "pre"? - so for releases (e.g. change of version to 2.0a1) it would automatically not be built, and hence one less thing for us to do at release time?

Comment 38

[Mark Banner (:standard8)]

Attachment: Automatically disable debugQA for non-"pre" builds of suite.

Patch based on my comment 37. This will only build the debugQA extension if "pre" is found in the current version.txt file.

The windows installer will already cope with not having the debugQA extension built, so there should be no issues there.

This would mean that a) we can close this bug, b) we can keep building debugQA on branches (or trunk) until very close to the actual release, c) we don't have to do anything to the build apart from change the version number.

Comment 39

[Robert Kaiser]

Comment on attachment 281099 [details] [diff] [review]
Automatically disable debugQA for non-"pre" builds of suite.

IIRC, we always did ship the debug UI for alpha and beta build as well.

Additionally, it would be really nice to have a simple way for someone doing custom builds to still build the extension, even from release code.

Comment 40

[Mark Banner (:standard8)]

(In reply to comment #39)
> (From update of attachment 281099 [details] [diff] [review])
> IIRC, we always did ship the debug UI for alpha and beta build as well.

IMHO if we're going to ship the debug UI in alpha and beta builds then we should be shipping it finals as well. Reasoning: alpha and beta builds are previews of what will be in the final builds. Its very rare you'd remove a feature between beta and final. If we ship debug UI for alpha and beta, and not final, then I can see some confused users coming.

> Additionally, it would be really nice to have a simple way for someone doing
> custom builds to still build the extension, even from release code.

Agreed, any suggestions on how we do this within the limits of the build system?

Comment 41

[Robert Kaiser]

I think we shipped alphas and betas with the debug UI as long as we've had those for the suite, and I know for sure we did so for SeaMonkey. I don't remember any complaints about not having it in stable releases.

For enabling it, we could do that with some Makefile variable, probably.

Comment 42

[Mark Banner (:standard8)]

Comment on attachment 281099 [details] [diff] [review]
Automatically disable debugQA for non-"pre" builds of suite.

I'm not sure of how we want to deal with this currently, or of the best fix for it. Maybe we should just leave this bug open and patch it manually each time.

Comment 43

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

-> somebody else

Comment 44

[Robert Kaiser]

Attachment: only build debugQA in alpha and nightly builds

I came across this again and had an idea for a way to do this without even calling into a subshell, and for alphas and nightlies (i.e. those with "pre" in the version) only.
Here is the patch to do that, I tested it with a couple of different version strings.

Comment 45

[[e-mail]]

Comment on attachment 388091 [details] [diff] [review]
only build debugQA in alpha and nightly builds

>+ifneq (,$(filter xxx,$(subst a, xxx ,$(subst pre, xxx ,$(MOZ_APP_VERSION)))))
Does $(filter a pre,$(MOZ_APP_VERSION)) work?

Comment 46

[Stefan [:stefanh]]

So, this means that debugQA won't be in the upcoming beta, right? Hmm, then we need to fix bug 457548...

Comment 47

[Robert Kaiser]

(In reply to comment #45)
> (From update of attachment 388091 [details] [diff] [review])
> >+ifneq (,$(filter xxx,$(subst a, xxx ,$(subst pre, xxx ,$(MOZ_APP_VERSION)))))
> Does $(filter a pre,$(MOZ_APP_VERSION)) work?

No, as filter works on space-separated words.

(In reply to comment #46)
> So, this means that debugQA won't be in the upcoming beta, right? Hmm, then we
> need to fix bug 457548...

SeaMonkey always removed the debug UI for betas. And I thought  everything that could have been done so far has been done for that bug? If not, the beta will ship with the bug unless you fix it before Tuesday.

Comment 48

[Stefan [:stefanh]]

(In reply to comment #47)

> SeaMonkey always removed the debug UI for betas.

SeaMonkey have not "always removed debug UI for betas" according to your comment #41.

> And I thought  everything that
> could have been done so far has been done for that bug? If not, the beta will
> ship with the bug unless you fix it before Tuesday.

I requested blocking-2.0 in October 2008 (no reaction) because I assumed debug UI was not going to be removed until we shipped. If I had known that it was going to be removed for beta I would have fixed the bug for a long time ago :(

Comment 49

[Robert Kaiser]

(In reply to comment #48)
> SeaMonkey have not "always removed debug UI for betas" according to your
> comment #41.

I think it's right to only ship it in alphas and nightlies, esp. as that means that betas make a testing ground for things like the window title stuff you brought up.

> I requested blocking-2.0 in October 2008 (no reaction) because I assumed debug
> UI was not going to be removed until we shipped. If I had known that it was
> going to be removed for beta I would have fixed the bug for a long time ago :(

And I didn't think of it as I for one thing am not on a Mac and for the other, I thought all the patching in that bug had fixed the issue already.

Comment 50

[[e-mail]]

(In reply to comment #47)
> (In reply to comment #45)
> > (From update of attachment 388091 [details] [diff] [review] [details])
> > >+ifneq (,$(filter xxx,$(subst a, xxx ,$(subst pre, xxx ,$(MOZ_APP_VERSION)))))
> > Does $(filter a pre,$(MOZ_APP_VERSION)) work?
> No, as filter works on space-separated words.
I see I misunderstood, and I reread the manual, and perhaps this works:
($filter %a %pre,$(MOZ_APP_VERSION))

Comment 51

[[e-mail]]

Does this affect the installer at all?

Comment 52

[Robert Kaiser]

(In reply to comment #50)
> I see I misunderstood, and I reread the manual, and perhaps this works:
> ($filter %a %pre,$(MOZ_APP_VERSION))

It works for the "pre" part, but not for the "a", a version of e.g. 2.0a1 doesn't trigger it.

(In reply to comment #51)
> Does this affect the installer at all?

The installer is made to degrade gracefully if debugQA isn't built, from all I know.

Comment 53

[[e-mail]]

Comment on attachment 388091 [details] [diff] [review]
only build debugQA in alpha and nightly builds

>+# replacing the "a" or "pre" in the version with <sacpe>xxx<space> enables us
OK, so make sucks, but I don't like xxx - maybe <space>debugQA</space>?

P.S. Actually I think $(findstring a,$(MOZ_APP_VERSION:pre=a)) works.

Comment 54

[Robert Kaiser]

Attachment: only build debugQA in alpha and nightly builds, v1.1

OK, Neil's last suggestion works, thanks. Forwarding r+ and requesting approval for 2.0b1.

Comment 55

[Robert Kaiser]

Pushed as http://hg.mozilla.org/comm-central/rev/57a5d25e6406

This should finally fix this bug once and for all. yay!