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

Now custom configuration types are supported #4

Merged
merged 42 commits into from
Mar 24, 2019
Merged

Conversation

IgorMuzyka
Copy link
Collaborator

Now it all works.
You can define custom config types and use them within libraries and Package.swift (no external dependencies needed this time).
Updated readme.
@orta please check.

@IgorMuzyka
Copy link
Collaborator Author

Fixes #1 .

@IgorMuzyka
Copy link
Collaborator Author

IgorMuzyka commented Mar 16, 2019

Also, a nice improvement is that now if there is multiple libraries using PackageConfig to define their config they no longer depend on PackageConfig in the Package.swift file.

#if canImport(LibraryOneConfig)
import LibraryOneConfig

LibraryOneConfig().write()
#endif 

#if canImport(LibraryTwoConfig)
import LibraryTwoConfig

LibraryTwoConfig().write()
#endif

@IgorMuzyka
Copy link
Collaborator Author

But now there is another problem. If my package which depends on my dependency does not need PackageConfig then it's not in the build directory to depend on it in the first place. Will try to solve this by unmaking PackageConfig dynamic library(since we don't really need it as an external dependency now) and see what happens.

@orta
Copy link
Member

orta commented Mar 16, 2019

You got this 👍

@IgorMuzyka
Copy link
Collaborator Author

IgorMuzyka commented Mar 17, 2019

Made it all work. Now PackageConfig has product package-config which essentially is:

mkdir -p ./Sources/PackageConfigs/
touch ./Sources/PackageConfigs/PackageConfigs.swift
swift build --target PackageConfigs

And user is now expected to define any dependencies that require PackageConfig in this target in Package.swift:

.target(name: "PackageConfigs", dependencies: [
    "PackageConfig", 
    "SomeLibraryConfig",
]),

What this achieves is that being present in a target assures that we get all of the dylibs of the listed libraries built when we run the executable.
So in order to use executable libraries depending on PackageConfig user now needs to do this:

# after adding or changing the `PackageConfigs` target
swift run resolve
# creates empty source for the target and builds it which gives us all of it's dylibs
swift run package-config

# and now user can run the dependency he wished
swift run some-executable
  • Need to update README with all of the changes and then it's probably it if you approve.

@IgorMuzyka
Copy link
Collaborator Author

Done.
Now the end user of any library depending on PackageConfig needs to just run swift run package-config to ensure that all dylibs are built. This assumes that Package.swift already contains PackageConfigs target wrapping all of those required Configs.

After that user can run swift run any-executable-depending-on-package-config and would be able to get its config from Package.swift utilizing the dylibs built by swift run package-config.

So a correct definition in the Package.swift and one command to satisfy all of your dependencies with their dylibs so they can load theirs configs.

I think this way of achieving this functionality is ok.
If you have any specific ideas on how to improve further, I would gladly listen.

@IgorMuzyka
Copy link
Collaborator Author

IgorMuzyka commented Mar 17, 2019

Also considering #2 probably a nice feature for future would be to add the var searchPaths: [String] { get } property to PackageConfig protocol so that package config would link it from some more places than just .build directory.
Nevermind, this strikethrough idea is probably dumb as I'm not sure how it would work.

@IgorMuzyka
Copy link
Collaborator Author

Ohh. Figured out another problem 😒. Can not have two or more different packages PackageConfig in Package.swift since when building for either of them they would fail with Program used external function '_$whatever' which could not be resolved!.
Will need to solve this somehow, not yet sure how. Since the PackageConfig inside the dependent library provides us with knowledge of the dynamicLibraries it needs there is no way currently to know another dependency dynamicLibraries.
A really dumb way to solve this might be just linking every possible dylib in the .build directory.
Note that #if canImport(SomeConfig) does not solve this currently.
Maybe will try to make a dylib which exports any dependency in PackageConfigs. and get's linked
Not sure yet.

@IgorMuzyka
Copy link
Collaborator Author

Figured out since PackageConfig dependent configs needs to be listed under PackageConfigs target to build dylibs i'll just parse Package.swift by hand and extract them. This way we'll link all of the dylibs required for all the PackageConfigs to be linked when compiling Package.swift with swiftc.

@IgorMuzyka
Copy link
Collaborator Author

IgorMuzyka commented Mar 19, 2019

It works.
Now just need to:

  • Update README.md
  • Update Parser

@IgorMuzyka
Copy link
Collaborator Author

@orta I think it's done.

@IgorMuzyka
Copy link
Collaborator Author

IgorMuzyka commented Mar 20, 2019

Here is test example project with two of my own libraries used depending on PackageConfig from my fork.
To check how it works just run test.sh in the root.
It will do as follows:

rm -rf .build
rm -rf test.xcodeproj
rm -rf Package.resolved
swift run package-config
swift package generate-xcodeproj
swift run phase
swift run ignore

Package.swift looks like this.

// swift-tools-version:4.2
// The swift-tools-version declares the minimum version of Swift required to build this package.

import PackageDescription

let package = Package(
    name: "test",
    products: [
        .library(name: "test", targets: ["test"]),
    ],
    dependencies: [
        .package(url: "https://github.com/IgorMuzyka/phase", .branch("master")),
        .package(url: "https://github.com/IgorMuzyka/ignore", .branch("master")),
    ],
    targets: [
        .target(name: "test", dependencies: []),
        .testTarget(name: "testTests",dependencies: ["test"]),

        .target(name: "PackageConfigs", dependencies: [
            "PhaseConfig",
            "IgnoreConfig",
        ]),
    ]
)

#if canImport(PhaseConfig)
import PhaseConfig

PhaseConfig(phases: []).write()
#endif

#if canImport(IgnoreConfig)
import IgnoreConfig

IgnoreConfig(excludedTargets: ["test", "testTests"]).write()
#endif

This succeeds and results in xcode project with applied changes from running executables.

@IgorMuzyka
Copy link
Collaborator Author

Hmm running swift run package-config fails if PackageConfigs target is defined but no source files for it created.
Need to mention in readme that user should run this command prior to defining PackageConfigs target to get the emtpy source created for him.

@orta
Copy link
Member

orta commented Mar 20, 2019

( Sorry, I'm at the react native core dev summit hacking - will try get you a proper run through tomorrow. I sent you a collaborator invite too, looks interesting )

@IgorMuzyka
Copy link
Collaborator Author

Thats fine. Already accepted invite. Have fun at the conference.

@orta
Copy link
Member

orta commented Mar 24, 2019

Alright, cool, I've given this a review - I think it all makes sense. Let's call this version 0.9.0 to give some chance to make API breaking changes in production, but I think this makes sense!

@IgorMuzyka
Copy link
Collaborator Author

Awesome 👍🏻👍🏻👍🏻👍🏻👍🏻

@IgorMuzyka
Copy link
Collaborator Author

Would you add a tag with version?)

@orta
Copy link
Member

orta commented Mar 24, 2019

Shipped

Screen Shot 2019-03-24 at 8 43 00 AM

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.

2 participants