202772 - Port AppleSingle decoding in the XPInstall engine from Mac OS Classic to Mac OS X

Status: RESOLVED FIXED

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

Description

[Samir Gehani]

With the switch from CFM to Mach-O builds in 2003-01 we no longer compile the
AppleSingle decoding code in the XPInstall engine.  The fix for this defect will
result in restored support for AppleSingle decoding on Mac OS X.  This will
enable plugin vendors depending on this support to to continue using XPInstall
to deploy binaries that contain resource forks.

Comment 1

[Samir Gehani]

Attachment: Patch v.0.1

Support for standalone AppleSingle decoder (tested on files and folders
successfully) as well integrated in the XPInstall engine (not yet tested).

Comment 2

[Samir Gehani]

Attachment: Makefile to test STANDALONE_ASD

Comment 3

[timeless]

i think lordpixel did this recently..

Comment 4

[Simon Fraser [no longer active]]

lordpixel did an AS encoder, not decoder.
Note also that we already have a simple decoder in mozilla/config/asdecode.cpp

Comment 5

[Andrew Thompson]

What I did was bug 201868.

"ASEncoder" is the name of the application, but actually it both encodes and
decodes AppleSingle files and directories. Looking at Samir's patch here
briefly, there's obviously a lot of overlap. I was concentrating on just making
a straight Carbonization of the existing application, whereas he seems to have
added a Unix style main() to allow command line use.

Seems there are 3 things around:

1. /config/asdecode.cpp - written by Patrick Beard.
This is much much more basic than Samir's patch. eg, it doesn't restore Finder
creator and type codes, which causes issues with some tools. Samir's code seems
to implement most (all?) of the AppleSingle spec.

2. My patches, which make the "ASEncoder" Carbon app build in CodeWarrior and
run on OS X

3. Samir's changes, which build the "ASEncoder" app Mach-O using gcc, AFAICT.

The stuff I did is probably redundant, therefore.

I admit, I was surprised to discover there are no standalone AppleSingle or
AppleDouble decoders out there (the only one I found outputs your files as email
attachments). Stuffit will decode, I think, but not encode.

