Closed Bug 921148 Opened 6 years ago Closed 6 years ago

xpcshell has compiled-in MOZ_APP_NAME and other data which shouldn't be

Categories

(Firefox Build System :: General, defect)

x86_64
Windows 7
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla29

People

(Reporter: benjamin, Assigned: rstrong)

References

Details

Attachments

(1 file, 1 obsolete file)

I discovered while moving xpcshell into libxul that xpcshell on windows actually has compiled-in app-specific data such as MOZ_APP_NAME. This is a result of patches in bug 307181, but I'm not quite sure why it was necessary to build this logic into xpcshell itself rather than install a scripted dirserviceprovider to mock whatever update directories were necessary:

http://hg.mozilla.org/mozilla-central/rev/c20d415ef1b5#l9.131

Ehsan/rstrong, do you remember why this needed to be native code? XRE_UPDATE_ROOT_DIR is the key we're replying for here; looking through the tests it doesn't seem they in general have any app-specific behaviors that would require us to use a specific update directory.
Flags: needinfo?(robert.bugzilla)
Flags: needinfo?(ehsan)
Well, in order for our xpcshell based tests to work correctly, we need to make sure that xpcshell-initiated tests put the updates and look for them in the same directory as the rest of the update system does.

The reason I did this in the xpcshell.cpp code is that the shlwapi was easier to access from C++.  I guess you could use js-ctypes to call them from JS, in which case you should be able to remove this code and move it to a mock directory service provider.  Things were just easier this way...
Flags: needinfo?(ehsan)
I've been cleaning up the tests so I'll take this.

Benjamin, how important is this?
Assignee: nobody → robert.bugzilla
Flags: needinfo?(robert.bugzilla)
This is not super-important, it's just a reverse build system dependency of the sort that usually makes us cry later by accident. It will lock block the "hack Firefox frontend without a compile environment" bug, which I can't find right now.
(In reply to comment #3)
> This is not super-important, it's just a reverse build system dependency of the
> sort that usually makes us cry later by accident. It will lock block the "hack
> Firefox frontend without a compile environment" bug, which I can't find right
> now.

Sorry!  :-)
Benjamin, would you like me to remove XRE_EXECUTABLE_FILE as well?
Flags: needinfo?(benjamin)
That doesn't appear to require any app-specific defines; apart from those I don't really care.
Flags: needinfo?(benjamin)
Attached patch patch in progress (obsolete) — Splinter Review
Passes tests and makes a couple of tests that needed to run synchronously run in parallel. Just need to clean the patch up.
Attached patch patch rev 1Splinter Review
I'll land this at the same time as the patches in bug 951662.
Attachment #8349349 - Attachment is obsolete: true
Attachment #8355365 - Flags: review?(netzen)
Attachment #8355365 - Flags: review?(netzen) → review+
Pushed to mozilla-inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f4af26d0535

There are tests in bug 951662
Flags: in-testsuite+
Target Milestone: --- → mozilla29
https://hg.mozilla.org/mozilla-central/rev/3f4af26d0535
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.