Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PATCH] compile error after re-running Configure since AmigaOS merge #14923

Closed
p5pRT opened this issue Sep 24, 2015 · 24 comments
Closed

[PATCH] compile error after re-running Configure since AmigaOS merge #14923

p5pRT opened this issue Sep 24, 2015 · 24 comments
Labels

Comments

@p5pRT
Copy link

p5pRT commented Sep 24, 2015

Migrated from rt.perl.org#126152 (status was 'resolved')

Searchable as RT126152$

@p5pRT
Copy link
Author

p5pRT commented Sep 24, 2015

From @dcollinsn

dcollins@​nightshade​:~/perl$ git clean -dxf > /dev/null
dcollins@​nightshade​:~/perl$ ./Configure -Dusedevel -des > /dev/null
dcollins@​nightshade​:~/perl$ grep procselfexe config.sh
d_procselfexe='define'
procselfexe='"/proc/self/exe"'
dcollins@​nightshade​:~/perl$ ./Configure -Dusedevel -des > /dev/null
dcollins@​nightshade​:~/perl$ grep procselfexe config.sh
d_procselfexe='define'
procselfexe=''
dcollins@​nightshade​:~/perl$ make
...SNIP...
cc -c -DPERL_CORE -fwrapv -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2 -std=c89 -O2 -Wall -Werror=declaration-after-statement -Wextra -Wc++-compat -Wwrite-strings caretx.c
caretx.c​: In function ‘Perl_set_caret_X’​:
caretx.c​:102​:48​: error​: expected expression before ‘,’ token
  SSize_t len = readlink(PROCSELFEXE_PATH, buf, sizeof(buf) - 1);
  ^
make​: *** [caretx.o] Error 1

Bisect points at one of the amigaos changes to allow ./Configure to use hints of d_procselfexe, without actually allowing it to use hints of procselfexe. As a result, if there are hints of (d_)?procselfexe, the above behavior is observed.

It is trivial to move the line blanking procselfexe to inside the case statement, as in the attached trivial.patch. However, we never allowed d_procselfexe to hint before, we always checked since it is trivial to do so. Therefore, I propose the attached procselfexe.patch, which instead only accepts a hint of 'undef', and in any other case, exhibits the usual behavior of finding and checking the platform-specific exe for symbolic-link-ness.

%% GIT BISECT %%

(bisected on the return value of ./Configure -Dusedevel -des && ./Configure -Dusedevel -des; grep '/proc/self/exe' config.sh)

68b32a2 is the first bad commit
commit 68b32a2
Author​: Andy Broad <andy@​broad.ology.org.uk>
Date​: Thu Aug 13 19​:09​:25 2015 -0400

  amigaos4​: Configure​: allow hinting d_procselfexe
 
  Trying to access /proc causes the system to try to mount such a thing.
  (And there is no process file system in AmigaOS, anyway.)

:100755 100755 6eb478d7214816ced51aaee60438676996bcd888 d0ebb2eb35f5f017a6d61056c08585035d4778e5 M Configure
bisect run success

@p5pRT
Copy link
Author

p5pRT commented Sep 24, 2015

From @dcollinsn

procselfexe.patch
diff --git a/Configure b/Configure
index e12c8bb..5fe2fe1 100755
--- a/Configure
+++ b/Configure
@@ -17160,12 +17160,13 @@ EOM
 set readlink d_readlink
 eval $inlibc
 
-: Check if exe is symlink to abs path of executing program
+: Check if exe is symlink to abs path of executing program, but only if we
+: don't have a hint telling us not to. AmigaOS will attempt to mount /proc if
+: /proc/self/exe is referenced, and AmigaOS doesn't have that
 echo " "
 procselfexe=''
 val="$undef"
-case "$d_procselfexe" in
-'')
+if [ "$d_procselfexe" != "$undef" ]; then
 case "$d_readlink" in
     "$define")
        : NetBSD first as /proc/self is a symlink to /proc/curproc, and it feels