I'm some interest in wrapping a Cocoa GUI around these .cpp classes and adding
AppleDouble + drag & drop etc. Assuming nonone objects, its probably something
I'd fork and then post on my own webpages, rather than checking it into the tree
(don't think Mozilla needs it...) Naturally, anything I build would be
LGPL/GPL/MPL. Let me know if you have any thoughts.

Comment 6

[Simon Fraser [no longer active]]

Comment on attachment 121184 [details] [diff] [review]
Patch v.0.1

>Index: nsAppleSingleDecoder.cpp
>===================================================================


Can you attach the entire file? It's hard to see the diffs.

> 
>+#ifdef STANDALONE_ASD
>+int
>+main(int argc, char **argv)
>+{
>+  OSErr err;
>+  Str255 pEncodedName, pDecodedName;
>+  FSSpec encoded, decoded;
>+  nsAppleSingleDecoder *decoder = new nsAppleSingleDecoder;
>+  Boolean isDir;
>+
>+  if (argc < 2)
>+  {
>+    cout << "usage: " << argv[0] << " <file/folder>" << endl
>+         << "\t\tAppleSingle encoded file" << endl 
>+         << "\t\tor" << endl
>+         << "\t\tfolder containing AppleSingle encoded files" << endl
>+         << "\t\tnames of which are in cwd." << endl;
>+    exit(-1);
>+  }
>+
>+  CopyCStringToPascal(argv[1], pEncodedName);
>+  err = FSMakeFSSpec(0, 0, pEncodedName, &encoded);
>+  cout << "FSMakeFSSpec returned: " << err << endl;
>+  if (err != noErr) goto AU_REVOIR;

Shouldn't this return a non-zero error value (and not use goto).

>Index: nsInstall.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/xpinstall/src/nsInstall.cpp,v
>retrieving revision 1.210
>diff -w -u -6 -r1.210 nsInstall.cpp
>--- nsInstall.cpp	7 Mar 2003 06:07:52 -0000	1.210
>+++ nsInstall.cpp	21 Apr 2003 11:00:12 -0000
>@@ -73,13 +73,13 @@
> #include "nsWinProfile.h"
> #endif
> 
> #include "nsInstallFileOpEnums.h"
> #include "nsInstallFileOpItem.h"
> 
>-#ifdef XP_MAC
>+#ifdef XP_MACOSX

Please don't break XP_MAC. Use #if defined(XP_MAC) || defined(XP_MACOSX)

> #include "Gestalt.h"

<Gestalt.h>


>-#ifdef XP_MAC
>+#ifdef XP_MACOSX

Ditto.

> #define GESTALT_CHAR_CODE(x)          (((unsigned long) ((x[0]) & 0x000000FF)) << 24) \
>                                     | (((unsigned long) ((x[1]) & 0x000000FF)) << 16) \
>                                     | (((unsigned long) ((x[2]) & 0x000000FF)) << 8)  \
>                                     | (((unsigned long) ((x[3]) & 0x000000FF)))
>-#endif /* XP_MAC */
>+#endif /* XP_MACOSX */
> 
> PRInt32
> nsInstall::Gestalt(const nsString& aSelector, PRInt32* aReturn)
> {
>     *aReturn = nsnull;
> 
>@@ -917,13 +917,13 @@
> 
>     if (result != nsInstall::SUCCESS)
>     {
>         *aReturn = SaveError( result );
>         return NS_OK;
>     }
>-#ifdef XP_MAC
>+#ifdef XP_MACOSX

And again.

>     long    response = 0;
>     char    selectorChars[4];
>     int     i;
>     OSErr   err = noErr;
>     OSType  selector;
>@@ -941,13 +941,13 @@
> 
>     if (err != noErr)
>         *aReturn = err;
>     else
>         *aReturn = response;
> 
>-#endif
>+#endif /* XP_MACOSX */
>     return NS_OK;
> }
> 
> PRInt32
> nsInstall::GetComponentFolder(const nsString& aComponentName, const nsString& aSubdirectory, nsInstallFolder** aNewFolder)
> {
>@@ -2144,12 +2144,15 @@
> nsInstall::FileOpFileMacAlias(nsIFile *aSourceFile, nsIFile *aAliasFile, PRInt32* aReturn)
> {
> 
>   *aReturn = nsInstall::SUCCESS;
> 
> #ifdef XP_MAC
>+
>+// XXX port to Mac OS X
>+
>   nsInstallFileOpItem* ifop = new nsInstallFileOpItem(this, NS_FOP_MAC_ALIAS, aSourceFile, aAliasFile, aReturn);
>   if (!ifop)
>   {
>       *aReturn = SaveError(nsInstall::OUT_OF_MEMORY);
>       return NS_OK;
>   }
>@@ -2651,13 +2654,13 @@
>           case NS_ERROR_FILE_DISK_FULL:             return INSUFFICIENT_DISK_SPACE;
>           case NS_ERROR_FILE_TARGET_DOES_NOT_EXIST: return DOES_NOT_EXIST;
>           default:                                  return EXTRACTION_FAILED;
>         }
>     }
> 
>-#ifdef XP_MAC
>+#ifdef XP_MACOSX

Here too.

>     FSSpec finalSpec, extractedSpec;
> 
>     nsCOMPtr<nsILocalFileMac> tempExtractHereSpec;
>     tempExtractHereSpec = do_QueryInterface(extractHereSpec, &rv);
>     tempExtractHereSpec->GetFSSpec(&extractedSpec);
>

Comment 7

[Samir Gehani]

Attachment: Patch v.0.2

(*) Incorporate Simon's suggestions: don't break XP_MAC and address nits.
(*) Fix other XP_MAC sections to compile on Mac OS X including fileMacAlias()
API.
(*) Tested in XPInstall engine: passes for installing files and folders.

Comment 8

[Samir Gehani]

Simon,
Also note that patch v.0.2 (attachment 121394 [details] [diff] [review]) has the full source of the
modified nsAppleSingleDecoder.{h,cpp} files concatenated after the actual patch
per your request.

Comment 9

[Samir Gehani]

adt: nsbeta1+/adt2

Comment 10

[Samir Gehani]

Attachment: Patch v.0.3

(*) Switched from FSSpecs -> FSRefs in nsAppleSingleDecoder.
(*) Made fileMacAlias() work.

Comment 11

[Samir Gehani]

Comment on attachment 123088 [details] [diff] [review]
Patch v.0.3

Reviewers, 
I've attached the source of nsAppleSingleDecoder.{h,cpp} to the tail of the
patch for your reference (since the patch may be hard to read).

Comment 12

[Samir Gehani]

Attachment: Patch v.0.4

Address ssu's review comments:
(1) Change last param of FSRenameUnicode() to NULL and
    add comment regarding reason for following FSMakeFSRefUnicode() calls.
(2) Make DecoderFolder() be recursive.
(3) Change FSIterateContainer()'s wantFSSpec to be false now.
(4) Use APPLESINGLE_MAGIC defines in place of inlined 0x00051600 values.
(5) Remove irrelevant comment in ProcessFileDates() initial entry length check.

Comment 13

[Conrad Carlen (not reading bugmail)]

Comment on attachment 123354 [details] [diff] [review]
Patch v.0.4

>Index: nsLocalFileOSX.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/xpcom/io/nsLocalFileOSX.cpp,v
>retrieving revision 1.15
>diff -u -6 -r1.15 nsLocalFileOSX.cpp
>--- nsLocalFileOSX.cpp	21 Apr 2003 21:43:38 -0000	1.15
>+++ nsLocalFileOSX.cpp	15 May 2003 00:51:46 -0000
>@@ -1897,13 +1897,13 @@
>       mCachedFSRefValid = PR_TRUE;
>   }
>   if (mCachedFSRefValid) {
>     aFSRef = mCachedFSRef;
>     return NS_OK;
>   }
>-  return NS_ERROR_FAILURE;
>+  return NS_ERROR_FILE_NOT_FOUND;

See: http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsLocalFileOSX.cpp#941
This is done in 2 other places. At the time, I didn't want to move that into
GetFSRefInternal. It's an assumption (possible untruth) that I didn't want to
propagate. Now though, I think we do need to do that. So, remove the logic from
the other 2 places and add the same comment here.

>Index: Makefile.in
>===================================================================
>+# Support AppleSingle decoding on Mac OS X
>+ifeq ($(OS_ARCH),Darwin)

Don't test OS_ARCH to do this. Use:
ifneq (,$(filter mac cocoa,$(MOZ_WIDGET_TOOLKIT)))
Reason is, it's possible to build against X11 on OSX and, while OS_ARCH is
Darwin, it's not at all Mac.

>+REQUIRES	+= macmorefiles
>+
>+CPPSRCS		+= \
>+		nsAppleSingleDecoder.cpp \
>+		$(NULL)
>+
>+LIBS			+=	\
>+		$(DEPTH)/dist/lib/libmacmorefiles_s.a	\
>+		$(NULL)
>+endif

Looks like the following files have a mixture of indenting with tabs and
spaces.  Now would be a good time to de-tab them.

>Index: nsAppleSingleDecoder.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/xpinstall/src/nsAppleSingleDecoder.cpp,v
>retrieving revision 1.15
>diff -w -u -6 -r1.15 nsAppleSingleDecoder.cpp
>--- nsAppleSingleDecoder.cpp	23 Mar 2002 15:02:36 -0000	1.15
>+++ nsAppleSingleDecoder.cpp	14 May 2003 23:08:57 -0000
>@@ -22,58 +22,64 @@
> 
> 
> #ifndef _NS_APPLESINGLEDECODER_H_
> 	#include "nsAppleSingleDecoder.h"
> #endif
> 
>-#include "MoreFilesExtras.h"
>-#include "MoreDesktopMgr.h"
>-#include "IterateDirectory.h"
>+#include "MoreFilesX.h"
>+
>+#ifdef STANDALONE_ASD
>+#include <iostream>
>+using namespace std;
>+#undef MOZILLA_CLIENT
>+#endif
>+
> #ifdef MOZILLA_CLIENT
> #include "nsISupportsUtils.h"
> #endif
> 
> /*----------------------------------------------------------------------*
>  *   Constructors/Destructor
>  *----------------------------------------------------------------------*/
> #ifdef MOZILLA_CLIENT
> MOZ_DECL_CTOR_COUNTER(nsAppleSingleDecoder)
> #endif
> 
>-nsAppleSingleDecoder::nsAppleSingleDecoder(FSSpec *inSpec, FSSpec *outSpec)
>-: mInSpec(NULL), 
>-  mOutSpec(NULL),
>+nsAppleSingleDecoder::nsAppleSingleDecoder(const FSRef *inRef, FSRef *outRef)
>+: mInRef(NULL), 
>+  mOutRef(NULL),
>   mInRefNum(0),
>   mRenameReqd(false)
> {
> #ifdef MOZILLA_CLIENT
>     MOZ_COUNT_CTOR(nsAppleSingleDecoder);
> #endif
> 
>-    if (inSpec && outSpec)
>+  if (inRef && outRef)
> 	{
>-		/* merely point to FSSpecs, not own 'em */
>-		mInSpec = inSpec;
>-		mOutSpec = outSpec;
>+    /* merely point to FSRefs, not own 'em */
>+    mInRef = inRef;
>+    mOutRef = outRef;
> 	}
> }
> 


		   mOutSpec->name );
