'ac_add_options --with-windows-version=502' gives (me) { Bug 472093 comment 39: configure: error: Invalid value --with-windows-version, must be 400, 500 or 501 } due to old c-c and new m-c mismatch.

This part is safe to take as is, even if m-1.9.1 doesn't have it.

Why do we need to port this at all when 1.9.1 doesn't have it?

(In reply to comment #2) > Why do we need to port this at all when 1.9.1 doesn't have it? Because "I" can't build using m-c with the 'Windows Server 2003 R2' SDK anymore, and I guess we'll want to restore this option when c-c branches anyway: I can use an "ifndef 1.9.1" if you prefer.

Ah, so this is needed for building with mozilla-central but building with mozilla-1.9.1 still works with the current/old code? What I want to avoid is that c-c configure tells the user everything works but mozilla configure fails. Maybe I actually don't need to care about that though.

(In reply to comment #4) > Ah, so this is needed for building with mozilla-central Yes. > but building with mozilla-1.9.1 still works with the current/old code? Yes (it should): bug 472093 won't land on 1.9.1 :-| > What I want to avoid is that c-c configure tells the user everything works but > mozilla configure fails. Maybe I actually don't need to care about that though. Sure, I'm paying attention that c-c should continue to work with both m-1.9.1 and m-c.

Comment on attachment 377468 [details] [diff] [review] (Av1) Part 1, define MOZ_WINSDK_MAXVER to be highest Windows version supported by the SDK [Checkin: Comment 16] If it makes things work, I can't see a real reason not to take it. I still wonder why Thunderbird boxes aren't running into this problem you're seeing, and I don't really understand what we are setting this var for, so I'd like a second opinion here. Mark, can you check if it's OK to take this?

(In reply to comment #6) > why Thunderbird boxes aren't running into this problem you're seeing, I would guess because TB boxes do not (need to) disable the Vista features. > and I don't really understand what we are setting this var for For nothing yet: it will be used by following patches.

(In reply to comment #7) > (In reply to comment #6) > > and I don't really understand what we are setting this var for > > For nothing yet: it will be used by following patches. Erm, we need more to get things working? I'm not sure I like that! Can I at least get some clue of what you want to stuff in here?

Just the 3 patches from bug 472093: at least the blocking parts (until c-c branches).

Could you please give us that as one consolidated patch or at least attach and request review for everything needed at once? Going with one single thing and expecting us to swallow it and then say "oh, but we need more" is not really a good idea.

I had to move MOZILLA_1_9_1_BRANCH definition block up. (In reply to comment #10) > expecting us to swallow it and then say "oh, but we need more" ?! The first patch is explicitly labeled as 'Part 1', and my idea was only not to loose time preparing more patches if you would have say no to that first one... :-|

The 'switch code over to use these defines' will be a follow-up bug, to do after c-c has branched.

Comment on attachment 377985 [details] [diff] [review] (Bv1) Part 2, define MOZ_WINSDK_TARGETVER and error out if the SDK is too old [Checkin: Comment 19] I'd like Mark's opinion on this one as well, esp. for what it does with MOZILLA_1_9_1_BRANCH (move the block around, last hunk, etc.)

Comment on attachment 377468 [details] [diff] [review] (Av1) Part 1, define MOZ_WINSDK_MAXVER to be highest Windows version supported by the SDK [Checkin: Comment 16] sr=Standard8

Comment on attachment 377985 [details] [diff] [review] (Bv1) Part 2, define MOZ_WINSDK_TARGETVER and error out if the SDK is too old [Checkin: Comment 19] I need to take another look at this tomorrow, but in the meantime, I spotted this: >@@ -5516,16 +5591,18 @@ MOZ_ARG_DISABLE_BOOL(parental-controls, > [ --disable-parental-controls > Do not build parental controls], > MOZ_DISABLE_PARENTAL_CONTROLS=1, > MOZ_DISABLE_PARENTAL_CONTROLS=) > if test -n "$MOZ_DISABLE_PARENTAL_CONTROLS"; then > AC_DEFINE(MOZ_DISABLE_PARENTAL_CONTROLS) > fi > >+if test "$MOZILLA_1_9_1_BRANCH" = "1"; then >+ > dnl ======================================================== > dnl Vista SDK specific api > dnl ======================================================== > MOZ_ARG_DISABLE_BOOL(vista-sdk-requirements, > [ --disable-vista-sdk-requirements > Do not build Vista SDK specific code], > MOZ_DISABLE_VISTA_SDK_REQUIREMENTS=1, > MOZ_DISABLE_VISTA_SDK_REQUIREMENTS=) >@@ -5545,16 +5622,22 @@ case "$target" in > fi > ;; > esac > fi > fi > AC_SUBST(MOZ_DISABLE_PARENTAL_CONTROLS) > AC_SUBST(MOZ_DISABLE_VISTA_SDK_REQUIREMENTS) > >+else >+AC_SUBST(MOZ_DISABLE_PARENTAL_CONTROLS) >+dnl Dumb statement, not to have an empty 'else' block. >+MOZILLA_1_9_1_BRANCH=$MOZILLA_1_9_1_BRANCH >+fi MOZ_DISABLE_PARENTAL_CONTROLS applies whether or not we're on the 1.9.1 branch, so just move the AC_SUBST to after the check for --disable-parental-controls section.

Comment on attachment 377468 [details] [diff] [review] (Av1) Part 1, define MOZ_WINSDK_MAXVER to be highest Windows version supported by the SDK [Checkin: Comment 16] http://hg.mozilla.org/comm-central/rev/59bb99cbe97c

(In reply to comment #15) > MOZ_DISABLE_PARENTAL_CONTROLS applies whether or not we're on the 1.9.1 branch, > so just move the AC_SUBST to after the check for --disable-parental-controls > section. I'll do. (I did it that way not to touch the existing lines only...)

Comment on attachment 377985 [details] [diff] [review] (Bv1) Part 2, define MOZ_WINSDK_TARGETVER and error out if the SDK is too old [Checkin: Comment 19] sr=Standard8 (not my email address please) with the change mentioned in comment 15.

Comment on attachment 377985 [details] [diff] [review] (Bv1) Part 2, define MOZ_WINSDK_TARGETVER and error out if the SDK is too old [Checkin: Comment 19] http://hg.mozilla.org/comm-central/rev/dda9efe76799

Comment on attachment 377986 [details] [diff] [review] (Cv1) Part 3, add MOZ_NTDDI_* defines [Checkin: Comment 20] http://hg.mozilla.org/comm-central/rev/a4ee7c0f12ec

(In reply to comment #12) > The 'switch code over to use these defines' will be a follow-up bug, to do > after c-c has branched. I filed bug 496225.