@@ -17190,8 +17191,7 @@ esac
 $rm -f reflect
 set d_procselfexe
 eval $setvar
-;;
-esac
+fi
 
 : backward compatibility for d_hvfork
 if test X$d_hvfork != X; then

@p5pRT
Copy link
Author

p5pRT commented Sep 24, 2015

From @dcollinsn

trivial.patch
diff --git a/Configure b/Configure
index e12c8bb..ba1aaff 100755
--- a/Configure
+++ b/Configure
@@ -17162,10 +17162,10 @@ eval $inlibc
 
 : Check if exe is symlink to abs path of executing program
 echo " "
-procselfexe=''
 val="$undef"
 case "$d_procselfexe" in
 '')
+procselfexe=''
 case "$d_readlink" in
     "$define")
 	: NetBSD first as /proc/self is a symlink to /proc/curproc, and it feels

@p5pRT
Copy link
Author

p5pRT commented Sep 24, 2015

From @jkeenan

On Wed Sep 23 20​:00​:08 2015, dcollinsn@​gmail.com wrote​:

dcollins@​nightshade​:~/perl$ git clean -dxf > /dev/null
dcollins@​nightshade​:~/perl$ ./Configure -Dusedevel -des > /dev/null
dcollins@​nightshade​:~/perl$ grep procselfexe config.sh
d_procselfexe='define'
procselfexe='"/proc/self/exe"'
dcollins@​nightshade​:~/perl$ ./Configure -Dusedevel -des > /dev/null
dcollins@​nightshade​:~/perl$ grep procselfexe config.sh
d_procselfexe='define'
procselfexe=''
dcollins@​nightshade​:~/perl$ make
...SNIP...
cc -c -DPERL_CORE -fwrapv -fno-strict-aliasing -pipe -fstack-protector
-I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64
-D_FORTIFY_SOURCE=2 -std=c89 -O2 -Wall -Werror=declaration-after-
statement -Wextra -Wc++-compat -Wwrite-strings caretx.c
caretx.c​: In function ‘Perl_set_caret_X’​:
caretx.c​:102​:48​: error​: expected expression before ‘,’ token
SSize_t len = readlink(PROCSELFEXE_PATH, buf, sizeof(buf) -
1);
^
make​: *** [caretx.o] Error 1

Bisect points at one of the amigaos changes to allow ./Configure to
use hints of d_procselfexe, without actually allowing it to use hints
of procselfexe. As a result, if there are hints of (d_)?procselfexe,
the above behavior is observed.

It is trivial to move the line blanking procselfexe to inside the case
statement, as in the attached trivial.patch. However, we never allowed
d_procselfexe to hint before, we always checked since it is trivial to
do so. Therefore, I propose the attached procselfexe.patch, which
instead only accepts a hint of 'undef', and in any other case,
exhibits the usual behavior of finding and checking the platform-
specific exe for symbolic-link-ness.

%% GIT BISECT %%

(bisected on the return value of ./Configure -Dusedevel -des &&
./Configure -Dusedevel -des; grep '/proc/self/exe' config.sh)

68b32a2 is the first bad commit
commit 68b32a2
Author​: Andy Broad <andy@​broad.ology.org.uk>
Date​: Thu Aug 13 19​:09​:25 2015 -0400

amigaos4​: Configure​: allow hinting d_procselfexe

Trying to access /proc causes the system to try to mount such a thing.
(And there is no process file system in AmigaOS, anyway.)

:100755 100755 6eb478d7214816ced51aaee60438676996bcd888
d0ebb2eb35f5f017a6d61056c08585035d4778e5 M Configure
bisect run success

Would it be possible for you to do a 'git checkout' of the commit immediately before the one that broke the build, configure and build perl, and then provide us with the output of 'perl -V'?

That might help with diagnostics.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Sep 24, 2015

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Sep 24, 2015

From @dcollinsn

James,

