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

RESOLVED FIXED in mozilla1.9

Status

()

RESOLVED FIXED
11 years ago
11 years ago

People

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

Tracking

(Blocks: 1 bug)

Trunk
mozilla1.9
x86
Windows XP
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

11 years ago
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

11 years ago
Note that this also breaks MSVC w/o Vista SDK; see bug 412374 comment 51 and later.
Blocks: 412374
Component: Build Config → Build Config
Product: Firefox → Toolkit
QA Contact: build.config → build-config
(Assignee)

Comment 2

11 years ago
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?

Updated

11 years ago
Blocks: 203303
(Reporter)

Comment 3

11 years ago
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

11 years ago
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.
(Assignee)

Comment 6

11 years ago
Created attachment 312916 [details] [diff] [review]
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.
(Assignee)

Comment 7

11 years ago
oops, need to update the makefile changes...
(Assignee)

Updated

11 years ago
Attachment #312916 - Attachment is obsolete: true
(Assignee)

Comment 8

11 years ago
Created attachment 312918 [details] [diff] [review]
patch v.1
Created attachment 312921 [details] [diff] [review]
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.
Attachment #312921 - Flags: review?(benjamin)
(Assignee)

Comment 10

11 years ago
> 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.

Comment 12

11 years ago
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

11 years ago
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.

Updated

11 years ago
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?

Comment 16

11 years ago
(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.
(Assignee)

Comment 17

11 years ago
Created attachment 312962 [details] [diff] [review]
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.
Attachment #312918 - Attachment is obsolete: true
Attachment #312962 - Flags: review?(benjamin)
(Reporter)

Comment 18

11 years ago
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



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

11 years ago
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.
David: I think you didn't reconfigure.

Comment 22

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

True. My mistake.
(Assignee)

Comment 23

11 years ago
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. 
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.
(Reporter)

Comment 25

11 years ago
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

a1.9=beltzner
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)

Updated

11 years ago
Duplicate of this bug: 427544

Updated

11 years ago
Blocks: 393966

Comment 28

11 years ago
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)

Comment 29

11 years ago
> the configure flags

(and configure checks)

Comment 30

11 years ago
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.
Attachment #312962 - Flags: review?(benjamin) → review-

Comment 31

11 years ago
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

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

Comment 33

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

That's over in bug 426065.
(Assignee)

Comment 34

11 years ago
Created attachment 314368 [details] [diff] [review]
patch v.3
Attachment #312962 - Attachment is obsolete: true
(Assignee)

Updated

11 years ago
Attachment #314368 - Flags: review?(benjamin)

Updated

11 years ago
Attachment #314368 - Flags: review?(benjamin) → review+

Comment 35

11 years ago
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.
(Assignee)

Updated

11 years ago
Attachment #314368 - Flags: approval1.9?
Comment on attachment 314368 [details] [diff] [review]
patch v.3

a1.9=beltzner
Attachment #314368 - Flags: approval1.9? → approval1.9+
Jim: feel free to check in my patch at the same time as yours.
(Assignee)

Comment 39

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

Keywords: checkin-needed
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
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9

Comment 41

11 years ago
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!
Blocks: 428683

Comment 42

11 years ago
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

11 years ago
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

11 years ago
Ah, it seems bug 426065 addresses my comments sufficiently.
(Assignee)

Updated

11 years ago
Depends on: 428970
You need to log in before you can comment on or make changes to this bug.