nim-lang / zip

zip wrapper for Nim
MIT License
52 stars 37 forks source link

windows vs linux inconsistency #50

Open mark-summerfield opened 4 years ago

mark-summerfield commented 4 years ago

If I zip foo/bar.txt on linux or windows and unzip on linux or windows it works as expected with a foo directory created with bar.txt inside it.

However, if I zip foo\bar.txt on windows on linux it unzips as the file 'foo\bar.txt' (with \ part of the filename) and on windows it raises an IOError.

My workaround is to say when defined(windows): filename = filename.replace('\\', '/') but it seems to me that the zip library should handle this.

Araq commented 4 years ago

True.

krux02 commented 4 years ago

After all this is just a wrapper, so is this a problem of the original zip library or is this caused by bad wrapping of the library?

genotrance commented 4 years ago

@mark-summerfield - what code are you calling to do this extraction? Can you share some snippet? There's raw wrappers in this library and also some Nim helper code so it is unclear which procs you are calling.

mark-summerfield commented 4 years ago

I haven't looked at this in quite a while (I've now switched to D), however, I've attached the code (only 165 lines in a .zip). nzip.zip [edited]

{.experimental: "codeReordering".}

import os
import sequtils
import strformat
import strutils
import times
import unicode
import zip/zipfiles

when defined(windows):
  {.passl: "-lz".}

type Action = enum actCreate, actExtract, actList

main()

proc main() =
  let (action, filename, files) = readCommandLine() # May not return
  case action
  of actCreate: createArchive(filename, files)
  of actExtract: extractFiles(filename)
  of actList: listFiles(filename)

proc readCommandLine(): (Action, string, seq[string]) = # May not return
  var action = actList
  var filename = ""
  var files = newSeq[string]()
  let args = commandLineParams()
  let appname = lastPathPart(getAppFilename())
  if len(args) == 0 or args[0] in ["/?", "-h", "--help"]:
    let theYear = now().year
    let year = if theYear == 2019: "2019-20" else: &"2019-{theYear - 2000}"
    quit(&"""usage:
  {appname} [-l|-t|--list] file.zip
  {appname} <-e|-x|--extract> file.zip
  {appname} <-c|--create> file.zip file(s)
  {appname} </?|-h|--help>
  {appname} <-v|--version>
Action hyphens are optional.
[optional action] <required action>.
file.zip (or file.{{jar,od[gpst]}}) and file(s) are required where shown.
Copyright © {year} Mark Summerfield. All rights reserved.
License: Apache Version 2.0.""")
  for arg in args:
    case arg
    of "v", "-v", "--version": quit("1.0.0") # VERSION
    of "c", "-c", "--create", "create": action = actCreate
    of "l", "t", "-l", "-t", "--list", "list": action = actList
    of "e", "x", "-e", "-x", "--extract", "extract": action = actExtract
    else:
      if filename == "": # Will try to open anything, e.g., .zip, .jar,
        filename = arg   # .od[gpst], etc.
      else:
        if defined(windows) and '*' in arg:
          for name in walkPattern(arg):
            if not (name.startsWith('.') or isHidden(name)):
              files = files.concat(glob(name))
        else:
          files = files.concat(glob(arg))
  files = files.deduplicate()
  if action == actCreate and not filename.toLower().endsWith(".zip"):
    quit(&"can only create .zip archives, not: \"{filename}\"")
  if action in [actExtract, actList] and len(files) > 0:
    echo("ignoring files given after the archive's name")
  (action, filename, files)

proc listFiles(filename: string) =
  var archive: ZipArchive
  if open(archive, filename):
    defer: archive.close()
    let filenames = toSeq(archive.walkFiles())
    let prefix = commonPrefix(filenames)
    if len(prefix) > 3:
      echo(&"{prefix} = →")
    for name in filenames:
      if name.endsWith("/"):
        continue
      if len(prefix) > 3:
        echo(name.replace(prefix, "→ "))
      else:
        echo(name)
  else:
    echo(&"failed to list \"{filename}\"")

proc extractFiles(filename: string) =
  var archive: ZipArchive
  if open(archive, filename):
    defer: archive.close()
    let filenames = toSeq(archive.walkFiles())
    checkFilenames(filenames) # May not return
    var path = "."
    let prefix = commonPrefix(filenames)
    if prefix == "": # No common prefix so create one to avoid "splatting"
      path = changeFileExt(lastPathPart(filename), "") # current folder
    archive.extractAll(path)
    if path == ".":
      path = prefix
    if not path.endsWith("/"):
      path.add('/')
    echo(&"unzipped to \"{path}\"")
  else:
    echo(&"failed to extract \"{filename}\"")

proc commonPrefix*(paths: openArray[string], sep = "/"): string =
  if len(paths) == 0:
    return ""
  let first = paths[0]
  var index = -1
  block loop:
    for i in 0 .. first.high():
      for j in 1 .. paths.high():
        let path = paths[j]
        if i < path.high() and first[i] != path[i]:
          break loop
      index = i
  if index == -1:
    return ""
  else:
    return first[0 .. first.rfind(sep, 0, index)]

proc checkFilenames(names: openArray[string]) = # May not return
  for name in names:
    if name.isAbsolute():
      quit(&"won't unzip untrusted archive with absolute path: \"{name}\"")
    for path in name.parentDirs():
      if path == "..":
        quit(&"won't unzip untrusted archive with .. path: \"{name}\"")

proc glob(name: string): seq[string] =
  if existsFile(name):
    result.add(name)
  else:
    for file in walkDirRec(name):
      result.add(file)

proc createArchive(filename: string, files: openArray[string]) =
  var archive: ZipArchive
  if open(archive, filename, fmWrite):
    defer: archive.close()
    for filename in files:
      archive.addFile(relativised(filename), filename)
    echo(&"zipped to \"{filename}\"")
  else:
    echo(&"failed to create \"{filename}\"")

proc relativised(name: string): string =
  when defined(windows):
    result = name.replace('\\', '/')
  else:
    result = name
  if not result.isAbsolute():
    return
  if result.startsWith('/'):
    return result[1 .. ^1]
  if len(result) > 2:
    if result[0].isAlphaAscii():
      var index = 1
      while index < len(result) and result[index] in {'\\', '/', ':'}:
        inc index
      return result[index .. ^1]
krux02 commented 4 years ago

@mark-summerfield thank you for the source code. I edited your comment to include the source code directly, I hope you are not offended by it.

Generally speaking since you switched to D, what was the main motivation to not stay with Nim?

genotrance commented 4 years ago

This is using the zipfiles API so it's likely there's a bug in the Nim code rather than in zip itself. Is worth debugging to see if DirSep should be converted to the zip standard or something.

mark-summerfield commented 4 years ago

I switched to D because I felt that nim (and especially its standard library) was too immature.

D, despite being a lot older also turns out to be not that mature in its standard library either. However, there are books on D (especially "The D Programming Language" by Andrei Alexandrescu, and I prefer learning from books than online. (Nim's "in Action" book is pre-1.0 I believe.) OTOH I can build stand-alone GUIs with nim and can't with D, although NiGui is too limited (or was last time I used it) for anything but simple 2D games.

However, I haven't given up on nim entirely. Once I've finished the program I'm writing in D in a month or two I'll take another look at nim.