Closed Bug 974217 Opened 6 years ago Closed 6 years ago

Remove the non-standalone part of dirserviceprovider

Categories

(Firefox Build System :: General, defect)

x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla30

People

(Reporter: ehsan, Assigned: ehsan)

Details

Attachments

(1 file, 1 obsolete file)

Follow-up from bug 973142.
Note the only place where the standalone version is used is embedding/tests/winEmbed, and the only place where that is built is in xulrunner windows builds. Even then, it's installed in dist/xpi-stage/winembed, which makes it not packaged anywhere afaik. It doesn't even look like we have rules to run it on automation and logs seem it indicate it doesn't run during xulrunner builds.
http://ftp.mozilla.org/pub/mozilla.org/xulrunner/nightly/2014/02/2014-02-18-03-02-02-mozilla-central/mozilla-central-win32-xulrunner-nightly-bm85-build1-build6.txt.gz

So maybe we can just kill winEmbed tests and dirserviceprovider entirely?
Flags: needinfo?(benjamin)
Attached patch Patch (v1)Splinter Review
Attachment #8378051 - Flags: review?(mh+mozilla)
Note:

toolkit/profile/moz.build:17:UNIFIED_SOURCES += [ TOPSRCDIR + '/profile/dirserviceprovider/src/nsProfileLock.cpp' ]

so at least this code needs to move if we decide to kill this.
Comment on attachment 8378051 [details] [diff] [review]
Patch (v1)

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

Let's wait for bsmedberg's answer before landing, though.

::: profile/dirserviceprovider/src/moz.build
@@ +20,2 @@
>  
>  UNIFIED_SOURCES += modules_profiledirservice_src_csrcs

All the above can be expressed as:

UNIFIED_SOURCES += ['nsProfileDirServiceProvider.cpp']
if CONFIG['MOZ_PROFILELOCKING']:
    UNIFIED_SOURCES += ['nsProfileLock.cpp']

@@ +27,5 @@
> +DEFINES['XPCOM_GLUE'] = 1
> +
> +LOCAL_INCLUDES += [
> +    '../src',
> +]

../src in src is . ;)
Attachment #8378051 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8378077 [details] [diff] [review]
Remove the non-standalone part of dirserviceprovider; r=glandium

Sorry, a simple script went mad!
Attachment #8378077 - Attachment is obsolete: true
I'm torn. winembed is actually the best embedding example we have at the moment. I really don't want to remove it, although I understand the issue with it not actually being tested code.
Flags: needinfo?(benjamin)
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #7)
> I'm torn. winembed is actually the best embedding example we have at the
> moment. I really don't want to remove it, although I understand the issue
> with it not actually being tested code.

Should we land what we have here for now?  I don't see why we need to make that call right now.  If the code is useful just as an example, perhaps it's better to leave it in...
Flags: needinfo?(benjamin)
Yeah, I don't have any problem removing the other version that nobody's using. I'm only talking about the standalone version which might be valuable.
Flags: needinfo?(benjamin)
https://hg.mozilla.org/mozilla-central/rev/f5d15ac47466
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.