Closed Bug 425979 Opened 17 years ago Closed 17 years ago

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


(Toolkit Graveyard :: Build Config, defect)

Windows XP
Not set


(Not tracked)



(Reporter: bengt.erik.soderstrom, Assigned: jimm)




(2 files, 3 obsolete files)

From my Mingw build log:

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
Note that this also breaks MSVC w/o Vista SDK; see bug 412374 comment 51 and later.
Blocks: 412374
Product: Firefox → Toolkit
QA Contact: build.config → build-config
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?
Blocks: mingw
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.  

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.
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.
Attached patch patch v.1 (obsolete) — Splinter Review
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.
oops, need to update the makefile changes...
Attachment #312916 - Attachment is obsolete: true
Attached patch patch v.1 (obsolete) — Splinter Review
Attached patch configure bitsSplinter Review
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.
Attachment #312921 - Flags: review?(benjamin)
> 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.
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.
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.
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.
Attachment #312921 - Flags: review?(benjamin) → review+
Works for me. Either way, if these patches work properly, they should satisfy both mingw builds and msvc builds without the vista sdk.
Comment on attachment 312921 [details] [diff] [review]
configure bits

Part of a fix to get mingw/non-vista-sdk builds working again.
Attachment #312921 - Flags: approval1.9?
(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.
Attached patch patch v.2 (obsolete) — Splinter Review
Ok, one minor addition to the toolkit/components/build/ 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.
Attachment #312918 - Attachment is obsolete: true
Attachment #312962 - Flags: review?(benjamin)
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 build

5. Then got this error:

 nsIParentalControlsService.h: No such file or directory

In other words MinGw is still unhappy

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/` and see if it got added to DEFINES?
I get the same is the entry in my


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

True. My mistake.
I ran into the include error the first time myself. I don't use make build, so I had to regenerate configure using autoconf - autoconf-2.13 > configure before that flag was available and working. 
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.
OK, now it works even for me (after successful regeneration of configure)
MinGw is now happy again!
Comment on attachment 312921 [details] [diff] [review]
configure bits

Attachment #312921 - Flags: approval1.9? → approval1.9+
Assignee: nobody → jmathies
Summary: MinGW build error in ../mozilla/toolkit/components/parentalcontrols/src/nsParentalControlsServiceWin.cpp → MinGW build error in ../mozilla/toolkit/components/parentalcontrols/src/nsParentalControlsServiceWin.cpp (add --disable-parental-controls)
Blocks: tomtom
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.
Summary: MinGW build error in ../mozilla/toolkit/components/parentalcontrols/src/nsParentalControlsServiceWin.cpp (add --disable-parental-controls) → Win2003 SDK and MinGW build error in ../mozilla/toolkit/components/parentalcontrols/src/nsParentalControlsServiceWin.cpp (add --disable-parental-controls)
> the configure flags

(and configure checks)
Comment on attachment 312962 [details] [diff] [review]
patch v.2

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

>     ";1"

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/

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

Only ifdef src/... keep building the public/ directory in all cases.
Attachment #312962 - Flags: review?(benjamin) → review-
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.)
Re comment 13: Windows 2000 worked just fine until this issue occurred (at least for Thunderbird and Seamonkey builds).
> Configure does not yet check whether the Vista SDK is installed,

That's over in bug 426065.
Attached patch patch v.3Splinter Review
Attachment #312962 - Attachment is obsolete: true
Attachment #314368 - Flags: review?(benjamin)
Attachment #314368 - Flags: review?(benjamin) → review+
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.
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.
Attachment #314368 - Flags: approval1.9?
Comment on attachment 314368 [details] [diff] [review]
patch v.3

Attachment #314368 - Flags: approval1.9? → approval1.9+
Jim: feel free to check in my patch at the same time as yours.
"patch v.3" and "configure bits" should land at the same time.

Keywords: checkin-needed
Checking in;
/cvsroot/mozilla/,v  <--
new revision: 1.1976; previous revision: 1.1975
Checking in config/;
/cvsroot/mozilla/config/,v  <--
new revision: 3.460; previous revision: 3.459

Checking in toolkit/components/build/;
/cvsroot/mozilla/toolkit/components/build/,v  <--
new revision: 1.59; previous revision: 1.58
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
Checking in toolkit/components/parentalcontrols/;
/cvsroot/mozilla/toolkit/components/parentalcontrols/,v  <--
new revision: 1.2; previous revision: 1.1
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9
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!
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.
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.
Ah, it seems bug 426065 addresses my comments sufficiently.
Depends on: 428970
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.