Issue 297890043: Added build steps for Android clients
DescriptionCommit log for first patchset:
---------------
Added build steps for Android clients
---------------
Patch Set 1 #
Total comments: 4
MessagesTotal messages: 7
Thanks for the contribution. Please xref https://github.com/DynamoRIO/dynamorio/wiki/Contributing about contributing. Might I know if have signed the Contributor License Agreement? https://codereview.appspot.com/297890043/diff/1/api/docs/samples.dox File api/docs/samples.dox (right): https://codereview.appspot.com/297890043/diff/1/api/docs/samples.dox#newcode360 api/docs/samples.dox:360: cmake -DCMAKE_TOOLCHAIN_FILE=$DYNAMORIO_HOME/make/toolchain-android.cmake -DANDROID_TOOLCHAIN=/mytooldir/android-ndk-21 $DYNAMORIO_HOME/api/samples/ it only has the config step, no build step, xref example above.
Sign in to reply to this message.
On 2016/04/21 16:11:13, zhaoqin wrote: > Thanks for the contribution. > Please xref https://github.com/DynamoRIO/dynamorio/wiki/Contributing about > contributing. > Might I know if have signed the Contributor License Agreement? > > https://codereview.appspot.com/297890043/diff/1/api/docs/samples.dox > File api/docs/samples.dox (right): > > https://codereview.appspot.com/297890043/diff/1/api/docs/samples.dox#newcode360 > api/docs/samples.dox:360: cmake > -DCMAKE_TOOLCHAIN_FILE=$DYNAMORIO_HOME/make/toolchain-android.cmake > -DANDROID_TOOLCHAIN=/mytooldir/android-ndk-21 $DYNAMORIO_HOME/api/samples/ > it only has the config step, no build step, xref example above. Yes, I have signed the Contributor License Agreement.
Sign in to reply to this message.
On 2016/04/21 16:13:17, carolinetrippel wrote: > On 2016/04/21 16:11:13, zhaoqin wrote: > > Thanks for the contribution. > > Please xref https://github.com/DynamoRIO/dynamorio/wiki/Contributing about > > contributing. > > Might I know if have signed the Contributor License Agreement? > > > > https://codereview.appspot.com/297890043/diff/1/api/docs/samples.dox > > File api/docs/samples.dox (right): > > > > > https://codereview.appspot.com/297890043/diff/1/api/docs/samples.dox#newcode360 > > api/docs/samples.dox:360: cmake > > -DCMAKE_TOOLCHAIN_FILE=$DYNAMORIO_HOME/make/toolchain-android.cmake > > -DANDROID_TOOLCHAIN=/mytooldir/android-ndk-21 $DYNAMORIO_HOME/api/samples/ > > it only has the config step, no build step, xref example above. > > Yes, I have signed the Contributor License Agreement. Ah, I see, I missed the build step. Shall I add that?
Sign in to reply to this message.
On 2016/04/21 16:14:22, carolinetrippel wrote: > On 2016/04/21 16:13:17, carolinetrippel wrote: > > On 2016/04/21 16:11:13, zhaoqin wrote: > > > Thanks for the contribution. > > > Please xref https://github.com/DynamoRIO/dynamorio/wiki/Contributing about > > > contributing. > > > Might I know if have signed the Contributor License Agreement? > > > > > > https://codereview.appspot.com/297890043/diff/1/api/docs/samples.dox > > > File api/docs/samples.dox (right): > > > > > > > > > https://codereview.appspot.com/297890043/diff/1/api/docs/samples.dox#newcode360 > > > api/docs/samples.dox:360: cmake > > > -DCMAKE_TOOLCHAIN_FILE=$DYNAMORIO_HOME/make/toolchain-android.cmake > > > -DANDROID_TOOLCHAIN=/mytooldir/android-ndk-21 $DYNAMORIO_HOME/api/samples/ > > > it only has the config step, no build step, xref example above. > > > > Yes, I have signed the Contributor License Agreement. > > Ah, I see, I missed the build step. Shall I add that? IMHO, it is better to add to have a complete example. Also, for such reply, it might be better to reply in the code review comment (click reply on the comment). By doing that, it is easier to have a conversation esp if multiple round review.
Sign in to reply to this message.
> Commit log for first patchset: > --------------- > Added build steps for Android clients Please augment the commit message to clarify that this is adding this information to the *documentation*, rather than modifying DR's own build system. https://codereview.appspot.com/297890043/diff/1/api/docs/samples.dox File api/docs/samples.dox (right): https://codereview.appspot.com/297890043/diff/1/api/docs/samples.dox#newcode360 api/docs/samples.dox:360: cmake -DCMAKE_TOOLCHAIN_FILE=$DYNAMORIO_HOME/make/toolchain-android.cmake -DANDROID_TOOLCHAIN=/mytooldir/android-ndk-21 $DYNAMORIO_HOME/api/samples/ As you mentioned in the email, toolchain-android.cmake is not in the release package. DYNAMORIO_HOME is "set to the base of the DynamoRIO release package". The best solution would be to add a rule to export toolchain-android.cmake into the cmake directory.
Sign in to reply to this message.
https://codereview.appspot.com/297890043/diff/1/api/docs/samples.dox File api/docs/samples.dox (right): https://codereview.appspot.com/297890043/diff/1/api/docs/samples.dox#newcode360 api/docs/samples.dox:360: cmake -DCMAKE_TOOLCHAIN_FILE=$DYNAMORIO_HOME/make/toolchain-android.cmake -DANDROID_TOOLCHAIN=/mytooldir/android-ndk-21 $DYNAMORIO_HOME/api/samples/ On 2016/04/21 16:11:13, zhaoqin wrote: > it only has the config step, no build step, xref example above. Fixed the build step. Will add the toolchain-android.make file. https://codereview.appspot.com/297890043/diff/1/api/docs/samples.dox#newcode360 api/docs/samples.dox:360: cmake -DCMAKE_TOOLCHAIN_FILE=$DYNAMORIO_HOME/make/toolchain-android.cmake -DANDROID_TOOLCHAIN=/mytooldir/android-ndk-21 $DYNAMORIO_HOME/api/samples/ On 2016/04/21 16:26:01, bruening wrote: > As you mentioned in the email, toolchain-android.cmake is not in the release > package. DYNAMORIO_HOME is "set to the base of the DynamoRIO release package". > The best solution would be to add a rule to export toolchain-android.cmake into > the cmake directory. Acknowledged.
Sign in to reply to this message.
|
|||||||||||||||||||||||||||||||||||||||||||
