microsoft / vcpkg

C++ Library Manager for Windows, Linux, and MacOS
MIT License
23.03k stars 6.36k forks source link

[vcpkg upgrade] Displays `Value was null` instead of how many ports installed, failed, cascaded, etc #24029

Closed StarGate-One closed 2 years ago

StarGate-One commented 2 years ago

Describe the bug [vcpkg-tool] Displays Value was null instead of how many ports installed, failed, cascaded, etc

Environment

To Reproduce Steps to reproduce the behavior:

  1. Open Git Bash Shell
  2. cd /d/vcpkg-tool
  3. git pull
  4. cd /d/vcpkg
  5. git pull
  6. exit Git Bash
  7. Open vs2022 x64 Native command prompt
  8. cd d:\vcpkg
  9. Run cmake to build/compile/link vcpkg-tool release and copy to /d/vcpkg repo (I have script call vcpkg-tool to do this)
  10. ->vcpkg upgrade
    The following packages will be rebuilt:
    * shaderc[core]:x64-windows -> 2021.1#2
    * spirv-headers[core]:x64-windows -> 1.3.204.1
    * spirv-tools[core]:x64-windows -> 2022.1
    Additional packages (*) will be modified to complete this operation.
    If you are sure you want to rebuild the above packages, run this command with the --no-dry-run option.
  11. ->vcpkg upgrade --no-dry-run
    
    The following packages will be rebuilt:
    * shaderc[core]:x64-windows -> 2021.1#2
    * spirv-headers[core]:x64-windows -> 1.3.204.1
    * spirv-tools[core]:x64-windows -> 2022.1
    Additional packages (*) will be modified to complete this operation.
    Removing shaderc:x64-windows (1/6)...
    Elapsed time to handle shaderc:x64-windows: 23.08 ms
    Removing spirv-tools:x64-windows (2/6)...
    Elapsed time to handle spirv-tools:x64-windows: 19.53 ms
    ... ... ...
    -- Installing: D:/vcpkg/packages/shaderc_x64-windows/share/shaderc/copyright
    -- Performing post-build validation
    -- Performing post-build validation done
    Elapsed time to handle shaderc:x64-windows: 20.34 s

Total elapsed time: 3.81 min

RESULTS Value was null (in red bold type)

Expected behavior A clear and concise description of what you expected to happen. Normally something like the following is shown at the end of installing ports/features:

vcpkg-cmake:x64-windows: SUCCEEDED: 1.768 s
vcpkg-cmake-config:x64-windows: SUCCEEDED: 58.45 ms
7zip:x64-windows: SUCCEEDED: 47.52 s
bzip2:x64-windows: SUCCEEDED: 4.963 s
... ... ...
zlib-ng:x64-windows: SUCCEEDED: 11.73 s
zstr:x64-windows: SUCCEEDED: 223.3 ms

SUMMARY FOR x64-windows SUCCEEDED: 176 BUILD_FAILED: 0 POST_BUILD_CHECKS_FAILED: 0 FILE_CONFLICTS: 0 CASCADED_DUE_TO_MISSING_DEPENDENCIES: 0 EXCLUDED: 0 CACHE_MISSING: 0 DOWNLOADED: 0


**Failure logs**
No Failure logs as vcpkg-tool compiles nominally.

**Additional context**
Add any other context about the problem here.
This is observed from vcpkg-tool repo dated 2022-04-07 UTC with commit hash 48be135568c3930571adb03ff1632b8c159c2787 - not sure exactly which commit causes issue as I just had done a fresh complete delete/recreate of vcpkg/vcpkg-tool repos on 2022-04-06 UTC, so it could have been an earlier commit effecting the output of the `vcpkg upgrade` output.
JackBoosY commented 2 years ago

Same result with https://github.com/microsoft/vcpkg/issues/13933

BillyONeal commented 2 years ago

It seems like this can't be complete repro steps since vcpkg upgrade would try to upgrade things installed, but your repro steps don't have anything installed?

