1324838 - Enable TLS 1.3 short header extension in Firefox build

Status: RESOLVED FIXED

(NSS :: Build, defect)

Description

[Eric Rescorla (:ekr)]

Attachment: Makefile.in.txt

This is just a build flag, but we need it in Firefox 51 because that's pre-gyp

Ted, sorry but this is high priority because I need to uplift it as part of 
https://bugzilla.mozilla.org/show_bug.cgi?id=1305970

Comment 1

[Eric Rescorla (:ekr)]

Attachment: make.flags

Comment 2

[Eric Rescorla (:ekr)]

Comment on attachment 8820473 [details] [diff] [review]
make.flags

Review of attachment 8820473 [details] [diff] [review]:
-----------------------------------------------------------------

GPS, this is just frobbing a flag but I need a signoff from a build peer. It goes with:
https://bugzilla.mozilla.org/show_bug.cgi?id=1323104

Comment 3

[Eric Rescorla (:ekr)]

Attachment: make.flags

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 8820474 [details] [diff] [review]
make.flags

Review of attachment 8820474 [details] [diff] [review]:
-----------------------------------------------------------------

::: config/external/nss/Makefile.in
@@ +280,5 @@
>  NSS_XCFLAGS += $(filter -arch:%,$(CFLAGS))
>  endif
>  
> +# Enable short header experiment. Firefox only.
> +NSS_XCFLAGS += -DNSS_ENABLE_TLS13_SHORT_HEADERS

Any reason you went with NSS_XCFLAGS and not DEFAULT_GMAKE_FLAGS? This file is dead in central, so as long as it works, I don't really care about the semantic difference between the 2 variables.

Comment 5

[Eric Rescorla (:ekr)]

Per our IRC discussion, DEFAULT_GMAKE_FLAGS sets Make variables, but I need to set CFLAGS.

Comment 6

[Eric Rescorla (:ekr)]

Comment on attachment 8820474 [details] [diff] [review]
make.flags

Review of attachment 8820474 [details] [diff] [review]:
-----------------------------------------------------------------

Liz, Ritu, per our discussion earlier today, this is the trivial Firefox patch that goes with the NSS uplift which Kai will be doing shortly.

Comment 7

[Julien Cristau [:jcristau]]

Comment on attachment 8820474 [details] [diff] [review]
make.flags

AIUI the rest of this landed earlier today.  Enable tls1.3 short headers in aurora52.

Comment 8

[Carsten Book [:Tomcat]]

ekr landed this as https://hg.mozilla.org/releases/mozilla-aurora/rev/c3f401e19160ed979541940f21ff70a111cc5972 not sure if this need to land on trunk too

Comment 9

[Eric Rescorla (:ekr)]

Thanks for asking. No it does not because the build system changed between 52 and 53

Comment 10

[Eric Rescorla (:ekr)]

Comment on attachment 8820474 [details] [diff] [review]
make.flags

Review of attachment 8820474 [details] [diff] [review]:
-----------------------------------------------------------------

Now that we are uplifting NSS 3.28 to Beta, this should be uplifted as well. It is a no-op until the 3.28 uplift happens so it can happen now.

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8820474 [details] [diff] [review]
make.flags

Let's land this for beta 11 as discussed, as part of the NSS update/ TLS 1.3 move.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a6f1aa3ceb58