This is an archive of the discontinued LLVM Phabricator instance.

[zorg] [sanitizer] Update i686->i386 to match rename in compiler-rt
ClosedPublic

Authored by mgorny on Aug 28 2017, 11:52 AM.

Download Raw Diff

Details

Reviewers

eugenis
vitalybuka

Commits

rZORGa4d3ba59f684: [sanitizer] Update i686->i386 to match rename in compiler-rt
rL311925: [sanitizer] Update i686->i386 to match rename in compiler-rt

Summary

Update arch name used by the Android buildbot to reference "i386" rather
than "i686" since this is the only x86 library name post D26796 + D26764.
This is necessary so that the scripts push the correct library file.

Diff Detail

Event Timeline

mgorny created this revision.Aug 28 2017, 11:52 AM

Herald added subscribers: dberris, srhines. · View Herald TranscriptAug 28 2017, 11:52 AM

mgorny added a parent revision: D26764: [compiler-rt] [cmake] Remove i686 target that is duplicate to i386.Aug 28 2017, 11:52 AM

mgorny mentioned this in D26796: [Driver] Use arch type to find compiler-rt libraries (on Linux).

eugenis accepted this revision.Aug 28 2017, 12:49 PM

eugenis added inline comments.

zorg/buildbot/builders/sanitizers/buildbot_android.sh
80	Btw could you move this comment to line 72?

This revision is now accepted and ready to land.Aug 28 2017, 12:49 PM

eugenis added inline comments.Aug 28 2017, 12:58 PM

zorg/buildbot/builders/sanitizers/buildbot_android.sh
68	In fact, this might be wrong. Ndk arch should be i686, unlike the standalone toolchain path.

mgorny added inline comments.Aug 28 2017, 1:00 PM

zorg/buildbot/builders/sanitizers/buildbot_android.sh
68	FWICS ndk arch is actually 'x86' (the second argument). The first seems to be used only to construct the build directory path.
80	Sure, will do.

eugenis added inline comments.Aug 28 2017, 1:03 PM

zorg/buildbot/builders/sanitizers/buildbot_android.sh
68	Right. LGTM

Closed by commit rL311925: [sanitizer] Update i686->i386 to match rename in compiler-rt (authored by mgorny). · Explain WhyAug 28 2017, 1:32 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents

Path

Size

zorg/

buildbot/

builders/

sanitizers/

buildbot_android.sh

8 lines

Diff 112929

zorg/buildbot/builders/sanitizers/buildbot_android.sh

	Show First 20 Lines • Show All 59 Lines • ▼ Show 20 Lines
	fi			fi
	ninja -C llvm_build64 \|\| echo @@@STEP_FAILURE@@			ninja -C llvm_build64 \|\| echo @@@STEP_FAILURE@@

	# Android NDK has no iconv.h which is requred by LIBXML2.			# Android NDK has no iconv.h which is requred by LIBXML2.
	CMAKE_COMMON_OPTIONS="${CMAKE_COMMON_OPTIONS} -DLLVM_LIBXML2_ENABLED=OFF"			CMAKE_COMMON_OPTIONS="${CMAKE_COMMON_OPTIONS} -DLLVM_LIBXML2_ENABLED=OFF"

	build_android_ndk aarch64 arm64			build_android_ndk aarch64 arm64
	build_android_ndk arm arm			build_android_ndk arm arm
	build_android_ndk i686 x86			build_android_ndk i386 x86
				eugenisUnsubmitted Not Done Reply Inline Actions In fact, this might be wrong. Ndk arch should be i686, unlike the standalone toolchain path. eugenis: In fact, this might be wrong. Ndk arch should be i686, unlike the standalone toolchain path.
				mgornyAuthorUnsubmitted Not Done Reply Inline Actions FWICS ndk arch is actually 'x86' (the second argument). The first seems to be used only to construct the build directory path. mgorny: FWICS ndk arch is actually 'x86' (the second argument). The first seems to be used only to…
				eugenisUnsubmitted Not Done Reply Inline Actions Right. LGTM eugenis: Right. LGTM

	echo @@@BUILD_STEP run cmake@@@			echo @@@BUILD_STEP run cmake@@@
	configure_android aarch64 aarch64-linux-android			configure_android aarch64 aarch64-linux-android
	configure_android arm armv7-linux-androideabi			configure_android arm armv7-linux-androideabi
	configure_android i686 i686-linux-android			configure_android i386 i686-linux-android

	build_android aarch64			build_android aarch64
	build_android arm			build_android arm
	build_android i686			build_android i386

	# Testing armv7 instead of plain arm to work around			# Testing armv7 instead of plain arm to work around
	# https://code.google.com/p/android/issues/detail?id=68779			# https://code.google.com/p/android/issues/detail?id=68779
				eugenisUnsubmitted Not Done Reply Inline Actions Btw could you move this comment to line 72? eugenis: Btw could you move this comment to line 72?
				mgornyAuthorUnsubmitted Not Done Reply Inline Actions Sure, will do. mgorny: Sure, will do.
	test_android i686:x86 aarch64:arm64-v8a arm:armeabi-v7a			test_android i386:x86 aarch64:arm64-v8a arm:armeabi-v7a