>+    // delete encoded version of target file
>+    MAC_ERR_CHECK(FSDeleteObject( mInRef ));
>+    MAC_ERR_CHECK(FSGetParentRef( mOutRef, &parentOfOutRef ));
>+    MAC_ERR_CHECK(FSRenameUnicode( mOutRef, nameInRef.length, 
>+      nameInRef.unicode, kTextEncodingUnknown, NULL ));
>+    // make the new FSRef since the old one is invalid and 
>+    // FSRenameUnicode doesn't fill in newRef

Is this a bug in the OS? The documentation for FSRenameUnicode says it fills in
the new ref in the last optional param (for which you're passing NULL) The
MoreFilesX code uses this all over the place - hard to believe it's really
broken.

>+    MAC_ERR_CHECK(FSMakeFSRefUnicode( &parentOfOutRef, nameInRef.length,
>+      nameInRef.unicode, kTextEncodingUnknown, mOutRef ));
>+    mRenameReqd = false; 
> 	}
> 	
>  	return err;
> }
> 


> 
> OSErr
> nsAppleSingleDecoder::ProcessRealName(ASEntry inEntry)
> {

Same thing here with FSRenameUnicode

>+    // rename decoded file to ``newName''
>+    MAC_ERR_CHECK(FSRenameUnicode( mOutRef, newNameUni.length, 
>+      newNameUni.unicode, kTextEncodingUnknown, NULL ));
>+  }
>+
>+  // make the new FSRef since the old one is invalid and 
>+  // FSRenameUnicode doesn't fill in newRef
>+  MAC_ERR_CHECK(FSMakeFSRefUnicode(&parentOfOutRef, newNameUni.length, 
>+    newNameUni.unicode, kTextEncodingUnknown, mOutRef));
> 
> 	return err;
> }
> 
> OSErr
> nsAppleSingleDecoder::ProcessFileDates(ASEntry inEntry)
> {

> #define YR_2000_SECONDS 3029529600
>-	pb.hFileInfo.ioFlCrDat = dates.create + YR_2000_SECONDS;
>-	pb.hFileInfo.ioFlMdDat = dates.modify + YR_2000_SECONDS;
>-	pb.hFileInfo.ioFlBkDat = dates.backup + YR_2000_SECONDS;
>-	/* Not sure if mac has the last access time */
>-	
>-	nsAppleSingleDecoder::PLstrncpy(name, mOutSpec->name, mOutSpec->name[0]);
>-	name[0] = mOutSpec->name[0];
>-	pb.hFileInfo.ioNamePtr = name;
>-	pb.hFileInfo.ioVRefNum = mOutSpec->vRefNum;
>-	pb.hFileInfo.ioDirID = mOutSpec->parID;
>-	pb.hFileInfo.ioFDirIndex = 0;	/* use ioNamePtr and ioDirID */
>-	err = PBSetCatInfo(&pb, false);

Are you sure about this? The same values are being used with the new HFS+ code,
which uses UTC time, and the old HFS code, which used local time. Which is
right?

>+  catInfo.createDate = (UTCDateTime) 
>+    {0, dates.create + YR_2000_SECONDS, 0};
>+  catInfo.contentModDate = (UTCDateTime)
>+    {0, dates.modify + YR_2000_SECONDS, 0};
>+  catInfo.accessDate = (UTCDateTime) 
>+    {0, dates.access + YR_2000_SECONDS, 0};
>+  catInfo.backupDate = (UTCDateTime)
>+    {0, dates.backup + YR_2000_SECONDS, 0};
>+  GetUTCDateTime(&catInfo.attributeModDate, kUTCDefaultOptions);  
>+
>+  MAC_ERR_CHECK(FSSetCatalogInfo(mOutRef, 
>+    kFSCatInfoCreateDate |
>+    kFSCatInfoContentMod |
>+    kFSCatInfoAttrMod    |
>+    kFSCatInfoAccessDate |
>+    kFSCatInfoBackupDate,
>+    &catInfo));
> 	
> 	return err;
> }
> 
>+Boolean
>+nsAppleSingleDecoder::UCstrcmp(const HFSUniStr255 *str1, 
>+                               const HFSUniStr255 *str2)
> {
>-	char *s;
>-	int i, j, sign, tmp;
>-	
>-	/* check sign and convert to positive to stringify numbers */
>-	if ( (sign = n) < 0)
>-		n = -n;
>-	i = 0;
>-	s = (char*) malloc(sizeof(char));
>+  OSStatus          status;
>+  Boolean           bEqual; 
>+  UCCollateOptions  opts = kUCCollateStandardOptions | 
>+                           kUCCollatePunctuationSignificantMask; 
>+

Is this guarenteed to be the right kind of comparison for what HFS+ uses
(canonically decomposed Unicode, case-insensitive)? See
http://developer.apple.com/technotes/tn/tn1150.html#UnicodeSubtleties

>+  status = UCCompareTextDefault(opts, str1->unicode, str1->length, 
>+                                str2->unicode, str2->length, &bEqual, NULL);
>+  if (status != noErr)
> 

Were complete copies of the patched files then appended onto this patch?

Comment 14

[Samir Gehani]

>>-  return NS_ERROR_FAILURE;
>>+  return NS_ERROR_FILE_NOT_FOUND;
> 
> See: http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsLocalFileOSX.cpp#941
> This is done in 2 other places. At the time, I didn't want to move that into
> GetFSRefInternal. It's an assumption (possible untruth) that I didn't want to
> propagate. Now though, I think we do need to do that. So, remove the logic 
> from the other 2 places and add the same comment here.

Sure.

> Looks like the following files have a mixture of indenting with tabs and
> spaces.  Now would be a good time to de-tab them.

Actually they don't have a mixture (only tabs).  Also, I did convert tabs to
spaces: see the appended full source files at the end of the patch.

>>+    MAC_ERR_CHECK(FSRenameUnicode( mOutRef, nameInRef.length, 
>>+      nameInRef.unicode, kTextEncodingUnknown, NULL ));
>>+    // make the new FSRef since the old one is invalid and 
>>+    // FSRenameUnicode doesn't fill in newRef
> 
> Is this a bug in the OS? The documentation for FSRenameUnicode says it fills 
> in the new ref in the last optional param (for which you're passing NULL) 
> The MoreFilesX code uses this all over the place - hard to believe it's 
> really broken.

It appeared broken while I was developing.  I retested now with the single
change and it works (i.e., newRef is valid).  Not sure why it was a problem
while developing but I've reverted to relying on newRef.  (I've since upgraded
from 10.2.5 to 10.2.6: could that have something to do with it?  Something for
QA to look out for.)

>> #define YR_2000_SECONDS 3029529600
>>-	pb.hFileInfo.ioFlCrDat = dates.create + YR_2000_SECONDS;
>>-	pb.hFileInfo.ioFlMdDat = dates.modify + YR_2000_SECONDS;
>>-	pb.hFileInfo.ioFlBkDat = dates.backup + YR_2000_SECONDS;
>>-	/* Not sure if mac has the last access time */
>>-	
>>-	nsAppleSingleDecoder::PLstrncpy(name, mOutSpec->name,
>>mOutSpec->name[0]);
>>-	name[0] = mOutSpec->name[0];
>>-	pb.hFileInfo.ioNamePtr = name;
>>-	pb.hFileInfo.ioVRefNum = mOutSpec->vRefNum;
>>-	pb.hFileInfo.ioDirID = mOutSpec->parID;
>>-	pb.hFileInfo.ioFDirIndex = 0;	/* use ioNamePtr and ioDirID */
>>-	err = PBSetCatInfo(&pb, false);
> 
> Are you sure about this? The same values are being used with the new HFS+
> code, which uses UTC time, and the old HFS code, which used local time. 
> Which is right?

Oops, nice catch.  I need to convert from local to UTC time.  Will change.

>>+  OSStatus          status;
>>+  Boolean           bEqual; 
>>+  UCCollateOptions  opts = kUCCollateStandardOptions | 
>>+                           kUCCollatePunctuationSignificantMask; 
>>+
> 
> Is this guarenteed to be the right kind of comparison for what HFS+ uses
> (canonically decomposed Unicode, case-insensitive)? See
> http://developer.apple.com/technotes/tn/tn1150.html#UnicodeSubtleties
> 
>>+  status = UCCompareTextDefault(opts, str1->unicode, str1->length, 
>>+                                str2->unicode, str2->length, &bEqual, NULL);
>>+  if (status != noErr)
 
I chatted with ftang who mentioned that this routine will handle normalization.
 He said it's OK since we are using this routine simply for comparison, not
collation.

> Were complete copies of the patched files then appended onto this patch?

Yes, per Simon's request: see comment 11 and comment 8.  Also, this shows that
indenting is now consistent throughout the nsAppleSingleDecoder.* source files.

Comment 15

[Samir Gehani]

Attachment: Patch v.0.5

Address Conrad's comments.

Comment 16

[Asa Dotzler [:asa]]

Comment on attachment 123460 [details] [diff] [review]
Patch v.0.5

a=asa (on behalf of drivers) for checkin to 1.4

Comment 17

[Samir Gehani]

Patch v.0.5 landed.