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.

Sign up for GitHub

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

Jump to bottom

Improving examples build configs and documentation. #155

Merged
merged 4 commits into from
Jul 15, 2024
Merged

Improving examples build configs and documentation. #155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ab46995
Improving examples build configs and documentation.
jferreyra-sc Dec 15, 2023
fb63bbc
reverted to sdk 30.0.2
jferreyra-sc Dec 15, 2023
0e5bd48
Set min deployment target on iOS 11 for the example xcode project
jferreyra-sc Dec 19, 2023
f2143f8
Added npm instructions and examples server instructions.
jferreyra-sc Dec 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ to build and run Objective-C and Java tests.

### Building and running the mobile example apps

The Android example app can be build with bazel: `bazel build
//examples:android-app`, and then install to a device with `adb install
bazel-bin/examples/android-app.apk`
You need to install the Android SDK 30.0.2 and the NDK 21.4.7075529. You need both `ANDROID_SDK_HOME` and `ANDROID_NDK_HOME` env variables set.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason we require these versions? I don't remember the example app uses any new features that's only available in these versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the sdk version is hardcoded here:

djinni/bzl/android_configure.bzl

Line 27 in c7b0a39

build_tools_version="30.0.2",

The NDK versions here:
https://github.com/Snapchat/djinni/blob/c7b0a3978ebe34cc7b5e8323c0ecab5b75017ba7/bzl/android_configure.bzl#L37C17-L37C17
21.4.7075529 specifically is the latest update to that version.
A later NDK should be backwards compatible, however the bazel version specified to build this project only supports up to version 21, and warns (fiercely) that errors can be introduced by using a later NDK version.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just did a quick test and find that android example build still works if I complete remove the hard coded versions

--- a/bzl/android_configure.bzl
+++ b/bzl/android_configure.bzl
@@ -23,8 +23,6 @@ def _android_autoconf_impl(repository_ctx):
     native.android_sdk_repository(
         name="androidsdk",
         path="{}",
-        api_level=30,
-        build_tools_version="30.0.2",
     )
 """.format(sdk_home)
 
@@ -33,8 +31,7 @@ def _android_autoconf_impl(repository_ctx):
         ndk_rule = """
     native.android_ndk_repository(
         name="androidndk",
-        path="{}",
-        api_level=21,
+        path="{}"
     )
 """.format(ndk_home)

I think we probably should just remove these as we don't really use anything that requires a specific version.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just think we should avoid putting hard coded versions in here because they needlessly prevent people from building the code with whatever they already have in their system.

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 agree, that is a better solution.
Let me try that.

The Android example app can be build with bazel: `bazel build //examples:android-app`,
and then install to a device with `adb install bazel-bin/examples/android-app.apk`

The iOS example app are built with Xcode. Simply open the project in Xcode and
it should work.
Expand Down Expand Up @@ -301,9 +301,11 @@ Notable differences when comparing to the Java/ObjC support:
easily add extension methods (by add functions to prototype) without having to
derive from a base class.

The command to run Wasm/TypeScript unit tests is `bazel run
//test-suite:server-ts`. You will need the `tsc` compiler and the `browserify`
tool to run these tests.
Use `bazel run //test-suite:server-ts` to run the Wasm/TypeScript unit tests.
You will need `npm` and run `npm install` in the `test-suite` folder.
You need as well the `tsc` compiler and the `browserify` tool to run these tests.

To run the examples server: `bazel run //examples:server`

## Async interface support

Expand Down
2 changes: 1 addition & 1 deletion examples/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ android_library(
srcs = glob(["handwritten-src/java/com/dropbox/textsort/*.java"]),
custom_package = "com.dropbox.textsort",
manifest = "android/app/src/main/AndroidManifest.xml",
resource_files = glob(["android/app/src/main/res/**"]),
resource_files = glob(["android/app/src/main/res/**"], exclude=["android/app/src/main/res/.DS_Store"]),
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this cause builds to fail?

Copy link
Contributor Author

@jferreyra-sc jferreyra-sc Dec 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

yeah, basically if Mac Os adds these .DS_Store files, then the glob is picking them up and failing the build.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is strange. Do you mean Macos will put files in a source tree that cause bazel based projects to fail?

Copy link
Contributor Author

@jferreyra-sc jferreyra-sc Dec 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Exactly
Those files in the gitignore already:
https://github.com/Snapchat/djinni/blob/main/.gitignore#L1
So they do not get checked in by mistake, but still they can break the build process.

deps = [":textsort-java",],
)

Expand Down
2 changes: 2 additions & 0 deletions examples/ios/TextSort.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,7 @@
GCC_PRECOMPILE_PREFIX_HEADER = YES;
GCC_PREFIX_HEADER = "TextSort/TextSort-Prefix.pch";
INFOPLIST_FILE = "TextSort/TextSort-Info.plist";
IPHONEOS_DEPLOYMENT_TARGET = 11.0;
LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks";
LIBRARY_SEARCH_PATHS = "$(inherited)";
PRODUCT_BUNDLE_IDENTIFIER = "Dropbox.${PRODUCT_NAME:rfc1034identifier}";
Expand All @@ -449,6 +450,7 @@
GCC_PRECOMPILE_PREFIX_HEADER = YES;
GCC_PREFIX_HEADER = "TextSort/TextSort-Prefix.pch";
INFOPLIST_FILE = "TextSort/TextSort-Info.plist";
IPHONEOS_DEPLOYMENT_TARGET = 11.0;
LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks";
LIBRARY_SEARCH_PATHS = "$(inherited)";
PRODUCT_BUNDLE_IDENTIFIER = "Dropbox.${PRODUCT_NAME:rfc1034identifier}";
Expand Down
Loading