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

Include armv7 symbols in boringssl vendoring #228

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

orobio
Copy link

@orobio orobio commented Apr 2, 2024

Updates the BoringSSL vendoring to include armv7 specific symbols.

I had to do a work-around that specifically checks out the version of BoringSSL that is currently used. This was necessary due to that the very latest on their main branch doesn't work properly with the current vendoring script. I suggest to not merge this until the vendoring script is updated to work with the latest BoringSSL version again, then the work-around can be removed.

The armv7 build is done using the following container: orobio/swift-armv7-cross-bullseye:5.10
It was created from orobio/swift-armv7.

This container is hosted on my DockerHub account. Would that be acceptable, or would you prefer to host it yourself?

Most of this is new territory for me, so feel free to mention anything that can be improved.

@orobio orobio mentioned this pull request Apr 2, 2024
@Lukasa
Copy link
Contributor

Lukasa commented Sep 30, 2024

Sorry for letting this languish!

@shahmishal is there any way we can get armv7 Swift images upstream?

@orobio
Copy link
Author

orobio commented Oct 19, 2024

Any news on this? It would be nice to finish this up in one way or another. Let me know if there’s anything I can do!

@xtremekforever
Copy link

Maybe we just need to generate a Swift 6.0.1 unofficial container for armv7 now? Well, there are a couple of blockers to generating an SDK as was suggested previously:

  • I looked at using the swift-sdk-generator to create an SDK but it's simply not ready to be able to generate an SDK for armv7 in its current state. It will require some updates, such as supporting Debian 11 or 12 to grab armv7 debs to generate an SDK. Then, it would need to be able to somehow grab the armv7 build from "somewhere" (maybe locally) and put the files in the correct place. Right now the SDK generator places target .swiftmodules and libraries in the swift.xctoolchain directory, which is not quite right since they should go into the SDK directory for the target!!!
  • I had been previously investigating adding support into swiftlang/swift to cross compile for armv7, but ran into quite a few issues and a Swift runtime that didn't actually work properly on the target. Since then this effort has been paused due to other commitments on my end.

However, if we can just use some of the existing swift-armv7 scripts to build Swift 6.0.1 and generate a container, would that be enough to at least get this done? It would be simple enough to update what we have working and generate something that could be used.

@orobio
Copy link
Author

orobio commented Oct 20, 2024

Well, we have a working solution here, so unless there is any indication that this solution is not acceptable, I don’t think we need to do anything else.

Regarding the Swift version. This provides 5.10 for armv7, while the other architectures are still on 5.9, so I think that’s fine for now.

@Lukasa
Copy link
Contributor

Lukasa commented Oct 21, 2024

Yeah, I don't foresee an issue with the Swift version. If you can bring this up to current main I'm happy enough to merge this as-is.

@orobio orobio force-pushed the include-armv7-symbols-in-boringssl-vendoring branch from 34a7d45 to 7a349c1 Compare October 25, 2024 18:38
@orobio
Copy link
Author

orobio commented Oct 25, 2024

@Lukasa : I rebased this on main. Some notes:

  • I ran into an issue where running the script a second time causes an issue with the 5.9 containers (error: invalid tool type in 'tools' map). It looks like 5.10 leaves some data in the .build directory that 5.9 doesn't like. I didn't see this issue earlier when they were still on 5.8. Removing the .build directory will fix this, so it shouldn't be a big problem. If this is not acceptable (and there's no specific reason that they are still on 5.9), I can update the other containers to 5.10 as well.
  • I'm running this on Linux and assume it would be difficult (impossible?) to get the MacOS and iOS builds working, so I disabled these locally. Could you please verify the complete script on your end?
  • For the above reason I did not include the result of the script either. I'll leave it to you to either add the updated files to this branch before merging, or just take the new symbols in when you're updating BoringSSL in the future.

@xtremekforever
Copy link

@Lukasa any idea when you can review 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.

3 participants