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

packaging plugin #254

Merged
merged 18 commits into from
Jun 1, 2022
Merged

packaging plugin #254

merged 18 commits into from
Jun 1, 2022

Conversation

tomerd
Copy link
Contributor

@tomerd tomerd commented Mar 18, 2022

motivation: an easy way to archive lambda for uploading to AWS

changes:

  • add SwiftPM plugin with the verb "archive"
  • on Amazon Linux, build and package directly using the plugin bridge into SwiftPM
  • on other platforms use docker to build and package the lambda(s)

@tomerd tomerd force-pushed the feature/packaging-plugin branch 2 times, most recently from 9866b0b to 4d09892 Compare March 18, 2022 07:40
@tomerd tomerd added this to the 1.0.0-alpha.1 milestone Mar 18, 2022
@tomerd tomerd added the 🔼 needs-minor-version-bump For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0 label Mar 18, 2022
@tomerd tomerd force-pushed the feature/packaging-plugin branch 2 times, most recently from 27dece5 to c75825c Compare March 18, 2022 07:44
@tomerd
Copy link
Contributor Author

tomerd commented Mar 18, 2022

cc @abertelrud - this requires escaping the sandbox because of docker. would appreciate your feedback on the direction here

@tomerd tomerd force-pushed the feature/packaging-plugin branch 2 times, most recently from c5963ad to 907da4e Compare March 18, 2022 08:19
Copy link
Member

@fabianfett fabianfett left a comment

Choose a reason for hiding this comment

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

Great start. Love to see this arriving! A couple of quick thoughts.

Examples/Echo/Package.swift Outdated Show resolved Hide resolved
Package@swift-5.2.swift Outdated Show resolved Hide resolved
Plugins/AWSLambdaPackager/Plugin.swift Outdated Show resolved Hide resolved
Plugins/AWSLambdaPackager/Plugin.swift Show resolved Hide resolved
@tomerd tomerd force-pushed the feature/packaging-plugin branch 4 times, most recently from cd04fc7 to a882987 Compare March 18, 2022 23:52
@tomerd tomerd requested a review from yim-lee March 19, 2022 20:48
Package@swift-5.4.swift Outdated Show resolved Hide resolved
Package@swift-5.5.swift Outdated Show resolved Hide resolved
Plugins/AWSLambdaPackager/Plugin.swift Show resolved Hide resolved
Plugins/AWSLambdaPackager/Plugin.swift Show resolved Hide resolved
Plugins/AWSLambdaPackager/Plugin.swift Show resolved Hide resolved
Plugins/AWSLambdaPackager/Plugin.swift Outdated Show resolved Hide resolved
@tomerd tomerd force-pushed the feature/packaging-plugin branch 4 times, most recently from eb9923a to ab8c681 Compare April 15, 2022 19:10
@tomerd
Copy link
Contributor Author

tomerd commented Apr 15, 2022

@fabianfett I think this is ready to merge as iteration 1. next iteration we can explore how to integrate ziplib

Copy link
Member

@fabianfett fabianfett left a comment

Choose a reason for hiding this comment

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

As part of my review, I tried to run it in the EchoHandler... However I got the following output:

➜  Echo git:(feature/packaging-plugin) ✗ swift package archive     
-------------------------------------------------------------------------
building "echo" in docker
-------------------------------------------------------------------------
error: processFailed(126)

I think we should at least print the processes output, in the error case. This error is undebugable for me.

Examples/Echo/Package.swift Outdated Show resolved Hide resolved
@tomerd tomerd changed the title [wip] packaging plugin packaging plugin Apr 18, 2022
@tomerd
Copy link
Contributor Author

tomerd commented Apr 18, 2022

I think we should at least print the processes output

fixed

@fabianfett
Copy link
Member

Interesting, just pulled the newest version and tried to rerun:

➜  Echo git:(feature/packaging-plugin) ✗ swift package archive
-------------------------------------------------------------------------
building "echo" in docker
-------------------------------------------------------------------------
docker: Got permission denied while trying to connect to the Docker daemon socket at unix:///var/run/docker.sock: Post "http://%2Fvar%2Frun%2Fdocker.sock/v1.24/containers/create": dial unix /var/run/docker.sock: connect: operation not permitted.
See 'docker run --help'.
error: /usr/local/bin/docker run --rm -v /Users/fabian/Developer/swift-server/swift-aws-lambda-runtime/Examples/Echo:/workspace -w /workspace swift:amazonlinux2 bash -cl swift build -c release --show-bin-path failed with code 126
➜  Echo git:(feature/packaging-plugin) ✗ docker container create swift:5.6-focal
98decac8da409010202c45428d18e06f9761f1b130c649afc14778b6d74aa72b
➜  Echo git:(feature/packaging-plugin) ✗ 

Apparently from the plugin I don't have enough rights to create a new container, but just from the shell I have. Do you have any idea what this might be related to @tomerd.

@stevapple
Copy link
Contributor

stevapple commented Apr 19, 2022

@tomerd
Copy link
Contributor Author

tomerd commented Apr 19, 2022

right, this is because talking to docker is networking which is denied by the sandbox. for local testing I have been using the plugin without the sandbox, e.g: swift package --disable-sandbox --package-path Examples/Echo archive

@abertelrud would be interesting to see if we can poke a hole for docker in the sandbox, or be able to express it as a plugin requirement that the user can approve

@tomerd
Copy link
Contributor Author

tomerd commented May 10, 2022

@fabianfett ?

tomerd and others added 12 commits May 16, 2022 09:50
motivation: add an easy wasy for lambda users to package their lambda and upload it to AWS

changes:
* add SwiftPM plugin with the verb "archive"
* use docker to build and package the lambda(s)
Co-authored-by: Yim Lee <yim_lee@apple.com>
@tomerd tomerd force-pushed the feature/packaging-plugin branch 3 times, most recently from 89a4ba9 to ebd3f27 Compare May 16, 2022 18:03
Copy link
Member

@fabianfett fabianfett left a comment

Choose a reason for hiding this comment

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

Looks good and works for me!

Package@swift-5.6.swift Outdated Show resolved Hide resolved
Comment on lines +174 to +178
// rename artifact to "bootstrap"
let relocatedArtifactPath = workingDirectory.appending(artifactPath.lastComponent)
let symbolicLinkPath = workingDirectory.appending("bootstrap")
try FileManager.default.copyItem(atPath: artifactPath.string, toPath: relocatedArtifactPath.string)
try FileManager.default.createSymbolicLink(atPath: symbolicLinkPath.string, withDestinationPath: relocatedArtifactPath.lastComponent)
Copy link
Member

Choose a reason for hiding this comment

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

Code comment does not match behavior. I wonder if just renaming the executable (like the code comment says), is actually the better alternative here. What do we gain through the symlink indirection?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I cant remember tbh, I think I tired it and it caused some issue, will need to try again. in either case its the same end result

tomerd and others added 2 commits May 16, 2022 18:53
Co-authored-by: Fabian Fett <fabianfett@apple.com>
@tomerd tomerd merged commit 81f8d27 into swift-server:main Jun 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔼 needs-minor-version-bump For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants