Page MenuHomeFreeBSD

bash: Export symbols for "enable -f"
ClosedPublic

Authored by cem on Jul 29 2015, 5:22 AM.

Details

Test Plan

make package; seems to work.

Manual testing with https://github.com/taviso/ctypes.sh/ .

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

cem updated this revision to Diff 7449.Jul 29 2015, 5:22 AM
cem retitled this revision from to bash: Export symbols for "enable -f".
cem updated this object.
cem edited the test plan for this revision. (Show Details)
cem added reviewers: markj, ehaupt.
ehaupt edited edge metadata.Jul 29 2015, 6:47 AM
This comment was removed by ehaupt.
ehaupt added a comment.EditedJul 29 2015, 8:29 AM

Can you give me a test case on how to test this? From what I can tell the dynamically linked version (shells/bash) already supports 'enable -f':

$ enable -f foo
enable .
enable :
enable [
enable alias
enable bg
...

while the static version (shells/bash-static) (IMO) correctly doesn't:

$ enable -f foo
-bash: enable: dynamic loading not available

It appears to me that this is already handled correctly.

cem added a comment.Jul 29 2015, 1:45 PM
In D3231#65128, @ehaupt wrote:

Can you give me a test case on how to test this? From what I can tell the dynamically linked version (shells/bash) already supports 'enable -f':

$ enable -f foo
enable .
enable :
enable [
enable alias
enable bg
...

It doesn't actually work, though, if foo references any bash-provided symbols (and plugins expect to). Here's an example extension that does not load without this patch:

https://github.com/taviso/ctypes.sh

I get:

bash: enable: cannot open shared object /usr/local/lib/ctypes.so: /usr/local/lib/ctypes.so: Undefined symbol "list_optarg".

https://github.com/taviso/ctypes.sh/pull/6 for more context.

I've tried the example you've provided. Unfortunately I don't get the expected result:

root@portjail:~/ctypes.sh # pkg info libffi binutils
libffi-3.2.1
binutils-2.25.1

root@portjail:~/ctypes.sh # source /usr/local/bin/ctypes.sh 
-bash: enable: cannot open shared object /usr/local/lib/ctypes.so: /usr/local/lib/ctypes.so: Undefined symbol "ffi_type_pointer"
can't find the ctypes.so library, run make install?

root@portjail:~/ctypes.sh # ldconfig -r | grep ffi
        129:-lffi.6 => /usr/local/lib/libffi.so.6

root@portjail:~/ctypes.sh # ls -l /usr/local/lib/ctypes.so 
-rwxr-xr-x  1 root  wheel  53650 Jul 30 09:20 /usr/local/lib/ctypes.so

To build/install https://github.com/taviso/ctypes.sh I had to patch the Makefile:

--- Makefile.patch begins here ---
diff --git a/Makefile b/Makefile
index 0cc654d..ff85695 100644
--- a/Makefile
+++ b/Makefile
@@ -1,7 +1,7 @@
-CFLAGS  = -std=gnu99 -fPIC -O0 -ggdb3 -Wall -Wextra -fvisibility=hidden
-CPPFLAGS= -Iinclude $(shell pkg-config --cflags libffi)
+CFLAGS  = -std=gnu99 -fPIC -O0 -ggdb3 -Wall -Wextra -fvisibility=hidden -I/usr/local/include
+CPPFLAGS= -Iinclude $(shell pkg-config --cflags libffi) -I/usr/local/include
 UNAME   = $(shell uname -s)
-LDLIBS  = $(shell pkg-config --libs libffi)
+LDLIBS  = $(shell pkg-config --libs libffi) -L/usr/local/lib
 PREFIX = /usr/local
 
 ifeq ($(UNAME), Linux)
--- Makefile.patch ends here ---

Are there any additional steps/dependencies required?

cem added a comment.Jul 30 2015, 7:43 AM
In D3231#65425, @ehaupt wrote:

root@portjail:~/ctypes.sh # source /usr/local/bin/ctypes.sh
-bash: enable: cannot open shared object /usr/local/lib/ctypes.so: /usr/local/lib/ctypes.so: Undefined symbol "ffi_type_pointer"
can't find the ctypes.so library, run make install?

To build/install https://github.com/taviso/ctypes.sh I had to patch the Makefile:

--- Makefile.patch begins here ---
diff --git a/Makefile b/Makefile
index 0cc654d..ff85695 100644
--- a/Makefile
+++ b/Makefile
@@ -1,7 +1,7 @@
-CFLAGS  = -std=gnu99 -fPIC -O0 -ggdb3 -Wall -Wextra -fvisibility=hidden
-CPPFLAGS= -Iinclude $(shell pkg-config --cflags libffi)
+CFLAGS  = -std=gnu99 -fPIC -O0 -ggdb3 -Wall -Wextra -fvisibility=hidden -I/usr/local/include
+CPPFLAGS= -Iinclude $(shell pkg-config --cflags libffi) -I/usr/local/include
 UNAME   = $(shell uname -s)
-LDLIBS  = $(shell pkg-config --libs libffi)
+LDLIBS  = $(shell pkg-config --libs libffi) -L/usr/local/lib
 PREFIX = /usr/local
 
 ifeq ($(UNAME), Linux)
--- Makefile.patch ends here ---

Are there any additional steps/dependencies required?

Do you have pkgconf installed? It is needed for pkg-config invocations. For example, pkg-config --libs libffi emits:

-L/usr/local/lib -lffi

on my system (obviating the added -L/usr/local/lib).

Do you have pkgconf installed? It is needed for pkg-config invocations. For example, pkg-config --libs libffi emits:

There we go, that's what I was missing.

ehaupt requested changes to this revision.Jul 30 2015, 8:57 AM
ehaupt edited edge metadata.

Could you please make the following changes.

shells/bash/Makefile
7 ↗(On Diff #7449)

The revision should be bumped to 3.

58 ↗(On Diff #7449)

While it is simply ignored in the static version of the port it would be nicer to move it below line 77 in the if block along with a short comment.

This revision now requires changes to proceed.Jul 30 2015, 8:57 AM
cem updated this revision to Diff 7512.Jul 30 2015, 3:14 PM
cem edited edge metadata.

Per ehaupt@, bump revision and make -Wl,-export-dynamic conditional on
not-static build.

ehaupt accepted this revision.Jul 30 2015, 5:21 PM
ehaupt edited edge metadata.

Looks good, please commit.

This revision is now accepted and ready to land.Jul 30 2015, 5:21 PM
markj accepted this revision.Jul 30 2015, 5:32 PM
markj edited edge metadata.
This revision was automatically updated to reflect the committed changes.