DotNetZip code cleanup

Jul 21, 2009 at 11:09 AM

Hi Cheeso,

Do you use Resharper?

It found in ZipFile.cs some possible real problems and some code cleanliness problems.

Possible problems:

  • Possible null ref in WriteCentralDirectoryFooter(...) bytes[i+j] = block[j]
  • Wrong ArgumentException parameter in Save(string filename): "ZipFilename" should be "filename"

Redundant code:

  • ReadCentralDirectoryFooter(ZipFile zf) has 10 lines referring to int i, but it is never used.
  • Redundant base interface declerations. enough to write: partial class ZipFile { ...

 

And various code style suggestions (which you may not necessarily agree with) such as:

  • use string not System.String   (was code automatically converted from VB)
  • use bool?  instead of Nullable<bool>
  • use const for bufferLength 
  • leave the <use... in remarks on same line so resharper can check method name
  • remove "else"  after "return" aborts from method   
  • use lambda in EntryFileNames prop: (e) => e.FileName  instead of (e)=> { return e.FileName; }

   Moshe

Coordinator
Jul 21, 2009 at 12:41 PM

no, I don't have resharper.  Thanks for the analysis.

 

Coordinator
Jul 21, 2009 at 12:59 PM

I looked at the code.

The possible null is not a problem.  The "wrong argumentexception parameter" is a valid problem.

The unused integer is also a good catch - I've removed that.  And I've removed the redundant declarations on the partial class.

As for the style suggestions - most of those are good. 

Jul 22, 2009 at 12:12 PM

Its a great tool. 

Once I'm able to compile, I'll continue the style discussion ...

I'm not able to compile the Zip Full DLL - but don't really understand how that project works in the first place:
It tries to merge the CF projects and says that the pfx key is missing.
But I had previously removed the CF projects from the solution, since I don't wish to work on them,
and unmarked "sign the assembly" in all the other projects, and changed the references to use the "project" created DLLs, instead of the signed ones.
ZLib, ZLibTest, 'Zip partial' and 'Zip reduced' all build OK.

I did not find any reference to the CF projects in the Zip Full project: No build events, and the two references are to the projects,
which as I said compile OK.

Do you have any idea, how or why the Zip Full DLL is merging the CF files? No mention of them in the project properties!
Also, when building the 'Zip Tests', it tries to build (merge) the Zip Full DLL, which fails as above?

Is Ionic.CopyData part of the problem?  ...Which project creates that?

Without compiling the Full DLL I cannot change the Zip Tests test configuration file, because I have no DLL to give it...

Yours, Moshe

Jul 22, 2009 at 1:45 PM

OK, VS search box won't give result for project files, even if I say file type *.*

So opening the Zip Full DLL .csproj file in notepad showed me the culprit ILMerge calls.

I'll see if I know how to fix that.

Coordinator
Jul 22, 2009 at 3:25 PM
Edited Jul 22, 2009 at 3:29 PM

Ionic.CopyData is used during the running of tests.  It is not used in the Full DLL.  The CopyData dll should be included in the "Zip tests\Resources" directory.

DotNetZip uses ILMerge to merge multiple assemblies into one.  The goal is to allow flexibility in deployment:

  • the ZLIB logic can be deployed in a single DLL
  • the DotNetZip logic, which requires ZLIB, can be deployed in a single DLL

There is no "ILMerge" step defined in Visual Studio, so it is done as a custom build step, in two projects:  ZIP Full DLL and ZIP CF Full Dll.  

In each case, ILMerge signs the resulting merged assembly with the .pfx file, which of course you don't have.   I think you just need to remove the PFX file from the ILMerge command line to avoid problems.

Jul 22, 2009 at 9:29 PM

Thanks for the answer!

By the way, I think that the ILMerge steps could be defined in the Build Events (before and after), of the project, and then would be viewable by the VS,
and would also enable easier debugging, when the build fails.

I didn't have the time to pursue.  I hope I'll get to it tomorrow. 

Jul 26, 2009 at 11:26 AM

Do you have any idea how I can build without the CF projects?

Jul 26, 2009 at 1:56 PM

pashute forgot to mention that Resharper is free for open source developers :)

Coordinator
Jul 26, 2009 at 2:11 PM

To build without the CF projects, you could just remove them.     ?? 

Aug 12, 2009 at 2:46 PM

