Last Comment Bug 338987 - SoC Tracking - Scriptable Zipwriter
: SoC Tracking - Scriptable Zipwriter
Status: RESOLVED WONTFIX
:
Product: Other Applications
Classification: Client Software
Component: CCK (show other bugs)
: unspecified
: x86 Windows XP
: P1 normal (vote)
: ---
Assigned To: Benjamin Smedberg [:bsmedberg]
: Mike Kaply [:mkaply] (Out June 27-July 5)
Mentors:
Depends on:
Blocks: 338972
  Show dependency treegraph
 
Reported: 2006-05-23 08:07 PDT by chris hofmann
Modified: 2006-12-01 13:29 PST (History)
14 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
my code (15.62 KB, application/octet-stream)
2006-08-03 05:57 PDT, Lan Qiang
no flags Details
code snapshot (44.85 KB, application/octet-stream)
2006-08-21 06:00 PDT, Lan Qiang
no flags Details
compress level added (44.87 KB, application/octet-stream)
2006-08-24 08:01 PDT, Lan Qiang
no flags Details

Description chris hofmann 2006-05-23 08:07:50 PDT
Title/Summary 	Application for ZipWriter
Student 	Lan Qiang
Student Email 	jameslan@gmail.com

Component is not right but might be the closest general category
Comment 1 Mike Kaply [:mkaply] (Out June 27-July 5) 2006-05-23 08:18:00 PDT
if we have information to help the SoC people, should we give it? Or are they supposed to do this on their own?
Comment 2 Benjamin Smedberg [:bsmedberg] 2006-05-23 08:20:16 PDT
Help is good! This is not a test, it's an experience.
Comment 3 Mike Kaply [:mkaply] (Out June 27-July 5) 2006-05-23 08:34:23 PDT
The MAF extension created a scriptable zipwriter, but only released binaries

http://maf.mozdev.org/

Here's a dump of their XPT file if you want a starting point on interface (they didn't release an IDL):

Header:
   Major version:         1
   Minor version:         2
   Number of interfaces:  3
   Annotations:
      Annotation #0 is empty.

Interface Directory:
   - ::nsISupports (00000000-0000-0000-c000-000000000046):
      [Unresolved]
   - ::nsIProperties (78650582-4e93-4b60-8e85-26ebd3eb14ca):
      [Unresolved]
   - ::IZipWriterComponent (ece655ef-47c7-43aa-ada1-3c6cc77d78d5):
      Parent: ::nsISupports
      Flags:
         Scriptable: TRUE
         Function: FALSE
      Methods:
   G     uint32 CURR_COMPRESS_LEVEL(out retval int16);
    S    uint32 CURR_COMPRESS_LEVEL(in int16);
   G     uint32 currzipfile(out retval nsISupports);
    S    uint32 currzipfile(in nsISupports);
   G     uint32 zipdelta(out retval nsIProperties);
    S    uint32 zipdelta(in nsIProperties);
   G     uint32 password(out retval wstring);
    S    uint32 password(in wstring);
   G     uint32 basepath(out retval nsISupports);
    S    uint32 basepath(in nsISupports);
         uint32 init(in nsISupports);
         uint32 deleteEntry(in wstring);
         uint32 deleteEntiesStartingWith(in wstring);
         uint32 add(in nsISupports);
         uint32 addEntry(in wstring, in nsISupports);
         uint32 commitUpdates(out retval boolean);
      Constants:
         int16 COMPRESS_LEVEL0 = 0;
         int16 COMPRESS_LEVEL1 = 1;
         int16 COMPRESS_LEVEL2 = 2;
         int16 COMPRESS_LEVEL3 = 3;
         int16 COMPRESS_LEVEL4 = 4;
         int16 COMPRESS_LEVEL5 = 5;
         int16 COMPRESS_LEVEL6 = 6;
         int16 COMPRESS_LEVEL7 = 7;
         int16 COMPRESS_LEVEL8 = 8;
         int16 COMPRESS_LEVEL9 = 9;

And here is an example usage from the CCK:

