Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Status of Linux support #50

Open
felix91gr opened this issue Apr 23, 2018 · 10 comments
Open

Status of Linux support #50

felix91gr opened this issue Apr 23, 2018 · 10 comments

Comments

@felix91gr
Copy link
Collaborator

felix91gr commented Apr 23, 2018

tl;dr

This is the current support status.


There are 4 tests being skipped:

Here, here, here and here.

There are 2 tests not being compiled: here and here.

Why is that? What is missing?
How can I help?

Also, maybe what we should have for the 2 tests that are not being compiled is a skip, like for the other 4. That way at least they are being visibly skipped.

@kylef
Copy link
Owner

kylef commented Apr 23, 2018

The reason that some of these tests had been skipped in the past is because Foundation on Linux wasn't implemented to either behave the same way or to work at all.

For example, one is because the underlying function used for delete (isDeletableFile(atPath:)) is (perhaps was) unimplemented:

// fatal error: isDeletableFile(atPath:) is not yet implemented

I am not sure if it is warranted that all of these should still be skipped. It could be that Swift versions since they have been implemented resolved the behaviour inconsistencies on Linux, I just hadn't tested these on Linux in a while.

I believe the only place in PathKit where the actual implementation in PathKit (not counting for how underlying APIs are implemented) is isFSCaseSensitiveAt, which could be implemented if needed. Just hadn't been a priority for me:

// URL resourceValues(forKeys:) is not supported on non-darwin platforms...
// But we can (fairly?) safely assume for now that the Linux FS is case sensitive.
// TODO: refactor when/if resourceValues is available, or look into using something
// like stat or pathconf to determine if the mountpoint is case sensitive.
return true

@felix91gr
Copy link
Collaborator Author

For example, one is because the underlying function used for delete (isDeletableFile(atPath:)) is (perhaps was) unimplemented:

It still is unimplemented u.u

I believe the only place in PathKit where the actual implementation in PathKit (not counting for how underlying APIs are implemented) is isFSCaseSensitiveAt, which could be implemented if needed. Just hadn't been a priority for me:

I'm not sure if I understand what you mean. You mean that it could be implemented in PathKit itself even though the usually necessary function may not be available?

(I looked it up and indeed, resourceValues(forKeys: ) is also not implemented)

@kylef
Copy link
Owner

kylef commented Apr 24, 2018

I'm not sure if I understand what you mean. You mean that it could be implemented in PathKit itself even though the usually necessary function may not be available?

I mean, the code for PathKit in is hitting FileManager on all platforms in the same way. So in 99% of the cases the implementation differs due to FileManager implementation in OSS Foundation vs closed Objective-C Foundation.

The one exception is the isFSCaseSensitiveAt method.

@felix91gr
Copy link
Collaborator Author

Btw, I found why these two tests fail:

Sort of.

    $0.it("throws an error on failure writing data") {
      #if os(Linux)
      throw skip()
      #else
      let path = Path("/")
      let data = "Hi".data(using: String.Encoding.utf8, allowLossyConversion: true)

      try expect {
        try path.write(data!)
      }.toThrow()
      #endif
    }

    $0.it("throws an error on failure writing a String") {
      #if os(Linux)
      throw skip()
      #else
      let path = Path("/")

      try expect {
        try path.write("hi")
      }.toThrow()
      #endif
    }

They fail when you call normalize() in PathKit.swift. In particular, here:

  public func normalize() -> Path {
    return Path(NSString(string: self.path).standardizingPath)
  }

It's this step the one that fails:

NSString("/").standardizingPath

I tried it in the REPL for both / and /.cache and got this:

  1> import Foundation 
  2> let leString = NSString(string: "/") 
leString: Foundation.NSString = {
  Foundation.NSObject = {}
  _cfinfo = {
    info = 1920
    pad = 0
  }
  _storage = "/"
}
  3> leString.standardizingPath 
Execution interrupted. Enter code to recover and continue.
Enter LLDB commands to investigate (type :help for assistance.)
Process 12343 stopped
* thread #1, name = 'repl_swift', stop reason = signal SIGILL: illegal instruction operand
    frame #0: 0x00007ffff3d0902f libFoundation.so`Foundation.NSString.resolvingSymlinksInPath.getter : Swift.String + 2463
