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

Fix build on Cygwin #6

Closed
wants to merge 1 commit into from
Closed

Conversation

stilor
Copy link
Contributor

@stilor stilor commented Feb 23, 2017

The -lcygwin -lc actually breaks the build: elf2flt picks up the symbols
for getopt/optarg via <getopt.h> in binutils-X.Y/include, where optarg
is declared without dllimport attribute. Therefore it pulls in getopt()
from libc/libcygwin, but since optarg is not prefixed with imp_, it is
pulled from libiberty. But the object file in libiberty also contains
getopt() thus resulting in multiple definitions thereof.

Signed-off-by: Alexey Neyman stilor@att.net

@stilor
Copy link
Contributor Author

stilor commented Feb 23, 2017

To clarify: yes, I saw the comments in the deleted code. However, what I am seeing while trying to build elf2flt (via crosstool-ng) on cygwin is exactly the opposite: these workarounds result in libcygwin/libiberty clash for getopt. I am not sure how the original problem was manifested - was it some ancient binutils that didn't have getopt.h, or what?

stilor added a commit to stilor/elf2flt that referenced this pull request Feb 23, 2017
The -lcygwin -lc actually breaks the build: elf2flt picks up the symbols
for getopt/optarg via <getopt.h> in binutils-X.Y/include, where optarg
is declared without dllimport attribute.  Therefore it pulls in getopt()
from libc/libcygwin, but since optarg is not prefixed with _imp__, it is
pulled from libiberty. But the object file in libiberty also contains
getopt() thus resulting in multiple definitions thereof.

Upstream: uclinux-dev#6
Signed-off-by: Alexey Neyman <stilor@att.net>
The -lcygwin -lc actually breaks the build: elf2flt picks up the symbols
for getopt/optarg via <getopt.h> in binutils-X.Y/include, where optarg
is declared without dllimport attribute.  Therefore it pulls in getopt()
from libc/libcygwin, but since optarg is not prefixed with _imp__, it is
pulled from libiberty. But the object file in libiberty also contains
getopt() thus resulting in multiple definitions thereof.

Signed-off-by: Alexey Neyman <stilor@att.net>
stilor added a commit to stilor/crosstool-ng that referenced this pull request Feb 23, 2017
The -lcygwin -lc actually breaks the build: elf2flt picks up the symbols for getopt/optarg via
<getopt.h> in binutils-X.Y/include, where optarg is declared without dllimport attribute.
Therefore it pulls in getopt() from libc/libcygwin, but since optarg is not prefixed with
_imp__, it is pulled from libiberty. But the object file in libiberty also contains getopt()
thus resulting in multiple definitions thereof.

While there, kill extraneous -ldl passed into configure - configure detects -ldl successfully.

Upstream: uclinux-dev/elf2flt#6
Signed-off-by: Alexey Neyman <stilor@att.net>
@vapier
Copy link
Member

vapier commented Feb 23, 2017

the full history is in git, so you can simply look at the commit log to see where this code was added -- in commit 4565de8.

deleting this code outright won't work, sorry. you're breaking one system to fix another.

@stilor
Copy link
Contributor Author

stilor commented Feb 23, 2017

What about adding binutils headers dirs after the system dirs (e.g. with -idirafter)? That would match the order of headers with that of the libraries. I haven't tested that yet, but I think it should be another way of fixing Cygwin build.

Or, those blocks can be specifically made dependent on OS X.

The problem is, I don't have a OS X to test.

@vapier
Copy link
Member

vapier commented Feb 23, 2017

getopt is a mess.

the getopt.h include was explicitly added to support mingw. that shouldn't be needed though -- mingw headers provide unistd.h. however, deleting that wouldn't help as mingw will include getopt.h from its unistd.h.

i'm not keen on adding -idirafter as it's not exactly a "standard" flag.

how about we change how we utilize the path from --with-bfd-include-dir. instead of adding an -I flag directly to that path, we create a local dir and copy the few headers we actually care about out of it. then we add a -I flag to that path. it'd avoid any conflicts with the getopt.h header from binutils, and we can keep the symbol conflict logic in place that's in configure.ac now.

@stilor
Copy link
Contributor Author

stilor commented Mar 1, 2017

Ok, the suggested approach: #7

@stilor stilor closed this Mar 1, 2017
@stilor stilor deleted the fix-cygwin-build branch March 20, 2017 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants