Skip to content

Conversation

@pvieito
Copy link
Contributor

@pvieito pvieito commented Apr 10, 2019

  • Added Windows support
  • Added both executable and library main executable layouts
  • Added test for reverse bundle lookup (required for all implicitly loaded bundles, eg. Bundle.main)

@pvieito
Copy link
Contributor Author

pvieito commented Apr 10, 2019

@compnerd @millenomi

#else
return ""
#endif
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we want to have some constants for this in FileManager as we keep wanting to do this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea :\ probably a SPI picking up that information from CF (like: Bundle._platformLibraryPrefix/Suffix, Bundle._platformExecutablePrefix/Suffix or similar).

@millenomi
Copy link
Contributor

I'm not blocking this on the prefix/suffix SPI.

@millenomi
Copy link
Contributor

@swift-ci please test and merge

@millenomi
Copy link
Contributor

@pvieito This is good to merge after resolving conflicts.

- Added Windows support
- Added both executable and library main executable layouts
- Added test for reverse bundle lookup (required for all implicitly loaded bundles, eg. Bundle.main)
@pvieito
Copy link
Contributor Author

pvieito commented Apr 11, 2019

@millenomi I have merged @compnerd changes and added SPI on CF to check if Freestanding Bundles are supported.

@spevans
Copy link
Contributor

spevans commented Apr 11, 2019

@swift-ci please test and merge

@pvieito
Copy link
Contributor Author

pvieito commented Apr 11, 2019

@spevans I think Jenkins has ignored the build request, could you request another one? Thanks!

@spevans
Copy link
Contributor

spevans commented Apr 11, 2019

@swift-ci please test and merge

@pvieito
Copy link
Contributor Author

pvieito commented Apr 12, 2019

@compnerd this Swift CI failure is resolved?

@spevans
Copy link
Contributor

spevans commented Apr 12, 2019

@swift-ci please test and merge

1 similar comment
@compnerd
Copy link
Member

@swift-ci please test and merge

@pvieito
Copy link
Contributor Author

pvieito commented Apr 12, 2019

@millenomi Is the CI broken? I don't understand the error: https://ci.swift.org/job/swift-corelibs-foundation-PR-Linux/2451/console

@spevans
Copy link
Contributor

spevans commented Apr 12, 2019

@pvieito CI has been breaking all day for issues unrelated to the s-c-f PRs being tested

@pvieito
Copy link
Contributor Author

pvieito commented Apr 13, 2019

@spevans It seems to be working now, could you request a test-and-merge? Thanks!

@spevans
Copy link
Contributor

spevans commented Apr 13, 2019

@swift-ci please test and merge

@swift-ci swift-ci merged commit 7c007bb into swiftlang:master Apr 13, 2019
_bundle = result
}

public convenience init?(_executableURL: URL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pvieito @millenomi Should this method be public? also _supportsFHSBundles and _supportsFreestandingBundle above? If its just for testing shouldn't they be internal?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spevans If we mark it internal we would have to guard the tests with NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT, which means that they would not be executed on the Linux CI (and I would like them to be executed on the Linux CI, as I can only test them locally on macOS).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT works for Linux CI because it uses the debug-foundation flag. @millenomi Is that not the case?

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.

5 participants