Last Comment Bug 747470 - Cannot launch applications in Windows XP/Vista
: Cannot launch applications in Windows XP/Vista
Status: VERIFIED FIXED
[marketplace-beta+]
:
Product: Firefox Graveyard
Classification: Graveyard
Component: Web Apps (show other bugs)
: 14 Branch
: x86 Windows XP
: -- critical
: Firefox 15
Assigned To: Tim Abraldes [:TimAbraldes] [:tabraldes]
: Jason Smith [:jsmith]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-20 11:07 PDT by Jason Smith [:jsmith]
Modified: 2016-02-04 15:00 PST (History)
7 users (show)
jsmith: in‑moztrap+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1 - Add USE_STATIC_LIBS to webapprt makefile (916 bytes, patch)
2012-04-23 15:57 PDT, Tim Abraldes [:TimAbraldes] [:tabraldes]
no flags Details | Diff | Review
Patch v2 - include correct makefile (1.13 KB, patch)
2012-04-24 11:49 PDT, Tim Abraldes [:TimAbraldes] [:tabraldes]
benjamin: review+
Details | Diff | Review

Description Jason Smith [:jsmith] 2012-04-20 11:07:47 PDT
Steps:

1. Go to https://apps.mozillalabs.com/appdir/
2. Install an application
3. Launch an application

Expected:

The application should start up in a chromeless window.

Actual:

The command prompt appears, but an error dialog is shown saying:

BarFight.exe - Unable to Locate Component

The application has failed to start because MSVCR100.dll was not found. Re-installing the application may fix this problem.
Comment 1 Tim Abraldes [:TimAbraldes] [:tabraldes] 2012-04-20 14:15:22 PDT
Verified with Jason that the issue goes away if msvcr100.dll is copied from the Firefox installation dir to the webapp's installation dir.

One option for fixing this issue is to copy msvcr100.dll from the Firefox installation dir to webapp installation dirs when installing the webapps.

However, the right way to fix this is probably to statically link against the CRT.  I'll investigate that approach and see if I hit any roadblocks.
Comment 2 Benjamin Smedberg [:bsmedberg] 2012-04-23 07:47:20 PDT
I don't think this is a valid bug. Web apps change all the time, so we don't really know or care what the version or publisher information is. I believe that the current behavior is most correct.
Comment 3 Benjamin Smedberg [:bsmedberg] 2012-04-23 07:47:45 PDT
Oops, this comment was meant for a different bug.
Comment 4 Jason Smith [:jsmith] 2012-04-23 12:20:09 PDT
Also getting a reproduction of this issue on Windows Vista.
Comment 5 Ted Mielczarek [:ted.mielczarek] 2012-04-23 12:35:19 PDT
Presumably it'd reproduce on any system < Windows 8 (which I'd guess ships this CRT) which hasn't had the CRT installed already (by installing VC2010 or something else that installs it as a dependency).

You can link your stub with USE_STATIC_LIBS = 1 in the Makefile, or you can copy the runtime files over. If you do the latter, don't hardcode the names, pull them from the build system somewhere, like, say:
http://mxr.mozilla.org/mozilla-central/source/build/win32/Makefile.in#72
Comment 6 Benjamin Smedberg [:bsmedberg] 2012-04-23 13:14:32 PDT
We should just add USE_STATIC_LIBS to /webapprt/win/Makefile.in. I thought I mentioned that somewhere but unless it causes some weird build bustage it is the correct way to do this.
Comment 7 Tim Abraldes [:TimAbraldes] [:tabraldes] 2012-04-23 15:57:37 PDT
Created attachment 617685 [details] [diff] [review]
Patch v1 - Add USE_STATIC_LIBS to webapprt makefile

I tried adding "USE_STATIC_LIBS = 1" on Friday but it seemed to have no effect on my local builds (the command line still showed "/MDd" and the built executable still tried to load msvcr100.dll at runtime).  Running the change through try; hopefully there was just something weird about my local builds.

https://tbpl.mozilla.org/?tree=Try&rev=e4704a8b0013
Comment 8 Tim Abraldes [:TimAbraldes] [:tabraldes] 2012-04-24 08:41:30 PDT
"USE_STATIC_LIBS = 1" seems to have had no effect on the try build.  I'll keep investigating...
Comment 9 Tim Abraldes [:TimAbraldes] [:tabraldes] 2012-04-24 11:49:50 PDT
Created attachment 617972 [details] [diff] [review]
Patch v2 - include correct makefile

This makefile included "$(top_srcdir)/config/config.mk" instead of including "$(DEPTH)/config/autoconf.mk"

Correcting this issue made the "USE_STATIC_LIBS = 1" line have the intended effect.

Running this through try:
https://tbpl.mozilla.org/?tree=Try&rev=bb42284bf942
Comment 10 Myk Melez [:myk] [@mykmelez] 2012-04-24 12:15:59 PDT
(In reply to Tim Abraldes from comment #9)
> This makefile included "$(top_srcdir)/config/config.mk" instead of including
> "$(DEPTH)/config/autoconf.mk"

It used to include both, but I changed it to include only the former:

https://github.com/michaelrhanson/mozilla-central/commit/145a776343b377ad5a1e62c3564f1f052ffa241d#diff-19

Because the former includes the latter:

http://mxr.mozilla.org/mozilla-central/source/config/config.mk#59

So I don't understand why this change has the desired effect.  Also, are you sure we don't need anything in $(top_srcdir)/config/config.mk?
Comment 11 Benjamin Smedberg [:bsmedberg] 2012-04-24 12:22:54 PDT
The correct format for a makefile is:

DEPTH = .. (etc)

include $(DEPTH)/config/autoconf.mk

# Set variables like PROGRAM/CPPSRCS/etc

include $(topsrcdir)/config/config.mk # this is optional, only necessary if you need to modify the variables it sets. rules.mk automatically includes config.mk

# override variables here

include $(topsrcdir)/config/rules.mk # required

# override rules here

So tim's patch is correct and necessary, because you are setting variables (in particular USE_STATIC_LIBS) after including config.mk, which means it will have no effect.
Comment 12 Tim Abraldes [:TimAbraldes] [:tabraldes] 2012-04-25 09:55:45 PDT
The previous try run failed, here's the retry:
https://tbpl.mozilla.org/?tree=Try&rev=bc3790fc9b6b
Comment 13 Jason Smith [:jsmith] 2012-04-25 11:27:23 PDT
Doing some bare bones testing with try build in comment 12 revealed the following behavior on XP/Vista:

- I can launch applications that were installed with the try build
- I cannot launch applications that were installed with the other nightly build that produced this bug
Comment 14 Tim Abraldes [:TimAbraldes] [:tabraldes] 2012-04-25 11:39:37 PDT
Comment on attachment 617972 [details] [diff] [review]
Patch v2 - include correct makefile

(In reply to Jason Smith from comment #13)
> Doing some bare bones testing with try build in comment 12 revealed the
> following behavior on XP/Vista:
> 
> - I can launch applications that were installed with the try build

Hooray!

> - I cannot launch applications that were installed with the other nightly
> build that produced this bug

This is expected.  Any broken app installations will have to be reinstalled.
Comment 15 :Felipe Gomes (needinfo me!) 2012-04-26 07:48:02 PDT
https://hg.mozilla.org/mozilla-central/rev/b952bb042f12
Comment 16 Jason Smith [:jsmith] 2012-04-26 14:15:29 PDT
Felipe - This lands in tomorrow's nightly build, correct? (Double checking, cause this isn't working on today's nightly build)

Note You need to log in before you can comment on or make changes to this bug.