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

locate_tools: add detailed warnings for vcvarsall.bat errors #320

Merged
merged 9 commits into from
May 25, 2023

Conversation

Javagedes
Copy link
Contributor

Adds additional logging to QueryVcVariables() when calling vcvarsall.bat. command prompt arguments are limited to 8191 characters, which includes ENV variables. On top of that, any ENV variable that exceeds 8191 characters itself is not passed to a command.

This causes two different errors when confusing:

  1. When the total env is greater than 8191 characters (but a specific variable is not), the command fails with an ambigious error Exception: The input line is too long. The syntax of the command is incorrect.

  2. When a specific variable is greater than 8191 characters (commonly the PATH variable), it is not passed to the command. This means that the command does not fail, however specifically for vcvarsall.bat, we might fail to find what we are searching for because the folder it resides in could set in PATH.

This commit adds the following:

  1. When the PATH variable is greater than 8191 characters, a warning will notify the user that PATH will not be included when searching for their keys.
  2. When an exception occurs stating that the input line is too long, we expand on the exception to provide a clear message as to why the error is occurring.

@Javagedes Javagedes added the bug Something isn't working label May 4, 2023
@Javagedes Javagedes added this to the 0.15.0 milestone May 4, 2023
@Javagedes Javagedes self-assigned this May 4, 2023
@Javagedes Javagedes linked an issue May 4, 2023 that may be closed by this pull request
@codecov
Copy link

codecov bot commented May 4, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.03 🎉

Comparison is base (933c6ce) 78.75% compared to head (3ea9b52) 78.79%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #320      +/-   ##
==========================================
+ Coverage   78.75%   78.79%   +0.03%     
==========================================
  Files          47       47              
  Lines        6703     6710       +7     
==========================================
+ Hits         5279     5287       +8     
+ Misses       1424     1423       -1     
Impacted Files Coverage Δ
edk2toollib/windows/locate_tools.py 87.28% <100.00%> (+0.85%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Javagedes Javagedes requested a review from makubacki May 12, 2023 17:11
@Javagedes Javagedes merged commit 7491524 into tianocore:master May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Exception in locate_tools with specific environments
3 participants