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

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Benjamin Smedberg]

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.

Comment 1

[(no longer active)]

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...

Comment 2

[Robert Strong (they/them - no direct email)]

I've been cleaning up the tests so I'll take this.

Benjamin, how important is this?

Comment 3

[Benjamin Smedberg]

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.

Comment 4

[(no longer active)]

(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!  :-)

Comment 5

[Robert Strong (they/them - no direct email)]

Benjamin, would you like me to remove XRE_EXECUTABLE_FILE as well?

Comment 6

[Benjamin Smedberg]

That doesn't appear to require any app-specific defines; apart from those I don't really care.

Comment 7

[Robert Strong (they/them - no direct email)]

Attachment: patch in progress

Passes tests and makes a couple of tests that needed to run synchronously run in parallel. Just need to clean the patch up.

Comment 8

[Robert Strong (they/them - no direct email)]

Try push of the patch in progress
https://tbpl.mozilla.org/?tree=Try&rev=601cfb6693a3

Comment 9

[Robert Strong (they/them - no direct email)]

Attachment: patch rev 1

I'll land this at the same time as the patches in bug 951662.

Comment 10

[Robert Strong (they/them - no direct email)]

Pushed to mozilla-inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f4af26d0535

There are tests in bug 951662

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3f4af26d0535