178546 - regression: addFile (AppleSingle) doesn't work on Mac OS X

Status: RESOLVED FIXED

(Core Graveyard :: Installer: XPInstall Engine, defect)

Description

[anthonyd]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.0.1) Gecko/20020823 Netscape/7.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.0.1) Gecko/20020823 Netscape/7.0

regression: xpi install not copying binaries to location, even though it is
reporting success.  The install.js file in the example does find the correct
directory, but it seems the addFile method is failing.

Reproducible: Always

Steps to Reproduce:
1.  Have a clean install of a Geck-based browser on your Mac OS X system
2.  Drop the Musicnotes.xpi xpi installer on to a browser window (the file can
be found in the musicnotes.zip file in the url link above).
3.  Give permission for the xpi installer to install the plugin

Actual Results:  
xpi install file does not copy the binary to the system

Expected Results:  
copied the plug-in binary to the plugins folder

I am marking this major as it is a regression and it effects plugin installs on mac.

Comment 1

[anthonyd]

cc'ng aruner and peterl.  I am also gonna confirm this bug as I ahve confirmed
it several times already.

Comment 2

[anthonyd]

Attachment: new testcase

this is my modified testcase that contains an alert showing you which directory
the install.js is attempting to copy the binaries to.

Comment 3

[Arun Ranganathan]

Since this is a regression, I'm putting regression keyword, and FWIW, I'm
putting nomination keyword.  Let's try not to have regressions :-(
I'm also changing the Component from plugins to installer.

Comment 4

[Arun Ranganathan]

Re-assigning to default component owner.

Comment 5

[Daniel Veditz [:dveditz]]

Please include the section of your install log that shows the failing install
(snip out any other installs to avoid confusion and reduce bloat).

By regression you mean this worked on OS X in Mozilla 1.0 or 1.0.1 and does not
work in 1.0.2? Or do you mean it works on Mac Classic and not OS X? I ask
because the user agent you attached was for 1.0.1 so it's not clear what this is
a regression from.

Comment 6

[anthonyd]

I'm not sure where to look to find this install log on ox x, but I dont think
there will be there anything recorded anywqay as the xpi installer reports
success (falsely).

Comment 7

[Arun Ranganathan]

Run a Unix "find" from the command prompt for install.log -- it should in fact
be in your application's directory.  Whether you fail or are successful,
install.log should have *something.*

Comment 8

[Jimmy Lee]

Here's what I get from my install.log:

-------------------------------------------------------------------------------
file:///jimmymac/Users/jimmylee/Desktop/Musicnotes%20Folder/Musicnotes_OSX.xpi 
--  11/06/2002 11:42:47
-------------------------------------------------------------------------------


     Install **FAILED** with error -207  --  11/06/2002 11:42:50

-207 means Can't Read Archive.

Comment 9

[Jimmy Lee]

The following install.log shows that I can add a file though.  I am able to
install to the Plugins directory(a_getfolder_2_plugins), and I am able to
install using the same form of addFile (a_addfile_1).  The only problem I know
of installing binaries on OS X can be seen in Bug 175401.  Hope this helps.


-------------------------------------------------------------------------------
http://jimbob.mcom.com/jars/a_addfile_1.xpi  --  11/06/2002 12:00:20
-------------------------------------------------------------------------------

     Acceptance: a_addfile_1
     -----------------------

     [1/1]	Installing:
jimmymac:Projects:nscp110502_osx_trunk:Netscape.app:Contents:MacOS:smrtupdt.txt

     Install completed successfully  --  11/06/2002 12:00:20

-------------------------------------------------------------------------------
http://jimbob/jars/a_getfolder_2_plugins.xpi  --  11/06/2002 12:00:31
-------------------------------------------------------------------------------

     Acceptance: a_getfolder_2_plugins
     ---------------------------------

     [1/2]	Create Folder:
jimmymac:Projects:nscp110502_osx_trunk:Netscape.app:Contents:MacOS:Plug-ins:getfolder_plugins
     [2/2]	Installing:
jimmymac:Projects:nscp110502_osx_trunk:Netscape.app:Contents:MacOS:Plug-ins:getfolder_plugins:smrtupdt.txt

     Install completed successfully  --  11/06/2002 12:00:31

Comment 10

[Daniel Veditz [:dveditz]]

In the interest of shorter and faster-loading bug pages please put install log
snippets and the like in attachments.

Anthony: even if the install reported success the install log may have
additional information that can help. Depending on how the install script was
written many naive authors ignore errors from commands like addDirectory() and
call performInstall() anyway, which will then likely report that it was
successful at doing nothing.

