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

Also create symlinks when running on Windows #100

Closed
wants to merge 1 commit into from

Conversation

sykhro
Copy link
Contributor

@sykhro sykhro commented Sep 29, 2023

The code used to ignore symlinks on Windows, breaking support for clang-cl. I have added code to handle them, however this might change the behaviour of the application as creating symlinks on Windows requires the SeCreateSymbolicLinkPrivilege privilege (meaning the user needs to be Administrator or have Developer Mode enabled).

In my opinion, users with no such privilege should pass the --disable-symlinks flag (or maybe make that the default on Windows?)

@sykhro
Copy link
Contributor Author

sykhro commented Sep 29, 2023

pinging @Jake-Shadle because github suppresses notifications on draft PRs, sorry about any noise

@Jake-Shadle
Copy link
Owner

I don't understand, how does not creating syminks break clang-cl on windows? The reason for syminks is to fix casing issues on case-sensitive file systems, which windows does not have AFAIK.

@sykhro
Copy link
Contributor Author

sykhro commented Sep 29, 2023

I don't understand, how does not creating syminks break clang-cl on windows? The reason for syminks is to fix casing issues on case-sensitive file systems, which windows does not have AFAIK.

The /vctoolsdir and /winsdkdir folders need the versioned folders, which are symlinks (#47)

@Jake-Shadle Jake-Shadle marked this pull request as ready for review November 7, 2023 07:38
@Jake-Shadle Jake-Shadle self-requested a review as a code owner November 7, 2023 07:38
@Jake-Shadle
Copy link
Owner

I've merged this in #105

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