Skip to content

Conversation

LKedward
Copy link
Member

@awvwgk, response files seem to work on Windows if the objects paths are written in unix style.

Fixes #427.

@awvwgk
Copy link
Member

awvwgk commented Apr 17, 2021

The error on Windows seemed like an issue with the path of the response file, and I was planning to look into changing the directory first before creating the response file, but if the same issue also applies for the content of the response file than this won't work at all.

Now I would be intrigued how well this plays together with #442 when lib instead of ar is used.

@LKedward
Copy link
Member Author

Now I would be intrigued how well this plays together with #442 when lib instead of ar is used.

Looks like response files are supported with lib but we'll probably have to add some ugly logic in the backend to choose between unix_path/windows_path when writing the response file.

@awvwgk
Copy link
Member

awvwgk commented Apr 17, 2021

Maybe creating an archiver_t that knows how to create a static library would work. The logic to determine which path to choose than goes in the instantiation of the object rather than the target evaluation.

@everythingfunctional everythingfunctional self-requested a review June 4, 2021 17:01
@everythingfunctional
Copy link
Member

I hit this issue on two of my projects in the last two days, so I'm keen to get this merged in. I think this looks good enough to me, but I'll give @LKedward and @awvwgk a chance to hold it back if they think it needs to be.

@awvwgk
Copy link
Member

awvwgk commented Jun 4, 2021

Brad @everythingfunctional, would you mind testing this with lib instead of ar if you have access to a Windows machine (I don't have one). I might have time this weekend to get #451 up to speed, which gives a more robust way to handle response files (they don't work on OSX if I recall correctly).

@everythingfunctional
Copy link
Member

@awvwgk , I've got a couple of Windows environments, unfortunately both have things installed and setup with ar available in the PATH, so I can't be sure that it will work with lib, but I can say that it does work with ar. My understanding is that the response file is a feature of the Windows "command shell", not of the executables, so I don't see why they would behave any different.

@LKedward
Copy link
Member Author

LKedward commented Jun 5, 2021

Thanks for updating and reviewing @everythingfunctional - I'm happy for this to move forward 👍
I'm not able to test with lib unfortunately since I don't have visual studio installed.

Copy link
Member

@awvwgk awvwgk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this forward and revisit it in case it breaks with lib.

@LKedward
Copy link
Member Author

LKedward commented Jun 5, 2021

Thanks both - I'll now merge then.

@LKedward LKedward merged commit e0e6afe into fortran-lang:master Jun 5, 2021
@LKedward LKedward deleted the response-files branch June 5, 2021 12:33
@everythingfunctional
Copy link
Member

Brad @everythingfunctional, would you mind testing this with lib instead of ar if you have access to a Windows machine (I don't have one). I might have time this weekend to get #451 up to speed, which gives a more robust way to handle response files (they don't work on OSX if I recall correctly).

I did get a chance to set up a Windows environment and test with lib. It works, so we're all good.

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.

Command line is too long
3 participants