search

kylef / heroku-buildpack-swift

Heroku build pack for Swift
BSD 3-Clause "New" or "Revised" License
508 stars 0 forks source link

Clone swiftenv manually if .git folder has been removed. #37

Open wileykestner opened 6 years ago

wileykestner commented 6 years ago

I was trying to use heroku-buildpack-swift as the buildpack for a server-side swift web app when deploying it to Pivotal Web Services (PWS), a large instance of Pivotal Cloud Foundry.

I try to deploy my app using something like:

cf push my-special-app -b https://github.com/wileykestner/heroku-buildpack-swift.git

It appears that PWS removes the .git folder when it downloads the custom buildpack, so asking the buildpack to initialize it's submodules to bring in swiftenv doesn't work and the bin/compile script dies without successfully configuring the instance or deploying the web app.

I'm proposing a work-around in this PR that checks for the existence of the .git folder before attempting to initialize the git submodules. If it can't find them, it goes and manually clones swiftenv and gets it onto the PATH in the same way initializing swiftenv as a submodule would. Using this fork/PR I was able to successfully deploy my swift app on PWS.

I'm a total novice at buildpacks so please feel free to reject or request any amendments. As it stands now, the diff is quite small and seems to allow a single buildpack to deploy swift 4.0 applications to both heroku and PWS (and possibly other cloud foundry instances, haven't tested).

CC'ing @drnic @briancroom because it appears that You Might Care™ based on Cursory Internet Research®

wileykestner commented 6 years ago

Also CC'ing some folks at Pivotal who may care:

@tylerschultz @mhoran

wileykestner commented 6 years ago

@kylef Any interest in merging this PR? Any comments?

kylef commented 6 years ago

Hi @wileykestner, thanks for debugging this problem. The one concern here is you might end up with a different version of swiftenv that what is tested and known to work. The submodule allows us to lock to a specific commit so we can test that a version works before rolling out to swiftenv versions. As the clone would end up with latest it could be that an unforeseen bug may catch you. Ideally that wouldn't the the case, but I'd hate to introduce a change which breaks deploys for someone.

With some alterations, I think perhaps we could checkout a specific version after the clone such as the latest 1.3.0 release and update this at the same time as the submodule.

CloudFoundry-community are maintaining a fork of the Heroku build pack which adds other changes aided to make the build pack better work on CloudFoundry. You might be better of trying that out inside, I am not entirely sure how it differs from Heroku build pack as I do not use CloudFoundry. This build pack is developed specifically for Heroku.

https://github.com/cloudfoundry-community/swift-buildpack

wileykestner commented 6 years ago

@kylef Thanks for your review and comments. Guaranteeing a specific revision of swiftenv seems like a better idea for the reasons you mention.

I can think of two ways this could be accomplished:

  1. Add a single line like git checkout my-special-tag-v1.0 after the git clone command.
  2. Vendor the appropriate version of swiftenv into the buildpack itself and remove the deploy-time dependency on git.

My impulse would be to select the second option. It removes github's availability as a point of failure for application deployment. Of course, if github isn't available, then Heroku's main deployment model would break anyway as I understand it, so perhaps this is a moot point from your perspective.

If I update this pull request to vendor in the currently pinned version of swiftenv in lieu of the deploy-time git clone command would you consider merging?

Or since the charter of this fork is deployment on heroku (as you point out), would it be more appropriate for me to maintain my own fork and close this current pull request?

kylef commented 6 years ago

I think I agree with 2, perhaps it would make sense to vendor swiftenv into the build pack itself. Then there is no need to download it (either via git or submodule) and less room for networking problems preventing deploys. As I believe Heroku will cache the build pack itself and the other binaries like Swift and Clang are cached after first deploy (until changed). I can take a go at implementing this tonight (unless you wanted to in the PR).

I'm happy to accept some tweaks in how the build pack works to facilitate CloudFoundation (providing it isn't a huge re-haul of the buildpack) but as I do not use it I cannot test properly. I have no idea what the status is on the other forks.