536451 - [OS/2] cannot handle mozsqlite3.dll

Status: VERIFIED FIXED

(Toolkit :: Storage, defect)

Description

[Walter Meinl]

User-Agent:       Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.9.3a1pre) Gecko/20091221 Minefield/3.7a1pre
Build Identifier: 

On OS/2 it's a must that dll follow the 8+3 naming convention. Since the internal sqlite3.dll was renamed to mozsqlite3.dll, the latter dll cannot be used. The way to work around is to use SHORT_LIBNAME. In the patch I'll attach I suggest the designation mozsqlt3.dll.


Reproducible: Always

Comment 1

[Walter Meinl]

Attachment: USE short_libname and move short_libname below import_library in rules.mk

The patch does beside using a short_libname for mozsqlite3.dll move the short_libname definition in rules.mk below the import_library definition in order to prevent that the import_library name gets shortened as well. Since import_library names are allowed to have more than 8 alphanumerics, it is not necessary to shorten them, the same applies for static libs. If we would let rules.mk as it is, we would have to have an extra definition for the link line, because it wants to link against -lmozsqlite3.lib. I think the move in rules.mk would help us also in possible future renamings of dlls, when a component links against the corresponding importlib, cause then we would have only to define a short_libname in the respective Makefile and let the name of the import_library alone.
Here for example the import_library gets expanded to mozsqlite3.lib, the final dll, it's map and def file names are mozsqlt3.dll mozsqlt3.map and mozsqlt3.def, respectively.
It appears to work, Dave you are probably most experienced with linking under gcc. You think, it's correct, that import_libnames are different than those of the dlls?

Comment 2

[Dave Yeo]

