Closed Bug 536451 Opened 10 years ago Closed 10 years ago

[OS/2] cannot handle mozsqlite3.dll

Categories

(Toolkit :: Storage, defect)

x86
OS/2
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla1.9.3a1

People

(Reporter: wuno, Assigned: wuno)

References

Details

Attachments

(2 files, 2 obsolete files)

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
Blocks: 513747
Version: unspecified → Trunk
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?
(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.
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.
(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.
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"
Status: NEW → ASSIGNED
Attached patch modify short_libname handling (obsolete) — Splinter Review
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.
Assignee: nobody → dragtext
Attachment #418905 - Attachment is obsolete: true
Ooops... I changed the assignee by accident - I'm restoring it to Walter.
Assignee: dragtext → wuno
(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.
(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.
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.
Attachment #419017 - Attachment is obsolete: true
This should fix SM and TB c-c builds overlayed to m-c (or 1.9.2)
Attachment #421353 - Flags: review?(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.
Attachment #421353 - Flags: review?(ted.mielczarek) → review+
(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 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
Attachment #421355 - Flags: review?(bugzilla)
Keywords: checkin-needed
Attachment #421355 - Flags: review?(bugzilla) → review+
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.
(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.
http://hg.mozilla.org/mozilla-central/rev/550709f67284
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Keywords: checkin-needed
Whiteboard: needs comm-central landing
Attachment #421353 - Attachment description: modify short_libname, strip an additonal line → modify short_libname, strip an additonal line [Checkin: Comment 17]
Flags: in-testsuite-
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
Attachment #421355 - Attachment description: fix for c-c to find xpcom_core.lib with m-c → fix for c-c to find xpcom_core.lib with m-c [Checkin: Comment 18]
Walter, have you considered to get a hg access?
Blocks: 543382
Depends on: 521523
Keywords: checkin-needed
Whiteboard: needs comm-central landing
Depends on: 545534
Depends on: 534408
Status: RESOLVED → VERIFIED
Depends on: 544321
Blocks: 534408
No longer depends on: 534408
You need to log in before you can comment on or make changes to this bug.