Closed Bug 1082777 Opened 8 years ago Closed 8 years ago

Can't run xpcshell from the command line


(Core :: XPCOM, defect)

Not set





(Reporter: bzbarsky, Assigned: spohl)



(2 files)

When I try to run $objdir/dist/bin/xpcshell, I get:

###!!! ABORT: You can't dereference a NULL nsCOMPtr with operator->().: 'mRawPtr != 0', file ../../dist/include/nsCOMPtr.h, line 829
#01: nsCOMPtr<nsIFile>::operator->() const (nsCOMPtr.h:828, in XUL)
#02: XRE_XPCShellMain (XPCShellImpl.cpp:1281, in XUL)
#03: main (xpcshell.cpp:43, in xpcshell)

Line 1281 looks like this:


where the code around that line is:

        nsCOMPtr<nsIFile> tmpDir;
        XRE_GetFileFromPath(argv[0], getter_AddRefs(tmpDir));

In my case, what I see is that argv[0] is "../obj-firefox/dist/bin/xpcshell" and XRE_GetFileFromPath returns a null tmpDir.

Same thing if I try to run ../obj-firefox/dist/, by the way.
In what context is xpcshell run directly rather than via mozharness or mach? I'm sure it's a legitimate use case, just never heard about it... I'm happy to look into this.
Assignee: nobody → spohl.mozilla.bugs
Flags: needinfo?(bzbarsky)
> In what context is xpcshell run directly rather than via mozharness or mach?

I just wanted a repl in the xpcshell environment to test a few things about that particular environment faster than I can by doing the full run/modify cycle on an xpcshell test.  This is not exactly a rare use case.
Flags: needinfo?(bzbarsky)
Attached patch PatchSplinter Review
Just to make sure that this was mentioned somewhere: this doesn't appear to be a regression. Unless I'm missing something, I don't believe invoking xpcshell as described here ever worked.

This patch fixes the issue when invoking xpcshell inside the .app bundle. I'm not sure if running the xpcshell binary from dist/bin is something we want to support. If we do, we should file a separate bug as this will need more invasive changes to support both the v2 bundle structure for the .app bundle as well as the dist/bin structure where all files live in the same directory.
Attachment #8508201 - Flags: review?(smichaud)
Comment on attachment 8508201 [details] [diff] [review]

This looks fine to me -- I can't see how it would cause any harm.
Attachment #8508201 - Flags: review?(smichaud) → review+
Running dist/bin/xpcshell absolutely used to work.
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #5)
> Running dist/bin/xpcshell absolutely used to work.

With a relative path for its invocation on the Terminal, i.e. "../obj-firefox/dist/bin/xpcshell" or "../obj-firefox/dist/"?

Are you saying that it isn't sufficient to have the xpcshell inside the .app bundle working, but we also need to be able to run it out of dist/bin? If that's the case I may as well just fix it as part of this bug.
Flags: needinfo?(benjamin)
Just to clarify why I didn't think this way (comment 0) of invoking xpcshell ever worked is due to the platform implementation of |nsLocalFile::InitWithNativePath| [1]. If the path is empty, or does not start with a '/', we return |NS_ERROR_FILE_UNRECOGNIZED_PATH|. The reason why is given in nsIFile.idl [2], i.e. relative paths are treated as error.
I'm not sure what that has to do with this, though. I certainly have run just ./xpcshell before successfully. Not this year, though: I don't hack on mac that much.
Flags: needinfo?(benjamin)
|nsLocalFile::InitWithNativePath| is called indirectly by |XRE_GetFileFromPath|, and since it doesn't support relative paths, it returned a null |tmpDir| in comment 0.

I'm still not clear whether we want to support both running xpcshell out of dist/bin as well as the .app bundle though. In order to support that, we'd need to change the XPCShellImpl to allow for a GreD pointing at dist/bin, avoid loading libraries at a relative path of ../MacOS etc. The only reason why I'm a bit hesitant is because it increases code complexity, which will make it more difficult to maintain (especially for someone who may not be familiar with the history of the v2 signing changes). I'm happy to make this change if we deem it necessary, so just let me know.
Went ahead and landed the patch here, since we'll want it no matter what decision we make about running xpcshell out of dist/bin. Marking this bug as leave-open until I hear back.
Keywords: leave-open
This would make it possible to run xpcshell out of dist/bin. Feel free to r- if we don't want this and I'll close the bug. Thanks!
Attachment #8509592 - Flags: review?(benjamin)
Comment on attachment 8509592 [details] [diff] [review]
Enable running out of dist/bin

As long as the user can run xpcshell from somewhere, I don't care whether it's dist/bin or dist/

But if dist/bin isn't going to work, it would be nice if it at least showed a reasonable error message. Since this patch also appears to work, we can go with this.
Attachment #8509592 - Flags: review?(benjamin) → review+
Thanks, Benjamin!

Pushed to inbound:
Keywords: leave-open
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.