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

Use direct call to external tool #29

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

Et7f3
Copy link

@Et7f3 Et7f3 commented Nov 8, 2021

I put here because it touch internal of compiler.

I am ok if I have to fix project before we take decision.

@Et7f3
Copy link
Author

Et7f3 commented Nov 8, 2021

I forced pushed to remove my commit and GitHub closed it 🤦‍♂️

@Et7f3 Et7f3 reopened this Nov 8, 2021
@Et7f3 Et7f3 changed the title Create Just guess the number Use direct call to external tool Nov 8, 2021
@Et7f3 Et7f3 marked this pull request as ready for review November 8, 2021 22:23
@nojb
Copy link

nojb commented Nov 8, 2021

Another con: -ccopt and -cclib will no longer work in general.

@Et7f3
Copy link
Author

Et7f3 commented Nov 9, 2021

Why ?

  -cclib <opt>  Pass option <opt> to the C linker
  -ccopt <opt>  Pass option <opt> to the C compiler and linker

I see pass option in singular form. The argument are already splited and we concatenate them to reconstitute command line.

I have checked with clang:

% clang -Wl,'-Z -Z'
ld: unknown option: -Z -Z
clang: error: linker command failed with exit code 1 (use -v to see invocation)
% clang -Wl,-Z
ld: library not found for -lSystem
clang: error: linker command failed with exit code 1 (use -v to see invocation)

And for gcc:

% x86_64-w64-mingw32-gcc -Wl,'-E -E'
/usr/lib/gcc/x86_64-w64-mingw32/7.4.0/../../../../x86_64-w64-mingw32/bin/ld: unrecognized option '-E -E'
/usr/lib/gcc/x86_64-w64-mingw32/7.4.0/../../../../x86_64-w64-mingw32/bin/ld: use the --help option for usage information
collect2: error: ld returned 1 exit status
% x86_64-w64-mingw32-gcc -Wl,-E
/usr/lib/gcc/x86_64-w64-mingw32/7.4.0/../../../../x86_64-w64-mingw32/bin/ld: warning: --export-dynamic is not supported for PE+ targets, did you mean --export-all-symbols?
/usr/x86_64-w64-mingw32/sys-root/mingw/lib/../lib/libmingw32.a(lib64_libmingw32_a-crt0_c.o): In function `main':
/usr/src/debug/mingw64-x86_64-runtime-6.0.0-1/crt/crt0_c.c:18: undefined reference to `WinMain'
collect2: error: ld returned 1 exit status

Ok if we want the behavior of go then it might be an issue:

-extldflags flags
	Set space-separated flags to pass to the external linker.

The workaround is to split on space maybe in bonus respect quotation or ask to escape space with \x32 for instance

@nojb
Copy link

nojb commented Nov 9, 2021

See the discussion at ocaml/ocaml#9482

@dra27
Copy link
Member

dra27 commented Nov 9, 2021

It's not clear to me from the esy issue you cite - are you simply offended that the process tree is deep, or does it actually cause an issue? The invocation of the linker is not - or certainly shouldn't be - the slow step of builds. It's mentioned in the discussions @nojb's linked, I think a prerequisite to calling these executables directly would be having a complete story for what to do with existing uses of -ccopt and -cclib to allow those fields to be stored as string list in .cma/.cmxa files.

@Et7f3
Copy link
Author

Et7f3 commented Nov 9, 2021

I am simply offended. Ok so first I need to convert this in string list ocaml/ocaml#9482 (comment) it should be a simple refactoring.

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.

3 participants