Jimmy: could you work with Anthony on this? A -207 error (usually a corrupt
archive) is not likely to have looked like a success to him so maybe the two of
you are testing different archives or under different conditions.

Comment 11

[Daniel Veditz [:dveditz]]

What's the date of the attached page? He says

"I'm the guy who crafted the XPI installer for Netscape 6.2 on Windows. I also
wrote the Mac version of the plugin, which is now in beta test. The XPI
installer for Mac OS X that worked flawlessly a month ago no longer works. The
symptoms are that the download occurs, the install.js script runs, the thing
reports success, but no plugin binary is to be found on disk. I have added alert
statements to my script so I know it is being run. I have cleared my disk cache
so I know it is downloading a fresh XPI. I already figured out that XPI installs
plugins inside the App's package directory, not in /Library/Internet Plugins. 
I'm attaching the XPI Package and a test HTML page in a ZIP file."

Steve: should Mac OS X plugins be installed in /Library/Internet Plugins? We
could do that. (We ask the directory service, so maybe more places need to
change as well?)

Comment 12

[Jimmy Lee]

I'm still investigating if anybody is wondering.  I can install a text file with
the same install.js. 

Comment 13

[Steve Dagley]

Dan,

Yes, ~/Library/Internet Plug-Ins would be preferred as the location to install
plugins rather than the application bundle as that way they aren't lost when
someone updates the browser (and they're available to all installed browsers). 
Note that I said ~/Library rather that /Library though - to install into the
root /Library rather than the user's local ~/Library we'd have to add support to
authenticate to the OS before the copy would be allowed.  I also added ccarlen
to the cc: list as I don't know which directory the special system directory
service will return

Comment 14

[Daniel Veditz [:dveditz]]

What does that mean -- so does the attached install a file and it doesn't run,
or it doesn't install it at all?

Comment 15

[Jimmy Lee]

My own test at http://jimbob.mcom.com/jars/f_addfile_macsmpltxt.xpi installs a
binary successfully.  I think there is more to it than just binary.

Comment 16

[Jimmy Lee]

This behavior is observed in 7.0 as well.  This is not a regression from 7.0.

Comment 17

[Syd Logan]

Ok, so the log file shows "success" for me, but the file isn't there.

weeeeeeird.

Making a debug build to see if I can get lucky and figure out what is happening.

[1/1]   Installing: Mac
OS:Netscape:Netscape.app:Contents:MacOS:Plug-ins:Musicnotes Install completed
successfully  --  11/06/2002 15:58:32

Comment 18

[Arun Ranganathan]

Syd's Comment 17 goes straight to the heart of the problem -- it shows success,
but it looks like nothing's happening.  FWIW, here's what the developer said:

"By the way, I have NS 6.2, NS 7.0 and Mozilla 1.0 on my Classic Mac OS 9.2
system, and XPI install works beautifully on all of those browsers."

So it seems to be an OSX problem.  But I guess what we need to do is figure out
why Jimmy's test case works, whereas Syd's fails. Tony's test cases fail also.

Comment 19

[Syd Logan]

I'm running MacOS X 10.1 by the eway. And I am installing to an HFS volume.

If you are failing, pease please please tell me what version os MacOS X you are
running and the file system type you installed the browser to.

Comment 20

[Steve Dagley]

Ok, grasping at straws here.  The path being reported is appropriate to be
installing the plugin inside the app bundle.  Has anyone tried re-booting and
then looking inside the app bundle to see if the file got installed?  Sometimes
the OS X Finder doesn't notice a folder's contents have changed so if you have a
view of that folder open while installing that might bite you.

Comment 21

[Conrad Carlen (not reading bugmail)]

I'm not familiar with this code. When the log file shows "success," what is
actually done to determine success? That no error was returned along the way?
Or, a stat is done on the destination file?

Comment 22

[Michael Buckland]

anthonyd, what is the basis you are  using to call this a regression? 
Specifically, do you see this problem using MacOSX and Netscape 7.0?  If so, has
there been any change to the plugin itself since about 8/25/02?

Comment 23

[anthonyd]

I called this a regression because the music notes contact stated that this
worked prior to 7.0.  His report also states that this is the exact same xpi
install file that worked on gecko-based browsers prior to 7.0.

Comment 24

[anthonyd]

Syd, that sounds exactly like my install of os x.

Steve, I tried rebooting, and installing, and reinstalling, and rebooting.  I
dont have the particular folder open when I install either.

I'm wondering if maybe this is something specific to os x though and this
particular binary - maybe 2 great flavors that dont go great together.

Comment 25

[Jimmy Lee]

Attachment: Installs SimpleText to browser plugin directory

Attached is a xpi I created using the same install.js as the MusicNotes xpi. 
Instead of installing MusicNotes, I changed the binary to SimpleText.

The install.log reflects a successfull installation as expected.  The
SimpleText binary is found my Plugin-ins folder as expected.  Is there
something distinctly different between MusicNotes and SimpleText?

Comment 26

[Michael Buckland]

Could commenters say if using 10.1 or 10.2?  

Comment 27

[Daniel Veditz [:dveditz]]

Based on Jimmy's comment 25 I don't see how this could be stop-ship, it's
something very particular to this example. My prime suspect at this point would
be that the apple single decoding is failing and not returning an error -- but
that's purely a guess.

Can we get the actual OS X binary for this plugin so we can package it ourselves?

Has anyone tried installing this xpi on Mac Classic? We know that works in
general so the answer would help point us at an OS X problem or a package problem.

Comment 28

[Conrad Carlen (not reading bugmail)]

> Is there something distinctly different between MusicNotes and SimpleText?

The SimpleText installed by your test script is a Classic app and, as such, the
contents are in the rsrc fork and not the data fork. Data: 0 Bytes, Resource:
58,770 Bytes. If Music Notes is a modern OSX app, it's probably the other way
around. Possibly that difference horks the installation because we're not
dealing properly with various file forks or the lack thereof?

Comment 29

[Steve Dagley]

Maybe MusicNotes is a plugin bundle?

Comment 30

[Syd Logan]

The install script is executing. The engine does detect the contents as being
apple single, and it is creating a file called "musicnotes" in the plugins
folder that is in my debug build dir. Sherlock identifies the file as type
"text" when it locates it. If I just continue in the debugger, that file gets
wiped; I'm trying to identify the best I can what is causing that to happen. 

Comment 31

[Jimmy Lee]

For what it's worth Syd, I can install a text file as mentioned in Comment #12.

Comment 32

[Syd Logan]

Line 2673 in nsInstall.cpp seems to be where we delete the file that was created
(see the line with the ---> below). Unclear to me if this is evil or not, just
reporting what I am seeing as I debug. Dan, and comments?

    if ( nsAppleSingleDecoder::IsAppleSingleFile(&extractedSpec) )
    {
        nsAppleSingleDecoder *asd = new nsAppleSingleDecoder(&extractedSpec,
&finalSpec);
        OSErr decodeErr = fnfErr;

        if (asd)
            decodeErr = asd->Decode();

        if (decodeErr != noErr)
        {
            if (asd)
                delete asd;
            return EXTRACTION_FAILED;
        }

        if ( !(extractedSpec.vRefNum == finalSpec.vRefNum) ||
             !(extractedSpec.parID   == finalSpec.parID)   ||
             !(nsAppleSingleDecoder::PLstrcmp(extractedSpec.name, finalSpec.name)) )
        {
            // delete the unique extracted file that got renamed in AS decoding
--->            FSpDelete(&extractedSpec);

            // "real name" in AppleSingle entry may cause file rename
            tempExtractHereSpec->InitWithFSSpec(&finalSpec);
            extractHereSpec = do_QueryInterface(tempExtractHereSpec, &rv);
        }
    }

Comment 33

[Michael Buckland]

Discussed in bbird team meeting.  We know this is not a regression.  We don't
see this as a stop ship bug.  Minusing, removing regression keyword, nominating
for buffy and linking to our metabug in case we need to revisit.

Comment 34

[Syd Logan]

After the above function executes, and musicnotes is removed, there is a file
called "decode" which is left in the plugins folder. This all happens during the
prep stage. When we go to finalize the install, we find ourselves in the
following function:

PRInt32 ReplaceFileNowOrSchedule(nsIFile* replacementFile, nsIFile* doomedFile,
PRInt32 aMode)

The intent I believe is to rename the file named "decode" but all that happens
apparently is the file gets removed.

Going to run through another pass in the debugger to see if I can figure out why.

Comment 35

[Syd Logan]

Hrmmm,

In ReplaceFileNow() (ScheduleTasks.cpp), the call to MoveToNative causes the
file "decode" to be renamed to "Musicnotes" as expected, but immediately
following this, the call to to DeleteFileNowOrSchedule() blows "Musicnotes"
away. Major problem there! Perhaps the logic that surrounds this code in
ReplaceFileNow() has some flaw. The primitive calls themselves seem to be being
do what they are supposed to...

        rv = doomedFile->GetNativeLeafName(leafname);
        if ( NS_SUCCEEDED(rv))
            rv = replacementFile->MoveToNative(parentofReplacementFile, leafname);

        if ( NS_SUCCEEDED(rv) )
        {
            // we replaced the old file OK, now we have to
            // get rid of it if it was renamed out of the way
            result = DeleteFileNowOrSchedule( renamedDoomedFile );
        }

Comment 36

[Syd Logan]

In otherwords, the bug is we are renaming a file and then immediately blowing it
away.

Comment 37

[Jimmy Lee]

Ran some installs of MusicNotes.xpi on earlier versions.  Netscape 6.23, 6.22,
6.21, and 6.2 on Mac OS 10.1.5 all reported in the Install Log that the plugin
installed successfully.  However, in every version, the plugin file did not install.

To answer Dan's earlier question, I was unsuccessful with installing
MusicNotes.xpi on Mac OS 9.2.2 with today's branch build.  Again, the Install
Log reported success, but the file was not found in the Plugin-ins folder.

Interesting, eh?

Comment 38

[Syd Logan]

Attachment: Proposed patch to fix this

Declare a booling flag variable for each use, so the first doesn't clobber the
second. Running with this causes plugin to get installed, and it shows up in
about:plugins.

Comment 39

[Syd Logan]

--> syd (I want credit for this fix :-)

Comment 40

[Sean Su]

Comment on attachment 105524 [details] [diff] [review]
Proposed patch to fix this

r=ssu for this patch.  

This patch will help with the error checking and prevent this particular
problem, but not necessarily why the file did not get decoded into it's final
name.

Arun, can you find out from the developer how to verify his plugin was properly
installed?

Syd, Jimmy also has a test case now with a simpler file that might help with
verifying if this patch wors.  You should try to hook up with him to run his
test case both under Mac Os X and 9.x.

Comment 41

[Syd Logan]

Well, I'd swear watching in the debugger that everything got done exactly right.
The file was identified as apple single, the apple single decoder decoded it and
saved it as "decode", it was converted to the proper file name (so, it was
converted to the final name, contrary to sean's assertion), and then it was
(mistakenly) deleted because of the double use of that flag variable. With the
patch, that last problem is removed, and about:plugins reports the plugin being
installed. I think the final (and only) remaining step for now is to see that
the plugin actually works, and for that, we will need some test case from the
plugin designer.

So, reporter, what exactly does this plugin do, and what are the steps to verify
that it is working?

Thanks for the r= sean.

Comment 42

[anthonyd]

The reporter does not as of yet have a bugzilla account, but I will send them an
email with the questions you seek answers to Syd.

Comment 43

[Peter Lubczynski]

re: comment #40 - when the plugin appears in about:plugins it is properly
installed. The mime info would only appear if we were able to extract it from
the plugin.

Comment 44

[Michael Buckland]

Removing bbird metabug blockage, as bbird has shipped.

Comment 45

[Sean Su]

over to me

Comment 46

[Samir Gehani]

Installer triage team: nsbeta1+/adt1

Comment 47

[Arun Ranganathan]

I'm changing the bug description.  I've been hacking away at this for ages,
slowly going nuts, and I'm quite happy confirming anthonyd's original findings.
 I wish this was a stop ship :-( but we have time to nail it yet.
The bottom line is, this bug stops XPInstall from working on OSX.  Test cases
and install.js coming up as attachments!

Comment 48

[Arun Ranganathan]

Attachment: install.js file that installs to the plugins directory

This is an install.js file that puts a file that I create (myfile.xyz) in the
Plugins directory.  Thus, my XPI file (called test.xpi) consists of myfile.xyz
and install.js.  When I run the install.js file, I get SUCCESS from addFile,
but nothing shows up in the standard plugins directories.

Comment 49

[Arun Ranganathan]

Attachment: Actual XPInstall file that runs the install.js in Attachment 115688 and returns SUCCESS on addFile (but really fails).

This is the XPInstall file which contains the install.js file which is
Attachment 115688 [details] .  Note that it installs a file, myfile.xyz, to the Plugins
directory.  It also throws up a few alerts.

Comment 50

[Arun Ranganathan]

Incidentally, this works beautifully with OS 9.2 -- my XPInstall script installs
to the application's Plugins directory.  My OS X version of Netscape doesn't
appear to have its own application bundle; in other words, it doesn't seem to
have it's own plugins directory other than /Library/Internet Plug-ins and
/Users/aruner/Library/Internet Plug-ins.

Comment 51

[Daniel Veditz [:dveditz]]

cc:syd -- this sounds awfully familiar, I think he tracked this down once.

IIRC it was something about Applesingle decoding having a nameclash with the
original temp file, and then the resulting file was deleted as temp trash.

Someone please try
http://www.mozilla.org/quality/smartupdate/xpis/acceptance/a_addfiledel.xpi

If it's the same as the old problem it should work since that's a simple text
file that doesn't go through applesingle decoding. It will narrow down the problem.

Comment 52

[Arun Ranganathan]

I have actually been an alarmist in this bug so I must tidy up some of the
damage I've caused :-)

1.  I spent some time with peterl and sdagley and I now realize that in fact
getFolder("Plugins") returns the right folder and that invocation to addFile
works to install the random myfile.xyz to the right folder, namely the app.
bundle's Plugins directory (say in Netscape.app).  

2. dveditz's test case is also successful -- under MacOS folder in Contents, I
notice addfiledelete with smardupdt.txt

3. the real bug is why the MusicNotes.xpi doesn't work (anthonyd's original
bug).  It doesn't appear to install to the application bundle's plugins folder.

4. (sidenote) I've asked for RFE so that we can make installations to locations
such as /Library/Internet Plugin-ins.  We can do this now (perhaps by using
getFolder("file:///") and then working to the right location, but there's no
convenient syntactic sugar to getFolder for this.  This is the subject of bug 195148

Comment 53

[Sean Su]

I wonder if bug 195109 has anything to do with this.

Comment 54

[Sean Su]

I just tried anothonyd's MusicNotes.xpi and it did fail with -207.

I then, under Mac OSX, unzipped it and simply rezipped it and tried installing
again.  This time it worked.  I'm not sure why, but maybe our libjar isn't as
forgiving as unzip? Running a zip -T on the original .xpi file shows no problems.

Closing as worksforme, not sure what else to close it as (maybe invalid?).

Comment 55

[Grace Bush]

verified on build 2003040403
(had to unzip and rezip the attachment but it installed)

Comment 56

[Daniel Veditz [:dveditz]]

This is still happening to people. WFM isn't right.

Comment 57

[Daniel Veditz [:dveditz]]

In order to fail the AppleSingle file either needs to have no embedded name, or
a name different from the as-installed filename. If the embedded name is the
same as the target filename given to nsInstallFile then it works OK.

The symptoms are exactly as described by Syd, dunno why this patch was never
applied.

Comment 58

[Johnny Stenback (:jst)]

Attachment: Test XPI file (uses addDirectory)

This XPI file shows that there's still a problem with AppleSingle encoded
files. Specifically the problem seems to be with AS files that don't encode the
original name into the AS file. This XPI file installs a file called foo into
the plugins directory. The foo file is named foo in the XPI file, and it
doesn't contain the original name, so it should be undecoded into a temp file,
and then moved back to foo, and it seems like that happens, but after that, the
foo file is deleted, and we're left with no file at all.

If the AS file contains the original name, the file is decoded and it's not
deleted, so that's a workaround for this problem, but the problem still exists
when the original name is not part of the AS file.

Comment 59

[Peter Van der Beken [:peterv]]

Attachment: Updated patch to trunk

The patch does fix the problem with the jst's XPI.

Comment 60

[Johnny Stenback (:jst)]

Comment on attachment 160661 [details] [diff] [review]
Updated patch to trunk

sr=jst

Comment 61

[Daniel Veditz [:dveditz]]

Comment on attachment 160661 [details] [diff] [review]
Updated patch to trunk

r=dveditz

Comment 62

[Peter Van der Beken [:peterv]]

Comment on attachment 160661 [details] [diff] [review]
Updated patch to trunk

Simple fix to a bug in XPInstall that makes us delete a file that was just
installed but claim the install succeeded anyway. Low risk.

Comment 63

[Daniel Veditz [:dveditz]]

Comment on attachment 160661 [details] [diff] [review]
Updated patch to trunk

a=dveditz for the 1.7 branch

Comment 64

[Asa Dotzler [:asa]]

Comment on attachment 160661 [details] [diff] [review]
Updated patch to trunk

a=asa for aviary checkin.

Comment 65

[Peter Van der Beken [:peterv]]

Checked in on trunk and branches.