Well, that's what I did. Just removed them.
And then got all those build errors from the ILMerge :-(

 

And Mark Hor was correct!!  Resharper is free for open source projects:
http://www.jetbrains.com/resharper/buy/opensource_license.html 

I thought he was joking (with the smiley at the end of his message)

Coordinator
Aug 12, 2009 at 3:15 PM

Thanks for pointing that out about Resharper.

The ILMerge performed by the Zip Full DLL.csproj does not merge CF DLLs. 

Maybe you haven't removed all the CF projects?  There is  "ZIP CF Full DLL" project that performs ILMerge on the CF dlls.  This will obviously fail if you have removed some of the CF projects.  But if you have removed all the CF projects then you don't have "ZIP CF Full DLL".

I just tried this and it works here.

  • remove all CF projects and references to same, from the .sln file
  • remove all mention of Ionic.pfx from the various .csproj files

The resulting solution builds, successfully, EXCEPT, there are missing resource directories, which cause WinFormsExample to fail to build, and the Zip Tests fail to build.  Hmmm...  That's a problem.  But not the problem you are having. I''ll fix that separately.

 

Coordinator
Aug 12, 2009 at 3:16 PM
This discussion has been copied to a work item. Click here to go to the work item and continue the discussion.
Aug 12, 2009 at 4:22 PM
OK, so could you tell me when you change those.
Meanwhile besides the test and example, do I get a working DLL so I can try out myself?
(In which case I'll add the NULLSTREAM stuff for discovering zip size)

Thanks, Moshe


On Wed, Aug 12, 2009 at 5:15 PM, Cheeso <notifications@codeplex.com> wrote:

From: Cheeso

Thanks for pointing that out about Resharper.

The ILMerge performed by the Zip Full DLL.csproj does not merge CF DLLs. 

Maybe you haven't removed all the CF projects?  There is  "ZIP CF Full DLL" project that performs ILMerge on the CF dlls.  This will obviously fail if you have removed some of the CF projects.  But if you have removed all the CF projects then you don't have "ZIP CF Full DLL".

I just tried this and it works here.

  • remove all CF projects and references to same, from the .sln file
  • remove all mention of Ionic.pfx from the various .csproj files

The resulting solution builds, successfully, EXCEPT, there are missing resource directories, which cause WinFormsExample to fail to build, and the Zip Tests fail to build.  Hmmm...  That's a problem.  But not the problem you are having. I''ll fix that separately.

 

Read the full discussion online.

To add a post to this discussion, reply to this email (DotNetZip@discussions.codeplex.com)

To start a new discussion for this project, email DotNetZip@discussions.codeplex.com

You are receiving this email because you subscribed to this discussion on CodePlex. You can unsubscribe on codePlex.com.

Please note: Images and attachments will be removed from emails. Any posts to this discussion will also be available online at codeplex.com


Coordinator
Aug 12, 2009 at 6:02 PM

> OK, so could you tell me when you change those.

I'll update the workitem when I check-in a change for the missing "Resources" directories.

 

> Meanwhile besides the test and example, do I get a working DLL so I can try out myself? (In which case I'll add the NULLSTREAM stuff for discovering zip size)

Yes, you will get a bunch of DLLs from a .sln modified this way. Yank out all the CF projects, and you'll be able to build Ionic.Zip.Reduced.dll, Ionic.Zip.Partial.dll Ionic.Zip.dll, Ionic.Zlib.dll, and all  of the example applications, except for the WinForms example which is missing a Resources directory.

Coordinator
Aug 15, 2009 at 12:04 AM

Just FYI, I did a bunch of FxCopy cleanup in the source, lots of changes.  Thanks for reminding me about this. 

 

Sep 1, 2009 at 4:31 PM

Great job!

First of all, in 1.8.4.22 there is no SNK or PFX. So I only had to remove the 3 CF's.
ZLib compiles and builds OK.
Zip Partial DLL builds OK too.

Now the problem is that the Zip Full DLL is not building, so I don't get Ionic.Zip.dll.

The message is :

  ...Ilmerge.exe /t:library /xmldocs/out:"obj\Debug\Ionic.Zip.dll" "c:\path to zip\Zip Partial DLL\bin\Debug\Ionic.Zip.Partial.dll" "c:\path to zip\Zlib\bin\Debug\Ionic.Zlib.dll"
  Exited with code 3.

Any ideas?

Coordinator
Sep 1, 2009 at 5:08 PM

do you have ilmerge installed?

Did you download ilmerge and install it?

 

Sep 2, 2009 at 12:18 PM

oi! ouch

:'''(