UPower isnt linux-only

RESOLVED FIXED in mozilla15

Status

()

Core
Hardware Abstraction Layer (HAL)
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gaston, Assigned: gaston)

Tracking

Trunk
mozilla15
Other
OpenBSD
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
While looking for other things under hal/ i noticed the following chunk in hal?Makefile.in, within ifeq (Linux,$(OS_TARGET))


79679: ifdef MOZ_ENABLE_DBUS
79679: CPPSRCS += UPowerClient.cpp
92976: else
92976: CPPSRCS += FallbackBattery.cpp
79679: endif

That's a bit wrong, since upower is not linux only. It also builds and runs fine on OpenBSD and FreeBSD (also probably builds on NetBSD but there's no dedicated backend yet, see http://cgit.freedesktop.org/upower/tree/src). I dont know if MOZ_ENABLE_DBUS can be set on osx (i guess so), so maybe the same ifdef MOZ_ENABLE_DBUS -then-enable-upower chunk should be added to the last else chunk, so that poor other platforms get a chance to talk to upower too ?

It doesnt seem to me that this battery api is used for anything else on desktop firefox, but who knows, in the future it might ?
(Assignee)

Updated

5 years ago
Depends on: 696041
(Assignee)

Comment 1

5 years ago
Created attachment 627453 [details] [diff] [review]
Buld upowerclient on BSDs too if dbus is enabled

That patch correctly builds UPowerClient.o here if dbus is enabled.
Assignee: nobody → landry
Attachment #627453 - Flags: review?(mounir)
Landry, what about just adding a line for BSD's instead?
(Assignee)

Comment 3

5 years ago
Created attachment 627656 [details] [diff] [review]
Buld upowerclient on BSDs too if dbus is enabled

Something like that ?
Attachment #627453 - Attachment is obsolete: true
Attachment #627453 - Flags: review?(mounir)
Attachment #627656 - Flags: review?(mounir)
Comment on attachment 627656 [details] [diff] [review]
Buld upowerclient on BSDs too if dbus is enabled

Review of attachment 627656 [details] [diff] [review]:
-----------------------------------------------------------------

r=me but avoid the change at the end if unnecessary.

::: hal/Makefile.in
@@ +135,5 @@
>  ifeq (gonk,$(MOZ_WIDGET_TOOLKIT))
>  # So that we can call nsScreenManagerGonk::GetConfiguration().
>  LOCAL_INCLUDES += -I$(topsrcdir)/widget/gonk
>  LOCAL_INCLUDES += -I$(topsrcdir)/widget/xpwidgets
> +endif

What are you changing here?
Attachment #627656 - Flags: review?(mounir) → review+
(Assignee)

Comment 5

5 years ago
(In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #4)
> Comment on attachment 627656 [details] [diff] [review]
> Buld upowerclient on BSDs too if dbus is enabled
> 
> Review of attachment 627656 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me but avoid the change at the end if unnecessary.
> 
> ::: hal/Makefile.in
> @@ +135,5 @@
> >  ifeq (gonk,$(MOZ_WIDGET_TOOLKIT))
> >  # So that we can call nsScreenManagerGonk::GetConfiguration().
> >  LOCAL_INCLUDES += -I$(topsrcdir)/widget/gonk
> >  LOCAL_INCLUDES += -I$(topsrcdir)/widget/xpwidgets
> > +endif
> 
> What are you changing here?

Hmm. i guess that's mercurial/vim that added that, dunno if there's a policy wrt newline at EOF in the codebase..

-endif
\ No newline at end of file
+endif
(Assignee)

Comment 6

5 years ago
Created attachment 627881 [details] [diff] [review]
Same patch without EOF change

New patch without that chunk, setting checkin-needed.
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
Status: NEW → ASSIGNED
Flags: in-testsuite-
Keywords: checkin-needed
Target Milestone: --- → mozilla15
Attachment #627881 - Flags: checkin+
(Assignee)

Comment 7

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/ce3d4fd7033b
Status: ASSIGNED → NEW
Target Milestone: mozilla15 → ---
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla15

Comment 8

5 years ago
https://hg.mozilla.org/mozilla-central/rev/ce3d4fd7033b
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Blocks: 788414
You need to log in before you can comment on or make changes to this bug.