425979 - Win2003 SDK and MinGW build error in ../mozilla/toolkit/components/parentalcontrols/src/nsParentalControlsServiceWin.cpp (add --disable-parental-controls)

Status: RESOLVED FIXED

(Toolkit Graveyard :: Build Config, defect)

Description

[Bengt-Erik Soderstrom]

From my Mingw build log:

/cygdrive/d/mozilla/mozilla/toolkit/components/parentalcontrols/src/nsParentalControlsServiceWin.cpp
In file included from d:/mozilla/mozilla/toolkit/components/parentalcontrols/src/nsParentalControlsServiceWin.cpp:39:
d:/mozilla/mozilla/toolkit/components/parentalcontrols/src/nsParentalControlsServiceWin.h:53:20: wpcapi.h: No such file or directory
d:/mozilla/mozilla/toolkit/components/parentalcontrols/src/nsParentalControlsServiceWin.h:54:22: wpcevent.h: No such file or directory

.....

nsParentalControlsServiceWin.cpp makes use of the Windows Vista SDK, which is not recognized by Mingw.

One possible workaround would be to disable the Parental Control feature when Mingw is used. 
For example in .mozconfig:
  
ac_add_options --disable-parentalcontrol

Comment 1

[:Mook]

Note that this also breaks MSVC w/o Vista SDK; see bug 412374 comment 51 and later.

Comment 2

[Jim Mathies [:jimm]]

We could just ifdef the whole thing out for mingw builds. Why is it that mingw isn't up to date, is newer sdk stuff like this being worked on or do we expect this to be the case for the foreseeable future?

Comment 3

[Bengt-Erik Soderstrom]

I don't think it is just Mingw being out. I found (painfully) that my VC8 build also choked on this point. After updating to the Vista SDK, at least My VC8 build was OK.
There are probably other compilers which also will choke on this point. It is my opinion that we should, as far as possible, avoid introducing Microsoft-only features. 
If we, however, like to make use of of various Vista features - there will certainly be more of them in the future as their service packs are introduced - that is OK, but only if it is checked first that the required components are present, and if not, appropriate steps are taken.  

 

Comment 4

[Benjamin Smedberg]

We will not automatically flip off features based on compile-time feature detection: if you are building with an old SDK (or with w32api, which is equivalent to an old SDK), we should have a configure test which produces an early error message. But choices such as --disable-parentalcontrols should be an explicit choice to work around the broken SDK.

Comment 5

[(not currently active) Ted Mielczarek]

Jim: could you provide a patch that conditionally compiles the troublesome bits of the parental controls service? You can just make them #ifndef MOZ_DISABLE_PARENTAL_CONTROLS. I can provide a simple configure patch to give a --disable-parental-controls switch that generates this define.

Comment 6

[Jim Mathies [:jimm]]

Attachment: patch v.1

It'd be great if we could test this out on a ming build before comitting. I don't have that setup but can to test if need be.

Comment 7

[Jim Mathies [:jimm]]

oops, need to update the makefile changes...

Comment 8

[Jim Mathies [:jimm]]

Attachment: patch v.1

Comment 9

[(not currently active) Ted Mielczarek]

Attachment: configure bits

Here's the corresponding configure bits. Jim: can you apply this patch, add to your mozconfig:
ac_add_options --disable-parental-controls
and ensure everything builds ok?

Also, would be good for someone who doesn't have the Vista SDK to apply both patches, add that mozconfig line, and ensure that they can build.

Comment 10

[Jim Mathies [:jimm]]

> Also, would be good for someone who doesn't have the Vista SDK to apply both
> patches, add that mozconfig line, and ensure that they can build.

Just some clarification here to be sure we're all on the same page. The point here was to make things work for mingw which apparently does not yet support the vista sdk. However, we now rely on that sdk to build a complete Fx. In the past we've copy / pasted ms code into our source to get around this, but that's now being frowned upon which is why oartially the reason this broke mingw in the first place. (I've made a mental note to ifdef out stuff for mingw builds or add configure options in the future.) I wonder how long we'll be able to keep that up though going forward.

People also seem to be assuming this is designed to fix build problems in VC 7.1 with the vista sdk. From my understanding, the Vista SDK "may work" with VS2003, but it is not officially supported by MS. mozilla-tools is definitely broken in this case, and will not compile as it stands now. I don't know if we plan on trying to hack that to get it to work, or remove support for it. This patch will not fix that problem though.

