1391467 - Remove --enable-necko-protocols

Status: RESOLVED FIXED

(Core :: Networking, enhancement)

Description

[u408661]

This build config option is totally broken with e10s (ipdl doesn't support ifdef, so we can't conditionally compile in protocol support there). So, let's just get rid of this build option (and all the #defines it's responsible for).

Comment 2

[Enrico Weigelt, metux IT consult]

Please, don't. 

Instead we should teach xpidlgen to use the preprocessor.

I've already did some investigations on that, but haven't sorted out
how to properly retrieve the build config (more precisely: how to
get the currrent obj-* dir pathname) - any advice is highly appreciated.

xpidlgen just has to filter the *.idl's through CPP, then we can easily
fix problems like #1383587 via #ifdef. That also gives lots of other
possibilities (anything you could do w/ preprocessor ;-)). 

I already have working patches (not just for ftp, but lots of other things)
here, just yet missing the xpidlgen part.

The really good thing w/ such switches is that you can easily customize
for scenarios where you don't want certain things (for whatever reason).

I don't ask you to officially support this, but please don't kill it entirely.

Comment 3

[u408661]

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

Comment 4

[u408661]

Attachment: Bug 1391467 - Remove --enable-necko-protocols.

This patch removes the ability to select which protocols you want
included in necko, a wholly untested configuration that is broken in
practice. We have no need of this kind of configurability in necko.

In addition, this removes the final vestiges of rtsp support, which was
originally removed in bug 1295885 but still had some stuff hanging
around behind some ifdefs (that were never true).

Review commit: https://reviewboard.mozilla.org/r/170334/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/170334/

Comment 5

[Gregory Szorc [:gps]]

Comment on attachment 8898997 [details]
Bug 1391467 - Remove --enable-necko-protocols.

https://reviewboard.mozilla.org/r/170334/#review175590

Always happy to review a patch that deletes code :)

Hopefully this is obvious, but my r+ is for the build system bits, not the C++. I also didn't look at the moz.build changes in too much detail, as we typically defer review of those to the module they relate to. But the changes seemed reasonable.

Comment 6

[u408661]

(In reply to Gregory Szorc [:gps] from comment #5)
> Comment on attachment 8898997 [details]
> Bug 1391467 - Remove --enable-necko-protocols.
> 
> https://reviewboard.mozilla.org/r/170334/#review175590
> 
> Always happy to review a patch that deletes code :)

They're my favorite patches to write! :)

> Hopefully this is obvious, but my r+ is for the build system bits, not the
> C++. I also didn't look at the moz.build changes in too much detail, as we
> typically defer review of those to the module they relate to. But the
> changes seemed reasonable.

Indeed. I originally asked mcmanus, but he's on PTO this week, and I'd like to get this landed, so redirect to valentin.

Comment 7

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8898997 [details]
Bug 1391467 - Remove --enable-necko-protocols.

https://reviewboard.mozilla.org/r/170334/#review176030

This is some great stuff! Thanks!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ce6e2f914c96
Remove --enable-necko-protocols. r=gps,valentin

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/ce6e2f914c96