463075 - building xul app --with-libxul-sdk fails lacking nspr-config

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Sergey Yanovich]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b2pre) Gecko/20081104 Firefox/3.1b2pre
Build Identifier: trunk

configure.in is hardcoding $(DEPTH)/nsprpub/config/nspr-config into $(NSPR_CFLAGS) and $(NSPR_LIBS), but the file isn't created. Looks like nsprpub/configure isn't called anymore.

This is a regression against 1.9.0

Reproducible: Always




There maybe several solutions:
* actually create nspr-config in the tree in --with-libxul-sdk builds;
* ship nspr-config with sdk and use that one.

Comment 1

[(not currently active) Ted Mielczarek]

Regression from bug 462405.

Comment 2

[Benjamin Smedberg]

In the xulrunner (SDK) side, can we copy nspr-config to dist/lib or someplace like that, and run it from there instead of directly from objdir/nsprpub/config ?

Comment 3

[Sergey Yanovich]

Copying nspr-config to dist/bin, and setting _USE_SYSTEM_NSPR=1 on "${with_libxul_sdk+set}" = set should be enough for uninstalled case.

In that case, nspr-config should get installed to $(libdir) on 'make install'.

Comment 4

[Benjamin Smedberg]

No, we don't want to install NSPR as the system NSPR... the nspr-config should be isolated within sdkdir/lib

Comment 5

[Sergey Yanovich]

For reference, js-config is installed in dist/bin when building. 'make install' puts it into $(prefix)/bin

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: install nspr-config into $(dist_bindir)

This was the intention of revision 1.2.28.8 of nsprpub/config/Makefile.in in 2001, but that was backed out for some tinderbox issue.
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=NSPR&branch=NSPRPUB_CLIENT_BRANCH&branchtype=match&dir=&file=mozilla%2Fnsprpub%2Fconfig%2FMakefile.in&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=all&mindate=&maxdate=&cvsroot=%2Fcvsroot

Probably time to try this again.

Comment 7

[Karl Tomlinson (:karlt)]

