[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Merging feature/android
From: |
Po Lu |
Subject: |
Re: Merging feature/android |
Date: |
Thu, 02 Mar 2023 18:19:50 +0800 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
Eli Zaretskii <eliz@gnu.org> writes:
> How long was this branch kept in its finished state, and how many
> people tried it? Does it build on the main supported systems?
It builds on GNU/Linux and Haiku, I've not tested this on other systems
yet. I plan to fix the MS-DOS build a while after it is merged.
> 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.
Thanks, then I guess I'll ask in April. It's been finished for about a
week now.
> 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.
Sure.
> . 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.
What would be a good place to put these patch files? admin/notes
perhaps?
> 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.
Okay. Paul, the Android port really needs the `printf-posix' and
`vasprintf-posix' modules (as Android's printf ranges from ``completely
broken'' to ``just missing %td'' depending on the OS version being
used), but stpncpy and getline are only ``nice-to-have''s. Is there any
downside to depending on those additional gnulib modules? And will they
build on MS Windows as well?
> -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"?
Because the modules code has a dependency on the GCC cleanup function
extension, and fails to compile when it is not present. I rewrote the
configury to detect the presence of the extension and not build with
modules when that is in effect.
> 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.
OK, thanks for catching this.
> 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 <pwd.h>])
>
> This could fail the MS-Windows build -- did you check that it doesn't
> have any adverse effect on that?
It should not, since the result of the check is only used on Unix
systems.
> CM_OBJ setting in configure.ac:
>
> -CM_OBJ="cm.o"
>
> Why was this deleted?
It wasn't, I simply moved it further up.
> + Does Emacs use Android? ${ANDROID}
>
> This should say "Is Emacs being built for Android?" instead.
OK, thanks.
> 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?
I tried multiple times, but the gnulib stuff kept trying to include
generated headers from the wrong copy of gnulib, so in the end I
couldn't find any way around having to keep two copies of gnulib
in-tree.
In the past cross/lib was also used to hold patches for Android, but the
gnulib folks have now fixed all of the problems which required patches.
> 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.
I will fix these too, thanks.
- Merging feature/android, Po Lu, 2023/03/01
- Re: Merging feature/android, Eli Zaretskii, 2023/03/02
- Re: Merging feature/android,
Po Lu <=
- Re: Merging feature/android, Eli Zaretskii, 2023/03/02
- Re: Merging feature/android, Po Lu, 2023/03/02
- Re: Merging feature/android, Eli Zaretskii, 2023/03/02
- Re: Merging feature/android, Po Lu, 2023/03/02
- Re: Merging feature/android, Eli Zaretskii, 2023/03/03
- Re: Merging feature/android, Po Lu, 2023/03/03
- Re: Merging feature/android, Eli Zaretskii, 2023/03/03
- Re: Merging feature/android, Po Lu, 2023/03/03
- Re: Merging feature/android, Eli Zaretskii, 2023/03/03
- Re: Merging feature/android, Po Lu, 2023/03/03