http://lxr.mozilla.org/seamonkey/source/extensions/cck/browser/resources/content/cckwizard/cckwizard.js#1094
Comment 4 Nickolay_Ponomarev 2006-05-27 11:24:50 PDT
I wonder if anyone actually contacted the author of MAF. The extension is GPL and the zip writer is created by him too, so maybe he'll be willing to share the code.
Comment 5 Lan Qiang 2006-07-05 09:18:31 PDT
Is this idl file ok?

[scriptable, uuid(FB7B97BE-7D20-4213-9BE0-5510AA749ADC)]
interface nsIZippingProgress : nsISupports
{
	readonly attribute string          zipEntry;
	readonly attribute nsIFile         inFile;
	readonly attribute PRUint64        bytesTotal;
	readonly attribute PRUint64        bytesRead;
	readonly attribute PRUint64        bytesWritten;
	readonly attribute unsigned short  compression;
	readonly attribute boolean         translateCRLF;
};

[scriptable, uuid(05ABA2E7-AC72-4ba3-8F7B-64DC5F07AB72)]
interface nsIZipFileIO : nsIZipReader
{
	void create(in nsIFile zipFile);
	nsOutputStream addEntry(in string zipEntry, unsigned short compressoin, boolean translateCRLF);
	void compress(in string zipEntry, in nsIFile inFile, unsigned short compressoin, boolean translateCRLF);
	void removeEntries(in nsIUTF8StringEnumerator zipEntries);

	readonly attribute nsIZippingProgress compressing;
};
Comment 6 Benjamin Smedberg [:bsmedberg] 2006-07-05 09:48:08 PDT
All of these methods need significant javadoc-style documentation. See questions below:

>         void create(in nsIFile zipFile);

Does this differ from nsIZipReader.open() in that it creates a zipfile if one does not already exist?

