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

Updated to Swift 5.1 and fixed deprecations #46

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

john-mueller
Copy link

I noticed, while building Publish with Swift 5.1 on Linux, that ShellOut was giving build warnings related to launchPath and launch() being deprecated. As I was investigating, I found that several of the tests were not running on Linux, and were actually failing when included. This PR addresses the following issues:

  • Added missing tests to ShellOutTests.allTests.
  • The #if os checks in Process.launchBash(with:outputHandle:errorHandle:) are no longer needed, as FileHandle.readabilityHandler was implemented and merged in Swift 5.1. This means both macOS and Linux can use the same code paths (this was not true when d18b7b6 was committed). This also fixes most of the tests.
  • Added if #available(OSX 10.13, *) checks to use executableURL and run() on current macOS and Linux targets. Defaults to the older API on earlier versions of macOS. This gets rid of the build warnings.
  • Changed testSingleCommandAtPathContainingTilde() to run at ~/.. (typically /Users on macOS and /home on Linux) instead of ~. This is because Swift docker images, which are one of the easier ways to test Swift on Linux, have an empty home directory by default, and thus fail this test. By contrast, ~/.. must contain at least ~, and still tests that paths with ~ work.

I think this should cut a new version release, as it changes the minimum Swift version from 4.2 to 5.1.

Remove "#if os" checks, since `readabilityHandler` was implemented on Linux in Swift 5.1.
Added "#if available" checks to use `executableURL` and `run()` instead of `launchPath` and `launch()` on macOS 10.13+ and Linux.
Added missing tests to `allTests` so they run on Linux.
Changed `testSingleCommandAtPathContainingTilde()` as it formerly failed with an empty home directory (common on Linux and Docker images).
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

Successfully merging this pull request may close these issues.

None yet

1 participant