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

Make vcvarsall.bat detection more robust #25225

Closed
MeFisto94 opened this issue Dec 10, 2019 · 5 comments
Closed

Make vcvarsall.bat detection more robust #25225

MeFisto94 opened this issue Dec 10, 2019 · 5 comments

Comments

@MeFisto94
Copy link
Contributor

@MeFisto94 MeFisto94 commented Dec 10, 2019

On my system, there are two installs of Visual Studio:

  • Visual Studio 2015
  • Visual Studio 2017 Community

Now it seems that I don't even have a proper Visual Studio 2015 installed, which may be some kind of source for the problem, but using 2015 probably won't work either.
Windows says I only have VS 2015 Shell (Isolated) and VS 2015 Tools for Unity installed.

The problem is, that Visual Studio 2015 contains C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat.
This is what mach is picking up (VS140COMNTOOLS=C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\Tools\ is set) and thus it fails to locate MSBuild ("Can't find MSBuild.exe installation under C:\Program Files (x86)\Microsoft Visual Studio.")

The real path should be E:\Programme\Microsoft Visual Studio\2017\Community\VC\Auxiliary\Build\vcvarsall.bat

Maybe mach could just search all paths, launch the vcvarsall.bat, try to invoke msbuild.exe and use it to query the version of VS and then use the most recent one. I have to look into the registry as well, if there is something pointing to the VS 2017 Install Directory.

Workaround Number 2 from https://github.com/servo/servo/blob/master/README.md#troubleshooting-a-windows-environment makes MSBuild.exe available, but isn't a solution for me either, because I couldn't add python to the path, because Windows seems to truncate env vars to 1024 characters [and %PATH% got much longer than that when using said prompt]

Apart from that I'd want to go the unsupported way of making mach work under mozilla-build, i.e. the regular firefox environment, because I'm all set there (python installed, git installed, several tools and configurations).
It seems it suffers the same problem as mach.bat: just being unable to locate the vcvarsall.bat file.

I understand there might be no official support for my undertaking, but maybe someone else is willing to help out or my findings can help them with their environment somehow, because given my current situation, I am unable to build servo either way.

Edit1: Do note that multiple folders within the Visual Studio Folder are on the Path by default, so it might be an idea to scan %PATH% for vcvarsall.bat

@MeFisto94
Copy link
Contributor Author

@MeFisto94 MeFisto94 commented Dec 10, 2019

Actually, I've found a/the solution, but it seems @indygreg has already applied it at least to the part where firefox tries to locate cl.exe etc.

It was in this https://developercommunity.visualstudio.com/content/problem/2813/cant-find-registry-entries-for-visual-studio-2017.html arcticle:

The solution was:

For potentially multiple installs of VS2017, e.g. Enterprise, Preview, Community all installed, each install will have a unique directory under C:\ProgramData\Microsoft\VisualStudio\Packages\_Instances. In each directory, there are json files containing data on each install.

e.g. I have a directory called "6bb4a47b" under the _Instances folder which contains information on my VS2017 Enterprise Preview I've installed with the RTM version. In here, there is a large state.json file which includes a value installationPath which is C:\Program Files (x86)\Microsoft Visual Studio\Preview\\Enterprise in my case.

It requires json parsing, but for me it was also able to work (the folder is there, even when you've installed VS 2017 on a completely different partition).

Now before I dive deep into that, is there some mach API which we can use to use the location?

@jdm
Copy link
Member

@jdm jdm commented Dec 10, 2019

What sort of API are you looking for? mach is pretty small: https://searchfox.org/mozilla-central/source/python/mach/mach

@MeFisto94
Copy link
Contributor Author

@MeFisto94 MeFisto94 commented Dec 12, 2019

Actually I am looking for the part where mach (if it's really mach) is detecting the VS Source Path as described in that article. Maybe it's not in mach but it's definitely in some python code, so it can't be part of configure/autoconf.
Gecko uses this to determine the path to CL.exe, which is close to the MSBuild.exe/vcvarsall that we need

Note that this issue is about allowing non-hardcoded VS Installation Paths, which for me seems common, especially when the Partition C is a small SSD and most data resides on another Partition (VS with all Options is just large)

As a note to myself:

@MeFisto94
Copy link
Contributor Author

@MeFisto94 MeFisto94 commented Dec 13, 2019

So I found what I'm looking for, but I'd like some feedback:

Essentially we can take the way gecko does this, see here: https://searchfox.org/mozilla-central/rev/923eec8d2fb8078ebc7a33a9e1ce73eac01f7446/build/moz.configure/toolchain.configure#581

We can then walk the extra mile to analyse the installed packages, so we could warn the user if they didn't install the correct payloads.
Technically it's just another json parse.
I also assume we don't have anything like gecko's python-ish configure, as we build with cargo?

@jdm
Copy link
Member

@jdm jdm commented Dec 13, 2019

Correct, there is no configure step. Just mach commands, then cargo.

bors-servo added a commit that referenced this issue Dec 17, 2019
Mach: Improve Visual Studio detection for non-standard-path installations

Improve locating the Visual Studio installation

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25225
- [X] These changes do not require tests because testing build infrastructure is difficult
bors-servo added a commit that referenced this issue Dec 17, 2019
Mach: Improve Visual Studio detection for non-standard-path installations

Improve locating the Visual Studio installation

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25225
- [X] These changes do not require tests because testing build infrastructure is difficult
bors-servo added a commit that referenced this issue Dec 17, 2019
Mach: Improve Visual Studio detection for non-standard-path installations

Improve locating the Visual Studio installation

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25225
- [X] These changes do not require tests because testing build infrastructure is difficult
bors-servo added a commit that referenced this issue Dec 17, 2019
Mach: Improve Visual Studio detection for non-standard-path installations

Improve locating the Visual Studio installation

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25225
- [X] These changes do not require tests because testing build infrastructure is difficult
bors-servo added a commit that referenced this issue Dec 17, 2019
Mach: Improve Visual Studio detection for non-standard-path installations

Improve locating the Visual Studio installation

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25225
- [X] These changes do not require tests because testing build infrastructure is difficult
bors-servo added a commit that referenced this issue Dec 17, 2019
Mach: Improve Visual Studio detection for non-standard-path installations

Improve locating the Visual Studio installation

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25225
- [X] These changes do not require tests because testing build infrastructure is difficult
bors-servo added a commit that referenced this issue Jan 8, 2020
Mach: Improve Visual Studio detection for non-standard-path installations

Improve locating the Visual Studio installation

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25225
- [X] These changes do not require tests because testing build infrastructure is difficult
@bors-servo bors-servo closed this in 9f971f7 Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.