Page MenuHomePhabricator
Differential D14434

[CI] update NDK to 23 and bump Corrosion
ClosedPublic
Actions

Authored by kamil on Mar 10 2025, 6:27 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 7, 11:28 PM
Unknown Object (File)
Fri, Apr 4, 9:07 PM
Unknown Object (File)
Thu, Apr 3, 5:05 PM
Unknown Object (File)
Thu, Apr 3, 9:50 AM
Unknown Object (File)
Mon, Mar 31, 11:26 PM
Unknown Object (File)
Fri, Mar 28, 5:17 PM
Unknown Object (File)
Mon, Mar 24, 9:12 PM
Unknown Object (File)
Sun, Mar 23, 4:34 PM
View All 31 Files
Subscribers
None

Details

Reviewers
ashoat
angelika
bartek
tomek
Commits
rCOMM7bb11205a532: [CI] update NDK to 23 and bump Corrosion
Summary

ENG-10302

Android

The issue
There was a new, major version of rustup (1.28.0) with two breaking changes:

  1. rustup will no longer automatically install the active toolchain if it is not installed - this makes our CI to fail on most of our pipelines
  2. rustup show's output format has been cleaned up, making it easier to find out about your toolchains' status - this command is used by Corrosion, which we use to integrate Rust on Android

Fix for 1
In D14429, there is an example of how to do it explicitly, and it works, but because this major update caused many CI's to fail, there is a hotfix for this in 1.28.1

Fix for 2
To achieve that, we need to update Corrosion version to 0.5.1 which is prepared for new output format, v0.5.1.

Why NDK update?
We need to bump Corrosion from 0.2.1 to 0.5.1, which is a lot, and the minimal Rust version has been changed. According to this blog post, the minimum supported NDK version is 23. I took it from 0.71-stable. It should be fine as we're probably going to have 27 soon after the react-native upgrade (cc. @angelika). There is some context on this in D5900 (cc. @ashoat).

Commtest

It is going to be fixed, but in a not-yet published version 1.28.2, we can patch it by explicitly adding a target in the Dockerfile.

Test Plan

CI - not sure if I need to test something else; not familiar with Android configuration

Diff Detail

Repository
rCOMM Comm
Branch
fix-ci
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil created this revision.Mar 10 2025, 6:27 AM
kamil held this revision as a draft.
kamil edited the summary of this revision. (Show Details)Mar 10 2025, 6:42 AM
kamil edited the test plan for this revision. (Show Details)
Harbormaster failed remote builds in B33701: Diff 47378!Mar 10 2025, 6:42 AM
kamil updated this revision to Diff 47381.Mar 10 2025, 6:45 AM

Add the Rust target for ARM64 architecture

kamil edited the summary of this revision. (Show Details)Mar 10 2025, 6:50 AM
kamil edited the test plan for this revision. (Show Details)
Harbormaster failed remote builds in B33704: Diff 47381!Mar 10 2025, 6:56 AM
kamil edited the summary of this revision. (Show Details)Mar 10 2025, 6:58 AM
Harbormaster completed remote builds in B33704: Diff 47381.Mar 10 2025, 7:16 AM
kamil published this revision for review.Mar 10 2025, 7:16 AM
ashoat added inline comments.Mar 10 2025, 7:20 AM
native/android/build.gradle
10

Wondering why we're regressing from 24 to 23 for aarch64 here. Does this make performance worse for M1 users?

ashoat added a comment.Mar 10 2025, 7:20 AM

CI - not sure if I need to test something else; not familiar with Android configuration

I think it would be good to check the Android app for both emulator and physical device targets, and play around with it a little bit just to make sure nothing is broken and performance isn't degraded

kamil added a comment.Mar 10 2025, 7:28 AM
In D14434#401075, @ashoat wrote:

CI - not sure if I need to test something else; not familiar with Android configuration

I think it would be good to check the Android app for both emulator and physical device targets, and play around with it a little bit just to make sure nothing is broken and performance isn't degraded

Okay, makes sense

native/android/build.gradle
10

I did the same as react-native did for 0.71 compared to 0.70, so I think it is fine. It was done in this PR.

I can try doing 24 for aarch64 if diverging with react-native is okay

ashoat added inline comments.Mar 10 2025, 8:20 AM
native/android/build.gradle
10

Ah, seems fine if React Native is doing it

angelika added inline comments.Mar 10 2025, 8:27 AM
native/android/build.gradle
10

I'm not sure if downgrading it from 24 to 23 for M users is ok. Maybe RN didn't support building on M1 at that time (0.71 was a bit ago)?
Also note that this is about what system are you using to build the project. The CI runs on Linux. Have you tried building the app on M1 machine?

