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

optionally include active templating library #59

Merged
merged 1 commit into from
Sep 7, 2022

Conversation

pascalkuthe
Copy link
Contributor

The microsoft active templating library (ATL) is used by some c++ libraries that rust crates might wish to link to (the lld linker for my usecase). This PR adds an optional flag that includes ATL with the installation.
I currently did not add any symlinks, because I am not sure if they are required (I personally do not work on windows), before merging it should be resolved whether any links are needed

@Jake-Shadle
Copy link
Owner

The links are not needed on windows, they are only present to resolve casing differences on case-sensitive file systems.

@pascalkuthe
Copy link
Contributor Author

I know that symlinks are just for emulating a case insensitive filesystem. I mean with my comment that I personally do not develop on windows so I would not know if there is any sourcecode that uses uppercase names for atl.

However it seems that atl is internally all lowercase so I think that symlinks are not required.
I have remove the comment (which should make clippy happy).
I have also fixed the testsuite so the testsuite will hopefulkly pass now.

In the meantime I have found a way to remove the atl dependency from my personal usease but the option might be nice for other people nontheless

Copy link
Owner

@Jake-Shadle Jake-Shadle left a comment

Choose a reason for hiding this comment

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

Thanks! I think I'll merge this as is, if there are issues with casing someone else who actually hits those can provide any fixes.

@Jake-Shadle Jake-Shadle merged commit 3b63e4f into Jake-Shadle:main Sep 7, 2022
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