Paths

Table of Contentst

-
native/android/app/
-
android/
-
app/
1/1
CMakeLists.txt

[native] fix path to native rust library directory
ClosedPublic
Actions

Authored by varun on Oct 25 2022, 12:10 PM.

Details

Reviewers

atul
• jon
• max

Commits

rCOMM8687152bd65c: [native] fix path to native rust library directory

Summary

We were incorrectly including a directory containing build artifacts from a previous Xcode build. This happened to work by coincidence, but is not the right directory. Now we include the directory created by corrosion containing the correct native_rust_library source files.

Test Plan

cd native/android && ./gradlew bundleDebug && ./gradlew bundleRelease

Diff Detail

Repository

rCOMM Comm

Branch

master

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

varun created this revision.Oct 25 2022, 12:10 PM

Herald added subscribers: • abosh, tomek, ashoat. · View Herald TranscriptOct 25 2022, 12:10 PM

I cherry-picked this change to D5369 and it worked locally

Added @jon as a reviewer because I'm not sure if there's a more idiomatic way to do this

Harbormaster completed remote builds in B13037: Diff 17906.Oct 25 2022, 12:23 PM

varun requested review of this revision.Oct 25 2022, 12:23 PM

Pre-empting any future CI issues because of case-sensitivity on certain platforms

Harbormaster completed remote builds in B13040: Diff 17909.Oct 25 2022, 12:56 PM

• max accepted this revision.Oct 25 2022, 4:46 PM

• max added inline comments.

native/android/app/CMakeLists.txt
134 ↗	(On Diff #17909)	Looks monstrous, but seems we can't get rid of this.

atul accepted this revision.Oct 26 2022, 10:29 AM

atul added inline comments.

native/android/app/CMakeLists.txt
134 ↗	(On Diff #17909)	+1

This revision is now accepted and ready to land.Oct 26 2022, 10:29 AM

I want to make sure @jon takes a look since he's the in-house CMake expert

This revision now requires review to proceed.Oct 26 2022, 11:14 AM

• jon accepted this revision.Oct 26 2022, 12:02 PM

• jon added inline comments.

native/android/app/CMakeLists.txt
134 ↗	(On Diff #17909)	If this path is generated at build time, using `${CMAKE_CURRENT_BINARY_DIR}/.cxx` is probably the best practice. This should only make a difference if we do `add_subdirectory(native/android/app)`. But it's just avoid that potential pain.

This revision is now accepted and ready to land.Oct 26 2022, 12:02 PM

Created linear task to follow up on Jon's comment

native/android/app/CMakeLists.txt
134 ↗	(On Diff #17909)	https://linear.app/comm/issue/ENG-2121/change-relative-directory-paths-in-android-cmakeliststxt

Closed by commit rCOMM8687152bd65c: [native] fix path to native rust library directory (authored by varun). · Explain WhyOct 26 2022, 12:22 PM

This revision was automatically updated to reflect the committed changes.

varun added a commit: rCOMM8687152bd65c: [native] fix path to native rust library directory.

ashoat mentioned this in D5604: [native] Avoid hardcoding path to rust_native_library lib.rs.h file.Nov 10 2022, 12:41 PM

ashoat mentioned this in rCOMMf7a17e3c25fa: [native] Avoid hardcoding path to rust_native_library lib.rs.h file.Nov 10 2022, 1:38 PM

Revision Contents
Changeset List

Path

Size

native/

android/

app/

CMakeLists.txt

2 lines

Diff 17906

View Options

native/android/app/CMakeLists.txt

	Show First 20 Lines • Show All 130 Lines • ▼ Show 20 Lines
	const styles = StyleSheet.create({			const styles = StyleSheet.create({
	dummyMessage: {			dummyMessage: {
	paddingHorizontal: 12,			paddingHorizontal: 12,
	paddingVertical: 6,			paddingVertical: 6,
	},			},
	message: {			message: {
	overflow: 'hidden',			overflow: 'hidden',
	paddingHorizontal: 12,			paddingHorizontal: 12,
	paddingVertical: 6,			paddingVertical: 8,
	},			},
	text: {			text: {
	fontFamily: 'Arial',			fontFamily: 'Arial',
	fontSize: 18,			fontSize: 14,
				benschacAuthorUnsubmitted Done Inline Actions I actually think this is a bit too small. But that's what's in the design. https://www.figma.com/file/L675ETKDnGaSwlpZAw4MIC/Mobile-App?node-id=351%3A455 @ashoat - I think 16px looks better. benschac: I actually think this is a bit too small. But that's what's in the design. https://www.figma.
				lineHeight: 22,
	},			},
	});			});

	export { InnerTextMessage, dummyNodeForTextMessageHeightMeasurement };			export { InnerTextMessage, dummyNodeForTextMessageHeightMeasurement };

[native] fix path to native rust library directoryClosedPublicActions