bug#10155: OSX breakage

[Stefan Monnier]

> --- .bzrignore 2011-09-29 14:19:11 +0000
> +++ .bzrignore 2011-10-17 01:22:19 +0000
> @@ -53,6 +53,7 @@
> lib/c++defs.h
> lib/getopt.h
> lib/inttypes.h
> +lib/stdalign.h
> lib/stdbool.h
> lib/stdio.h
> lib/stdint.h

Shouldn't these be covered by a glob lib/*.h instead?

> + Use Gnulib stdalign module (Bug#9772, Bug#9960).

As already explained, we don't want this change now. Syncing gnulib
shouldn't impose such a change.


Stefan

[Stefan Monnier]

[Stefan Monnier]

[Eli Zaretskii]

> Date: Tue, 29 Nov 2011 01:28:30 -0800
> From: Paul Eggert <egg...@cs.ucla.edu>
> CC: Eli Zaretskii <el...@gnu.org>, ma...@lassnig.net,
> 97...@debbugs.gnu.org, 99...@debbugs.gnu.org, 10...@debbugs.gnu.org
>
> On 11/28/11 21:00, Chong Yidong wrote:
> > As Eli pointed out in the discussion on bug#10155, there is already code
> > in lib/unistd.in.h that apparently ought to handle this, but itsn't
> > working.
>
> Here's a proposed patch to fix that, by having Emacs use Gnulib's
> environ module. This syncs from Gnulib, and so it also brings
> in the patches for Bug#9772 and Bug#9960. (It is possible to
> pull out just the environ fix by hand, but that's an error-prone process
> and I'd rather avoid it.) I have tested this on Fedora 15,
> but not on OSX nor on Windows.

FWIW, I'd very much prefer not to install such large changes a day or
two before the next pretest. If the only practical way of fixing this
within gnulib is the proposed changeset, I'd say let's just add a
declaration of environ conditioned on OS X and FreeBSD. I don't like
this solution, but I think on balance it has less potential for
destabilizing the build.

[Eli Zaretskii]

[Eli Zaretskii]

[Eli Zaretskii]

> Date: Tue, 29 Nov 2011 08:58:34 -0800
> From: Paul Eggert <egg...@cs.ucla.edu>
> CC: Chong Yidong <c...@gnu.org>, ma...@lassnig.net, 97...@debbugs.gnu.org,
> 99...@debbugs.gnu.org, 10...@debbugs.gnu.org,
> Eli Zaretskii <el...@gnu.org>
>
> You're right, on further thought Emacs itself needn't use stdalign,
> and I've prepared a simpler patch (below) that omits all changes to
> src/*. Gnulib now uses stdalign, though, and Emacs uses
> multiple Gnulib modules such as sha256 that use stdalign, so
> this patch still brings in the stdalign module for Gnulib's own
> purposes.

I understand that the only part needed for solving the problems with
the missing declaration of `environ' is this:

> === added file 'm4/environ.m4'
> --- m4/environ.m4 1970-01-01 00:00:00 +0000
> +++ m4/environ.m4 2011-11-29 16:38:21 +0000
> @@ -0,0 +1,47 @@
> +# environ.m4 serial 6
> +dnl Copyright (C) 2001-2004, 2006-2011 Free Software Foundation, Inc.
> +dnl This file is free software; the Free Software Foundation
> +dnl gives unlimited permission to copy and/or distribute it,
> +dnl with or without modifications, as long as this notice is preserved.
> +
> +AC_DEFUN_ONCE([gl_ENVIRON],
> +[
> + AC_REQUIRE([gl_UNISTD_H_DEFAULTS])
> + dnl Persuade glibc <unistd.h> to declare environ.
> + AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS])
> +
> + AC_CHECK_HEADERS_ONCE([unistd.h])
> + gt_CHECK_VAR_DECL(
> + [#if HAVE_UNISTD_H
> + #include <unistd.h>
> + #endif
> + /* mingw, BeOS, Haiku declare environ in <stdlib.h>, not in <unistd.h>. */
> + #include <stdlib.h>
> + ],
> + [environ])
> + if test $gt_cv_var_environ_declaration != yes; then
> + HAVE_DECL_ENVIRON=0
> + fi
> +])

IOW, the reason why lib/unistd.in.h didn't do its job is that
HAVE_DECL_ENVIRON was not defined to zero.

If that is true, I wonder if we could have a much smaller change that
fixes just that single problem.

[Eli Zaretskii]

