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

Add podspec #20

Closed
wants to merge 6 commits into from
Closed

Add podspec #20

wants to merge 6 commits into from

Conversation

igor-makarov
Copy link

Hi, this PR adds a .podspec file to the package.

The podspec has been tested with a working app and is functioning.

I've also run pod lib lint which integrates the pod into a blank app target and executes its tests. This also passes.

This closes #19.

@swift-server-bot
Copy link

Can one of the admins verify this patch?

Copy link
Member

@weissi weissi left a comment

Choose a reason for hiding this comment

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

Thanks for your PR! I can't take this right now yet because this claims version 0.0.1 but we haven't tagged a version yet (but will soon after Swift 5 gets released).

Regarding the podspec: Do we need to check the podspec in? We could just use a script like SwiftNIO does which automatically generates & uploads the podspecs with versions from SwiftPM. Otherwise we'll have to manage the versions in two places: the git tags for SwiftPM as well as the podspec file, no?

@chrisballinger
Copy link

@weissi You generally need to check the podspec in because a Podfile can reference a git repo, in which case it will look in the root for a podspec.

The podspec file is expected to be in the root of the repository

http://guides.cocoapods.org/syntax/podfile.html#pod

@weissi
Copy link
Member

weissi commented Apr 15, 2019

@chrisballinger what’s wrong with the way NIO does it? The pods look just fine to me: https://cocoapods.org/pods/SwiftNIO

The reason I don’t want to check it in is that there’s then two places for the version information: the git tags and the podspec and that’s not good.

@igor-makarov
Copy link
Author

In addition to publishing the podspec in the spec repo, it is used for one more purpose: direct git pod integration. Right now, I'm referring to swift-log in my Podfile like so:

pod 'Logging', git: 'https://github.com/igor-makarov/swift-log.git', branch: 'add-podspec'

This way, the podspec is retrieved directly from the repo and the version tags are ignored, allowing for usage of latest dev version.

@prafsoni prafsoni mentioned this pull request Jul 2, 2019
@igor-makarov igor-makarov force-pushed the add-podspec branch 3 times, most recently from 9af83c0 to 42c32ce Compare August 25, 2019 07:20
@igor-makarov
Copy link
Author

Hi! It's been a long time but I've been recently doing similar integration with swift-llbuild and I've gotten a new angle on the whole podspec versioning thing.
The full explanation is over at swiftlang/swift-llbuild#545.

I think it covers all cases and is compatible with SwiftPM versioning, in a way. Let me know what you think!

@weissi
Copy link
Member

weissi commented Aug 25, 2019

Wow, this looks great. I’m +1 on this.

@weissi
Copy link
Member

weissi commented Aug 25, 2019

Thank you!

Logging.podspec Outdated
def self.infer_version_from_git
return nil unless Dir.exist?('.git')

`git tag --list '*.*.*' --sort=committerdate`.split.select do |tag|
Copy link
Member

Choose a reason for hiding this comment

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

Couldn’t we sort this ‘by version’? Just in case we tag 1.2.0 and later we need to add a 1.1.2. I think sort can sort by version and git might be able to too?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for pointing it out!

I've replaced it with a git command that correctly looks for the tags in the current commit tree only.

Copy link
Author

Choose a reason for hiding this comment

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

This way if you have 1.2.0 and you later branch 1.1.2, if will get handled correctly.

Copy link
Member

@weissi weissi left a comment

Choose a reason for hiding this comment

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

Thank you, looks great to me. You might want to remove the old build_podspec script or maybe have it just use this podspec.

@igor-makarov
Copy link
Author

I've removed the old podspec builder as suggested.

@danramteke
Copy link

Any updates on getting this merged?

@tomerd
Copy link
Member

tomerd commented Sep 25, 2019

@weissi i know we added https://github.com/apple/swift-log/blob/master/scripts/build_podspec.sh instead, have we pushed the podspec?

@weissi
Copy link
Member

weissi commented Sep 25, 2019

@danramteke / @tomerd yes, the podspec is live as cocoapods.org/pods/Logging

@danramteke
Copy link

@weissi thanks for the direct link. I was not able to find it by searching "logging"

@igor-makarov
Copy link
Author

@danramteke yeah, the search indexer on the CP website broke a while ago, and because it's a volunteer project, nobody got around to fixing it yet.

@weissi
Copy link
Member

weissi commented Oct 8, 2019

thanks @igor-makarov , I was wondering why it doesn't show up.

