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

Move to Swift 5.1 #275

Merged
merged 26 commits into from
Dec 21, 2019
Merged

Move to Swift 5.1 #275

merged 26 commits into from
Dec 21, 2019

Conversation

f-meloni
Copy link
Member

Is quite early for moving danger to swift 5.1, but just wanted to start having a look at how it would work and if the problems that Danger had on linux (#214) are still there on 5.1

@bofeizhu
Copy link

@f-meloni On Linux in Swift 5.1, URLSession moved to the FoundationNetworking module.

@f-meloni
Copy link
Member Author

@bofeizhu I know, this is why I've opened pointfreeco/swift-snapshot-testing#254 and asked on nerdishbynature/RequestKit#64 if I can help :)

@f-meloni f-meloni marked this pull request as ready for review December 4, 2019 21:07
.travis.yml Outdated Show resolved Hide resolved
@ellneal
Copy link
Member

ellneal commented Dec 11, 2019

@f-meloni I've made some changes to the Dockerfile in my fork to build the container –and by extension the GitHub Action– with Swift 5.1 (currently on Docker Hub as ellneal/danger-swift:2.0.6).

FROM swift:5.1

MAINTAINER Orta Therox

LABEL "com.github.actions.name"="Danger Swift"
LABEL "com.github.actions.description"="Runs Swift Dangerfiles"
LABEL "com.github.actions.icon"="zap"
LABEL "com.github.actions.color"="blue"

# Install nodejs
RUN apt-get update -q \
    && apt-get install -qy curl \
    && mv /usr/lib/python2.7/site-packages /usr/lib/python2.7/dist-packages; ln -s dist-packages /usr/lib/python2.7/site-package \
    && curl -sL https://deb.nodesource.com/setup_10.x |  bash - \
    && apt-get install -qy nodejs \
    && rm -r /var/lib/apt/lists/*

# Install danger-swift globally
COPY . _danger-swift
RUN cd _danger-swift && make install

# Run Danger Swift via Danger JS, allowing for custom args
ENTRYPOINT ["npx", "--package", "danger", "danger-swift", "ci"]

The new Swift containers don't have curl installed by default, and there's some bizarre issue that requires the mv /usr/lib/python2.7/site-packages /usr/lib/python2.7/dist-packages; ln -s dist-packages /usr/lib/python2.7/site-package statement (source).

I've also changed it to copy the source from the repository rather than cloning the repo, although I'm not sure if there's a specific reason for that? If left to copy though, it's a good idea to add a .dockerignore file to ignore the SwiftPM build directory.

# .dockerignore
.build

@f-meloni
Copy link
Member Author

f-meloni commented Dec 12, 2019

@ellneal I will wait for this to build then I can try the dockerfile directly with github actions and try to fix it.

Thank you for having a look :)

@f-meloni
Copy link
Member Author

f-meloni commented Dec 19, 2019

This is now passing CI 🎉🎉🎉
Thank you @ellneal and @JosephDuffy for the help with the RequestKit and Octokit swift 5.1 update!
Now I will focus on the Dockerfile :)

@younatics
Copy link

I also want to use it in Swift 5.1.3. Currently, Danger/swift is not working in Xcode 11, Swift 5.1.3 because of module compiled with Swift 5.0 cannot be imported by the Swift 5.1.3 compiler

@f-meloni
Copy link
Member Author

@younatics Did you compile Danger with a different xcode version?

@f-meloni
Copy link
Member Author

f-meloni commented Dec 21, 2019

The dockerfile is working, thank you @ellneal for your suggestions :)
Also the build time is now down to 3 mins

Schermata 2019-12-21 alle 17 09 11

Possibly even less given the released version has less dependencies

@f-meloni f-meloni merged commit fd78cbd into master Dec 21, 2019
@f-meloni f-meloni deleted the swift_5.1 branch December 21, 2019 17:23
@ellneal
Copy link
Member

ellneal commented Dec 21, 2019

That's awesome @f-meloni 😁 Can you tag a new release when you get chance?

@f-meloni
Copy link
Member Author

@ellneal to be fair my plan was to finish #303 and make a new 3.0.0 release, with swift 5.1 and the new dependencies resolver.

@ellneal
Copy link
Member

ellneal commented Dec 21, 2019

That makes sense. I'll just switch my actions to use the master branch until then 👍

@f-meloni
Copy link
Member Author

It shouldn't take much longer anyway, I think it will be out max at the beginning of next year I think.

@f-meloni
Copy link
Member Author

@ellneal You should now be able to use the version 3.0.0, that should also be 30 seconds faster on the build phase.

Schermata 2020-01-12 alle 14 35 52

@ellneal
Copy link
Member

ellneal commented Jan 13, 2020

Awesome, thanks @f-meloni 🎉

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.

None yet

5 participants