kamil added a comment.Tue, Mar 11, 5:25 AM

Tested this on a physical device and emulator, everything seems to be fine

native/android/build.gradle
10

I tried building this on a physical device and an emulator on my M3 machine, in both dev and release modes, everything seems to work

ashoat accepted this revision.Tue, Mar 11, 6:22 AM
angelika accepted this revision.Tue, Mar 11, 6:44 AM
This revision is now accepted and ready to land.Tue, Mar 11, 6:44 AM
Closed by commit rCOMM7bb11205a532: [CI] update NDK to 23 and bump Corrosion (authored by kamil). · Explain WhyTue, Mar 11, 8:21 AM
This revision was automatically updated to reflect the committed changes.
kamil added a commit: rCOMM7bb11205a532: [CI] update NDK to 23 and bump Corrosion.

Revision Contents
Changeset List

PathSize
native/
android/
app/
2 lines
10 lines
services/
commtest/
3 lines

Diff 47381

View Options

native/android/app/CMakeLists.txt

// @flow // @flow
import { useNavigation, useRoute } from '@react-navigation/native'; import { useNavigation, useRoute } from '@react-navigation/native';
import invariant from 'invariant';
import * as React from 'react'; import * as React from 'react';
import { View, useWindowDimensions } from 'react-native'; import { View, useWindowDimensions } from 'react-native';
import type { ViewStyleProp } from 'react-native/Libraries/StyleSheet/StyleSheet'; import type { ViewStyleProp } from 'react-native/Libraries/StyleSheet/StyleSheet';
import { FlatList } from 'react-native-gesture-handler'; import { FlatList } from 'react-native-gesture-handler';
import { useFetchThreadMedia } from 'lib/actions/thread-actions.js'; import { useFetchThreadMedia } from 'lib/actions/thread-actions.js';
import type { MediaInfo, Media } from 'lib/types/media-types'; import type { MediaInfo, Media } from 'lib/types/media-types';
import { import {
ThreadSettingsCategoryActionHeader, ThreadSettingsCategoryActionHeader,
ThreadSettingsCategoryFooter, ThreadSettingsCategoryFooter,
} from './thread-settings-category.react.js'; } from './thread-settings-category.react.js';
import GestureTouchableOpacity from '../../components/gesture-touchable-opacity.react.js'; import GestureTouchableOpacity from '../../components/gesture-touchable-opacity.react.js';
import type { ImageModalParams } from '../../media/image-modal.react.js';
import Multimedia from '../../media/multimedia.react.js'; import Multimedia from '../../media/multimedia.react.js';
import type { VideoPlaybackModalParams } from '../../media/video-playback-modal.react.js';
import { import {
ImageModalRouteName, ImageModalRouteName,
VideoPlaybackModalRouteName, VideoPlaybackModalRouteName,
} from '../../navigation/route-names.js'; } from '../../navigation/route-names.js';
import { useStyles } from '../../themes/colors.js'; import { useStyles } from '../../themes/colors.js';
import type { import type {
LayoutCoordinates, LayoutCoordinates,
VerticalBounds, VerticalBounds,
▲ Show 20 Lines • Show All 196 Lines • ▼ Show 20 Lines const navigateToMedia = React.useCallback(() => {
ref.current?.measure((x, y, width, height, pageX, pageY) => { ref.current?.measure((x, y, width, height, pageX, pageY) => {
const initialCoordinates: LayoutCoordinates = { const initialCoordinates: LayoutCoordinates = {
x: pageX, x: pageX,
y: pageY, y: pageY,
width, width,
height, height,
}; };
invariant(
verticalBounds,
'verticalBounds should be set in navigateToMedia',
);
const params: ImageModalParams | VideoPlaybackModalParams = {
presentedFrom: route.key,
mediaInfo,
initialCoordinates,
verticalBounds,
};
navigation.navigate<'VideoPlaybackModal' | 'ImageModal'>({ navigation.navigate<'VideoPlaybackModal' | 'ImageModal'>({
name: name:
mediaInfo.type === 'video' || mediaInfo.type === 'encrypted_video' mediaInfo.type === 'video' || mediaInfo.type === 'encrypted_video'
? VideoPlaybackModalRouteName ? VideoPlaybackModalRouteName
: ImageModalRouteName, : ImageModalRouteName,
key: `multimedia|${threadID}|${mediaInfo.id}`, key: `multimedia|${threadID}|${mediaInfo.id}`,
params: { params,
presentedFrom: route.key,
mediaInfo,
item,
initialCoordinates,
verticalBounds,
},
}); });
}); });
}, [navigation, route, threadID, mediaInfo, item, verticalBounds]); }, [navigation, route, threadID, mediaInfo, verticalBounds]);
const containerStyle = const containerStyle =
index % numColumns === 0 index % numColumns === 0
? memoizedStyles.mediaContainer ? memoizedStyles.mediaContainer
: memoizedStyles.mediaContainerWithMargin; : memoizedStyles.mediaContainerWithMargin;
return ( return (
<View key={item.id} style={containerStyle} onLayout={onLayout} ref={ref}> <View key={item.id} style={containerStyle} onLayout={onLayout} ref={ref}>
Show All 25 Lines
View Options

native/android/build.gradle

// @flow // @flow
import { useNavigation, useRoute } from '@react-navigation/native'; import { useNavigation, useRoute } from '@react-navigation/native';
import invariant from 'invariant';
import * as React from 'react'; import * as React from 'react';
import { View, useWindowDimensions } from 'react-native'; import { View, useWindowDimensions } from 'react-native';
import type { ViewStyleProp } from 'react-native/Libraries/StyleSheet/StyleSheet'; import type { ViewStyleProp } from 'react-native/Libraries/StyleSheet/StyleSheet';
import { FlatList } from 'react-native-gesture-handler'; import { FlatList } from 'react-native-gesture-handler';
import { useFetchThreadMedia } from 'lib/actions/thread-actions.js'; import { useFetchThreadMedia } from 'lib/actions/thread-actions.js';
import type { MediaInfo, Media } from 'lib/types/media-types'; import type { MediaInfo, Media } from 'lib/types/media-types';
import { import {
ThreadSettingsCategoryActionHeader, ThreadSettingsCategoryActionHeader,
ThreadSettingsCategoryFooter, ThreadSettingsCategoryFooter,
} from './thread-settings-category.react.js'; } from './thread-settings-category.react.js';
import GestureTouchableOpacity from '../../components/gesture-touchable-opacity.react.js'; import GestureTouchableOpacity from '../../components/gesture-touchable-opacity.react.js';
import type { ImageModalParams } from '../../media/image-modal.react.js';
import Multimedia from '../../media/multimedia.react.js'; import Multimedia from '../../media/multimedia.react.js';
import type { VideoPlaybackModalParams } from '../../media/video-playback-modal.react.js';
import { import {
ImageModalRouteName, ImageModalRouteName,
VideoPlaybackModalRouteName, VideoPlaybackModalRouteName,
} from '../../navigation/route-names.js'; } from '../../navigation/route-names.js';
import { useStyles } from '../../themes/colors.js'; import { useStyles } from '../../themes/colors.js';
import type { import type {
LayoutCoordinates, LayoutCoordinates,
VerticalBounds, VerticalBounds,
▲ Show 20 Lines • Show All 196 Lines • ▼ Show 20 Lines const navigateToMedia = React.useCallback(() => {
ref.current?.measure((x, y, width, height, pageX, pageY) => { ref.current?.measure((x, y, width, height, pageX, pageY) => {
const initialCoordinates: LayoutCoordinates = { const initialCoordinates: LayoutCoordinates = {
x: pageX, x: pageX,
y: pageY, y: pageY,
width, width,
height, height,
}; };
invariant(
verticalBounds,
'verticalBounds should be set in navigateToMedia',
);
const params: ImageModalParams | VideoPlaybackModalParams = {
presentedFrom: route.key,
mediaInfo,
initialCoordinates,
verticalBounds,
};
navigation.navigate<'VideoPlaybackModal' | 'ImageModal'>({ navigation.navigate<'VideoPlaybackModal' | 'ImageModal'>({
name: name:
mediaInfo.type === 'video' || mediaInfo.type === 'encrypted_video' mediaInfo.type === 'video' || mediaInfo.type === 'encrypted_video'
? VideoPlaybackModalRouteName ? VideoPlaybackModalRouteName
: ImageModalRouteName, : ImageModalRouteName,
key: `multimedia|${threadID}|${mediaInfo.id}`, key: `multimedia|${threadID}|${mediaInfo.id}`,
params: { params,
presentedFrom: route.key,
mediaInfo,
item,
initialCoordinates,
verticalBounds,
},
}); });
}); });
}, [navigation, route, threadID, mediaInfo, item, verticalBounds]); }, [navigation, route, threadID, mediaInfo, verticalBounds]);
const containerStyle = const containerStyle =
index % numColumns === 0 index % numColumns === 0
? memoizedStyles.mediaContainer ? memoizedStyles.mediaContainer
: memoizedStyles.mediaContainerWithMargin; : memoizedStyles.mediaContainerWithMargin;
return ( return (
<View key={item.id} style={containerStyle} onLayout={onLayout} ref={ref}> <View key={item.id} style={containerStyle} onLayout={onLayout} ref={ref}>
Show All 25 Lines
View Options

services/commtest/Dockerfile

// @flow // @flow
import { useNavigation, useRoute } from '@react-navigation/native'; import { useNavigation, useRoute } from '@react-navigation/native';
import invariant from 'invariant';
import * as React from 'react'; import * as React from 'react';
import { View, useWindowDimensions } from 'react-native'; import { View, useWindowDimensions } from 'react-native';
import type { ViewStyleProp } from 'react-native/Libraries/StyleSheet/StyleSheet'; import type { ViewStyleProp } from 'react-native/Libraries/StyleSheet/StyleSheet';
import { FlatList } from 'react-native-gesture-handler'; import { FlatList } from 'react-native-gesture-handler';
import { useFetchThreadMedia } from 'lib/actions/thread-actions.js'; import { useFetchThreadMedia } from 'lib/actions/thread-actions.js';
import type { MediaInfo, Media } from 'lib/types/media-types'; import type { MediaInfo, Media } from 'lib/types/media-types';
import { import {
ThreadSettingsCategoryActionHeader, ThreadSettingsCategoryActionHeader,
ThreadSettingsCategoryFooter, ThreadSettingsCategoryFooter,
} from './thread-settings-category.react.js'; } from './thread-settings-category.react.js';
import GestureTouchableOpacity from '../../components/gesture-touchable-opacity.react.js'; import GestureTouchableOpacity from '../../components/gesture-touchable-opacity.react.js';
import type { ImageModalParams } from '../../media/image-modal.react.js';
import Multimedia from '../../media/multimedia.react.js'; import Multimedia from '../../media/multimedia.react.js';
import type { VideoPlaybackModalParams } from '../../media/video-playback-modal.react.js';
import { import {
ImageModalRouteName, ImageModalRouteName,
VideoPlaybackModalRouteName, VideoPlaybackModalRouteName,
} from '../../navigation/route-names.js'; } from '../../navigation/route-names.js';
import { useStyles } from '../../themes/colors.js'; import { useStyles } from '../../themes/colors.js';
import type { import type {
LayoutCoordinates, LayoutCoordinates,
VerticalBounds, VerticalBounds,
▲ Show 20 Lines • Show All 196 Lines • ▼ Show 20 Lines const navigateToMedia = React.useCallback(() => {
ref.current?.measure((x, y, width, height, pageX, pageY) => { ref.current?.measure((x, y, width, height, pageX, pageY) => {
const initialCoordinates: LayoutCoordinates = { const initialCoordinates: LayoutCoordinates = {
x: pageX, x: pageX,
y: pageY, y: pageY,
width, width,
height, height,
}; };
invariant(
verticalBounds,
'verticalBounds should be set in navigateToMedia',
);
const params: ImageModalParams | VideoPlaybackModalParams = {
presentedFrom: route.key,
mediaInfo,
initialCoordinates,
verticalBounds,
};
navigation.navigate<'VideoPlaybackModal' | 'ImageModal'>({ navigation.navigate<'VideoPlaybackModal' | 'ImageModal'>({
name: name:
mediaInfo.type === 'video' || mediaInfo.type === 'encrypted_video' mediaInfo.type === 'video' || mediaInfo.type === 'encrypted_video'
? VideoPlaybackModalRouteName ? VideoPlaybackModalRouteName
: ImageModalRouteName, : ImageModalRouteName,
key: `multimedia|${threadID}|${mediaInfo.id}`, key: `multimedia|${threadID}|${mediaInfo.id}`,
params: { params,
presentedFrom: route.key,
mediaInfo,
item,
initialCoordinates,
verticalBounds,
},
}); });
}); });
}, [navigation, route, threadID, mediaInfo, item, verticalBounds]); }, [navigation, route, threadID, mediaInfo, verticalBounds]);
const containerStyle = const containerStyle =
index % numColumns === 0 index % numColumns === 0
? memoizedStyles.mediaContainer ? memoizedStyles.mediaContainer
: memoizedStyles.mediaContainerWithMargin; : memoizedStyles.mediaContainerWithMargin;
return ( return (
<View key={item.id} style={containerStyle} onLayout={onLayout} ref={ref}> <View key={item.id} style={containerStyle} onLayout={onLayout} ref={ref}>
Show All 25 Lines