Comment 11

[(not currently active) Ted Mielczarek]

FWIW, I don't personally care much about the MinGW builds, but that change also broke a lot of other developers, so it's good to have a workaround other than just "install the vista sdk", especially when it's something like parental controls that people wouldn't mind disabling in their own builds.

Comment 12

[Matthew Gertner]

Also, as far as I can tell it is not possible to install the Vista SDK unless you have at least XP SP2. So we should be aware that, if use of the new SDK is mandatory, we are creating a significant new restriction on who can build Mozilla.

Comment 13

[Benjamin Smedberg]

Why is XPSP2 significant? AFAICT we don't support building with win2k, and winxpsp2 is a $0.00 install.

Ted, I care about mingw builds now, mainly for bug 421095, so I'd like us to keep that going and soon am going to try to get a regular tinderbox/buildbot set up with a cross-compile toolchain.

Comment 14

[(not currently active) Ted Mielczarek]

Works for me. Either way, if these patches work properly, they should satisfy both mingw builds and msvc builds without the vista sdk.

Comment 15

[(not currently active) Ted Mielczarek]

Comment on attachment 312921 [details] [diff] [review]
configure bits

Part of a fix to get mingw/non-vista-sdk builds working again.

Comment 16

[Matthew Gertner]

(In reply to comment #13)
> Why is XPSP2 significant? AFAICT we don't support building with win2k, and
> winxpsp2 is a $0.00 install.

It seemed significant to me to require people to upgrade their operating system. I don't think this is a showstopper, just wanted to make sure people were aware of this.

Comment 17

[Jim Mathies [:jimm]]

Attachment: patch v.2

Ok, one minor addition to the toolkit/components/build/Makefile.in. With that - VC7.1, Windows SDK 2003 RC2, Windows XPSP2, --disable-parental-control added -> clean build. VC8, Vista SDK, Vista, no --disable-parental-control added -> clean build.

Comment 18

[Bengt-Erik Soderstrom]

It seems that we are not yet quite ready. I did the following with MinGw:

1.  Applied patches configure bits and patch v.2

2.  Added  ac_add_options --disable-parental-controls  in .mozconfig

3.  Nuked the object directory (just for safety)

4.  make -f client.mk build

5. Then got this error:

d:/mozilla/mozilla/toolkit/components/downloads/src/nsDownloadManager.cpp:85:40:
 nsIParentalControlsService.h: No such file or directory

In other words MinGw is still unhappy

Comment 19

[(not currently active) Ted Mielczarek]

Bengt-Erik: hm, there's only one instance of that include in that C++ file, and it's surrounded with an ifndef MOZ_DISABLE_PARENTAL_CONTROLS, so maybe that define isn't working properly for you? can you run `grep PARENTAL objdir/config/autoconf.mk` and see if it got added to DEFINES?

Comment 20

[David G King]

I get the same error....here is the entry in my autoconf.mk

MOZ_DISABLE_PARENTAL_CONTROLS = @MOZ_DISABLE_PARENTAL_CONTROLS@

Not really what I expected.

Comment 21

[(not currently active) Ted Mielczarek]

David: I think you didn't reconfigure.

Comment 22

[David G King]

(In reply to comment #21)
> David: I think you didn't reconfigure.
> 

True. My mistake.

Comment 23

[Jim Mathies [:jimm]]

I ran into the include error the first time myself. I don't use make client.mk build, so I had to regenerate configure using autoconf - autoconf-2.13 configure.in > configure before that flag was available and working. 

Comment 24

[(not currently active) Ted Mielczarek]

Oh yeah, I forgot to mention that you'd need to run autoconf locally. Oops! In MozillaBuild, you can just type `autoconf-2.13` in the mozilla directory and it will regenerate configure.

Comment 25

[Bengt-Erik Soderstrom]

OK, now it works even for me (after successful regeneration of configure)
MinGw is now happy again!

Comment 26

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

Comment on attachment 312921 [details] [diff] [review]
configure bits

a1.9=beltzner

Comment 28

[Ben Bucksch (:BenB)]

Adjusting title to make clear that it not only applies to MinGW, but also to Windows Server 2003 SDK, i.e. the common SDK for Windows XP.
Superreviewers, next time, please make sure the configure flags happen before such features with new dependencies get checked in.

Comment 29

[Ben Bucksch (:BenB)]

> the configure flags

(and configure checks)

Comment 30

[Benjamin Smedberg]

Comment on attachment 312962 [details] [diff] [review]
patch v.2

>Index: toolkit/components/build/nsToolkitCompsCID.h

>+#ifndef MOZ_DISABLE_PARENTAL_CONTROLS
> #define NS_PARENTALCONTROLSSERVICE_CONTRACTID \
>     "@mozilla.org/parental-controls-service;1"
>+#endif

This is incorrect. The contract should still be valid even if we've disabled the actual parental controls service.

>Index: toolkit/components/downloads/src/nsDownloadManager.cpp

Shouldn't need any changes in this file.

>Index: toolkit/components/parentalcontrols/Makefile.in

>+ifndef MOZ_DISABLE_PARENTAL_CONTROLS
> DIRS = public
>-
> ifeq (WINNT,$(OS_ARCH))
> DIRS += src
> endif
>+endif


Only ifdef src/... keep building the public/ directory in all cases.

Comment 31

[Ben Bucksch (:BenB)]

Configure does not yet check whether the Vista SDK is installed, makes people bail very late in the build process, thus wasting a lot of time. (I don't have configure fu to code that myself, but I think you'd just need to check for existence of the header file.)

Comment 32

[Kent James (:rkent)]

Re comment 13: Windows 2000 worked just fine until this issue occurred (at least for Thunderbird and Seamonkey builds).

Comment 33

[Jim Mathies [:jimm]]

> Configure does not yet check whether the Vista SDK is installed,

That's over in bug 426065.

Comment 34

[Jim Mathies [:jimm]]

Attachment: patch v.3

Comment 35

[Ben Bucksch (:BenB)]

Jim, that bug suggests to tell users to install the Vista SDK. I think that the Win2003 SDK should still be supported and specific headers checked and only specific features disabled.

Comment 36

[(not currently active) Ted Mielczarek]

Ben: that bug should add a test to configure that lets you know that your build will not succeed without either the Vista SDK, or using the --disable-parental-controls flag from this bug. We don't want to silently disable features in configure, that just leads to confusion in the end.

Comment 37

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

Comment on attachment 314368 [details] [diff] [review]
patch v.3

a1.9=beltzner

Comment 38

[(not currently active) Ted Mielczarek]

Jim: feel free to check in my patch at the same time as yours.

Comment 39

[Jim Mathies [:jimm]]

"patch v.3" and "configure bits" should land at the same time.

Comment 40

[Reed Loden [:reed]]

Checking in configure.in;
/cvsroot/mozilla/configure.in,v  <--  configure.in
new revision: 1.1976; previous revision: 1.1975
done
Checking in config/autoconf.mk.in;
/cvsroot/mozilla/config/autoconf.mk.in,v  <--  autoconf.mk.in
new revision: 3.460; previous revision: 3.459
done

Checking in toolkit/components/build/Makefile.in;
/cvsroot/mozilla/toolkit/components/build/Makefile.in,v  <--  Makefile.in
new revision: 1.59; previous revision: 1.58
done
Checking in toolkit/components/build/nsToolkitCompsModule.cpp;
/cvsroot/mozilla/toolkit/components/build/nsToolkitCompsModule.cpp,v  <--  nsToolkitCompsModule.cpp
new revision: 1.54; previous revision: 1.53
done
Checking in toolkit/components/parentalcontrols/Makefile.in;
/cvsroot/mozilla/toolkit/components/parentalcontrols/Makefile.in,v  <--  Makefile.in
new revision: 1.2; previous revision: 1.1
done

Comment 41

[Kent James (:rkent)]

I can verify that with the 2008-04-09 CVS download (including this patch), Thunderbird trunk will now compile on a Windows 2000-based system, using msys compiles, without the Vista SDK, when the --disable-parental-controls option is added. Thanks!

Comment 42

[jag (Peter Annema)]

It'd be nice if we could add a configure test that will just automatically disable the feature when the SDK isn't found. Or, if that's not verbose enough, disable it when not on Vista, and fail with a clear error message when on Vista.

Comment 43

[jag (Peter Annema)]

Hrm, on second thought, is the Vista SDK available on XP? That'd complicate matters a bit.

My point is though that for a non-essential feature like this the build shouldn't fail with the default build settings just because an SDK required for that feature can't be found.

Comment 44

[jag (Peter Annema)]

Ah, it seems bug 426065 addresses my comments sufficiently.