I can when I get back to my computer but maybe it would be easier to
explain if you let me know what you don't understand. If d_procselfexe is
defined in config.sh when you run ./Configure, after that commit,
d_procselfexe will be preserved as 'define' and procselfexe will be empty.
This patch fixes that by finding procselfexe when d_procselfexe is hinted
as 'define'.
On Sep 24, 2015 6​:48 PM, "James E Keenan via RT" <perlbug-followup@​perl.org>
wrote​:

On Wed Sep 23 20​:00​:08 2015, dcollinsn@​gmail.com wrote​:

dcollins@​nightshade​:~/perl$ git clean -dxf > /dev/null
dcollins@​nightshade​:~/perl$ ./Configure -Dusedevel -des > /dev/null
dcollins@​nightshade​:~/perl$ grep procselfexe config.sh
d_procselfexe='define'
procselfexe='"/proc/self/exe"'
dcollins@​nightshade​:~/perl$ ./Configure -Dusedevel -des > /dev/null
dcollins@​nightshade​:~/perl$ grep procselfexe config.sh
d_procselfexe='define'
procselfexe=''
dcollins@​nightshade​:~/perl$ make
...SNIP...
cc -c -DPERL_CORE -fwrapv -fno-strict-aliasing -pipe -fstack-protector
-I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64
-D_FORTIFY_SOURCE=2 -std=c89 -O2 -Wall -Werror=declaration-after-
statement -Wextra -Wc++-compat -Wwrite-strings caretx.c
caretx.c​: In function ‘Perl_set_caret_X’​:
caretx.c​:102​:48​: error​: expected expression before ‘,’ token
SSize_t len = readlink(PROCSELFEXE_PATH, buf, sizeof(buf) -
1);
^
make​: *** [caretx.o] Error 1

Bisect points at one of the amigaos changes to allow ./Configure to
use hints of d_procselfexe, without actually allowing it to use hints
of procselfexe. As a result, if there are hints of (d_)?procselfexe,
the above behavior is observed.

It is trivial to move the line blanking procselfexe to inside the case
statement, as in the attached trivial.patch. However, we never allowed
d_procselfexe to hint before, we always checked since it is trivial to
do so. Therefore, I propose the attached procselfexe.patch, which
instead only accepts a hint of 'undef', and in any other case,
exhibits the usual behavior of finding and checking the platform-
specific exe for symbolic-link-ness.

%% GIT BISECT %%

(bisected on the return value of ./Configure -Dusedevel -des &&
./Configure -Dusedevel -des; grep '/proc/self/exe' config.sh)

68b32a2 is the first bad commit
commit 68b32a2
Author​: Andy Broad <andy@​broad.ology.org.uk>
Date​: Thu Aug 13 19​:09​:25 2015 -0400

amigaos4​: Configure​: allow hinting d_procselfexe

Trying to access /proc causes the system to try to mount such a thing.
(And there is no process file system in AmigaOS, anyway.)

:100755 100755 6eb478d7214816ced51aaee60438676996bcd888
d0ebb2eb35f5f017a6d61056c08585035d4778e5 M Configure
bisect run success

Would it be possible for you to do a 'git checkout' of the commit
immediately before the one that broke the build, configure and build perl,
and then provide us with the output of 'perl -V'?

That might help with diagnostics.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Sep 25, 2015

From @jkeenan

On Thu Sep 24 16​:49​:39 2015, dcollinsn@​gmail.com wrote​:

James,

I can when I get back to my computer but maybe it would be easier to
explain if you let me know what you don't understand.

It's not a question as to whether or not *I* understand your problem. We have long recommended that people include the output of 'perl -V' when filing a bug report; the 'perlbug' utility does this automatically. So we're only asking you to supply the same type of information as anybody else, and we do so to maximize the chance that someone reading the P5P list can help you or evaluate your patches.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Sep 25, 2015

From @dcollinsn

Happy to provide​:

%% PERL -V %%

