From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: Merging feature/android Date: Thu, 02 Mar 2023 11:23:04 +0200 Message-ID: <83pm9reccn.fsf@gnu.org> References: <87edq7ztks.fsf.ref@yahoo.com> <87edq7ztks.fsf@yahoo.com> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="31929"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org To: Po Lu Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Thu Mar 02 10:23:33 2023 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1pXf9u-0007zZ-Tt for ged-emacs-devel@m.gmane-mx.org; Thu, 02 Mar 2023 10:23:31 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pXf9I-0005Vq-2n; Thu, 02 Mar 2023 04:22:52 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pXf9G-0005Vd-8s for emacs-devel@gnu.org; Thu, 02 Mar 2023 04:22:50 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pXf9G-0002fm-0a; Thu, 02 Mar 2023 04:22:50 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=tV7ErcPF8myK1Q7d40N2c7PSH+acDMYOMgbhUctQN6E=; b=bQ7/RlaHW6Mk tM176cHlGBj5AtGQBKAOYSA4avSipaUQ7ynAtbmQXLQpdgvNmre14WTSZcqjSa5nrBNUYW6AOgj4f /UmLDgBoiwcC2JEOFGHxHJzdGXvEZuF8E/o8G/N3/yImISle050zVsS88JUaXFb2MuzMzKQE6ekQ3 6YOtkb9BJivrVwJph8AU9ftRss6OmQVYTvN4THKY1voOgAmdLEDFmOojLJiOig7m2rRWu7S+r9CK8 0DGAwfMuUFraTXcZ2LZRyIvJTvt/HGtHbMl/SEY5udZcYctMtlfamdEQxeoUECal6oVFRM874AJKG kUT+ktr5P9G1eKnENv0/YQ==; Original-Received: from [87.69.77.57] (helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pXf9F-0006p9-GM; Thu, 02 Mar 2023 04:22:49 -0500 In-Reply-To: <87edq7ztks.fsf@yahoo.com> (message from Po Lu on Thu, 02 Mar 2023 12:05:23 +0800) X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:303890 Archived-At: > From: Po Lu > Date: Thu, 02 Mar 2023 12:05:23 +0800 > > Please take a look at the feature/android branch. > > I would like to merge it into master in the next couple of days, but > before I do so I want everyone else to be happy with its contents as > well. So would everyone please do the usual with the diff between the > branch and master? How long was this branch kept in its finished state, and how many people tried it? Does it build on the main supported systems? If the branch in its current state is relatively "young", I'd prefer to delay merging it for a month or so, to give more people chance to build and try it. There's no rush in landing it. Some specific comments below. The INSTALL.android file: . It should be in a subdirectory, like we do with nt/INSTALL (and NEWS should be updated to that effect). . The NDK BUILD SYSTEM IMPLEMENTATION section doesn't belong in INSTALL, IMO. It should be a separate file, since it's mainly of interest to Emacs developers. . The PATCH FOR LIBXML2 part and similar parts for patching other libraries and components should also be in separate files, suitable for submitting to Patch or similar utilities, and INSTALL should only mention the need for these patch and refer to those files. . The copying conditions should be really at the end, not in the middle. admin/merge-gnulib: I don't think we should change this without a review from Paul Eggert. The additional modules you add may need to be disabled in nt/gnulib-cfg.mk if for some reason they are compiled without being needed. -OPTION_DEFAULT_ON([modules],[don't compile with dynamic modules support]) +OPTION_DEFAULT_IFAVAILABLE([modules],[don't compile with dynamic modules support]) Why was this changed to "ifavailable"? The changes in configure.ac that disable various warning options: + nw="$nw -Wunknown-warning-option" # Let #pragma GCC ignore work properly + # even when the compiler in use doesn't + # support the option. + # If Emacs is being built for Android and many functions are + # currently stubbed out for operation on the build machine, disable + # -Wsuggest-attribute=noreturn. + + nw="$nw -Wsuggest-attribute=noreturn" + gl_WARN_ADD([-Wno-shift-overflow]) If these are specific to Android, they should have suitable conditions for when to apply them. The gecos test in configure.ac: +# Check for pw_gecos in struct passwd; this is known to be missing on +# Android. + +AC_CHECK_MEMBERS([struct passwd.pw_gecos], [], [], [#include ]) This could fail the MS-Windows build -- did you check that it doesn't have any adverse effect on that? CM_OBJ setting in configure.ac: -CM_OBJ="cm.o" Why was this deleted? + Does Emacs use Android? ${ANDROID} This should say "Is Emacs being built for Android?" instead. The files in cross/lib/ seem to be from Gnulib? If so, do we really need another copy of them in the tree? any way to reuse the sources in lib/ instead? General remark about *.texi files: it looks like you used TABs there; you should only use spaces for alignment in Texinfo. The code in from_unicode_buffer that is used only on Android should be under an appropriate #ifdef. Likewise with other such code, if any. Thanks.