341654 - search bookmarks broken on trunk if you do --disable-places

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[(not reading, please use [e-mail] instead)]

search bookmarks broken on trunk if you do --disable-places.  (see related bug #336488)

note, I've already landed my fix for #336488 on the trunk.

the issue may be that we are not building localsearch service for firefox / trunk / disable places.

Comment 1

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

(In reply to comment #0)
> the issue may be that we are not building localsearch service for firefox /
> trunk / disable places.

Indeed, that is the issue.

I don't think that we want to fix this, since --disable-places is a temporary measure that was introduced to ease the landing of Places, and isn't officially supported.

Comment 2

[(not reading, please use [e-mail] instead)]

we are planning to disable places very soon, so I'll go fix this ASAP.

Comment 3

[(not reading, please use [e-mail] instead)]

Attachment: patch

note, for the 1.8 branch, this was done in mozilla/Makefile.in:

ifneq (,$(filter browser suite,$(MOZ_BUILD_APP)))
tier_99_dirs += xpfe/components/search
endif

Comment 4

[(not reading, please use [e-mail] instead)]

note, when I build xpfe/components/search on the Mac trunk, I get the following warnings:

1)

/Users/sspitzer/Desktop/trunk-no-places/mozilla/xpfe/components/search/src/nsInternetSearchService.cpp: In member function 'nsresult InternetSearchDataSource::AddSearchEngineInternal(const char*, const char*, const PRUnichar*, const PRUnichar*, nsIRDFResource*)':
/Users/sspitzer/Desktop/trunk-no-places/mozilla/xpfe/components/search/src/nsInternetSearchService.cpp:2326: warning: enumeral mismatch in conditional expression: 'nsIInternetSearchContext::<anonymous enum>' vs 'nsIInternetSearchContext::<anonymous enum>'
/Users/sspitzer/Desktop/trunk-no-places/mozilla/xpfe/components/search/src/nsInternetSearchService.cpp:2348: warning: enumeral mismatch in conditional expression: 'nsIInternetSearchContext::<anonymous enum>' vs 'nsIInternetSearchContext::<anonymous enum>'

2)

/usr/bin/ld: warning multiple definitions of symbol _poll
../../../../dist/bin/libnspr4.dylib(unix.o) definition of _poll
/usr/lib/gcc/i686-apple-darwin8/4.0.1/../../../libSystem.dylib(poll.So) definition of _poll

note, I see the first warning on the branch.

Comment 5

[(not reading, please use [e-mail] instead)]