StarGate-One commented 2 years ago

I used the ellipsis (... ... ...) after removing port shaderc and before installing shaderc as there was several hundred lines of normal output when vcpkg update --no-dry-run removes all the ports in a certain order and reinstalls them in reverse of the removal order.

My normal process was have a script do vcpkg upgrade without the --no-dry-run to a file that manipulated the output into 2 lines, one did a vcpkg remove against each port and then another doing a vcpkg install against each port, but having to do this manually now since a change was made where vcpkg remove port must have the features brackets blank, before you could leave the features within the output line and vcpkg would remove them. So I just have not got around to rewriting/modifying the script to parse out the features on the vcpkg remove line and leave the features in on the vcpkg install line.

The ouput in the Expected behavior was just the tail of the last vcpkg install done the day prior but was using the output to illustrate what vcpkg upgrade normally puts out at the end but that did not happen this time due to the Value is null message.

Note: The ports did successfully upgrade (compile, link and install) just the message of how many successfully upgraded, etc was pre-empted by the value is null message.

I am not sure this has any relevance, but I did see in this commit for vcpkg-tool file include/vcpkg/build.h a null_value counter was removed in lines 34 and 200.

StarGate-One commented 2 years ago

See results today with on a new clone from earlier in the day 04/08/2022 UTC with a later version update to zziplib. Attached contains the compile of vcpkg-tool and the vcpkg upgrade --no-dry-run --keep-going --debug console outputs. vcpkg-upgrade-20220408.log

StarGate-One commented 2 years ago

Ok more testing -