(In reply to comment #1)
> Dave you are probably most experienced with linking under
> gcc. You think, it's correct, that import_libnames are different than those > of the dlls?

Yes, the import library and static libs need the long name for linking to find the import (static) library. As long as the import library internally refers to the short name everything is fine.
The static part of the patch is correct as long as the OS/2 makefiles refer to the long name when linking. I haven't actually looked but if it is working for you then the makefiles must be being generated with the right name.
Quickly looking, a lot of the static library names are defined in various makefile.in with the _s suffix.
Anyways I'll apply your patch to my half built tree and see if it causes problems with the static libraries.

Comment 3

[Rich Walsh]

I just tried a debug build with --disable-libxul (which used to be the debug default) and got this error trying to link xpcom.dll: 
  weakld: cannot open library file 'G:\TK45\LIB\xpcomcor_s.a'.
On the command line, the library was specified by its shortname, 'xpcomcor', rather than 'xpcom_core'.

FWIW... with libxul enabled, an earlier build with this patch worked OK.

Comment 4

[Walter Meinl]

(In reply to comment #3)
> I just tried a debug build with --disable-libxul (which used to be the debug
> default) and got this error trying to link xpcom.dll: 
>   weakld: cannot open library file 'G:\TK45\LIB\xpcomcor_s.a'.
> On the command line, the library was specified by its shortname, 'xpcomcor',
> rather than 'xpcom_core'.
> 
> FWIW... with libxul enabled, an earlier build with this patch worked OK.

Ah, I've looked in xpcom/stub/Makefile.in
105 ifeq ($(OS_TARGET),OS2)
106 EXTRA_DSO_LIBS = xpcomcor
107 DEPENDENT_LIBS_LIST += xpcomcor.dll
108 else
109 EXTRA_DSO_LIBS = xpcom_core
110 DEPENDENT_LIBS_LIST += $(LIB_PREFIX)xpcom_core$(DLL_SUFFIX)
111 endif

Here we specify the short_libname explicitely, cause rules.mk without the patch here shortens also the importlib names. We have to fix it with the new patch to look like

109 EXTRA_DSO_LIBS = xpcom_core
105 ifeq ($(OS_TARGET),OS2)
107 DEPENDENT_LIBS_LIST += xpcomcor.dll
108 else
110 DEPENDENT_LIBS_LIST += $(LIB_PREFIX)xpcom_core$(DLL_SUFFIX)
111 endif
Rich, can you try whether this works (hopefully you still have the broken build around, then a make -C objdir/xpcom/stub should give you a quick answer).
I'll attach an update of the patch later.

Comment 5

[Walter Meinl]

Ok, we would have to change in configure.in as well after DYNAMIC_XPCOM_LIBS to read "$(LIBXUL_DIST)/lib/xpcom_core.lib" rather than "$(LIBXUL_DIST)/lib/xpcomcor.lib"

Comment 6

[Rich Walsh]

Attachment: modify short_libname handling

MY build is still running but it got past xpcom.dll OK.  Beside the changes you recommended, I found a few more that seemed advisable.

In xpcom/stub/makefile.in which we were already changing, DEPENDENT_LIBS_LIST had a reference to mozsqlite3 which I modified.  Also, js/src/configure.in had a reference to xpcomcor which I changed to match the main configure.in.

Comment 7

[Rich Walsh]

Ooops... I changed the assignee by accident - I'm restoring it to Walter.

Comment 8

[Walter Meinl]

(In reply to comment #6)
> Created an attachment (id=419017) [details]
> modify short_libname handling
> 
> MY build is still running but it got past xpcom.dll OK.  Beside the changes you
> recommended, I found a few more that seemed advisable.
> 
> In xpcom/stub/makefile.in which we were already changing, DEPENDENT_LIBS_LIST
> had a reference to mozsqlite3 which I modified.
Yeah, strange that this didn't affect the build. When I wrote the patch, I'd expected that it would make trouble, but as it didn't (at least in non-debug builds) I forgot to correct it later.

>  Also, js/src/configure.in had
> a reference to xpcomcor which I changed to match the main configure.in.
Yes, the configure.in have to be as much in sync as possible (just hadn't the time this morning to overhaul the patch ;-).

Nevertheless, we must not forget comm-central builds based on mozilla-central. I'll add an attachment probably after the holidays. Merry Christmas everyone.

Comment 9

[Dave Yeo]

(In reply to comment #6)
> In xpcom/stub/makefile.in which we were already changing, DEPENDENT_LIBS_LIST 
> had a reference to mozsqlite3 which I modified.

This,

+ifeq ($(OS_TARGET),OS2)
+DEPENDENT_LIBS_LIST += mozsqlt3.dll
+else
+DEPENDENT_LIBS_LIST += $(LIB_PREFIX)mozsqlite3$(DLL_SUFFIX)
+endif

should be this, allowing us to change something like $LIB_PREFIX in the future

+ifeq ($(OS_TARGET),OS2)
+DEPENDENT_LIBS_LIST += $(LIB_PREFIX)mozsqlt3$(DLL_SUFFIX)
+else
+DEPENDENT_LIBS_LIST += $(LIB_PREFIX)mozsqlite3$(DLL_SUFFIX)
+endif

Really the whole library naming is screwed up, the above should have $SHARED_LIBRARY_NAME or $SHORT_LIBNAME instead of a hard coded name.

Comment 10

[Walter Meinl]

Attachment: modify short_libname, strip an additonal line [Checkin: Comment 17]

Rich I found that your patch contained a line too much in js/src/configure.in as there is still short_libname used also for static libs.

Comment 11

[Walter Meinl]

Attachment: fix for c-c to find xpcom_core.lib with m-c [Checkin: Comment 18]

This should fix SM and TB c-c builds overlayed to m-c (or 1.9.2)

Comment 12

[(not currently active) Ted Mielczarek]

Comment on attachment 421353 [details] [diff] [review]
modify short_libname, strip an additonal line
[Checkin: Comment 17]

I really wish OS/2 didn't have this lame limitation.

Comment 13

[Steve Wendt]

(In reply to comment #12)
> I really wish OS/2 didn't have this lame limitation.

As do we all, but it's what we are stuck with.

Comment 14

[Walter Meinl]

Comment on attachment 421355 [details] [diff] [review]
fix for c-c to find xpcom_core.lib with m-c
[Checkin: Comment 18]

Mark, once the other patch has landed on M-C we would need sth like this to be able to build C-C either with M-C or 1.9.2

Comment 15

[Mark Banner (:standard8)]

Comment on attachment 421355 [details] [diff] [review]
fix for c-c to find xpcom_core.lib with m-c
[Checkin: Comment 18]

r=Standard8. You may want to update the packaging manifests as well.

Comment 16

[Walter Meinl]

(In reply to comment #15)
> (From update of attachment 421355 [details] [diff] [review])
> r=Standard8. You may want to update the packaging manifests as well.

You're right at least for thunderbird we should do it. I will open a follow up, cause I want to check whether we need to change some ifdefs more.

Comment 17

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/550709f67284

Comment 18

[Serge Gautherie (:sgautherie)]

Comment on attachment 421355 [details] [diff] [review]
fix for c-c to find xpcom_core.lib with m-c
[Checkin: Comment 18]


http://hg.mozilla.org/comm-central/rev/74377b3a4cd5

Comment 19

[Serge Gautherie (:sgautherie)]

Walter, have you considered to get a hg access?