dcollins@​nightshade​:~/perl$ ./perl -Ilib -V
Summary of my perl5 (revision 5 version 23 subversion 4) configuration​:
  Commit id​: e120c24
  Platform​:
  osname=linux, osvers=2.6.32-5-686, archname=i686-linux
  uname='linux nightshade 2.6.32-5-686 #1 smp tue may 13 16​:33​:32 utc
2014 i686 gnulinux '
  config_args='-Dusedevel -des'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=undef, usemultiplicity=undef
  use64bitint=undef, use64bitall=undef, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector
-I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64
-D_FORTIFY_SOURCE=2',
  optimize='-O2',
  cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector
-I/usr/local/include'
  ccversion='', gccversion='4.4.5', gccosandvers=''
  intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234,
doublekind=3
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12,
longdblkind=3
  ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t',
lseeksize=8
  alignbytes=4, prototype=define
  Linker and Libraries​:
  ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib/gcc/i486-linux-gnu/4.4.5/include-fixed
/usr/lib /lib/../lib /usr/lib/../lib /lib /usr/lib/i486-linux-gnu /usr/lib64
  libs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  libc=libc-2.22.so, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version='2.22'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib
-fstack-protector'

Characteristics of this binary (from libperl)​:
  Compile-time options​: HAS_TIMES PERLIO_LAYERS PERL_COPY_ON_WRITE
  PERL_DONT_CREATE_GVSV
  PERL_HASH_FUNC_ONE_AT_A_TIME_HARD PERL_MALLOC_WRAP
  PERL_PRESERVE_IVUV PERL_USE_DEVEL USE_LARGE_FILES
  USE_LOCALE USE_LOCALE_COLLATE USE_LOCALE_CTYPE
  USE_LOCALE_NUMERIC USE_LOCALE_TIME USE_PERLIO
  USE_PERL_ATOF
  Built under linux
  Compiled at Sep 24 2015 22​:39​:24
  @​INC​:
  lib
  /usr/local/lib/perl5/site_perl/5.23.4/i686-linux
  /usr/local/lib/perl5/site_perl/5.23.4
  /usr/local/lib/perl5/5.23.4/i686-linux
  /usr/local/lib/perl5/5.23.4
  .

On Thu, Sep 24, 2015 at 9​:03 PM, James E Keenan via RT <
perlbug-followup@​perl.org> wrote​:

On Thu Sep 24 16​:49​:39 2015, dcollinsn@​gmail.com wrote​:

James,

I can when I get back to my computer but maybe it would be easier to
explain if you let me know what you don't understand.

It's not a question as to whether or not *I* understand your problem. We
have long recommended that people include the output of 'perl -V' when
filing a bug report; the 'perlbug' utility does this automatically. So
we're only asking you to supply the same type of information as anybody
else, and we do so to maximize the chance that someone reading the P5P list
can help you or evaluate your patches.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=126152

@p5pRT
Copy link
Author

p5pRT commented Oct 13, 2015

From @tonycoz

On Wed Sep 23 20​:00​:08 2015, dcollinsn@​gmail.com wrote​:

dcollins@​nightshade​:~/perl$ git clean -dxf > /dev/null
dcollins@​nightshade​:~/perl$ ./Configure -Dusedevel -des > /dev/null
dcollins@​nightshade​:~/perl$ grep procselfexe config.sh
d_procselfexe='define'
procselfexe='"/proc/self/exe"'
dcollins@​nightshade​:~/perl$ ./Configure -Dusedevel -des > /dev/null
dcollins@​nightshade​:~/perl$ grep procselfexe config.sh
d_procselfexe='define'
procselfexe=''
dcollins@​nightshade​:~/perl$ make
...SNIP...
cc -c -DPERL_CORE -fwrapv -fno-strict-aliasing -pipe -fstack-protector
-I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64
-D_FORTIFY_SOURCE=2 -std=c89 -O2 -Wall -Werror=declaration-after-
statement -Wextra -Wc++-compat -Wwrite-strings caretx.c
caretx.c​: In function ‘Perl_set_caret_X’​:
caretx.c​:102​:48​: error​: expected expression before ‘,’ token
SSize_t len = readlink(PROCSELFEXE_PATH, buf, sizeof(buf) -
1);
^
make​: *** [caretx.o] Error 1