(In reply to comment #4)
> No, we don't want to install NSPR as the system NSPR... the nspr-config should
> be isolated within sdkdir/lib

I don't think I fully understand this.
Would installing nspr-config into dist/bin be making it the system NSPR?

Comment 8

[Karl Tomlinson (:karlt)]

Attachment: use nspr-config from $(LIBXUL_DIST)/bin

changes to Mozilla's configure.in to use nspr-config exported by attachment 376174 [details] [diff] [review].

Comment 9

[Karl Tomlinson (:karlt)]

Attachment: install nspr-config into $(dist_bindir)

export needs to depend on $(TARGETS) for nsinstall.

Comment 10

[Karl Tomlinson (:karlt)]

Do we want 1.9.1 to be able to build xul apps without --with-system-nspr?
(Or can xul apps be built without --with-libxul-sdk?)

Comment 11

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Wouldn't block, but would take -- looks like an easy review for Ted.

Comment 12

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

(In reply to comment #10)
> (Or can xul apps be built without --with-libxul-sdk?)

They can be built with:
ac_add_app_options <project> --with-libxul-sdk=<relative path to xulrunner objdir>/dist

in a multi-project build (which is what we're doing for Fennec). This bug is only about using a pre-built SDK, right?

Comment 13

[Karl Tomlinson (:karlt)]

(In reply to comment #12)
> They can be built with:
> ac_add_app_options <project> --with-libxul-sdk=<relative path to xulrunner
> objdir>/dist
> 
> in a multi-project build (which is what we're doing for Fennec).

"ac_add_app_options mobile --with-libxul-sdk=../xulrunner/dist" didn't work for me (with mk_add_options MOZ_BUILD_PROJECTS="xulrunner browser mobile").

Comment 14

[Karl Tomlinson (:karlt)]

Actually, it was probably "ac_add_app_options browser --with-libxul-sdk=../xulrunner/dist" that wasn't working for me.  I probably didn't get as far as mobile.  Maybe mobile doesn't use NSPR?

Comment 15

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

(In reply to comment #14)
> Maybe mobile doesn't use NSPR?

Indeed it doesn't (apart from on WinCE, which doesn't use nspr-config).

Comment 16

[Dave Townsend [:mossop]]

Attachment: rollup patch

Just taken the liberty of rolling up those two patches since both are needed and adding a fix to stop nspr-config being packaged in Firefox on OSX.

Comment 17

[Karl Tomlinson (:karlt)]

What's the process for getting (the NSPR part of these) changes into NSPR?

Comment 18

[Dave Townsend [:mossop]]

Comment on attachment 377979 [details] [diff] [review]
install nspr-config into $(dist_bindir)

Ted, this is identical to the NSPR part of the patch you reviewed and is what needs to land there.

Comment 19

[Dave Townsend [:mossop]]

Attachment: patch avoiding nspr [Checkin: Comment 21 & 25]

We can avoid needing to make changes to NSPR (which will take a while to get onto branch) by simply copying across nspr-config ourselves. The only new part of this patch is in config/nspr/Makefile.in

Comment 20

[Dave Townsend [:mossop]]

We've basically broken the documented ways to build XUL apps with our build system so any of those apps that include their own binary components can't update to 1.9.1 without this. The fix is pretty trivial build config stuff, any regressions should be almost immediately apparent.

Comment 21

[Dave Townsend [:mossop]]

Pushed to trunk: http://hg.mozilla.org/mozilla-central/rev/dc1c75480d85

Comment 22

[Dave Townsend [:mossop]]

Comment on attachment 385091 [details] [diff] [review]
patch avoiding nspr
[Checkin: Comment 21 & 25]

This has been on trunk for nearly a week now with no issues, and it is the sort of thing that would completely break the build if it was wrong, so I think this is safe to take on branch

Comment 23

[Samuel Sidler (old account; do not CC)]

We won't *block* 1.9.1.1 on this, but may take the patch. It's more like that we'll block 1.9.1.2 on it and take it then, however.

Comment 24

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

Comment on attachment 385091 [details] [diff] [review]
patch avoiding nspr
[Checkin: Comment 21 & 25]

a1912=beltzner

Comment 25

[Dave Townsend [:mossop]]

Fixed on 1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d58f788d601b

Comment 26

[Todd Whiteman]

I've noticed there is no "nspr-config" in the xulrunner SDK (for both 1.9.1 and 1.9.2 - at least in the latest Linux nightly builds):
  http://ftp.mozilla.org/pub/mozilla.org/xulrunner/nightly/latest-trunk/
  http://ftp.mozilla.org/pub/mozilla.org/xulrunner/nightly/latest-mozilla-1.9.1/

It seems like there should be right?

Comment 27

[Dave Townsend [:mossop]]

For some reason this is only working on OSX.

Comment 28

[Dave Townsend [:mossop]]

Attachment: fix packaging [Checkin: Comment 29 & 36]

me == fail

Comment 29

[Dave Townsend [:mossop]]

Landed, this should work now: http://hg.mozilla.org/mozilla-central/rev/00470cd4569c

Comment 30

[Karl Tomlinson (:karlt)]

(Thanks, Dave.
 I guess we'll want that change on 1.9.1 too, in a few days maybe.)

Comment 31

[Samuel Sidler (old account; do not CC)]

Dave: Does that patch apply to 1.9.1? Can you request approval if so?

Comment 32

[Phil Ringnalda (:philor)]

Attachment: fix comm-central packaging [Checkin: Comment 33]

Comment 33

[Phil Ringnalda (:philor)]

Comment on attachment 393665 [details] [diff] [review]
fix comm-central packaging
[Checkin: Comment 33]

http://hg.mozilla.org/comm-central/rev/1a8ed893eaf2

Comment 34

[Dave Townsend [:mossop]]

Comment on attachment 393545 [details] [diff] [review]
fix packaging
[Checkin: Comment 29 & 36]

Verified that the mozilla-central nightlies are giving SDKs with nspr-config in it on all platforms, we should take this patch on the branch too

Comment 35

[Samuel Sidler (old account; do not CC)]

Comment on attachment 393545 [details] [diff] [review]
fix packaging
[Checkin: Comment 29 & 36]

Approved for 1.9.1.3. a=ss

Comment 36

[Dave Townsend [:mossop]]

Pushed: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/51445c32fb91