erlang / otp

Erlang/OTP
http://erlang.org
Apache License 2.0
11.4k stars 2.95k forks source link

zip:extract/2 with keep_old_files does not respect cwd #9087

Open jonatanklosko opened 2 days ago

jonatanklosko commented 2 days ago

Describe the bug

When calling zip:extract/2 with keep_old_files and cwd options, the cwd option is effectively ignored for the old file check. As a result, the old files are always replaced.

To Reproduce

# Create test.zip
$ mkdir test && touch test/foo.txt test/bar.txt && zip -r test.zip test && rm -rf test
  adding: test/ (stored 0%)
  adding: test/foo.txt (stored 0%)
  adding: test/bar.txt (stored 0%)
$ ls
test.zip
$ erl
Erlang/OTP 27 [erts-15.0] [source] [64-bit] [smp:10:10] [ds:10:10:10] [async-threads:1] [jit]

Eshell V15.0 (press Ctrl+G to abort, type help(). for help)
# With cwd (1)
1> zip:extract("test.zip", [verbose, keep_old_files, {cwd, "nested"}]).
extracting: "test/foo.txt"
extracting: "test/bar.txt"
{ok,["nested/test/foo.txt","nested/test/bar.txt"]}
# With cwd (2) - replaces existing files, unexpected (!)
2> zip:extract("test.zip", [verbose, keep_old_files, {cwd, "nested"}]).
extracting: "test/foo.txt"
extracting: "test/bar.txt"
{ok,["nested/test/foo.txt","nested/test/bar.txt"]}
# Without cwd (1)
3> zip:extract("test.zip", [verbose, keep_old_files]).
extracting: "test/foo.txt"
extracting: "test/bar.txt"
{ok,["test/foo.txt","test/bar.txt"]}
# Without cwd (2) - keeps existing files as expected
4> zip:extract("test.zip", [verbose, keep_old_files]).
{ok,[]}
# With cwd (3) - now it keeps existing files, because it checks for the "test" top-level directory
5> zip:extract("test.zip", [verbose, keep_old_files, {cwd, "nested"}]).
{ok,[]}

Expected behavior

keep_old_files to respect cwd.

Affected versions

Tested with 27.0.

garazdawi commented 2 days ago

Yes, that seems very wrong. A quick look suggests that it should not be very hard to fix. There seems to be very little testing of keep_old_files in general so not surprising that it is broken.

I added it to my todo list, a PR would help speed things along.

jonatanklosko commented 1 day ago

@garazdawi PR opened :)