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

Support for embedding resources in an executable #6067

Merged
merged 1 commit into from
Feb 8, 2023
Merged

Conversation

neonichu
Copy link
Contributor

@neonichu neonichu commented Jan 20, 2023

Basic support for a new .embed resource rule which will allow embedding the contents of the resource into the executable code by generating a byte array, e.g.

@_implementationOnly import struct Foundation.Data
    
struct PackageResources {
static let best_txt = Data([104,101,108,108,111,32,119,111,114,108,100,10])
}

Note that the current naïve implementaton will not work well for larger resources as it is pretty memory inefficient.

@neonichu neonichu self-assigned this Jan 20, 2023
@MaxDesiatov
Copy link
Contributor

MaxDesiatov commented Jan 20, 2023

Would it be better for performance if it was embedded as a [UInt8] array? That way you don't need to decode base64 at run time. You also don't need to rely on Foundation.Data then.

@neonichu
Copy link
Contributor Author

I updated the PR, but I think we'd still want to rely on Data instead of offering just a byte array?

@abertelrud
Copy link
Contributor

I updated the PR, but I think we'd still want to rely on Data instead of offering just a byte array?

Although the bundle form of this code generation does rely on Foundation, I think it makes sense for the byte array form to not do so. That makes it useful on, say, new platforms that doesn't have Foundation yet (or is having Foundation now a requirement for any new platform that claims to support Swift? — I've lost track of how core it is now considered to the Swift standard libraries).

@neonichu
Copy link
Contributor Author

Updated naming to embedInCode and changed the generated code to bare [UInt8].

Copy link
Contributor

@tomerd tomerd left a comment

Choose a reason for hiding this comment

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

cool 👍

@tomerd
Copy link
Contributor

tomerd commented Jan 20, 2023

looks good. I guess its draft since we need some tests?

Basic support for a new `.embed` resource rule which will allow embedding the contents of the resource into the executable code by generating a byte array, e.g.

```
struct PackageResources {
static let best_txt: [UInt8] = [104,101,108,108,111,32,119,111,114,108,100,10]
}
```

Note that the current naïve implementaton will not work well for larger resources as it is pretty memory inefficient.
@neonichu neonichu marked this pull request as ready for review February 8, 2023 01:42
@neonichu neonichu requested a review from elsh as a code owner February 8, 2023 01:42
@neonichu
Copy link
Contributor Author

neonichu commented Feb 8, 2023

Added a test now, so this is ready for review. Note that the test is part of FunctionalTests which is currently disabled so it won't run on CI at this time.

@neonichu
Copy link
Contributor Author

neonichu commented Feb 8, 2023

@swift-ci please smoke test

@neonichu neonichu changed the title WIP: Support for embedding resources in an executable Support for embedding resources in an executable Feb 8, 2023
@neonichu
Copy link
Contributor Author

neonichu commented Feb 8, 2023

Seeing another immediate failure for Windows here, cc @shahmishal

@shahmishal
Copy link
Member

@swift-ci test Windows

@neonichu
Copy link
Contributor Author

neonichu commented Feb 8, 2023

@swift-ci smoke test Windows

@neonichu
Copy link
Contributor Author

neonichu commented Feb 8, 2023

@swift-ci smoke test macOS

@neonichu neonichu merged commit 9fbd3fb into main Feb 8, 2023
@neonichu neonichu deleted the embed-resources branch February 8, 2023 16:31
@tomerd
Copy link
Contributor

tomerd commented Feb 8, 2023

thanks for adding this great feature @neonichu , should we add an entry in the change log / release note? seems important to communicate this one

@neonichu
Copy link
Contributor Author

neonichu commented Feb 8, 2023

Added in #6132

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.

5 participants