SwiftGen / StencilSwiftKit

A framework bringing additional nodes & filters to Stencil dedicated to Swift code generation
MIT License
289 stars 55 forks source link

Update to Stencil 0.14.0 (and Swift 5) #127

Closed fortmarek closed 3 years ago

fortmarek commented 3 years ago

Since Stencil has a new release, it'd be great to update it for StencilSwiftKit, too!

Apart from updating the version in Package.swift, some minor changes have been necessary

fortmarek commented 3 years ago

The build is failing due to Error untarring cache: Error extracting tarball - I can't find what the problem could be and have zero experience with CircleCI, would you have any insights maybe, @AliSoftware?

AliSoftware commented 3 years ago

Mmmh interesting, I got a very similar error in CircleCI for a completely different project and repo at work. Maybe CircleCI is being unstable recently about that? 🤔

Another reason might be if CircleCI updated something inside the VM images (like the version of bundler or the absolute path where the project is cloned inside the VM, changing the path of the cache being restored) without telling us…?

Two things to try:

  1. Simply re-trigger the CI build. Maybe that was a one-off bug of CircleCI itself
  2. Force-invalidate the cache so that build and subsequent ones use a new cache. You can do that by updating the cache key used in `.circleci/config.yml

I'll take care of trying both options in a minute.

AliSoftware commented 3 years ago

@fortmarek Although unlikely, this might also be because you're using a fork and not the main repo. Now that you're part of the Core Contributors to the SwiftGen project, you're supposed to be able to create branches directly on the main repo instead of having to use a fork. Can you confirm me that is the case? (at least for your future PRs in SwiftGen's repos)

fortmarek commented 3 years ago

Thanks for looking into it so quickly @AliSoftware! The fork could be why this is happening, but it seems I have not been added to core contributors - at least I don't see myself being part of the organization and I can not create a new branch in the repo itself.

AliSoftware commented 3 years ago

Ah, right, I've added you to the stencilproject org but not SwiftGen's. This should be fixed now 😅

AliSoftware commented 3 years ago

Ok, seems CI is working again.

Some jobs are still failing, but for good reason now (found 1 violation during lint, and compile error on usage of .components) that actually needs fixing in your PR.

AliSoftware commented 3 years ago

@fortmarek If you need to update the Xcode version for your build to work, you'll also need to change line 12 of .circleci/config.yml to tell CircleCI to use a VM which contains that Xcode pre-installed. See here for the list of VM images provided by CircleCI and their corresponding installed Xcode versions.

fortmarek commented 3 years ago

I think this is ready for another review, @AliSoftware. One thing that will need to be resolved at some point (but I would not do it in this PR) is that the currently-used docker image has ruby with version 2.3.1. But if you run bundle update, you'd get cocoapods-downloader with version 1.4.0 which requires ruby >= 2.4.0. Unfortunately, we'll have to spin up our own docker image or find a different alternative from the jazzy docker image to solve this.

djbe commented 3 years ago

Note that I made PRs for Cocoapods 1.7 and Swift 5 quite a while ago: #118 and #119 respectively. There may be some bits in there that may be usable here if needed (haven't checked yet).

fortmarek commented 3 years ago

The CI seems to be green now. I have added Swift 5.2 image to a new repo here. I can transfer it to SwiftGen organization, but the problem is that the docker image will still reference my docker account (and for that we'd need SwiftGen Docker ID which might not be worth the hassle ?). I was also not able to configure with chruby (I tried hard, but ultimately failed ... but feel free to try it out by yourself). The current ruby version it uses is 2.5.1, which is what I put in .ruby-version.

I think it'd also be great to have only one Package.swift as it's easy to forget about the other, so how much are we keen on supporting Swift 4.0? Right now, this PR drops the support for it and keeps it only for Swift >= 4.2, but I can change that.

Apart from that, I have tried to update the Swift and Pods version where I deemed appropriate, but it definitely deserves another look.

AliSoftware commented 3 years ago

I won't have time to review the PR today but just to answer your questions:

fortmarek commented 3 years ago

I don't know much about Docker tbh but I was surprised when looking at your Dockerfile to see mention of ruby-0.7.1; didn't we want 2.4?!

That was version of ruby-install, but come to think of it, it was not necessary anymore anyway; the current ruby version is installed with rubygems, so I simplified the current Dockerfile 👍

fortmarek commented 3 years ago

I think this is ready for another review @AliSoftware 🙂

AliSoftware commented 3 years ago

Just received an email from CircleCI about docker rate-limiting coming soon, and was wondering if there's any risk affecting us with our use of Docker in this PR…

Hi there,

On November 1st, Docker Hub will begin limiting anonymous image pulls. We want to make sure you know how you might be impacted and what you can do to avoid interruptions to your workflow.

Adding Docker authentication to your pipeline config is the easiest way to avoid any service disruptions. If you use the Docker executor or pull Docker images when using the machine executor on CircleCI, we encourage you to authenticate. Because the anonymous API rate limits are based on IP addresses, they will impact CircleCI cloud customers. Authenticated users get higher per-user rate limits, regardless of IP.

We are currently working on a partnership with Docker to minimize the impact of this change for our users and will share more details as we get them.

For more information or to leave a question for us, please head over to Discuss or contact support.

Thanks and happy building,

  • The CircleCI team

@fortmarek you seem to be more versed into Docker than I am, any idea of the potential impact for us here?

fortmarek commented 3 years ago

I believe the new Docker restrictions are OK as they limit the number of free anonymous pulls - but that's something we should not be doing very often, so I think we're fine

djbe commented 3 years ago

~Why are we using these custom docker images instead of the official Swift images? Missing software, such as ruby/gems, or some other reason?~

Never mind, just found your dockerfile: https://github.com/fortmarek/docker-StencilSwiftKit/blob/main/swift-5.2/Dockerfile

I've made a repo to manage our docker image, and automate the build&push process to docker: https://github.com/SwiftGen/swift-docker. It's slightly based on @fortmarek with some other things I've found. In the process, I created a SwiftGen docker organisation, so now we can have docker access tokens and never worry about usage limits.

AliSoftware commented 3 years ago

Why are we using these custom docker images instead of the official Swift images? Missing software, such as ruby/gems, or some other reason?

That's the reason indeed. See this comment thread above for more context 😉

I've made a repo to manage our docker image, and automate the build&push process to docker: https://github.com/SwiftGen/swift-docker. It's slightly based on @fortmarek with some other things I've found. In the process, I created a SwiftGen docker organisation, so now we can have docker access tokens and never worry about usage limits.

Great!

Just one thing to be aware of tho, given the authentication phase you added in our CI config uses secret env vars, that means that the CI won't be able to run and authenticate on forks (on which CI secrets are not available, for security reasons), right? Not sure if that was already the case before or not. but worth to keep in mind

djbe commented 3 years ago

Yeah the secrets aren't shared to forks, but shouldn't matter for such an "internal" repository. (talking about the docker repo)

The access tokens in CircleCI are set at a "team" level. I'm not 100% sure how that works with forks, this is what the docs say:

To use environment variables set on the Contexts page, the person running the workflow must be a member of the organization for which the context is set.

So seems like it'll only be used when we trigger the builds? Not that it really matters anyway, like @fortmarek mentioned, we wont be triggering too many builds on this repository.

AliSoftware commented 3 years ago

Ok. sgtm!

fortmarek commented 3 years ago

Thanks for taking this over the finish line @djbe! I've been swarmed with other stuff and couldn't get to it. Creating a new docker repo inside Swiftgen repository was something we were pondering but back then it did not seem to be worth it - which has changed with new restrictions, so well done 👏 Again, thanks for doing this 🙂