libFoundation.so`Foundation.NSString.resolvingSymlinksInPath.getter : Swift.String:
->  0x7ffff3d0902f <+2463>: ud2    
    0x7ffff3d09031 <+2465>: ud2    
    0x7ffff3d09033 <+2467>: ud2    
    0x7ffff3d09035 <+2469>: ud2    
Target 0: (repl_swift) stopped.
  4> let leOtherString = NSString(string: "/.cache")
leOtherString: Foundation.NSString = {
  Foundation.NSObject = {}
  _cfinfo = {
    info = 1920
    pad = 0
  }
  _storage = "/.cache"
}
  5> leOtherString.standardizingPath
$R0: String = "/.cache"

It's definitely related to that property. I'm gonna investigate further.

@felix91gr
Copy link
Collaborator Author

I think this might be the problem. It would be weird, though, because all other paths seem to work just fine. And they are all NSStrings as far as I can tell.

@felix91gr
Copy link
Collaborator Author

I found the bug. I'm gonna file a thing. Here it is:

  1. standardizingPath exists somewhere else. Here, in particular. I think that's the version being used here in PathKit.

  2. If we check the implementation and try to run it in the REPL:

Welcome to Swift version 4.1 (swift-4.1-RELEASE). Type :help for assistance.
  1> import Foundation 
  2> let leString = NSString(string: "/") 
leString: Foundation.NSString = {
  Foundation.NSObject = {}
  _cfinfo = {
    info = 1920
    pad = 0
  }
  _storage = "/"
}
  3> leString.expandingTildeInPath 
$R0: String = "/"
  4> let expanded = leString.expandingTildeInPath 
expanded: String = "/"
  5> var resolved = expanded._bridgeToObjectiveC() 
  6. 
resolved: Foundation.NSString = {
  Foundation.NSObject = {}
  _cfinfo = {
    info = 1920
    pad = 0
  }
  _storage = "/"
}
  6> var resolved = expanded._bridgeToObjectiveC().resolvingSymlinksInPath 
resolved: String = {
  _core = {
    _baseAddress = <extracting data from value failed>

    _countAndFlags = <extracting data from value failed>

    _owner = <extracting data from value failed>

  }
}
Execution interrupted. Enter code to recover and continue.
Enter LLDB commands to investigate (type :help for assistance.)
Process 15285 stopped
* thread #1, name = 'repl_swift', stop reason = signal SIGILL: illegal instruction operand
    frame #0: 0x00007ffff3dbeec3 libFoundation.so`Foundation.NSString.resolvingSymlinksInPath.getter : Swift.String + 2259
libFoundation.so`Foundation.NSString.resolvingSymlinksInPath.getter : Swift.String:
->  0x7ffff3dbeec3 <+2259>: ud2    
    0x7ffff3dbeec5 <+2261>: ud2    
    0x7ffff3dbeec7 <+2263>: ud2    
    0x7ffff3dbeec9 <+2265>: ud2    
Target 0: (repl_swift) stopped.
  7>  

It works fine with other paths. I'm gonna post it at the Swift JIRA.

I think both tests that use this call on / should be passing afterwards :)

@felix91gr
Copy link
Collaborator Author

felix91gr commented Apr 25, 2018

Continuation: posted it to JIRA. Currently trying to compile my changes to Foundation in order to confirm my theory and make a PR.

The last 2 tests

For the last 2 tests that fail:

  • conforms to SequenceType
    • without options
    • with options

Without options

Seems to be actually runnable and passing. I'm gonna make a PR soon if that's the case to enable it (I need to clean up my debug prints first).

Is passing. I'm gonna make a PR. Made a PR: #52

With options

It throws this error: Fatal error: Enumeration options not yet implemented is not yet implemented: file Foundation/FileManager.swift, line 637

In the current master branch of swift-corelibs-foundation, the NSUnimplemented line is still there, but a little below. Therefore, this test isn't passable yet.

@felix91gr
Copy link
Collaborator Author

Summary

There are 6 tests that are being skipped on non-Darwin platforms.

  1. Describe: "symlinking", It: "can create a relative symlink in the same directory": fails because there's missing functionality in the Linux port.
  2. Describe: "file info", It: "can test if a path is deletable": fails because isDeletableFile(atPath path: String) -> Bool is unimplemented.
  3. Describe: "writing", It: "throws an error on failure writing data": fails because of SR-7526.
  4. Describe: "writing", It: "throws an error on failure writing a String": fails because of SR-7526.
  5. Describe: "conforms to SequenceType", It: "without options": passes as of Opened test to non-Darwin platforms #52.
  6. Describe: "conforms to SequenceType", It: "with options": fails because FileManager.enumerator(...) is unimplemented.

I'm gonna forward this to the Foundation team. Some of them were interested in how Open Source libraries are using the framework, so that they can prioritize what pieces will be implemented next. FileManager and FileHandle seem to be the most commonly used classes across the board. In fact, even SPM needs it to eventually being able to discover tests without needing the LinuxMain.swift file.

@felix91gr
Copy link
Collaborator Author

Fixed it at apple/swift-corelibs-foundation#1536

@felix91gr
Copy link
Collaborator Author

felix91gr commented Jun 13, 2018

Update on this list: second skipping test may be able to pass soon if all goes well for the writer of that post.

Success there would leave PathKit with only 2 skipping tests on Linux: no.1 (which is missing functionality) and no.6 (which needs another unimplemented Foundation function)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants