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

Update to Stencil 0.14.0 (and Swift 5) #127

Merged
merged 47 commits into from
Oct 9, 2020
Merged

Conversation

fortmarek
Copy link
Member

@fortmarek fortmarek commented Aug 24, 2020

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
Copy link
Member Author

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
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link
Member Author

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
Copy link
Contributor

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

@AliSoftware
Copy link
Contributor

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
Copy link
Contributor

AliSoftware commented Aug 29, 2020

@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.

Copy link
Contributor

@AliSoftware AliSoftware left a comment

Choose a reason for hiding this comment

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

Just small nits and questions (but looks good overall!)

Package.swift Show resolved Hide resolved
Sources/StencilSwiftKit/Environment.swift Show resolved Hide resolved
StencilSwiftKit.xcodeproj/project.pbxproj Show resolved Hide resolved
StencilSwiftKit.xcodeproj/project.pbxproj Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
@AliSoftware
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Member

djbe commented Oct 8, 2020

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.

@djbe djbe requested a review from AliSoftware October 8, 2020 23:48
This was referenced Oct 9, 2020
This was referenced Oct 9, 2020
@djbe djbe changed the title Update Stencil to 0.14.0 Update to Stencil 0.14.0 (and Swift 5) Oct 9, 2020
@djbe djbe modified the milestones: 2.8.0, 2.7.3 Oct 9, 2020
@AliSoftware
Copy link
Contributor

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
Copy link
Member

djbe commented Oct 9, 2020

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
Copy link
Contributor

Ok. sgtm!

@djbe djbe merged commit 2737b0b into SwiftGen:stable Oct 9, 2020
@fortmarek
Copy link
Member Author

fortmarek commented Oct 9, 2020

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 🙂

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.

4 participants