>         nsOutputStream addEntry(in string zipEntry, unsigned short compressoin,

nsOutputStream is not a type. Perhaps you meant nsIOutputStream? Please document whether this is going to be a blocing or non-blocking stream. If non-blocking, how do callers receive notification that the non-blocking operation is complete (and successful)? Polling the zippingprogress object is insufficient.

There are a whole bunch of additional questions to ask if this is a non-blocking operation, but let's handle this set first ;-)

I am inclined to say that we should never do CRLF translation in this code; we can use streamconverters to do CRLF translation if necessary. Did somebody ask for CRLF translation in particular? Do zipfiles have a special flag that indicates that a file is text and needs CRLF conversion?

>         void compress(in string zipEntry, in nsIFile inFile, unsigned short
> compressoin, boolean translateCRLF);

Did you just mistype "compressoin" in both these methods?

>         void removeEntries(in nsIUTF8StringEnumerator zipEntries);

Seems to me that a single "removeEntry" method would be sufficient... is that incorrect? Is this method blocking or non-blocking?

>         readonly attribute nsIZippingProgress compressing;

If two nonblocking compression operations are being performed at the same time, is there still only a singleton nsIZippingProgress object?
Comment 7 Lan Qiang 2006-07-06 10:26:55 PDT
(In reply to comment #6)
> All of these methods need significant javadoc-style documentation. See
> questions below:
> 
> >         void create(in nsIFile zipFile);
> 
> Does this differ from nsIZipReader.open() in that it creates a zipfile if one
> does not already exist?

nsIZipFileIO.create will truncate file if it already exists, while nsIZipReader.open could fail when zipFile does not exist.

> 
> >         nsOutputStream addEntry(in string zipEntry, unsigned short compressoin,
> 
> nsOutputStream is not a type. Perhaps you meant nsIOutputStream? Please
> document whether this is going to be a blocing or non-blocking stream. If
> non-blocking, how do callers receive notification that the non-blocking
> operation is complete (and successful)? Polling the zippingprogress object is
> insufficient.
> 
> There are a whole bunch of additional questions to ask if this is a
> non-blocking operation, but let's handle this set first ;-)
> 

The stream is a blocking stream. Usually, it will be feed in working thread, and UI thread query its progress.

> I am inclined to say that we should never do CRLF translation in this code; we
> can use streamconverters to do CRLF translation if necessary. Did somebody ask
> for CRLF translation in particular? Do zipfiles have a special flag that
> indicates that a file is text and needs CRLF conversion?

Greate. Original PKZip was develped on DOS, for compatibility, such zip utilities as info-zip provides option of converting CRLF. Since morden application, including mozilla, can handle this, this option is obsolete.
> 
> >         void compress(in string zipEntry, in nsIFile inFile, unsigned short
> > compressoin, boolean translateCRLF);
> 
> Did you just mistype "compressoin" in both these methods?

ah! so many typo....

> >         void removeEntries(in nsIUTF8StringEnumerator zipEntries);
> 
> Seems to me that a single "removeEntry" method would be sufficient... is that
> incorrect? Is this method blocking or non-blocking?

There's something about efficiency. If remove one entry each time, I have to copy the unremoved entries every time.

> >         readonly attribute nsIZippingProgress compressing;
> 
> If two nonblocking compression operations are being performed at the same time,
> is there still only a singleton nsIZippingProgress object?
 
As Darin Fisher said (in mozilla.dev.platform), "nsIOutputStream does not need to
support writing from multiple threads simultaneously.  It only needs
to support being used from multiple threads", there will be up to only one nsIOutputStream at any moment. 

nsIZippingProgress represents progress of processing or last finished (total==read) activity. If there is no any activity yet, GetCompressing() will return nsnull.


Additional method ``setTime'' will be added to set date/time of an entry.

There are some features do not support by this version of nsIZipFileIO, such as encryption, comments, because they are not supported by nsIZipReader.
Comment 8 Benjamin Smedberg [:bsmedberg] 2006-07-10 09:16:25 PDT
I think we need to discuss how this interface is going to be used: most clients are not going to want to spin off worker threads to perform synchronous operations. For the most part, the client code that will want to use this interface will be running from the main thread and will want to add some files to a ZIP:

zip.addAStream("/application.ini", somestream, listener);
zip.addAStream("/chrome.manifest", somestream, listener);
etc...

// listener waits for all zip operations to complete, then notifies user

These operations cannot block (because the main thread is not allowed to block). Perhaps a separate service that queues ZIP changes and performs them asynchronously and proxies listener notifications back to the main thread is needed, but the interfaces as proposed here are insufficient for the clients that want to use this code.
Comment 9 Ted Mielczarek [:ted.mielczarek] 2006-07-26 05:08:54 PDT
I don't know what the status of this is, and I'm probably late to the party, but I'd personally like an interface that works somewhat like Perl's Archive::Zip:
http://search.cpan.org/dist/Archive-Zip/lib/Archive/Zip.pod

my $zip = Archive::Zip->new();
my $file_member = $zip->addFile( 'xyz.pl', 'AnotherName.pl' ); my $string_member = $zip->addString( 'This is a test', 'stringMember.txt' );
$string_member->desiredCompressionMethod( COMPRESSION_DEFLATED );

$zip->writeToFileNamed( 'someOtherZip.zip' );

Obviously with a more XPCOM flavor, but it's really easy to use, and pretty flexible.  Since this is write-only, you probably don't need to return the member from the add operation.

I think for the most usability, the interface needs to have an AddFile method taking an nsIFile, and an AddStream method.  It should also support writing the created zip file to a file or stream.
Comment 10 Benjamin Smedberg [:bsmedberg] 2006-07-31 12:51:09 PDT
James, do you have an updated status on this project? It's about time to get code finalized, but I don't know that the design issues are even worked out.
Comment 11 Lan Qiang 2006-07-31 19:54:23 PDT
(In reply to comment #9)
> I don't know what the status of this is, and I'm probably late to the party,
> but I'd personally like an interface that works somewhat like Perl's
> Archive::Zip:
> http://search.cpan.org/dist/Archive-Zip/lib/Archive/Zip.pod
> 
> my $zip = Archive::Zip->new();
> my $file_member = $zip->addFile( 'xyz.pl', 'AnotherName.pl' ); my
> $string_member = $zip->addString( 'This is a test', 'stringMember.txt' );
> $string_member->desiredCompressionMethod( COMPRESSION_DEFLATED );
> 
> $zip->writeToFileNamed( 'someOtherZip.zip' );
> 
> Obviously with a more XPCOM flavor, but it's really easy to use, and pretty
> flexible.  Since this is write-only, you probably don't need to return the
> member from the add operation.
> 
> I think for the most usability, the interface needs to have an AddFile method
> taking an nsIFile, and an AddStream method.  It should also support writing the
> created zip file to a file or stream.
> 

Is it necessary to zipping file in memory?
Now the nsIZipFileIO derived from nsIZipReader requires opening or creating a local file (the archive) first.
The writing part of this interface is an anolog to nsIZipReader.
Comment 12 Lan Qiang 2006-07-31 20:57:00 PDT
(In reply to comment #10)
> James, do you have an updated status on this project? It's about time to get
> code finalized, but I don't know that the design issues are even worked out.
> 

Sorry.

I plan to implement a nsIZipIOQueue to handle zipping requests, processing these requests in working thread and notify the finish event to client.

Now I'm working on nsZipFileIO and some other classes which provide a blocking IO. I think this work can be done in one week or 10 days.

I am not very familiar threads and synchronization in mozilla, I will try my best to finish it before the deadline.

I have built a host app for debug and unit test, but it seems there's no usable unit test framework of c++ in mozilla?
Comment 13 Benjamin Smedberg [:bsmedberg] 2006-08-01 06:29:29 PDT
James, thanks for the update. Where are you keeping your code at the moment? Could you prepare a patch or a ZIP of your source (even if it doesn't build or work)... it would help those watching to get a better sense of how all the pieces fit together.
Comment 14 Benjamin Smedberg [:bsmedberg] 2006-08-01 06:33:09 PDT
> Is it necessary to zipping file in memory?

At this stage, no. We might consider adding that feature as a second step later.
Comment 15 Lan Qiang 2006-08-03 05:57:39 PDT
Created attachment 231937 [details]
my code 

existing code comes from libjar mostly.
Comment 16 Ted Mielczarek [:ted.mielczarek] 2006-08-03 06:16:20 PDT
Do you have a simple example of using these interfaces?  I'm not sure I understand how nsIZipIOQueue fits into the picture.  I'd like to see this be easy to use for extension writers, since that's probably it's biggest target market.

For the "compression" parameter, I'd like to see constants defined on the interface at least for the most common values:
const unsigned short COMPRESS_NONE = ?
const unsigned short COMPRESS_FAST = ?
const unsigned short COMPRESS_MAX = ?
or something like that.

I also think that instead of having "addEntry" and "compress", they should be more similar, like "addEntry" and "addEntryFromFile".  (Possibly not the best method names, but hopefully you get my point.)
Comment 17 Dave Liebreich [:davel] 2006-08-04 07:02:59 PDT
(In reply to comment #12)
> I have built a host app for debug and unit test, but it seems there's no usable
> unit test framework of c++ in mozilla?
> 

I didn't find one either, which is why, as a proof of concept, I wrote a brain-dead simple unit test - see bug 346703

Rob Campbell and I are going to keep working on this, but we welcome any help.

Is there a particular c++ unit test framework you have used in the past?

If you post/attach the code for your host app, I'm willing to try and help you get enough of a framework in place to support unit tests for this project.

-Dave
Comment 18 Lan Qiang 2006-08-07 07:49:42 PDT
(In reply to comment #16)
> Do you have a simple example of using these interfaces?  I'm not sure I
> understand how nsIZipIOQueue fits into the picture.  I'd like to see this be
> easy to use for extension writers, since that's probably it's biggest target
> market.
> 
nsIZipIOQueue enqueues the operations to nsIZipFileIO and convert blocking operation into non-blocking ones.

pseudo code:

zipio = createZipIO();
zipio.open(zipfile);
zipioQueue = createZipIOQueue();
zipioQueue.Init(zipio);

zipioQueue.extract(entry, outFile1, listener);
zipioQueue.compress(entry, inFile1, listener);
zipioQueue.extract(entry, outFile2, listener);
zipioQueue.compress(entry, inFile2, listener);

Actually, I'm working on nsZipFileIO and nsZipIOQueue 
> For the "compression" parameter, I'd like to see constants defined on the
> interface at least for the most common values:
> const unsigned short COMPRESS_NONE = ?
> const unsigned short COMPRESS_FAST = ?
> const unsigned short COMPRESS_MAX = ?
> or something like that.
> 
It's defined as (in modules/libjar/zipstruct.h)

#define STORED            0
#define SHRUNK            1
#define REDUCED1          2
#define REDUCED2          3
#define REDUCED3          4
#define REDUCED4          5
#define IMPLODED          6
#define TOKENIZED         7
#define DEFLATED          8

but currently only STORED and DEFLATED are supported by libjar. These constants will added into nsIZipFileIO.idl, but only STORED and DEFLATED will be supported.

> I also think that instead of having "addEntry" and "compress", they should be
> more similar, like "addEntry" and "addEntryFromFile".  (Possibly not the best
> method names, but hopefully you get my point.)
> 
Thanks, "addEntry" and "compress" are confusing, I'll change them.
Comment 19 Lan Qiang 2006-08-21 06:00:04 PDT
Created attachment 234765 [details]
code snapshot

Here is the code snapshot.
The nsZipIOQueue and nsZipIOStatus have not been implement because I spent several days more than I planned to get zipped data correct. 

TODO:
  o Change ``compression'' paramter from compressing method to compressing level, and some constant to be defined.
      Mose zip software only supports DEFLATED and STORED methods, it's nonsense to allow user choice the methd.

  o Implement singleton zipIOStatus
  o Implement nsZipIOQueue
  o Add comprehensive test cases
  o Add javascript unit testing code
  o Tune classes(members, relationship, etc.) in this component
Comment 20 Benjamin Smedberg [:bsmedberg] 2006-08-21 09:11:01 PDT
Where did zipformat.h come from? I don't see it in the current tree or in the Mozilla classic tree, but it has a tri-license header.
Comment 21 Lan Qiang 2006-08-21 10:08:05 PDT
(In reply to comment #20)
> Where did zipformat.h come from? I don't see it in the current tree or in the
> Mozilla classic tree, but it has a tri-license header.
> 

It's origin is ``mozilla/modules/libjar/zipstruct.h''
Comment 22 Benjamin Smedberg [:bsmedberg] 2006-08-21 10:19:50 PDT
How do I build this code? Do I stick it in extensions/zipwriter and --enable-extensions=default,zipwriter? 
Comment 23 Lan Qiang 2006-08-21 11:29:37 PDT
(In reply to comment #22)
> How do I build this code? Do I stick it in extensions/zipwriter and
> --enable-extensions=default,zipwriter? 
> 

I modified mozilla/Makefile.in in my working copy. Using --enable-extensions could build the component but igonre the testing program, is there any build option handling it?
Comment 24 Lan Qiang 2006-08-24 08:01:23 PDT
Created attachment 235245 [details]
compress level added

In this version, ``compression'' parameter of method nsIZipFileIO.AddEntry and nsIZipFileIO.AddEntryFromFile represents the compressing level, which could be from -1 to 9.

For -1, it means default level (seems translate to 6 by zlib);
for 0, it means no compression, just store file in zip archive;
for 1, it is fastest compression level and 9 is the maximum compression.

The constants are:
	const nsZipLevel ZipLevelMax     = 9;
	const nsZipLevel ZipLevelSpeed   = 1;
	const nsZipLevel ZipLevelStore   = 0;
	const nsZipLevel ZipLevelDefault = -1;
Comment 25 Philip Chee 2006-12-01 06:36:58 PST
In comment 3 Michael Kaply (IBM) (mkaply) wrote:

>The MAF extension created a scriptable zipwriter, but only released binaries

FYI. The MAF IDLs appear to be in their CVS:
<http://www.mozdev.org/source/browse/maf/src/idl/>
Comment 26 Benjamin Smedberg [:bsmedberg] 2006-12-01 07:18:33 PST
This project never completed successfully. Resolving the bug.

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