Foundation: port NSData to Windows

[compnerd]

Adjust the API usage for the Windows environment.

[compnerd]

@swift-ci please test

[compnerd]

CC: @parkera @phausler @millenomi @spevans @jakepetroules

[spevans]

Would this be better as a #if !canImport(Darwin) as I dont think it is supported on any other platforms apart from Apple's?

[phausler]

That is true. The only other malloc implementation that has this feature iirc is jemalloc.

[compnerd]

It would be interesting to be able to use jemalloc on other targets. I know that there are environments running off of jemalloc. I don't think that we have a good way to identify that yet. Perhaps having a macro from the build system -DHAVE_MALLOC_GOOD_SIZE?

[spevans]

I think swift just needs a #if hasImported(<some function>) conditional :)

[spevans]

Couldnt this whole function just be replaced with:
return try _NSCreateTemporaryFile(dirPath._nsObject.appendingPathComponent("")
or something like that, they seem to have the same bodies of code expect for computing the initial template.

[compnerd]

I honestly don't know. I'm not sure why this is duplicated in the first place, as the same thing happens in the Linux case.

[compnerd]

Hmm, looking at this, no, it wouldn't work, since that would alter the template and give you a different result.

[spevans]

What about passing the template as the argument, eg:

internal func _NSCreateTemporaryFile(withTemplate template: String)

There only seems to be one caller of each method so generating the template at the call-site seems to solve the issue.

[spevans]

I think it might be better to try and rewrite this whole method using FileHandle and eliminate all of the Windows/Unix differences here. It will requires some new internal FileHandle methods (eg a getFilePermissions, setFilePermissions, and a throwable version of synchronizeFile) but I think it should be possible.
makeTemporaryFile could return a FileHandle instead of an fd as well. What do you think?

The helper methods could probably be reused in FileManager later on any

[compnerd]

I like that idea, if that is acceptable as per @parkera, @phausler, or @millenomi. The problem is that this is a public interface, no?

[spevans]

I just meant change the function body. Would something like this work (untested, and you will need to add an internal FileHandle.setFilePermissions(mode) method:

open func write(toFile path: String, options writeOptionsMask: WritingOptions = []) throws {
        let fm = FileManager.default

        let fh: FileHandle
        let useAuxiliaryFile = writeOptionsMask.contains(.atomic)
        var auxFilePath: String? = nil
        var mode: mode_t? = nil

        if useAuxiliaryFile {
            if let perms = try? fm.attributesOfItem(atPath: path)[.posixPermissions] as? NSNumber, let m = perms {
                mode = mode_t(m.uintValue)
            }

            let (newFD, path) = try self.makeTemporaryFile(inDirectory: path._nsObject.deletingLastPathComponent)
            fh = FileHandle(fileDescriptor: newFD, closeOnDealloc: true)
            auxFilePath = path
        } else {
            var flags = O_WRONLY | O_CREAT | O_TRUNC
            if writeOptionsMask.contains(.withoutOverwriting) {
                flags |= O_EXCL
            }

            guard let newfh = FileHandle(path: path, flags: flags, createMode: 0o666) else {
                throw _NSErrorWithErrno(errno, reading: false, path: path)
            }
            fh = newfh
        }

        try self.enumerateByteRangesUsingBlockRethrows { (buf, range, stop) in
            if range.length > 0 {
                do {
                    try NSData.write(toFileDescriptor: fh.fileDescriptor, path: path, buf: buf, length: range.length)
                    fh.synchronizeFile()
                } catch {
                    if let auxFilePath = auxFilePath {
                        try? fm.removeItem(atPath: auxFilePath)
                    }
                    throw error
                }
                fh.setFilePermissions(mode) // TODO: Add this as an internal Filehandle function 
            }
        }

        if let auxFilePath = auxFilePath {
            try fm._fileSystemRepresentation(withPath: path, { pathFsRep in
                try fm._fileSystemRepresentation(withPath: auxFilePath, { auxFilePathFsRep in
                    if rename(auxFilePathFsRep, pathFsRep) != 0 {
                        try? FileManager.default.removeItem(atPath: auxFilePath)
                        throw _NSErrorWithErrno(errno, reading: false, path: path)
                    }
                    if let mode = mode {
                        chmod(pathFsRep, mode)
                    }
                })
            })
        }
    }

[compnerd]

I can rewrite this in terms of FileHandle, but note that there will still be Windows specific paths here as FileHandle.fileDescriptor is not available on Windows, since it works in terms of HANDLEs. Additionally, a few of the operations that this is performing would need to be moved around (e.g. chmod). The makeTemporaryFile will still require Windows specific handling (FileHandle wants a HANDLE, not a file descriptor).

[spevans]

What about moving the two internal NSData.write() functions to be internal methods of FileHandle (its the only caller of them anyway, and it would match _readDataOfLength()) then you should be able to eliminate the fh.fileDescriptor.

[compnerd]

IMO, that is far more accessible for developers working on FileHandle. I remember looking at this and getting confused why it was split. @parkera @phausler - wanna chime in?

[phausler]

I think the only reason why this was two separate implementations is because they are transliterations from the objc implementations. The implementations for NSData live in CoreFoundation and NSFileHandle lives in Foundation. Plus from a historical case the implementations for NSFileHandle use NSExceptions heavily (whereas NSData does not). Frankly these are all implementation details that probably dont need to be maintained across to SCLF. It makes sense to me that file handle things like writing etc should be handled by FileHandle. @millenomi might have some insight here on which way to favor.

[spevans]

Ive done a refactor of the FileHandle write paths in #1876

[compnerd]

I believe that all the needed changes here have been consumed. I'm going to go ahead and close this for now, and if we hit anything related to this, we should be able to pull that in then.