Bisect points at one of the amigaos changes to allow ./Configure to
use hints of d_procselfexe, without actually allowing it to use hints
of procselfexe. As a result, if there are hints of (d_)?procselfexe,
the above behavior is observed.

It is trivial to move the line blanking procselfexe to inside the case
statement, as in the attached trivial.patch. However, we never allowed
d_procselfexe to hint before, we always checked since it is trivial to
do so. Therefore, I propose the attached procselfexe.patch, which
instead only accepts a hint of 'undef', and in any other case,
exhibits the usual behavior of finding and checking the platform-
specific exe for symbolic-link-ness.

I think I prefer the "trivial" patch.

The more complex patch doesn't let a hints file include​:

  d_procselfexe=define
  procselfexe=/some/strange/path

or even​:

  ./Configure -Dd_procselfexe -Dprocselfexe=/some/strange/path ...

Tony

%% GIT BISECT %%

(bisected on the return value of ./Configure -Dusedevel -des &&
./Configure -Dusedevel -des; grep '/proc/self/exe' config.sh)

68b32a2 is the first bad commit
commit 68b32a2
Author​: Andy Broad <andy@​broad.ology.org.uk>
Date​: Thu Aug 13 19​:09​:25 2015 -0400

amigaos4​: Configure​: allow hinting d_procselfexe

Trying to access /proc causes the system to try to mount such a thing.
(And there is no process file system in AmigaOS, anyway.)

:100755 100755 6eb478d7214816ced51aaee60438676996bcd888
d0ebb2eb35f5f017a6d61056c08585035d4778e5 M Configure
bisect run success

@p5pRT
Copy link
Author

p5pRT commented Oct 13, 2015

From @dcollinsn

On Mon Oct 12 17​:18​:04 2015, tonyc wrote​:

I think I prefer the "trivial" patch.

The more complex patch doesn't let a hints file include​:

d_procselfexe=define
procselfexe=/some/strange/path

or even​:

./Configure -Dd_procselfexe -Dprocselfexe=/some/strange/path ...

Tony

While I see your point, we've never allowed that before. That'd be a neat idea, but I think we need to do /something/ to fix this regression, because currently ./Configure; ./Configure doesn't work on any platform with a /proc/self/exe.

If you'd like to allow that, then you want something like this patch. However, this presently doesn't work because the second if statement doesn't work, even when procselfexe is '', it isn't entered. I can't figure out why, Configure just crashes when I try to debug anything.

@p5pRT
Copy link
Author

p5pRT commented Oct 13, 2015

From @dcollinsn

procselfexe.patch
diff --git a/Configure b/Configure
index e751dd9..2725d92 100755
--- a/Configure
+++ b/Configure
@@ -17100,12 +17100,17 @@ EOM
 set readlink d_readlink
 eval $inlibc
 
-: Check if exe is symlink to abs path of executing program
+: Check if exe is symlink to abs path of executing program. We will honor
+: hints of procselfexe, as well as hints of d_procselfexe=$undef, and will
+: search if we have no hints or if d_procselfexe=$define but
+: procselfexe=''. AmigaOS will attempt to mount /proc if /proc/self/exe
+: is referenced, and AmigaOS doesnt have that
 echo " "
-procselfexe=''
 val="$undef"
-case "$d_procselfexe" in
-'')
+if [ "$procselfexe" != '' && "$d_procselfexe" == '']; then
+    d_procselfexe = $define
+fi
+if [ "$procselfexe" == '' && "$d_procselfexe" != "$undef" ]; then
 case "$d_readlink" in
     "$define")
        : NetBSD first as /proc/self is a symlink to /proc/curproc, and it feels
@@ -17130,8 +17135,7 @@ esac
 $rm -f reflect
 set d_procselfexe
 eval $setvar
