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 with llvm-dlltool #101

Closed
wants to merge 1 commit into from
Closed

Conversation

neheb
Copy link

@neheb neheb commented Dec 21, 2021

It doesn't support the full versions of these options.

It doesn't support the full versions of these options.
@m417z
Copy link
Collaborator

m417z commented Nov 11, 2022

From what I see, llvm-dlltool supports --dllname and --output-lib, but not --def (--input-def can be used instead):
https://github.com/llvm/llvm-project/blob/99d3ead44cfb21dca24c63a0b0731eaad54b491e/llvm/lib/ToolDrivers/llvm-dlltool/Options.td#L6-L13

I see that GNU binutils also supports --input-def, and --def only exists for compatibility:
https://github.com/haiku/buildtools/blob/57c9179de2ca2850f82c28d34123b933ee24368e/binutils/binutils/dlltool.c#L3737

So if you change your PR to replace --def with --input-def, I'll merge it.

@neheb
Copy link
Author

neheb commented Nov 11, 2022

That link says -d is the same as input-def

edit: the second one that is.

@m417z
Copy link
Collaborator

m417z commented Nov 11, 2022

Yes, your change looks correct, but I see no reason to change to the short options. Replacing --def with --input-def would be a smaller change which still fixes the incompatibility with llvm-dlltool.

@m417z m417z closed this in 49d03ad Nov 12, 2022
fengjixuchui added a commit to fengjixuchui/minhook that referenced this pull request Mar 3, 2023
Use LLVM-compatible dlltool flags, fixes TsudaKageyu#101
dand-oss pushed a commit to dand-oss/minhook that referenced this pull request Aug 10, 2023
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.

2 participants