@tomerd
Copy link
Member

tomerd commented Dec 20, 2019

can we close this since we are already publishing a pod?

@igor-makarov
Copy link
Author

This is an improvement to the podspec, it removes the need for a separate script.

@tomerd
Copy link
Member

tomerd commented Dec 20, 2019

@weissi do you prefer the script or the spec file? i dont have a strong opinion

@weissi
Copy link
Member

weissi commented Dec 20, 2019

@tomerd definitely script because then there's one source of truth, right? The podspec needs to re-iterate the modules as well as the version number. So generating this from Package.swift sounds better to me

@weissi
Copy link
Member

weissi commented Dec 20, 2019

@tomerd actually, the podspec seems to be automatically detecting the version which we might be fine with. Still seems a little dangerous that you don't have to specify which version exactly to upload though. But better than nothing.

@igor-makarov
Copy link
Author

I thought that it would be nice to avoid having a generator script.
What are the modules you're talking about?

@weissi
Copy link
Member

weissi commented Dec 20, 2019

@igor-makarov well, the thing is that there might be new modules in the future. In that case we'd need to remember to change both the podspec and the Package.swift. A generator script wouldn't have this issue because it uses the real source of truth which is the Package.swift. For a more complicated example check the generator in swift-nio

@igor-makarov
Copy link
Author

The current generator doesn't read from Package.swift though, right?

My original problem was that if there's no podspec in the git repo, there's no way to integrate the latest master version into a project.

@igor-makarov
Copy link
Author

Did a rebase to clean up the commit history.

@chrisballinger
Copy link

@weissi This should be merged because even though the problem is semi-resolved by having a podspec version published to trunk, not having this merged prevents someone from pointing to the master branch (or another branch) on this repo. For example, this is a somewhat common workflow:

pod 'Logging', git: 'https://github.com/apple/swift-log.git', branch: 'master'

@FredrikAugust
Copy link

Any updates on this?

@weissi
Copy link
Member

weissi commented Jun 17, 2020

There is another option: We could check in a .podspec IF we have a script (that is run by CI) which regenerates the podspec in CI. Basically exactly like we do the LinuxMain.swift generation. If the CI detects that you haven't run the update script it fails. WDYT? @ktoso / @glbrntt / @tomerd

@glbrntt
Copy link
Contributor

glbrntt commented Jun 18, 2020

@weissi that sounds reasonable, would it infer the version as per this PR or would the script just have to be run after tagging a new version?

@weissi
Copy link
Member

weissi commented Jun 18, 2020

@weissi that sounds reasonable, would it infer the version as per this PR or would the script just have to be run after tagging a new version?

Oh man, I forgot about the version :. Could it use git in the podspec to find the version?

@glbrntt
Copy link
Contributor

glbrntt commented Jun 18, 2020

Yes: inferring from git is one of the changes proposed by this PR.

@weissi
Copy link
Member

weissi commented Jun 18, 2020

Yes: inferring from git is one of the changes proposed by this PR.

Cool! If we now get a generator script that generates exactly what this PR proposes and add that to the CI to double check it's not wrong, then I'm cool with this.

@tomerd
Copy link
Member

tomerd commented Jun 18, 2020

we do version inference in the jazzy docs generation script, you can steal it from there

@ktoso
Copy link
Member

ktoso commented Jun 19, 2020

Sounds good to me 👍

@ktoso ktoso added the stashed label Sep 3, 2020
@ktoso ktoso changed the base branch from master to main September 3, 2020 12:35
@ktoso
Copy link
Member

ktoso commented Oct 25, 2022

I'm going to be the bearer or news and close this ticket then I guess.

2 years ago, most of projects we maintain have stopped supporting CocoaPods - https://forums.swift.org/t/swiftnio-is-dropping-support-for-cocoapods/56840 - ever since SwiftPM and its ecosystem has kept growing, and we don't have the expertise to maintain pods to begin with.

I'm going to close this PR given the above context and lack of action on it since a long time.

@ktoso ktoso closed this Oct 25, 2022
sushichop added a commit to sushichop/Puppy that referenced this pull request Oct 31, 2022
because `Logging` framework no longer supports CocoaPods.
See apple/swift-log#20.
sushichop added a commit to sushichop/Puppy that referenced this pull request Oct 31, 2022
because `Logging` framework no longer supports CocoaPods.
See apple/swift-log#20.
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.

CocoaPods / Carthage support
9 participants