Port |Bug 513924 - remove tons of options from configure| to comm-central

RESOLVED FIXED in Thunderbird 3.3a1

Status

MailNews Core
Build Config
--
trivial
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: sgautherie, Assigned: Takanori MATSUURA)

Tracking

Trunk
Thunderbird 3.3a1
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

Comment hidden (empty)
(Reporter)

Comment 1

7 years ago
This would require a huge checking to add relevant "ifdef 192" :-(

Bug 513709 has already removed some of these vars.
(Reporter)

Updated

7 years ago
Whiteboard: [needs c-1.9.2 branch]
(Reporter)

Comment 2

7 years ago
Created attachment 426937 [details] [diff] [review]
(Av0-WIP1) Just copy it

This ports all of autoconf.mk.in, config.mk and rules.mk.
Will need to port the rest of configure.in too.

This is not meant for (future) "c-1.9.2": needs to branch first!

Comment 3

7 years ago
Those don't sound too bad for me to do them with ifdefs, actually. I guess you just don't want to do it that way?
(Reporter)

Comment 4

7 years ago
(In reply to comment #3)

As I implied in comment 1, I don't want to check what's needed to keep or not on 1.9.2.
But feel free to do it if you want.
(Reporter)

Updated

7 years ago
Depends on: 516758
(Assignee)

Comment 5

7 years ago
Created attachment 454466 [details] [diff] [review]
[WIP] Sync for comm-central

After checking in the attachment 454457 [details] [diff] [review] in bug 516758, I'll regenerate this patch.
(Assignee)

Comment 6

7 years ago
Created attachment 454467 [details] [diff] [review]
[WIP] Sync for comm-central v2

leaky was added by too old CVS 1.503 in mozilla. However comm-central don't have leaky.

This patch will remove boehm leak detector and add leaky memory tool.https://developer.mozilla.org/en/Debugging_memory_leaks#Leaky
Attachment #454466 - Attachment is obsolete: true
(Assignee)

Comment 7

7 years ago
I file a new bug 575509 for porting bug 516758.

I suggest "Depends on" -516758 +575509.
(Assignee)

Updated

7 years ago
Depends on: 575509
No longer depends on: 516758
(Assignee)

Comment 8

7 years ago
Created attachment 462031 [details] [diff] [review]
Sync for comm-central v3

Update Serge's patch for the latest comm-central.
And additional sync to attachment 399009 [details] [diff] [review] in bug 513924.

Serge, do you have any comments?
If the patch is OK for you, I'll add a flag for review request.
Attachment #426937 - Attachment is obsolete: true
Attachment #454467 - Attachment is obsolete: true
(In reply to comment #8)
> Serge, do you have any comments?
> If the patch is OK for you, I'll add a flag for review request.

Serge has had real life acting up lately, and has been [mostly] awol from mozilla stuff, not sure if waiting on an OK from him first is the best use of time here, but I'll leave the choice up to you for now.
(Assignee)

Comment 10

7 years ago
(In reply to comment #9)
I call Serge only because the initial patch is generated by him.
I have had a plan to request review to you after Serge's reply.

If you can review the attachment 462031 [details] [diff] [review], it would be happy.
(Assignee)

Updated

7 years ago
Attachment #462031 - Flags: review?(bugspam.Callek)
Comment on attachment 462031 [details] [diff] [review]
Sync for comm-central v3

O whops, I bitrotted your rules.mk change in Bug 558518. I'll review this regardless tonight. Whoever gets "done" first (as in landed) I'll be sure to correct the patch for the other bug.
Comment on attachment 462031 [details] [diff] [review]
Sync for comm-central v3

(In reply to Bug 513924 comment #17)
> Comment on attachment 399009 [details] [diff] [review]
> Patch v1.1
> 
>  if test "$MOZ_VIEW_SOURCE"; then
>      AC_DEFINE(MOZ_VIEW_SOURCE)
>  fi
> 
> Feels like we could just remove this completely and scrub the codebase. I'm
> fine with doing it in a followup, though.

With this please *either* remove this from us now, or file a followup for ted. (We can address both c-c and m-c in said followup) [if you remove now, we need it gone from autoconf.mk too]

>  if test "$MOZ_JSLOADER"; then
>      AC_DEFINE(MOZ_JSLOADER)
>  fi
> 
> Same with this.

Same.

Kill the |AC_SUBST(MOZ_NO_INSPECTOR_APIS)| line while your removing the option from our configure, nothing in our side of the codebase needs it

Also remove |XPCOM_USE_LEA| from autoconf.mk.in as well.

----

r+ with those things addressed, please attach a new patch and mark checkin-needed in the keywords. (Sorry I didn't get to this as early as I had hoped)
Attachment #462031 - Flags: review?(bugspam.Callek) → review+

Updated

7 years ago
Whiteboard: [needs c-1.9.2 branch]
Self note: Need to file Bugs for GC_LEAK_DETECTOR for directory/c-sdk, and nspr (including mozilla/config/nspr/build.mk)
(Reporter)

Comment 14

7 years ago
Comment on attachment 462031 [details] [diff] [review]
Sync for comm-central v3

(In reply to comment #6)
> leaky was added by too old CVS 1.503 in mozilla. However comm-central don't
> have leaky.

It doesn't because I removed it in
http://hg.mozilla.org/comm-central/rev/97196906ae5d
(and I don't see why it would need to be readded.)
Attachment #462031 - Flags: feedback-
(Reporter)

Updated

7 years ago
Assignee: nobody → t.matsuu
Status: NEW → ASSIGNED
Target Milestone: Future → ---
(Assignee)

Comment 15

7 years ago
Created attachment 466288 [details] [diff] [review]
Sync for comm-central v3 with address comment #12

(In reply to comment #12)
For MOZ_VIEW_SOURCE and MOZ_JSLOADER issue, I removed them from configure.in and config/autoconf.mk.in because definitions are not used in c-c part.

I also removed
MOZ_NO_INSPECTOR_APIS and XPCOM_USE_LEA from config/autoconf.mk.in
and
MOZ_NO_INSPECTOR_APIS from configure.in

(In reply to comment #14)
> It doesn't because I removed it in
> http://hg.mozilla.org/comm-central/rev/97196906ae5d
> (and I don't see why it would need to be readded.)

Sorry. I miss it.
I removed MOZ_LEAKY part from my previous patch.
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/311ea60cc2b8
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.2a1
(In reply to comment #13)
> Self note: Need to file Bugs for GC_LEAK_DETECTOR for directory/c-sdk, and nspr
> (including mozilla/config/nspr/build.mk)

Bug 589505 and Bug 589504 respectively.

(In reply to comment #12)
> Comment on attachment 462031 [details] [diff] [review]
> Sync for comm-central v3
> 
> (In reply to Bug 513924 comment #17)
> > Comment on attachment 399009 [details] [diff] [review] [details]
> > Patch v1.1
> > 
> >  if test "$MOZ_VIEW_SOURCE"; then
...
> > Feels like we could just remove this completely and scrub the codebase. I'm
> > fine with doing it in a followup, though.

Bug 589506

> >  if test "$MOZ_JSLOADER"; then
...
> > Same with this.

Same bug
You need to log in before you can comment on or make changes to this bug.