Revert "Fix empty folder missing problem when zip files. (#330)"

[joan38]

This reverts commit 90626df.
Fixes #349

I've spent some time trying to do a fix forward but I don't have much time on my hand.
Can we revert this in the meantime and @counter2015 can provide another PR for the empty folder issue?

[joan38]

@lihaoyi can you rerun the failing build?

[joan38]

Ah ok thanks 🙂

[lihaoyi]

Tagged this as 0.11.4-M5, will propagate it to Mill once it's published

[counter2015]

@joan38 Thanks for report. I'm not sure whether I understand correctly. It seems the commit break the unwritten rules. But in my persion thinking, the newer one is more intuitive. The former version seems handle folder and file differently. Do you except that when user pass a folder to zip parameter, we should access inside it ?

Then I have a quesion here, if user want to keep the outer folder, how can we do that?
In first glance, I think we can pass the parent folder but exclude other content (it's not convenient).

The common zip command in linux support pass muti source include files and folders and keep the outside folder sources

For example

$ tree
.
├── run.scala
└── sources
    ├── file1.txt
    └── file2.txt


$ zip archieve.zip sources run.scala
  adding: sources/ (stored 0%)
  adding: run.scala (deflated 41%)

$ tree
.
├── archieve.zip
├── run.scala
└── sources
    ├── file1.txt
    └── file2.txt

I acknowledge that my previous PR may have introduced breaking changes due to oversights in the implementation. However, from my perspective, retaining directory structures during compression aligns more closely with user intuition. What's your opinion ? @lihaoyi

[joan38]

@counter2015 The readme says:

This will create a new zip archive at dest containing file1.txt and everything inside sources.

But your point is fair and I thought about that too.

I'll give you an example taken from what broke downstream for a Mill plugin:

os.zip(dest / "docusaurus.zip", Seq(wd / "build"))

How would you implement this with your implementation?
My fix was:

os.zip(dest / "docusaurus.zip", os.list(wd / "build").map(os.zip.ZipSource.fromPath))

(BTW I think we need some conversions for Seq[Path] to Seq[ZipSource] to avoid .map(os.zip.ZipSource.fromPath))

Both are good to me but maybe we need a 0.12.0 version bump for such change? @lihaoyi WDYT?

Also I want to thank you for your contribution @counter2015, it's very much appreciated.

Thanks

[joan38]

I just opened #352
Maybe with that we could make the change but not sure about the breaking aspect of it.