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

Vendor libarchive and use SwiftPM to build #154

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

Conversation

cmcgee1024
Copy link
Member

@cmcgee1024 cmcgee1024 commented Aug 9, 2024

Use SwiftPM to compile and link in libarchive using a vendored
version of it. This will help to ensure that it is statically linked
into the final binary in both the dev environment and at release.
Also, it removes the need for a separate C compiler toolchain to
be present when producing a released binary, just the same swift
toolchain that is being used to make the swiftly binary.

Another benefit of making swift build just work for this project
is that it will make the supported platform matrix better reflect
reality with a passing Linux build on the swift package index site.

https://swiftpackageindex.com/swiftlang/swiftly

@cmcgee1024
Copy link
Member Author

@swift-ci test macOS

@rauhul
Copy link
Member

rauhul commented Aug 9, 2024

Is it possible to submodule libarchive?

@cmcgee1024
Copy link
Member Author

Is it possible to submodule libarchive?

Possible, probably. Git subtree seems like a better way to do this kind of vendoring. It will allow us to pull the latest in the future too. Specific instructions are in UPDATING.md.

@adam-fowler
Copy link
Contributor

Possible, probably. Git subtree seems like a better way to do this kind of vendoring

I'm interested to why you think this is the case. Both methods allow you to update to new versions. I'm fairly agnostic on which method to choose.

@rauhul
Copy link
Member

rauhul commented Aug 11, 2024

Possible, probably. Git subtree seems like a better way to do this kind of vendoring

I'm interested to why you think this is the case. Both methods allow you to update to new versions. I'm fairly agnostic on which method to choose.

Personally I prefer submodules because it doesn't include the +/- diff lines from the vendored project PRs and I'm just more familiar with submodules.

@cmcgee1024
Copy link
Member Author

I'm interested to why you think this is the case. Both methods allow you to update to new versions. I'm fairly agnostic on which method to choose.

There's a couple of things that I think make subtree a good option here.

When you clone the swiftly repository you'll automatically get the code for libarchive needed to compile the project right away without any extra commands (ie. submodule init) at the correct revision, also when you pull. It's a lower cognitive load when you're not explicitly working on libarchive adjacent code, or when you're unaware of it.

There are certain changes that we're making to libarchive itself, such as all of the SwiftPM logic and metadata. They can live here inside a subtree of this repo instead of becoming a separate repo/branch somewhere. I'm not sure where we would put it since I have no idea if it would ever be upstreamed. I hope that it can be upstreamed someday alongside the cmake code, but I don't think that can be counted on, so we would probably need to make our own repo so that swiftly wouldn't break if someone accidentally cleaned up the commit/branch/repo that we rely.

Here's a nice subtree guide from Atlassian with a much better overview of it, including the tradeoffs:

https://www.atlassian.com/git/tutorials/git-subtree

@adam-fowler
Copy link
Contributor

There are certain changes that we're making to libarchive itself, such as all of the SwiftPM logic and metadata.

Can you point me to the changes. Also if you are making changes how does subtree deal with changes being made to the subtree?

@cmcgee1024
Copy link
Member Author

cmcgee1024 commented Aug 20, 2024

@adam-fowler @rauhul this PR is just a draft at the moment and has gotten messy over time as I updated the branch. I will restructure this with a force push to better demonstrate how git subtree should be used in practice. For example, there will be a commit that adds libarchive squashed at a particular revision into the subdirectory, ignored for the most part. Then a second commit can make the vendored customizations to it (SwiftPM metadata, a config.h that matches it, module maps). Finally, the commit that adds the the vendored libarchive as a dependency, removing the custom shell scripting that exists today and must be run as a pre-build step.

I'm curious how everyone's feeling about the high-level idea of a SwiftPM built libarchive for the reasons mentioned in the PR description above. Does this seem like a reasonable idea if we can work out the mechanics of the best use of git?

@adam-fowler
Copy link
Contributor

I'm curious how everyone's feeling about the high-level idea of a SwiftPM built libarchive for the reasons mentioned in the PR description above.

I'm good with that

@cmcgee1024
Copy link
Member Author

@swift-ci test macOS

@cmcgee1024
Copy link
Member Author

cmcgee1024 commented Aug 22, 2024

I've reorganized this PR into the 4 commits. Note that these could be reorganized into one or more PR's if that ends up being a better workflow for reviewing the details of the vendoring.

The most relevant commit with the swiftly changes is in 11d5958

Next commit adds SwiftPM support to libarchive and an UPDATING.md with instructions to update the library 83661a4

The other two are the least interesting since they are the ones created by git subtree that squashed and merged a specific revision of libarchive (v3.7.4). They provide the source code and enough metadata for git subtree to upstream the SwiftPM support someday, perhaps, and also we can use subtree to pull and merge changes to libarchive in the future.

@cmcgee1024
Copy link
Member Author

@swift-ci test macOS

@cmcgee1024
Copy link
Member Author

@swift-ci test macOS

@cmcgee1024 cmcgee1024 marked this pull request as ready for review August 28, 2024 18:44
@cmcgee1024
Copy link
Member Author

Please don't merge this PR yet.

Using the Squash and Merge option will likely rewrite the metadata needed by git subtree. I'm looking at trying to allow "Create a Merge Commit" for this project.

@cmcgee1024 cmcgee1024 changed the title DRAFT: vendor libarchive and use SwiftPM to build Vendor libarchive and use SwiftPM to build Sep 5, 2024
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