[Eli Zaretskii]

[Paul Eggert]

On 11/29/11 09:30, Eli Zaretskii wrote:
> I wonder if we could have a much smaller change that
> fixes just that single problem.

Yes we could, though it would involve more of the patch
than the part that you've identified, and it would involve
some other stuff done by hand. The simplest way to think
about it is, I expect, is that it would be a fork of gnulib.
I could look into preparing such a patch but it would take
some time and would introduce other reliability concerns,
so I hope I don't have to do that....

[Paul Eggert]

[Paul Eggert]

[Chong Yidong]

Eli Zaretskii <el...@gnu.org> writes:

>> Here's a proposed patch to fix that, by having Emacs use Gnulib's
>> environ module. This syncs from Gnulib, and so it also brings
>> in the patches for Bug#9772 and Bug#9960. (It is possible to
>> pull out just the environ fix by hand, but that's an error-prone process
>> and I'd rather avoid it.) I have tested this on Fedora 15,
>> but not on OSX nor on Windows.
>
> FWIW, I'd very much prefer not to install such large changes a day or
> two before the next pretest. If the only practical way of fixing this
> within gnulib is the proposed changeset, I'd say let's just add a
> declaration of environ conditioned on OS X and FreeBSD. I don't like
> this solution, but I think on balance it has less potential for
> destabilizing the build.

Agreed. Paul, can you pick out just the environ fix? If not, we should
go with the ifdefs as Eli suggested, and leave any Gnulib synch for
post-24.1.

[Chong Yidong]

[Chong Yidong]

[Chong Yidong]

OK, then let's go with the ifdef conditioning directly in the Emacs
sources.

But is conditioning on OS X and FreeBSD the right thing? We don't know
if it fails on other BSDs. Since revno 106533 was intended to fix the
MS-WINDOWS build, I think it's better to condition it for WINDOWSNT for
now, as below:

Eli, WDYT?

=== modified file 'lib-src/emacsclient.c'
*** lib-src/emacsclient.c 2011-11-27 18:52:53 +0000
--- lib-src/emacsclient.c 2011-11-30 02:44:47 +0000
***************
*** 1635,1640 ****
--- 1635,1645 ----
/* Send over our environment and current directory. */
if (!current_frame)
{
+ #ifndef WINDOWSNT
+ /* This is defined in stdlib.h on MS-Windows. It's defined in
+ unistd.h on some POSIX hosts, but not all (Bug#10155). */
+ extern char **environ;
+ #endif
int i;
for (i = 0; environ[i]; i++)
{

[Chong Yidong]

[Chong Yidong]

[Eli Zaretskii]

> From: Chong Yidong <c...@gnu.org>
> Cc: Eli Zaretskii <el...@gnu.org>, ma...@lassnig.net, 99...@debbugs.gnu.org, 97...@debbugs.gnu.org, 10...@debbugs.gnu.org
> Date: Wed, 30 Nov 2011 10:45:42 +0800

>
> OK, then let's go with the ifdef conditioning directly in the Emacs
> sources.
>
> But is conditioning on OS X and FreeBSD the right thing? We don't know
> if it fails on other BSDs. Since revno 106533 was intended to fix the
> MS-WINDOWS build, I think it's better to condition it for WINDOWSNT for
> now, as below:
>
> Eli, WDYT?

Fine with me.

[Eli Zaretskii]

[Eli Zaretskii]

[Chong Yidong]

Eli Zaretskii <el...@gnu.org> writes:

>> OK, then let's go with the ifdef conditioning directly in the Emacs
>> sources.
>>
>> But is conditioning on OS X and FreeBSD the right thing? We don't know
>> if it fails on other BSDs. Since revno 106533 was intended to fix the
>> MS-WINDOWS build, I think it's better to condition it for WINDOWSNT for
>> now, as below:
>>
>> Eli, WDYT?
>
> Fine with me.

Done. Could someone check if compilation on Mac OS X works now?

[Chong Yidong]

[Chong Yidong]

[Jan Djärv]

It does.

Jan D.

[Jan Djärv]

[Jan Djärv]

[Paul Eggert]

On 11/30/11 03:15, Mario Lassnig wrote:
> Looks good to me. Compiles, installs, runs.

Thanks for checking. I'll mark Bug#10155 as done since the bug
itself is fixed. The cleaner fix for 'environ' can wait until
after Emacs 24 is out.

[Paul Eggert]

[Paul Eggert]