Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Package Download fails with message "Bad read of entry..." (LunchBox) #8982

Closed
archinate opened this issue Jul 6, 2018 · 20 comments
Closed

Comments

@archinate
Copy link

Hello,
I have recently received messages from my LunchBox users that they are unable to download the LunchBox package because of a "Bad read of entry..." error. This appears to pertain to one of its dependencies. I am unable to read the entire message so I have pasted it here.

The package rums correctly on my side. I have tried to re-publish LunchBox a few times and it appears to upload correctly and post as a new version. However, this message persists when trying to download any new version.

image

@mjkkirschner
Copy link
Member

Hi @archinate this looks like a bug in the zip library the package manager client uses.
I think the size of the package might be related to the bug. Fixing it will require a new release of dynamo AFAIK, or potentially a new package manager extension dll.

Also related - Lunchbox bundles some dynamo 1.x dlls (dynamoServices.dll) that cause issues when the package is loaded into dynamo 2.0. We can file an issue for this if that helps. (not sure where to do so). These dlls should not be distributed with packages as they will already be loaded by Dynamo.

@kronz
Copy link
Contributor

kronz commented Jul 7, 2018

@archinate @mjkkirschner How big is Lunchbox now? It can't be anywhere close to Meshtoolkit at ~80mb, unless you are uploading a bunch of backup files accidentally or Revit files, no?

@archinate
Copy link
Author

Hi @mjkkirschner and @kronz,
Here is a screenshot of the current bin folder for LunchBox. I don't see any indication that Dynamo libraries are shipping with any of the most recent versions that are giving "bad read of entry..." download error. The current package size is showing to be 25 mb on my side much of which is the Accord.NET libraries.

image

@kronz
Copy link
Contributor

kronz commented Jul 7, 2018

What I'm seeing is that v 2018.6.28, updated Jun 28 downloads fine, and the 2 versions 2018.7.* from July 6 both have the error shown above. Between last week and now you added back in the Accord libraries that were available in 2017.10 . . . .anything special in how you are structured the Accord libs now as opposed to 2017.10?

@kronz
Copy link
Contributor

kronz commented Jul 7, 2018

Ah, wait. @mjkkirschner , it looks like in Dynamo 1.3, @archinate had this set up with 2 high level categories for the Lunchbox tools, LunchBox and LunchBox ML
2018-07-07_1516
v 2018.6.28 doesn't have the ML stuff at all, and the tools in it work great in Dynamo 2 (multi-output ports behave nicely). So, is this a change in how to get categories showing up in the 2.0 Library? I don;t know of any other package that ships 2 categories in the same package, although maybe there are some

@mjkkirschner
Copy link
Member

mjkkirschner commented Jul 7, 2018 via email

@archinate
Copy link
Author

hey @kronz,
I had published a small update on 6.28 and it wasn't until later that a user asked where the Accord/ML nodes were at so for some reason I had a partial publish there? I'm not quite sure.

It was on 2018.7 that I retried publishing and then users started getting that download error. The only difference from 2017.10 and 2018.7 was that I published with Dynamo 2.0 although I did try to re-publish from Dynamo 1.3 and the error persists.

@mjkkirschner
Copy link
Member

Please see this thread:
haf/DotNetZip.Semverd#74

The thread states the bug from the old ionic.zip.dll (which I am guessing we used) is fixed in the latest dotnetzip packages. Publishing from or downloading with 1.3 vs 2.0 won't make a difference as we have always AFAIK used this older ionic dll for package zipping. The difference is the changing size of the package and the bug in this zipping library.

@kronz
Copy link
Contributor

kronz commented Jul 9, 2018

@archinate
ok, so we need to update our zipping library as it seems you have hit on some un-magical combination of file size and number of files. In the meantime, can you try adding some dummy bytes to your library, a .txt file? Seems like there is some specific, but undefined, combination of package size and number of files. @mjkkirschner any guidance on increments?

@mjkkirschner
Copy link
Member

mjkkirschner commented Jul 9, 2018

It is pretty difficult to find solid information on this bug since it is quite old. I think you should avoid packaging dlls which are multiples of 128kb... the accord .net math.core dll is quite close to these values.

128kb *11 = 1.408 vs. 1,376,256 on my machine

I might try leaving that dll out of an upload as a test... I assume your library requires it? Could you try a different version of accord?

@archinate
Copy link
Author

hi @mjkkirschner yes the node package requires accord and also depends on this version of accord.

I should note that the 2017.10 release of LunchBox also included Accord libraries and didn't have this issue.

-Nate

@mjkkirschner
Copy link
Member

mjkkirschner commented Jul 9, 2018

the 2017.1x release of lunchbox bundles 3.6.6031 of accord while the 2018 release bundles 3.8 as far as I can tell - this bug occurs with different sized files, I think it's possible the dlls have different sizes between the two releases.

@mjkkirschner
Copy link
Member

mjkkirschner commented Jul 9, 2018

some potential workarounds....

  • @teocomi used https://github.com/Fody/Costura to embed some dlls as resources in the Speckle dynamo package. You might be able to embed all these dlls inside one resource file which would change the file size.
  • You could ship an extension with your package which is capable of grabbing your dependencies when dynamo starts using github or nuget etc... - if they exist already, don't download them again.

@archinate
Copy link
Author

@kronz and @mjkkirschner This sounds like a rather serious bug in package deployment. Having users of Dynamo also track file sizes in multiples of 128kb is reaching.

Any word on when a real fix to package management will be implemented?

@erfajo
Copy link

erfajo commented Aug 10, 2018

Today came the message at LinkedIn that Lunchbox has left the Package Manager...
https://www.linkedin.com/feed/update/urn:li:activity:6433436294420008960
https://provingground.io/tools/lunchbox/download-lunchbox-for-grasshopper/

This is not one of the smaller players who is leaving, this is the package that has been holding a position in the top five for all times.
ac4466e16bf799e92a9d6f0ec050598dce613f82

@johnpierson
Copy link
Member

This is being tracked internally as QNTM-4781

@radumg
Copy link
Collaborator

radumg commented Nov 27, 2018

super-late jumping into this but maybe it's helpful or at least extra validation : about 2 years ago when i wrote a dynamopackages.com API client i ran into issues with its zip library, namely that no other NuGet zip package would unzip files downloaded from the package manager.

The only way to make it work was to use the exact same version Ionic zip library as used in the Greg NuGet package, so it seemed to me the culprit was indeed the Ionic zip library used in Greg, doing the zipping before the package is uploaded.

@mjkkirschner
Copy link
Member

we are moving to using system.io.compression where possible in master.

@johnpierson
Copy link
Member

@radumg PR #20 has some updates regarding this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants