658831 - Shared smime build in comm-1.9.2 fails: Makefile:84: *** missing separator.

Status: RESOLVED FIXED

(MailNews Core :: Security: S/MIME, defect)

Description

[Philipp Wagner [:imphil]]

Attachment: patch

The Makefile in mailnews/extensions/smime/build contains an error which prevents a build with the message 

Makefile:84: *** missing separator.

A patch is attached. Who is doing reviews in this component?

Comment 1

[Philipp Wagner [:imphil]]

Ah well, this does not yet completely solve the problem. There is another problem after that:

make export
make[1]: Entering directory `/home/philipp/src/comm-1.9.2/obj-debug/mailnews/extensions/smime/build'
make[1]: Nothing to be done for `export'.
make[1]: Leaving directory `/home/philipp/src/comm-1.9.2/obj-debug/mailnews/extensions/smime/build'
make libs
make[1]: Entering directory `/home/philipp/src/comm-1.9.2/obj-debug/mailnews/extensions/smime/build'
make[1]: *** No rule to make target `../../../base/util/libmsgbsutl_s.a', needed by `libmsgsmime.so'.  Stop.
make[1]: Leaving directory `/home/philipp/src/comm-1.9.2/obj-debug/mailnews/extensions/smime/build'
make: *** [default] Error 2



I currently don't have a clue where that's coming from. Any ideas?

Comment 2

[Philipp Wagner [:imphil]]

Attachment: patch v2

so, I took the approach as in import/build/Makefile and TB builds and works, looks like this code path has never been tested?

Comment 3

[Philipp Wagner [:imphil]]

Hm, coming back to this with a bit more sleep, things look this way:

* The whole code is changed in comm-central, and the problematic line is not there any more

* I mixed up static/shared, sorry.

* With this patch the build works with --enable-static/--disable-shared as well as without those two flags (using a minimal mozconfig)

* Since this whole code changed in comm-central, I'm not convinced if it's worth the effort to change this on the branch, since it seems to have been like that for ages.

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 534259 [details] [diff] [review]
patch v2

I think we might as well just get this landed.

+	EXTRA_DSO_LDOPTS += ../../base/util/$(LIB_PREFIX)msgbsutl_s.$(LIB_SUFFIX)


Note we don't normally indent here, but I'll fix that on checkin.

Comment 5

[Mark Banner (:standard8)]

Comment on attachment 534259 [details] [diff] [review]
patch v2

Actually, I'm not sure this is quite right. Taking a closer look.

Comment 6

[Mark Banner (:standard8)]

Attachment: Simpler fix

I took a bit of a look at this and realised that if the non-static-mail-build case we currently have two sets of options for linking against msgbaseutil - one using EXTRA_DSO_LIBS and one using SHARED_LIBRARY_LIBS. Trying it locally, we don't need the SHARED_LIBRARY_LIBS version.

Hence we can just drop SHARED_LIBRARY_LIBS and shouldn't have to worry about this. It doesn't affect our release builds as they are static builds with static mail.

David: just looking for an rs here, this will only land on 1.9.2 branch.

Comment 7

[David :Bienvenu]

Comment on attachment 558418 [details] [diff] [review]
Simpler fix

rs=me

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 558418 [details] [diff] [review]
Simpler fix

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

I should point out that I'm only familiar with the build system as a user.  I have little inkling about the ramifications of a change like this.

I can, however, confirm that this does not break my local buildage of TB 3.1.x.  So, r+ from me.

Comment 9

[Mark Banner (:standard8)]

Checked in for 3.1.15: http://hg.mozilla.org/releases/comm-1.9.2/rev/fee5369a4b8c