We have moved to GitHub Issues
Created by Shannon Deminick 04 May 2016, 09:49:03 Updated by Claus Jensen 25 Oct 2017, 10:29:46Tags: Up For Grabs Unscheduled PR Community Contrib
Subtask of: U4-7594
There is no reason to use this, .Net comes with it's own zip libraries that we should use, this also goes towards better supporting .Net Core, it's also worth noting that this lib is discontinued and simply not supported.
@Shandem I have some experience with the .NET zip libraries (png encoding/decoding) so I might be able to take this on. Is there anything I should keep an eye out for?
Nothing in particular, it should be pretty straight forward. All of the functionality we need will be available in the System.IO.Compression library, all that is being done in the core is zipping and unzipping folders on disk (IIRC)
Ace... I'll get on to it asap then.
I have some work done. No time to test this currently, need to get up early tomorrow.
This is the pull request: https://github.com/umbraco/Umbraco-CMS/pull/1336
Great! I think @JimbobSquarePants had also done some work on this, not sure if he submitted a PR already? If so please let me know so i can consolidate.
@Shandem @dampee I hadn't finished mine and would have had to revisit the code to fix my jetlagged attempt. I've commented in the PR on a couple of things.
@Shandem Is this still for grabs for a newbie in Umbraco? I checked the mentioned pull request and it seems like it wasn't accepted and won't be.
EDIT: After being finised with the PR - https://github.com/umbraco/Umbraco-CMS/pull/1845 , I realised I did the change fron dev7 and not dev 8 as "due in version" states is taht an issue?
@pijemcolu Awesome work! I've responded on the PR, just a few unit tests to fix up
@Shandem Did commit and fixed the three unit tests. A quick search for ICSharpCode.SharpZipLib still returns a couple of results though.
Can I feel free and fix those ones as well? All the references to sharpziplib nugget package could be removed afterwards.
Yes please do :) all code cleanup and removal (when possible) is what v8 is all about... And its pretty fun to do
@Shandem As an update I am sitting on a commit dealing with all of the above menioned issues. That doesn't make it possible to remove the nugget package ICSharpCode.ZipLib because of a dependency in LuceneNet. The dependency is getting resolved in 4.0 release. (https://issues.apache.org/jira/browse/LUCENENET-555)
Now I have to test the code which I am sitting on as it would be a small miracle if it worked straight out of the box mainly due to paths issues which I foresee.
A question: Are scripts RevertToClean/EmptyInstall.bat are intended to be used for ... what they imply by the name?
Ah, i didn't realize Lucene 3.x still shipped with
ICSharpCode.ZipLib, damn. OH well 4.x will be out very soon as I've been following the lucene mail list :)
I haven't used RevertToClean/EmptyInstall.bat for a long time but they are mainly there for testing new installs. So once you install, perform your tests, etc... you can run these batch files to revert all local changes to be back to their original state so you can start your testing from scratch with a 'new' website
Nothing to review for v8, for v7 need to review this new PR: https://github.com/umbraco/Umbraco-CMS/pull/2248
We are still referencing ISharpZip in a few places where Lucene still wants it, but otherwise we're clean.
We have some errors building now, left a comment on the PR.
I've updated the PR branch https://github.com/umbraco/Umbraco-CMS/pull/2248 , removing last three references of SharpZipLib.
One thing - in utill.cs, ZipPackage, it looks like you are creating the zip in an in-memory MemoryStream that's just... disposed at the end, so it's not going to create anything on-disk? Presumably you want to use a real file stream to write the zip content to disk?
(other files are ok)
@zpqrtbnk updated the PR and now writing the archive to the disk.
@Mikulas I've removed a couple of left over using statements (unused) and references to the SharpZipLib package. It will automatically get added by the Lucene.net dependency while it's needed there.. so we don't need to explicitly reference this package in our files since we don't have the dependency anymore.
When Lucene is updated - it should disappear along with it :)
Confirmed that everything still builds, works (can publish/render content) and that
ICSharpCode.SharpZipLib.dll is still in
/bin (added by Lucene I'd assume) after removing the last dependencies on our files.
Do we want to retarget this for v8 instead of a v7 minor, since due in version says v8? (@Shandem )
We have 2 options: we can retarget this due in version 7.8, all we are doing here is removing a dependency that we don't want to use and replacing the logic with native .NET code. Of course because of our current lucene version it will be part of the release but means that we are not actively using it and as of v8 it will just disappear
But... this isn't really an important fix that people will care about or even notice, so perhaps just re-targeting the PR to v8 is better
Have merged this to 7.8 - @zpqrtbnk has already manually merged this to v8 so we're good there.
Priority: Up for grabs
Backwards Compatible: True
Due in version: 8.0.0, 7.8.0
Sprint: Sprint 70
Story Points: 1