462579 - Strip off Private Browsing license headers when building

Status: RESOLVED WONTFIX

(Firefox :: Private Browsing, defect)

Description

[(no longer active)]

As requested in bug 411929 comment 38, we should use preprocessor rules to nuke the license headers for all of the files added in Private Browsing patches when building.  This will possibly make the code which we ship to users a bit more compact.

I'll get to this one after the main parts of the PB work have landed.

Comment 1

[(no longer active)]

Attachment: Patch (v1)

This is the only file which leaves the license headers in the built jar, as far as I can see.

Comment 2

[(no longer active)]

Landing this after beta 2.

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 347140 [details] [diff] [review]
Patch (v1)

a191=beltzner

Comment 4

[(no longer active)]

Attachment: hg bundle for check-in

Comment 5

[Shawn Wilsher :sdwilsh]

What's the point of doing this?  It can't possibly be installer size since a license block is going to be easily compressed.  What it is going to do is make for longer build times, which we've been trying to combat, as well as give useless line numbers for error messages now.

Comment 6

[(no longer active)]

This was a review comment from mconnor on bug 411929 comment 38.  I guess the rationale for taking it is taking up a bit less space on users' hard drives, at the expense of longer build times, and misleading line numbers like you suggest.

I'd WONTFIX this any day, so let's see what mconnor thinks.

Comment 7

[Shawn Wilsher :sdwilsh]

Also, if you build with --enable-chrome-format=symlink and turn off the xul cache then you can edit your tree and see your results without restarting.  Another downer for developers. :(

I'm going to take this out of the queue for the time being.

Comment 8

[:Gavin Sharp [email: [e-mail]]]

We already preprocess a bunch of stuff, there's no use holding back now for "developer ease". Does it really mess up line numbers? I thought that was at least partially fixed...

Comment 9

[(no longer active)]

(In reply to comment #7)
> Also, if you build with --enable-chrome-format=symlink and turn off the xul
> cache then you can edit your tree and see your results without restarting. 
> Another downer for developers. :(

Wow, I didn't know about this!  Nifty trick.  :-)

Comment 10

[Shawn Wilsher :sdwilsh]

(In reply to comment #8)
> We already preprocess a bunch of stuff, there's no use holding back now for
> "developer ease". Does it really mess up line numbers? I thought that was at
> least partially fixed...
I've never had error line numbers reported in preprocessed files correctly.  I know I hit this issue recently too.

Also, the argument that we already preproccess a bunch of stuff, so what's the big deal about preproccessing more doesn't follow.  That's like saying there are already a bunch of murders in the world, so what's the big deal about another one (exaggeration, yes)?  Making developers lives harder with no good reason to do so doesn't seem to make sense to me, which is why I raised the issue.  If there's a good reason to do this, I'll shutup, but this change feels more like "we do this elsewhere, so let's do it here".

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Your analogy is flawed - the incremental cost of murder is high, but the incremental cost of additional preprocessing is very low. Preprocessing in general might cause trouble for developers (though I doubt it's _that_ damaging given that you only heard about that trick on IRC today), but adding one more file to a long list of files already being preprocessed is not going to make a significant impact on developers. In this case specifically, we're *already* preprocessing this file, so the net cost is even lower than it normally would be.

That argument aside, I don't really think it's important that we take this patch. The win is hardly worth the effort or time we're spending arguing about it. We've had similar discussions in the past about removing license headers in a more systematic way, and the cost/benefit ratio has always been rather high. I'm fine with WONTFIXing this.

Comment 12

[(no longer active)]

Mass moving of all Firefox::General private browsing bugs to Firefox::Private Browsing.

Comment 13

[(no longer active)]

I'm WONTFIXing this bug, as I don't see the point for keeping this open any longer, as it doesn't have any kind of priority over existing "real" work.  :-)