-;;
-esac
+fi
 
 : backward compatibility for d_hvfork
 if test X$d_hvfork != X; then

@p5pRT
Copy link
Author

p5pRT commented Oct 13, 2015

From @jhi

How would the attached patch work for you?

@p5pRT
Copy link
Author

p5pRT commented Oct 13, 2015

From @jhi

0001-rt.perl.org-126152-Configure-proc-issues.patch
From 7fb5381d2121987ae02403bfcaa9d652c4daee1c Mon Sep 17 00:00:00 2001
From: Jarkko Hietaniemi <jhi@iki.fi>
Date: Tue, 13 Oct 2015 08:49:32 -0400
Subject: [PATCH] rt.perl.org 126152 Configure /proc issues

---
 Configure | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/Configure b/Configure
index e751dd9..56f777a 100755
--- a/Configure
+++ b/Configure
@@ -17100,17 +17100,26 @@ EOM
 set readlink d_readlink
 eval $inlibc
 
-: Check if exe is symlink to abs path of executing program
+: Check if exe is symlink to the abs path of executing program.
+: We will honor hints of procselfexe, as well as hints of
+: d_procselfexe=$undef, and will search if we have no hints or if
+: d_procselfexe=$define but procselfexe=''. Known problematic ones:
+: AmigaOS will attempt to mount proc: aka /proc, if /proc/... is
+: referenced, and AmigaOS does not have a proc filesystem anyway.
+: Similarly, /proc exists in IRIX, but it does not contain
+: a symlink to the executing program.
+
 echo " "
-procselfexe=''
+case "$procselfexe" in
+''|' ') ;;
+*) d_procselfexe="$define" ;;
+esac
 val="$undef"
-case "$d_procselfexe" in
-'')
-case "$d_readlink" in
-    "$define")
-	: NetBSD first as /proc/self is a symlink to /proc/curproc, and it feels
-	: more tidy to avoid an extra level of symlink
-	set NetBSD /proc/curproc/exe Linux /proc/self/exe FreeBSD /proc/curproc/file Solaris /proc/self/path/a.out
+case "$d_procselfexe:$d_readlink" in
+    "$define:$define")
+        : NetBSD first as /proc/self is a symlink to /proc/curproc,
+        : and it feels more tidy to avoid an extra level of symlink
+        set NetBSD /proc/curproc/exe Linux /proc/self/exe FreeBSD /proc/curproc/file Solaris /proc/self/path/a.out
 	while test $# -gt 0; do
 	    type=$1; try=$2
 	    shift; shift
@@ -17130,8 +17139,6 @@ esac
 $rm -f reflect
 set d_procselfexe
 eval $setvar
-;;
-esac
 
 : backward compatibility for d_hvfork
 if test X$d_hvfork != X; then
-- 
2.6.0

@p5pRT
Copy link
Author

p5pRT commented Oct 13, 2015

From [Unknown Contact. See original ticket]

How would the attached patch work for you?

@p5pRT
Copy link
Author

p5pRT commented Oct 14, 2015

From @dcollinsn

On Tue Oct 13 06​:08​:37 2015, jhi wrote​:

How would the attached patch work for you?

I see what you fixed about my patch ($procselfexe might == ' ' not ''), but without running yours (sorry, on a locked down office PC) I don't think your logic is right. It looks like if you call that with $d_pse and $pse both '', then the first case will do nothing, and then the second case will do nothing. I think the ideal flow is​:

1) Do we have a hinted $pse (I'm abbreviating procselfexe) but $d_pse is blank? If so, $d_pse = $define and do nothing else.
2) Do we have a hinted $pse and $d_pse == $define? If so, do nothing else.
3) Do we have a hinted $d_pse == $undef? If so, maybe $pse = '' and do nothing else.
4) Do we have readlink? If so, look for procselfexe.