fixed, using r=vlad.  (Would still like bsmedmerg's opinion on the Makefile.in change I made.)

Comment 6

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

So I'm not sure if this is the right fix now... this happens on gaius, it looks like xpfe/components/search depends on browser/components/bookmarks, but browser/components/bookmarks isn't even touched by this point?

/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/build/cygwin-wrapper ../../../../dist/bin/xpidl.exe -m typelib -w -I/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/xpfe/components/search/public -I../../../../dist/idl -e _xpidlgen/nsISearchService.xpt -d .deps/nsISearchService.pp /cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/xpfe/components/search/public/nsISearchService.idl
/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/build/cygwin-wrapper ../../../../dist/bin/xpt_link.exe _xpidlgen/search.xpt _xpidlgen/nsISearchService.xpt 
/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/build/cygwin-wrapper /home/cltbld/vc8-moztools/bin/nsinstall -m 644 _xpidlgen/search.xpt ../../../../dist/bin/components
make[7]: Leaving directory `/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/xpfe/components/search/public'
make[7]: Entering directory `/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/xpfe/components/search/src'
nsInternetSearchService.cpp
Building deps for nsInternetSearchService.cpp
/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/build/cygwin-wrapper cl -FonsInternetSearchService.obj -c  -DXPCOM_TRANSLATE_NSGM_ENTRY_POINT=1 -DMOZILLA_INTERNAL_API -D_IMPL_NS_GFX -D_IMPL_NS_MSG_BASE -D_IMPL_NS_WIDGET  -DOSTYPE=\"WINNT5.2\" -DOSARCH=\"WINNT\" -DBUILD_ID=2006092521  -I../../../../dist/include/xpcom -I../../../../dist/include/string -I../../../../dist/include/rdf -I../../../../dist/include/necko -I../../../../dist/include/pref -I../../../../dist/include/uconv -I../../../../dist/include/intl -I../../../../dist/include/unicharutil -I../../../../dist/include/bookmarks -I../../../../dist/include   -I../../../../dist/include/appcomps -I../../../../dist/include/nspr          -GR- -TP -nologo -Zc:wchar_t- -W3 -Gy -FdnsInternetSearchService.pdb  -DNDEBUG -DTRIMMED -Zi -O1 -UDEBUG -DNDEBUG -MD            -D_CRT_SECURE_NO_DEPRECATE=1 -D_CRT_NONSTDC_NO_DEPRECATE=1 -DWINVER=0x500 -D_WIN32_WINNT=0x500 -DX_DISPLAY_MISSING=1 -DMOZILLA_VERSION=\"1.9a1\"!
  -DMOZILLA_
VERSION_U=1.9a1 -DHAVE_SNPRINTF=1 -D_WINDOWS=1 -D_WIN32=1 -DWIN32=1 -DXP_WIN=1 -DXP_WIN32=1 -DHW_THREADS=1 -DSTDC_HEADERS=1 -DWIN32_LEAN_AND_MEAN=1 -DNO_X11=1 -D_X86_=1 -DD_INO=d_ino -DMOZ_EMBEDDING_LEVEL_DEFAULT=1 -DMOZ_EMBEDDING_LEVEL_BASIC=1 -DMOZ_EMBEDDING_LEVEL_MINIMAL=1 -DMOZ_PHOENIX=1 -DMOZ_BUILD_APP=browser -DMOZ_XUL_APP=1 -DMOZ_DEFAULT_TOOLKIT=\"cairo-windows\" -DMOZ_THEBES=1 -DMOZ_CAIRO_GFX=1 -DMOZ_DISTRIBUTION_ID=\"org.mozilla\" -DOJI=1 -DIBMBIDI=1 -DMOZ_VIEW_SOURCE=1 -DACCESSIBILITY=1 -DMOZ_XPINSTALL=1 -DMOZ_JSLOADER=1 -DNS_PRINTING=1 -DNS_PRINT_PREVIEW=1 -DMOZ_NO_XPCOM_OBSOLETE=1 -DMOZ_XTF=1 -DMOZ_MATHML=1 -DMOZ_ENABLE_CANVAS=1 -DMOZ_SVG=1 -DMOZ_SVG_FOREIGNOBJECT=1 -DMOZ_UPDATE_CHANNEL=nightly -DMOZ_FEEDS=1 -DMOZ_STORAGE=1 -DMOZ_SAFE_BROWSING=1 -DMOZ_URL_CLASSIFIER=1 -DMOZ_LOGGING=1 -DMOZ_USER_DIR=\"Mozilla\" -DHAVE_UINT64_T=1 -DMOZ_XUL=1 -DMOZ_PROFILELOCKING=1 -DMOZ_RDF=1 -DMOZ_MORK=1 -DMOZ_DLL_SUFFIX=\".dll\" -DJS_THREADSAFE=1 -DMOZILLA_LOCALE_VERSION=\"1.9a1\!
 " -DMOZILLA
_REGION_VERSION=\"1.9a1\" -DMOZILLA_SKIN_VERSION=\"1.8\"  -D_MOZILLA_CONFIG_H_ -DMOZILLA_CLIENT /cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/xpfe/components/search/src/nsInternetSearchService.cpp
nsInternetSearchService.cpp
c:/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/xpfe/components/search/src/nsInternetSearchService.cpp(80) : fatal error C1083: Cannot open include file: 'nsIBookmarksService.h': No such file or directory
make[7]: *** [nsInternetSearchService.obj] Error 2
make[7]: Leaving directory `/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/xpfe/components/search/src'
make[6]: *** [libs] Error 2
make[6]: Leaving directory `/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/xpfe/components/search'
make[5]: *** [libs] Error 2
make[5]: Leaving directory `/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/xpfe/components'
make[4]: *** [libs] Error 2
make[4]: Leaving directory `/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/xpfe'
make[3]: *** [libs_tier_toolkit] Error 2
make[3]: Leaving directory `/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla'
make[2]: *** [tier_toolkit] Error 2
make[2]: Leaving directory `/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla'
make[1]: *** [alldep] Error 2
make[1]: Leaving directory `/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla'
make: *** [alldep] Error 2

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Just backed this out; I'm not sure of the new toplevel Makefile.in bits, so I dunno where the fix should go...  I understand what the issue is better now, though!

Comment 8

[(not reading, please use [e-mail] instead)]

sorry about this vlad.

I wonder if I did something foolish like put search before bookmarks in the DIRS order?

Comment 9

[(not reading, please use [e-mail] instead)]

in bug #336488 comment #13, gavin wrote:  "Firefox bookmarks search depends on code in xpfe/components/search".

looking at the ff2 build log to see how this is achieved.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Probably blocking 1.9a1.

Comment 11

[(not reading, please use [e-mail] instead)]

for bug #317107, gavin implemented a new search service, so we don't need to build xpfe/components/search/src/nsInternetSearchService.cpp (the suite needs it, as it uses rdf:internetsearch, but firefox doesn't need it)

but for non-places bookmarks searching, we need the to build xpfe/components/search/src/nsLocalSearchService.cpp, as rdf:localsearch is still used.

so my plan is to build xpfe/components/search (if MOZ_PHOENIX and not PLACES), but not build nsInternetSearchService.cpp if MOZ_PHOENIX.

testing this now...

Comment 12

[(not reading, please use [e-mail] instead)]

that's not going to work, because the XPCOM module stuff all lives in nsInternetSearchService.cpp.

Comment 13

[(not reading, please use [e-mail] instead)]

Attachment: fix

Comment 14

[(not reading, please use [e-mail] instead)]

Attachment: diff from one level higher, to include the change to mozilla/xpfe/components/Makefile.in

Comment 15

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 240409 [details] [diff] [review]
diff from one level higher, to include the change to mozilla/xpfe/components/Makefile.in

looks fine, thanks!

Comment 16

[(not reading, please use [e-mail] instead)]

I checked in the fix for this on 2006-09-28 12:07, but forgot to mark it fixed.

Comment 17

[Adam Guthrie]

Verified FIXED using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060929 Minefield/3.0a1.

Comment 18

[Benjamin Smedberg]

Attachment: Fix build tiers, rev. 1

This bug undid bug 288640... search should be built in application tiers, not toolkit.

Comment 19

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

Comment on attachment 249382 [details] [diff] [review]
Fix build tiers, rev. 1

>Index: browser/build.mk

>+DIRS += xpfe/components/search

Should be tier_app_dirs.

Comment 20

[Benjamin Smedberg]

build tier fix committed.

Comment 21

[Phil Ringnalda (:philor)]

Was there a followup filed on the way attachment 249382 [details] [diff] [review] started Firefox-on-XULRunner burning, that I just can't find?