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

Resolve "Error finding an LLVM build" (#1831) #1832

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cmcneish
Copy link

Pass through Visual Studio version from build.sh into build/LLVM.lua, so that the downloaded version is consistent with build.sh's view rather than the host command prompt's view.

This should fix #1831.

Questions (1st time I've written any premake...):

  • Does this only need to go to premakes with --file=LLVM.lua?
  • Should the newoption exist? Premake didn't seem to need it.
  • Should the newoption have a list of supported versions? Seems like build.sh is responsible for validation here, but I'm not sure

Pass through Visual Studio version from build.sh into build/LLVM.lua, so that
the downloaded version is consistent with build.sh's view rather than the host
command prompt's view.
@cmcneish
Copy link
Author

@dotnet-policy-service agree

@tritao
Copy link
Collaborator

tritao commented Feb 20, 2024

The only problem I see with this is that now we will always set the vs option in Premake, so it will not act as an override.

I think you can rename the existing vs variable to action_vs and add a new vs that is empty by default. Then --vs=$vs will end up as --vs= and OPTIONS["vs"] will not be set anymore.

@cmcneish
Copy link
Author

Sorry about the delay, I don't have too much time to work on this. (If you want to finish this up I don't mind)

I think you can rename the existing vs variable to action_vs and add a new vs that is empty by default. Then --vs=$vs will end up as --vs= and OPTIONS["vs"] will not be set anymore.

Is the idea here to provide a different override for LLVM's VS version than the one used to build CppSharp? Would this warrant a new top-level argument in build.sh (say, --llvm_vs)?

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.

LLVM.lua does not get VS version from build.sh ("Error finding an LLVM build")
2 participants