I'm not clever enough to figure out what happens if we /don't/ have readlink, but it's probably a bad time. Anyway, if that's what the workflow is, then we'll combine the first two steps - which is actually easier if we do my step 3 first. So let me simplify that​:

A) Do we have a hinted $d_pse == $undef? If so, $pse = '' and do nothing else.
B) Do we have a hinted $pse? If so, $d_pse = $define and do nothing else.
C) Do we have readlink? If so, look for procselfexe.

So I'll attach that patch, with the caveat that I'm unable to test it until I get home.

@p5pRT
Copy link
Author

p5pRT commented Oct 14, 2015

From @dcollinsn

procselfexe.patch
diff --git a/Configure b/Configure
index e751dd9..2725d92 100755
--- a/Configure
+++ b/Configure
@@ -17100,14 +17100,20 @@ EOM
 set readlink d_readlink
 eval $inlibc

-: Check if exe is symlink to abs path of executing program
+: Check if exe is symlink to abs path of executing program. We will honor
+: hints of procselfexe, as well as hints of d_procselfexe=$undef, and will
+: search if we have no hints or if d_procselfexe=$define but
+: procselfexe=''. Known problematic ones: AmigaOS will attempt to mount
+: proc: aka /proc, if /proc/... is referenced, and AmigaOS does not have a
+: proc filesystem anyway. Similarly, /proc exists in IRIX, but it does not
+: contain a symlink to the executing program.
 echo " "
-procselfexe=''
 val="$undef"
-case "$d_procselfexe" in
-'')
-case "$d_readlink" in
-    "$define")
+if [ "$d_procselfexe" == "$undef" ]; then
+    procselfexe = ''
+elif [ "$procselfexe" != '' && "$procselfexe" != ' ' ]; then
+    d_procselfexe = $define
+elif [ "$d_readlink" == "$define" ]; then
        : NetBSD first as /proc/self is a symlink to /proc/curproc, and it feels
        : more tidy to avoid an extra level of symlink
        set NetBSD /proc/curproc/exe Linux /proc/self/exe FreeBSD /proc/curproc/file Solaris /proc/self/path/a.out
@@ -17125,13 +17131,10 @@ case "$d_readlink" in
                fi
            fi
        done
-       ;;
-esac
-$rm -f reflect
-set d_procselfexe
-eval $setvar
-;;
-esac
+       $rm -f reflect
+       set d_procselfexe
+       eval $setvar
+fi

 : backward compatibility for d_hvfork
 if test X$d_hvfork != X; then

@p5pRT
Copy link
Author

p5pRT commented Oct 14, 2015

From @jhi

Configure uses (well, tries to be consistent about) a particular (portable) shell dialect.

+if [ "$d_procselfexe" == "$undef" ]; then

You want $test not [].
You want a single =.
Usual idiom would be​:

if $test "X$d_pse" = "$undef"

+elif [ "$procselfexe" != '' && "$procselfexe" != ' ' ]; then

The above, and ...
You want -a, not &&.

+ set d_procselfexe
+ eval $setvar

Now d_pse is set (to define or undef) only if the scan is done. I would check if it is possible that it may end up being not set at all (empty string).

@p5pRT
Copy link
Author

p5pRT commented Oct 14, 2015

From [Unknown Contact. See original ticket]

Configure uses (well, tries to be consistent about) a particular (portable) shell dialect.

+if [ "$d_procselfexe" == "$undef" ]; then

You want $test not [].
You want a single =.
Usual idiom would be​:

if $test "X$d_pse" = "$undef"

+elif [ "$procselfexe" != '' && "$procselfexe" != ' ' ]; then

The above, and ...
You want -a, not &&.

+ set d_procselfexe
+ eval $setvar

Now d_pse is set (to define or undef) only if the scan is done. I would check if it is possible that it may end up being not set at all (empty string).

@p5pRT
Copy link
Author

p5pRT commented Oct 14, 2015

From @jhi

Usual idiom would be​:

if $test "X$d_pse" = "$undef"

Aargh. I of course meant​:

if $test "X$d_pse" = "X$undef"

