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

Add support for xbuild #212

Merged
merged 11 commits into from
Sep 4, 2023
Merged

Add support for xbuild #212

merged 11 commits into from
Sep 4, 2023

Conversation

notgull
Copy link
Sponsor Contributor

@notgull notgull commented Aug 29, 2023

xbuild is a tool for building iOS/Android apps in Rust.

xbuild is a tool for building iOS/Android apps in Rust.

Signed-off-by: John Nunley <dev@notgull.net>
@taiki-e
Copy link
Owner

taiki-e commented Aug 29, 2023

https://github.com/taiki-e/install-action/actions/runs/6014566895/job/16314554693

xbuild tool is deprecated and will be removed in future updates, use msbuild instead

🤔

@notgull
Copy link
Sponsor Contributor Author

notgull commented Aug 29, 2023

This is odd. The maintainer for rust-mobile just told me cargo-apk is deprecated in favor of xbuild.

@notgull
Copy link
Sponsor Contributor Author

notgull commented Aug 29, 2023

Oh wait, that's Mono's xbuild command.

It looks like I should install the xbuild binary under the name x, are there any existing controls for doing that?

@taiki-e
Copy link
Owner

taiki-e commented Aug 29, 2023

You can add a special case here.

installed_bin="${bin_dir}/$(basename "${bin_in_archive}")"

Probably something like.

case "${tool}" in
    build) installed_bin="${bin_dir}/x"  ;;
    *) installed_bin="${bin_dir}/$(basename "${bin_in_archive}")"  ;;
esac

And here too.

case "${tool}" in

Signed-off-by: John Nunley <dev@notgull.net>
Signed-off-by: John Nunley <dev@notgull.net>
@notgull
Copy link
Sponsor Contributor Author

notgull commented Aug 30, 2023

Ugggh, xbuild dynamically links to OpenSSL, doesn't it.

I've opened rust-mobile/xbuild#132 to deal with this. Until then I guess I'll just install libssl.

Signed-off-by: John Nunley <dev@notgull.net>
Signed-off-by: John Nunley <dev@notgull.net>
@notgull
Copy link
Sponsor Contributor Author

notgull commented Aug 30, 2023

Hmm, it looks like xbuild needs SSL 3, and many of the operating systems in the configuration at the moment just don't have SSL 3 available.

@taiki-e
Copy link
Owner

taiki-e commented Aug 30, 2023

For now, I think we can adjust https://github.com/taiki-e/install-action/blob/main/tools/ci/tool-list.sh and mark xbuild as glibc_pre_2_34_incompat.

many of the operating systems in the configuration

We test a number of environments in CI, but the ones that actually important for most users are the -latest variants of the GitHub-hosted runner: ubuntu-latest (ubuntu-22.04), macos-latest (macos-12), and windows-latest (windows-2022)

Signed-off-by: John Nunley <dev@notgull.net>
tools/codegen/base/xbuild.json Outdated Show resolved Hide resolved
main.sh Outdated Show resolved Hide resolved
Signed-off-by: John Nunley <dev@notgull.net>
@taiki-e
Copy link
Owner

taiki-e commented Aug 30, 2023

At least in their 0.1.x versions, the release tag does not seem to match the crate version.

https://crates.io/crates/xbuild/versions
https://github.com/rust-mobile/xbuild/releases

If that has been fixed in 0.2.x and up, that is fine, but otherwise it will be confusing because it will install something that does not match the crate version specified by the user.

@notgull
Copy link
Sponsor Contributor Author

notgull commented Aug 30, 2023

Hmm, yes, that seems problematic.

@dvc94ch When you make xbuild releases from this point on, can you make sure that the release tag matches the crate version?

Signed-off-by: John Nunley <dev@notgull.net>
manifests/xbuild.json Outdated Show resolved Hide resolved
@dvc94ch
Copy link

dvc94ch commented Aug 30, 2023

We already have a gh action: https://github.com/rust-mobile/setup-xbuild-action

Co-authored-by: Taiki Endo <te316e89@gmail.com>
@notgull
Copy link
Sponsor Contributor Author

notgull commented Aug 30, 2023

We already have a gh action: https://github.com/rust-mobile/setup-xbuild-action

It looks like that uses the GitHub CLI though, which needs a secret token to work. install-action works without any GitHub tokens.

Edit: it looks like there is also no control over the version installed, which is important for some cases.

@notgull
Copy link
Sponsor Contributor Author

notgull commented Sep 2, 2023

@dvc94ch Poke. Given the reasons above I feel that this is a good reason why xbuild should support install-action through proper versioning.

@dvc94ch
Copy link

dvc94ch commented Sep 2, 2023

Yeah, really annoying that gh cli requires a token to download a release, I know. xbuild has different components I think the release tag versions the sdk build and has nothing to do with the crate version

@notgull
Copy link
Sponsor Contributor Author

notgull commented Sep 2, 2023

Yeah, really annoying that gh cli requires a token to download a release, I know. xbuild has different components I think the release tag versions the sdk build and has nothing to do with the crate version

Hmm, so should this crate just ignore versions with + in it?

As xbuild updates with new SDKs unrelated to the underlying release,
this commit makes the manifest generator ignore those releases.

Signed-off-by: John Nunley <dev@notgull.net>
@notgull notgull requested a review from taiki-e September 3, 2023 15:36
Copy link
Owner

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

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

Thanks!

@taiki-e taiki-e merged commit b45f49a into taiki-e:main Sep 4, 2023
25 checks passed
@taiki-e
Copy link
Owner

taiki-e commented Sep 4, 2023

Published in 2.18.0.

@notgull notgull deleted the xbuild branch September 4, 2023 20:14
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