Skip to content

Deprecated response files. #547

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

Closed
wants to merge 1 commit into from
Closed

Deprecated response files. #547

wants to merge 1 commit into from

Conversation

VZout
Copy link

@VZout VZout commented Aug 24, 2020

To simplify the code I removed response files based on the suggestion of @alexcrichton. I now call the assemble commands multiple times.

This solves #496 as well.

Currently I'm using the same logic that splits the assemble command for all hosts. Let me know if you only want to split it only for Windows.

@alexcrichton
Copy link
Member

Thanks for this! Have you confirmed that this works on both gnu-like platforms and MSVC? I think we need to be sure to pass various flags to ar and lib.exe to get this to work, but I'm not sure if we do that by default. (if possible adding a test would be great)

@VZout
Copy link
Author

VZout commented Aug 25, 2020

Thanks for this! Have you confirmed that this works on both gnu-like platforms and MSVC? I think we need to be sure to pass various flags to ar and lib.exe to get this to work, but I'm not sure if we do that by default. (if possible adding a test would be great)

I was able to compile physx-rs for windows and for android without problem. But I did minimal testing. I'll double check to make sure everything works fine.

@alexcrichton
Copy link
Member

To confirm, when you say you compile for Windows, do you mean MSVC or MinGW?

@VZout
Copy link
Author

VZout commented Aug 25, 2020

MSVC. I'll try MinGW/GCC tomorrow. Compiling for android using clang worked.

@VZout
Copy link
Author

VZout commented Aug 31, 2020

Apparently this doesn't work just work 🤦‍♂️ Must have been building against the wrong local version... My bad.

I couldn't find any documentation on lld or lib.exe on how to do progressive linking like this. Do you have a reference for the various flags that need to be passed?

Also this solution is not 100% fool prove since in theory a single argument could exceed the command line limit.

@alexcrichton
Copy link
Member

Sorry I'm not super familiar with lib.exe, but with ar I'm pretty sure there's options to keep appending to the end of the archive but I'm unfortunately not sure what they are.

@Jake-Shadle
Copy link
Contributor

By default, LIB names the output file using the base name of the first object or library file and the extension .lib. The output file is put in the current directory. If a file already exists with the same name, the output file replaces the existing file. To preserve an existing library, use the /OUT option to specify a name for the output file.

The documentation for lib.exe seems to indicate that using /OUT will append if the library already exists, should be easy enough to test though by calling it twice with different object files then running /LIST.

@VZout
Copy link
Author

VZout commented Sep 17, 2020

By default, LIB names the output file using the base name of the first object or library file and the extension .lib. The output file is put in the current directory. If a file already exists with the same name, the output file replaces the existing file. To preserve an existing library, use the /OUT option to specify a name for the output file.

The documentation for lib.exe seems to indicate that using /OUT will append if the library already exists, should be easy enough to test though by calling it twice with different object files then running /LIST.

Huh I tried that a little while ago but didn't work. Will try again.

@maxded maxded mentioned this pull request Nov 17, 2020
@VZout VZout closed this Nov 17, 2020
@VZout
Copy link
Author

VZout commented Nov 17, 2020

@maxded Solved the issue I was having!

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