The X-prefix because of some broken shells, IIRC.

@p5pRT
Copy link
Author

p5pRT commented Oct 15, 2015

From @jhi

On Wed Oct 14 07​:22​:50 2015, dcollinsn@​gmail.com wrote​:

On Tue Oct 13 06​:08​:37 2015, jhi wrote​:

How would the attached patch work for you?

Take two attached, please see how it fares.

@p5pRT
Copy link
Author

p5pRT commented Oct 15, 2015

From @jhi

0001-PATCH-rt.perl.org-126152-Configure-proc-issues.patch
From 48c3928b00cf536c67e9da8afac821d0deab2e26 Mon Sep 17 00:00:00 2001
From: Jarkko Hietaniemi <jhi@iki.fi>
Date: Thu, 15 Oct 2015 08:39:42 -0400
Subject: [PATCH] [PATCH] rt.perl.org 126152 Configure /proc issues

---
 Configure | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/Configure b/Configure
index e751dd9..21f6dfd 100755
--- a/Configure
+++ b/Configure
@@ -17100,16 +17100,21 @@ EOM
 set readlink d_readlink
 eval $inlibc
 
-: Check if exe is symlink to abs path of executing program
+: Check if there is a /proc symlink to the abs path of
+: the executing program.  We will honor hints of d_procselfexe=$undef
+: or procselfexe being non-empty, otherwise will try to determine both
+: if we have readlink.
+: AmigaOS will attempt to mount proc: aka /proc, if /proc/... is
+: referenced, and AmigaOS does not have a proc filesystem anyway.
 echo " "
-procselfexe=''
 val="$undef"
-case "$d_procselfexe" in
-'')
-case "$d_readlink" in
-    "$define")
-	: NetBSD first as /proc/self is a symlink to /proc/curproc, and it feels
-	: more tidy to avoid an extra level of symlink
+if $test "X$d_procselfexe" = Xundef; then
+	procselfexe=''
+elif $test "X$procselfexe" != X -a "X$procselfexe" != 'X '; then
+	val="$define"
+elif $test "X$d_readlink" = Xdefine; then
+        : NetBSD first as /proc/self is a symlink to /proc/curproc,
+        : and it feels more tidy to avoid an extra level of symlink
 	set NetBSD /proc/curproc/exe Linux /proc/self/exe FreeBSD /proc/curproc/file Solaris /proc/self/path/a.out
 	while test $# -gt 0; do
 	    type=$1; try=$2
@@ -17125,13 +17130,10 @@ case "$d_readlink" in
 		fi
 	    fi
 	done
-	;;
-esac
+fi
 $rm -f reflect
 set d_procselfexe
 eval $setvar
-;;
-esac
 
 : backward compatibility for d_hvfork
 if test X$d_hvfork != X; then
-- 
2.6.0

@p5pRT
Copy link
Author

p5pRT commented Oct 15, 2015

From @dcollinsn

On Thu Oct 15 05​:45​:48 2015, jhi wrote​:

On Wed Oct 14 07​:22​:50 2015, dcollinsn@​gmail.com wrote​:

On Tue Oct 13 06​:08​:37 2015, jhi wrote​:

How would the attached patch work for you?

Take two attached, please see how it fares.

Works for me

@p5pRT
Copy link
Author

p5pRT commented Oct 16, 2015

From @jhi

On Thu Oct 15 15​:59​:53 2015, dcollinsn@​gmail.com wrote​:

On Thu Oct 15 05​:45​:48 2015, jhi wrote​:

On Wed Oct 14 07​:22​:50 2015, dcollinsn@​gmail.com wrote​:

On Tue Oct 13 06​:08​:37 2015, jhi wrote​:

How would the attached patch work for you?

Take two attached, please see how it fares.

Works for me

Thanks! I now went ahead and pushed this as b446945.

@p5pRT
Copy link
Author

p5pRT commented Oct 16, 2015

@jhi - Status changed from 'open' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant