add WinCE default configuration settings to configure.in

RESOLVED FIXED in mozilla1.9.3a1

Status

Firefox Build System
General
RESOLVED FIXED
10 years ago
2 months ago

People

(Reporter: wolfe, Assigned: wolfe)

Tracking

({mobile})

Trunk
mozilla1.9.3a1
ARM
Windows Mobile 6 Professional
mobile

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

10 years ago
Created attachment 341304 [details] [diff] [review]
v.1 patch

Two configuration settings which should be turned on by default for each WinCE build, so they do not have to appear in a mozconfig file.

ac_add_options --disable-vista-sdk-requirements

and 

ac_add_options --enable-default-toolkit=cairo-windows
Attachment #341304 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 1

10 years ago
Created attachment 341306 [details] [diff] [review]
v.2 patch - cleaner than before

Cleaner way than v.1 patch to set default configurations for WinCE builds.
Attachment #341304 - Attachment is obsolete: true
Attachment #341306 - Flags: review?(ted.mielczarek)
Attachment #341304 - Flags: review?(ted.mielczarek)

Comment 2

10 years ago
cairo-windows is correct, but I'm not so sure about the vista-sdk-requirements blanket. vista-sdk-requirements covers a lot more ground than just parental-controls, see http://mxr.mozilla.org/mozilla-central/search?string=vista_sdk_requirements
The Windows Mobile builds use their own SDK, so they don't have the Vista definitions (at least this is what Doug tells me). I suggested that we either #ifdef the relevant bits of code on WINCE, or flip the right defines in configure to make it happen. I don't know the actual scope of what needs to be disabled, though.
Duplicate of this bug: 458085
Duplicate of this bug: 458084
Comment on attachment 341306 [details] [diff] [review]
v.2 patch - cleaner than before

I'd prefer a patch that doesn't add $_PLATFORM_MOZ_DISABLE_VISTA_SDK_REQUIREMENTS, but it's not that big of a deal.
Attachment #341306 - Flags: review?(ted.mielczarek) → review+

Updated

9 years ago
Flags: wanted-fennec1.0+

Updated

9 years ago
Duplicate of this bug: 473826
(Assignee)

Comment 8

9 years ago
Created attachment 366964 [details] [diff] [review]
v.2.1 patch -- updated v.2 patch which applies cleanly

Nothing changed in this patch, except the surrounding context -- this new attachment just applies the changes cleanly to the current configure.in file.

As I understand it, the r+ given to patch v2.0 should still apply.
Attachment #341306 - Attachment is obsolete: true
(Assignee)

Comment 9

9 years ago
Comment on attachment 366964 [details] [diff] [review]
v.2.1 patch -- updated v.2 patch which applies cleanly

carried forward the r+ted from v.2 patch.
Attachment #366964 - Attachment description: v2.1 patch -- updated v2.0 patch which applies cleanly → v.2.1 patch -- updated v.2 patch which applies cleanly
Attachment #366964 - Flags: review+
Keywords: checkin-needed

Updated

9 years ago
Assignee: nobody → wolfe

Updated

9 years ago
Attachment #366964 - Flags: review+
http://hg.mozilla.org/mozilla-central/rev/2492fd7690c8
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Summary: WinCE add default configuration settings into configure.in → add WinCE default configuration settings to configure.in
Target Milestone: --- → mozilla1.9.3a1
Flags: in-testsuite-
Version: unspecified → Trunk
Bug 506493 comment 39:
{
Justin Wood (:Callek)      2010-04-04 21:38:23 PDT

I doubt this would work; given order of things, [_PLATFORM_MOZ_DISABLE_VISTA_SDK_REQUIREMENTS] is
AFTER where its used.
}

I noticed too but didn't care.
As Callek seems to care, could you confirm that/how it works?
(My guess would be that the var is missing |"| around itself where used!?)
(In reply to comment #11)
> Bug 506493 comment 39:
> {
> Justin Wood (:Callek)      2010-04-04 21:38:23 PDT
> 
> I doubt this would work; given order of things,
> [_PLATFORM_MOZ_DISABLE_VISTA_SDK_REQUIREMENTS] is
> AFTER where its used.
> }
> 
> I noticed too but didn't care.
> As Callek seems to care, could you confirm that/how it works?
> (My guess would be that the var is missing |"| around itself where used!?)


Actually the m-c order seemed right; so this doesn't belong as a comment here... I'll poke at it in our side's bug.
(In reply to comment #12)
> Actually the m-c order seemed right; so this doesn't belong as a comment
> here... I'll poke at it in our side's bug.

Slight correction; *this* patch was right, but something in m-c seems to have changed the order here... I'm investigating and will comment in our-sides bug and the "problem" bug.
Ok...

Dao/Wolfe, the problem is, the PATCH was right; but it bitrotted by the time it was landed...

This is currently not doing the right thing because of that. Do we want to reopen, use a new bug, or backout the change?
Reopen if you're going to back out the patch, use a new bug otherwise.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I'm going to back this out (tomorrow), unless comment 11 is answered.
I'm not sure what you want here? that configure variable isn't even used anymore, this patch is a year and a half old, things have changed.
Depends on: 557958
(In reply to comment #17)

Ah ... deprecated but not unused/removed:
I filed bug 557958: let's see that one first.
Based on last comment, lets just keep this fixed; and no need to do a "real" backout since it landed so long ago; we can however remove the offending winCE var/setting in trunk if we so desire (in a new bug).
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago8 years ago
Resolution: --- → FIXED
No longer blocks: 506493

Updated

2 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.