libtool -bindir support

[Jeremie Courreges-Anglas]

Making read(2) return EISDIR for directories breaks two ports, both
because they use libtool -bindir. cc(1) gets executed with an unknown
option, -bindir, and a path such as /usr/local/bin, which then gets
passed to ld(1). ld(1) copes with read(2) returning 0, not with EISDIR.
Thanks to Antoine who ran the bulk builds that exposed this problem.

-bindir support is meaningless on OpenBSD so handling that option should
be easy. The problem is that I don't know how to implement it in
libtool(1). GNU libtool recognizes -bindir among cc flags, while our
version seems to only handle options passed right after argv[0].

I plan to work around that problem by using GNU libtool for the ports
mentioned above, but someone else might want to poke at libtool(1)
internals. :)

--
jca | PGP: 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE

[Antoine Jacoutot]

I can have a look at it during g2k16 if no one beats me to it.
--
Antoine

[Antoine Jacoutot]

Hi Jeremie.

This seems to do the trick for me:

Index: LT/Getopt.pm
===================================================================
RCS file: /cvs/src/usr.bin/libtool/LT/Getopt.pm,v
retrieving revision 1.12
diff -u -p -r1.12 Getopt.pm
--- LT/Getopt.pm 19 Mar 2014 02:16:22 -0000 1.12
+++ LT/Getopt.pm 1 Aug 2016 10:18:03 -0000
@@ -292,6 +292,11 @@ MAINLOOP2:
if ($arg =~ m/^\-\-$/) {
next; # XXX ?
}
+ # ignore "-bindir /path/to/dir" on OpenBSD (matches GNU libtool)
+ if ($arg =~ m/^\-bindir$/) {
+ shift @main::ARGV;
+ next;
+ }
if ($arg =~ m/^\-/) {
for my $opt (@options) {
if ($opt->match($arg, $self)) {



--
Antoine

[Jeremie Courreges-Anglas]

+cc espie and jasper

Thanks for looking at it. Yup, that works, but I think I know
understand why I got confused first.

-bindir is just one option among others that should be recognized and
ignored in *link* mode. The following diff does just that, I think it
fits better in the existing code. I can successfully build
devel/libiscsi, -bindir /usr/local/bin doesn't get passed to cc(1).

Thoughts?


Index: Link.pm
===================================================================
RCS file: /cvs/src/usr.bin/libtool/LT/Mode/Link.pm,v
retrieving revision 1.31
diff -u -p -p -u -r1.31 Link.pm
--- Link.pm 27 Apr 2016 09:50:57 -0000 1.31
+++ Link.pm 1 Aug 2016 11:36:31 -0000
@@ -127,6 +127,7 @@ sub run
'all-static',
'allow-undefined', # we don't care about THAT one
'avoid-version',
+ 'bindir:',
'dlopen:',
'dlpreopen:',
'export-dynamic',
@@ -152,7 +153,7 @@ sub run
'version-info:',
'version-number:');

- # XXX options ignored: dlopen, dlpreopen, no-fast-install,
+ # XXX options ignored: bindir, dlopen, dlpreopen, no-fast-install,
# no-install, no-undefined, precious-files-regex,
# shrext, thread-safe, prefer-pic, prefer-non-pic

[Jeremie Courreges-Anglas]

I'm wondering about the second hunk: is this a mix of options that we
don't support on purpose and of options that could be useful?
(eg. -no-undefined.) So is the second hunk of this diff desirable?

[Marc Espie]

The main thing about that diff is that it has to go into a successful
bulk.

As for ignored options, it's informative. It tells us we ignore those
options. Which ones should actually be supported is another story.

[Antoine Jacoutot]

I think that goes without saying...
I'll do that in a few.

> As for ignored options, it's informative. It tells us we ignore those
> options. Which ones should actually be supported is another story.
>

--
Antoine

[Jeremie Courreges-Anglas]

It did, thanks to Antoine, no breakage. I already got an ok from
Antoine. Objections / ok?

> As for ignored options, it's informative. It tells us we ignore those
> options. Which ones should actually be supported is another story.

Fine with me.