Part One:

  1. I set vcpkg-tool repo back to commit [localization] more! remove system.print.h from vcpkg/base, tests (#479) a87c448d42e1acf9973ab5e40db0aebd5b7d3325
  2. Built, compiled and linked vcpkg-tool
  3. Set vcpkg repo back to [openssl] Fix building in Release only mode (https://github.com/microsoft/vcpkg/pull/24004[)](https://github.com/microsoft/vcpkg/commit/1b3a783ff15f57bad30bc5fe235e7bca86109706)
  4. git pull on vcpkg repo to make it look like it did after I merged in [spirv-tools,spirv-reflect,spirv-headers] update ports (https://github.com/microsoft/vcpkg/pull/23822[)](https://github.com/microsoft/vcpkg/commit/611dfc1864afbf9085d0cf3a19765c0cf01e8131)
  5. vcpkg upgrade --no-dry-run --keep-going --debug Value was null message was NOT present - see log: a87c448d42e1acf9973ab5e40db0aebd5b7d3325.log

Part Two:

  1. I set vcpkg-tool repo back to commit Implement published test results built in and reduce console chattine… …ss. (#437) 8e8fb17dde45aa1f5c05257377b959b8d583dcd5
  2. repeat steps 2-5 above with the following results in step 5 "Value was null message WAS displayed - see log: 8e8fb17dde45aa1f5c05257377b959b8d583dcd5.log

Something within the vcpkg-tool 8e8fb17dde45aa1f5c05257377b959b8d583dcd5 seems to have broken the vcpkg upgrade output results.

BillyONeal commented 2 years ago

I used the ellipsis (... ... ...) after removing port shaderc and before installing shaderc as there was several hundred lines of normal output when vcpkg update --no-dry-run removes all the ports in a certain order and reinstalls them in reverse of the removal order.

I understand what upgrade does. The problem is that I need to be able to repro the issue in order to fix it, and clearly this repro depends on the state of your installed tree before the upgrade command was issued, so I need to know how to get there. Like, can you include the initial install command at whatever the old SHA was in order to create the situation?

I am not sure this has any relevance, but I did see in this commit for vcpkg-tool file include/vcpkg/build.h a null_value counter was removed in lines 34 and 200.

Right, because the condition that could lead to null values was made structurally impossible. (All constructors setting that value were changed to prohibit nullness and call callers were updated to pass non-nullptr values)

Something within the vcpkg-tool 8e8fb17dde45aa1f5c05257377b959b8d583dcd5 seems to have broken the vcpkg upgrade output results.

Wouldn't be the first time I broke something...

BillyONeal commented 2 years ago
  • Vcpkg Version [2022-04-07-48be135568c3930571adb03ff1632b8c159c2787]

I also missed that we were talking about a not-yet-released tool version here, sorry. (I normally just look and go "that's not older than the last time we changed something big")

BillyONeal commented 2 years ago

OK I am able to repro now, stand by...

StarGate-One commented 2 years ago
BillyONeal commented 2 years ago
  • Ok I was hoping my last post would be enough to give you a starting point - if not, I will try to capture more details/information the next time.

Yeah, because I didn't realize it was a prerelease copy, I tried with the release copy, and it worked, so I assumed it had to be something special about your installed tree to create the condition. The actual issue was that I didn't realize it was prerelease :)

  • I know it is not a release version yet, and do not expect anyone to drop what they are doing to address it, as you all have way too many more important things on your plate, but I did want to let you all know before it somehow makes it into a release version and becomes a fire needing to be put out immediately after so many users start encountering the issue, as I have notice many are not so understanding or forgiving.

Sorry, I was explaining how I got to this place, not complaining about it being a prerelease version.

  • Does the CI do any vcpkg upgrade --no-dry-run tests?

Unfortunately not. I'm not sure exactly how to make that repeatable....

  • If not, it may be something to be added to the never ending list of nice to have features or not?

Yeah, if there's a practical way to turn that into an e2e test that would be nice.

  • I can work around it, and if my vcpkg upgrade list is too large, I just delete both the vcpkg and vcpkg-tool repos and build again from scratch, it only takes a few hours at most with the few ports I use.

Nah, we should fix it!

  • Again thank you and the rest of the team, you all put in a lot of hard work to keep this going and I for one appreciate it very much.

Thank you!

https://github.com/microsoft/vcpkg-tool/pull/500 should fix it I hope

BillyONeal commented 2 years ago

Actually I'm going to close this even though the tool release with the fix isn't out yet because the bug was never experienced in a released tool. Thanks for the report!

StarGate-One commented 2 years ago

@BillyONeal You are most welcome! I appreciate all you do. Have a wonderful week.

StarGate-One commented 2 years ago

@BillyONeal Works as advertised now - thanks a lot.

Total elapsed time: 4.945 min

RESULTS cpuinfo:x64-windows: REMOVED: 19.25 ms directxtk:x64-windows: REMOVED: 17.9 ms dxsdk-d3dx:x64-windows: REMOVED: 23.33 ms libgd:x64-windows: REMOVED: 16.61 ms fontconfig:x64-windows: REMOVED: 32.93 ms expat:x64-windows: REMOVED: 13.28 ms imgui-sfml:x64-windows: REMOVED: 6.677 ms ogre:x64-windows: REMOVED: 206.1 ms imgui:x64-windows: REMOVED: 17.77 ms glfw3:x64-windows: REMOVED: 8.979 ms xaudio2redist:x64-windows: REMOVED: 11.09 ms cpuinfo:x64-windows: SUCCEEDED: 13.29 s xaudio2redist:x64-windows: SUCCEEDED: 974.3 ms directxtk:x64-windows: SUCCEEDED: 37.62 s dxsdk-d3dx:x64-windows: SUCCEEDED: 734.4 ms expat:x64-windows: SUCCEEDED: 10.39 s fontconfig:x64-windows: SUCCEEDED: 31.08 s glfw3:x64-windows: SUCCEEDED: 5.415 s imgui:x64-windows: SUCCEEDED: 8.392 s imgui-sfml:x64-windows: SUCCEEDED: 5.751 s libgd:x64-windows: SUCCEEDED: 10.6 s ogre:x64-windows: SUCCEEDED: 2.859 min

SUMMARY FOR x64-windows SUCCEEDED: 11 REMOVED: 11