AcademySoftwareFoundation / rez

An integrated package configuration, build and deployment system for software
https://rez.readthedocs.io
Apache License 2.0
931 stars 329 forks source link

Visual Studio Build Tools prompt invocation is broken. #483

Open KelSolaar opened 6 years ago

KelSolaar commented 6 years ago

Hi,

I just noticed that lately and this seems to be happening since we moved to Rez 2.16.0 last year but invoking the Visual Studio Build Tools prompt (2015 or 2017) is currently busted.

I noticed it because I did not have cl and dumpbin when invoking the prompt. Before when issuing:

rez-env vs_buildtools -- vcvars64

I was put in the Visual Studio Build Tools 2017 prompt and had access to cl, etc... Now I actually need to do it staged:

rez-env vs_buildtools

Waiting for CMD prompt and then:

vcvars64

To properly spawn the Visual Studio Build Tools 2017 prompt.

I'm not exactly sure what is the issue, probably usual Windows pain but I wanted to flag it.

Cheers,

Thomas

KelSolaar commented 4 years ago

I was wrangling Jira on our side and this one is still not working as of Rez 2.47.2.

bfloch commented 4 years ago

I can not reproduce the problem. Can you share the contents of vs_buildtools and the location of vcvars64?

I created a package with env.PATH.append("{root}\\batch") In the batch subfolder I have a hello_world.bat file that echos the obvious.

I can run any of these commands just fine:

rez-env --shell=powershell issue_483_test -- hello_world
rez-env --shell=cmd issue_483_test -- hello_world
KelSolaar commented 4 years ago

Hi,

Here are some repro steps:

> ***>rez-env vs_buildtools -- vcvars64
**********************************************************************
** Visual Studio 2017 Developer Command Prompt v15.0
** Copyright (c) 2017 Microsoft Corporation
**********************************************************************
[vcvarsall.bat] Environment initialized for: 'x64'

> ***>dumpbin
'dumpbin' is not recognized as an internal or external command,
operable program or batch file.

> ***>rez-env vs_buildtools
resolved by ***, on Fri Sep 27 09:31:02 2019, using Rez v2.47.2

requested packages:
vs_buildtools
~platform==windows       (implicit)
~arch==AMD64             (implicit)
~os==windows-10.0.17763  (implicit)

resolved packages:
vs_buildtools-2017  ***\vs_buildtools\2017
>> ***>vcvars64
**********************************************************************
** Visual Studio 2017 Developer Command Prompt v15.0
** Copyright (c) 2017 Microsoft Corporation
**********************************************************************
[vcvarsall.bat] Environment initialized for: 'x64'

>> ***>dumpbin
Microsoft (R) COFF/PE Dumper Version 14.10.25019.0
Copyright (C) Microsoft Corporation.  All rights reserved.

usage: DUMPBIN [options] [files]

   options:

      /ALL
      /ARCHIVEMEMBERS
      /CLRHEADER
      /DEPENDENTS
      /DIRECTIVES
      /DISASM[:{BYTES|NOBYTES}]
      /ERRORREPORT:{NONE|PROMPT|QUEUE|SEND}
      /EXPORTS
      /FPO
      /HEADERS
      /IMPORTS[:filename]
      /LINENUMBERS
      /LINKERMEMBER[:{1|2}]
      /LOADCONFIG
      /NOLOGO
      /OUT:filename
      /PDATA
      /PDBPATH[:VERBOSE]
      /RANGE:vaMin[,vaMax]
      /RAWDATA[:{NONE|1|2|4|8}[,#]]
      /RELOCATIONS
      /SECTION:name
      /SUMMARY
      /SYMBOLS
      /TLS
(press <return> to continue)
      /UNWINDINFO

All the Visual Studio environment variables are not present too with the first invocation call.

For reference, our vs_buildtools package pretty much only add C:\\Program Files (x86)\\Microsoft Visual Studio\\2017\\BuildTools\\VC\\Auxiliary\\Build to %PATH%.

bfloch commented 4 years ago

Maybe this is a misunderstanding of the -c and -- parameters:

rez-env vs_buildtools -- vcvars64

... will resolve the environment, create a subprocess/shell with this environment, run the vcvars64 command and then exit the shell/subprocess.

This means that all the variable you would want to have will be gone after the execution. I think you can achieve what you want via:

rez-env vs_buildtools -c "vcvars64 & dumpbin"

However this will only work for the single command (your build entry point?).

If you want to just evaluate the context on your current shell you probably want to evaluate the resulting .bat file. I am not sure how to do it without a temporary file but this should work:

rez-env vs_buildtools -c "copy /Y %REZ_CONTEXT_FILE% C:\temp\context.bat" & C:\temp\context.bat

vcvars64
dumpbin

Let me know if this works for you.

KelSolaar commented 4 years ago

As far as I remember, it was not exiting the Visual Studio command prompt before we moved to 2.16.0, as a result, you would get inside the prompt and ready to use it.

bfloch commented 4 years ago

If it did not it just may mean that it was broken before. There has been some confusion on cmd closing behavior because its parameters act weird depending on what you want to do with the shell.

It definitely is behaving correctly now to my best knowledge.

To demonstrate I reduced your output. The first sample evaluates but throws you back into the original (unmodified) environment, as indicated by the single > prompt.

> ***>rez-env vs_buildtools -- vcvars64
> ***>dumpbin
'dumpbin' is not recognized as an internal or external command,
operable program or batch file.

This is keeping the shell open within the evaluated environment >>:

> ***>rez-env vs_buildtools
>> ***>vcvars64
>> ***>dumpbin

Rez was designed to work like this so that you could clean up environments properly. It should work like this across any OS and shell.

Sorry for the confusion.

KelSolaar commented 4 years ago

To be fair, from a user standpoint, I don't know if it is the behaviour you would expect naturally. Keep in mind that I'm purposely ignoring technical constraints here, but:

bfloch commented 4 years ago

I was maybe missing some context here, sorry for that.

I think what might be happening is that we hit a limitation in cmd. If I recall correctly we can either have the shell being kept open when calling a batch file via cmd as in your case illustrated here - but then rez env would always not close and we need to exit it manually as its own environments are also batch scripts. This fails most of the windows tests. In the past the setting has been bounced back and forth - lately here:

https://github.com/nerdvegas/rez/issues/691

... which I had to revert in https://github.com/nerdvegas/rez/pull/698/commits/07d07cce65b861faebb0b132d9c0ce0a767e82e6

I don't think we can fix it other then pressing Microsoft to finally convert their build environment to PowerShell. But I'll try to clarify and look into it next week. Maybe there is a way that we all have missed so far.

I wonder if something along this might work as a more sensible workaround (untested I don't use Windows beyond work):

rez-env vs_buildtools -- cmd /k vcvars64

This might also interest you: https://github.com/nerdvegas/rez/issues/703 I want to convert the vcvars.bat script into a static rez-env script. This way we don't need to rely on it and it would also work on any shell. There is also other attempts from cmake, microsoft_craziness.h etc. as mentioned in the thread.

Let's see if we can find a way. Again sorry for the confusion.

KelSolaar commented 4 years ago

rez-env vs_buildtools -- cmd /k vcvars64

Would be surely the way to go, it just does not feel natural. I can see how it is a tricky situation, Windows is full of quirks like that. Just spent my afternoon on #752 :)

bfloch commented 4 years ago

Bumping this buy back into view. I might risk another take on this - obviously we want batch execution to work like this. I just don't know what the cost would be, since the changes we had would break another half of the system. Maybe we need to introduce a hacky option for cmd sake only or similar. Not sure.

In general I tend to suggest people to move away from cmd and its broken design and no prospect to be ever fixed. But your use-case is a valid one until we finally get a better way to get the compiler/platform from Microsoft :-/ Hopefully we progress in #703 as mentioned before.