Skip to content

Conversation

@BillyONeal
Copy link
Member

@BillyONeal BillyONeal commented Oct 7, 2025

  • Audit for things that should be NuGet and not Nuget or nuget
  • system.process.h: Extract report_nonzero_exit_code and report_nonzero_exit_code_and_output, and teach to avoid printing the console output when echo_in_debug
  • binarycaching.cpp: This is the same DiagnosticContext plumbing done several times before. Specific notes:
    • We always print the console output now unless it's one of the failure modes we know about.
    • We check for the interactive console output after checking a nonzero exit code, not before.
    • We no longer claim --debug will do anything for the push operation. In particular that isn't safe in the face of pipelined uploads and we shouldn't be recommending --debug in any case. (We still claim that --debug does something for uploads)

Related: microsoft/vcpkg#43309

…caching.

Prior to this we were dropping NuGet console output on the floor requiring folks to use --debug.

There is a behavior change here in that if the exit code was 0 before but the console output included "Authentication may require manual action." we would print a warning anyway, now the warning is only printed on failure.

Drive by: Audit for 'Nuget' -> 'NuGet' for correct branding.
Comment on lines -856 to -861
if (!m_cmd.push(msg_sink, nupkg_path, nuget_sources_arg({&write_src, 1})))
{
msg_sink.println(Color::error,
msg::format(msgPushingVendorFailed, msg::vendor = "NuGet", msg::path = write_src)
.append_raw('\n')
.append(msgSeeURL, msg::url = docs::troubleshoot_binary_cache_url));
Copy link
Member Author

@BillyONeal BillyONeal Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously this replaced all rich error information from NuGet with the generic msgPushingVendorFailed. Ditto below

Comment on lines +826 to +827
WarningDiagnosticContext wdc{console_diagnostic_context};
m_cmd.install(wdc, packages_config, m_packages, m_src);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This previously entirely ignored all output from NuGet!

@BillyONeal BillyONeal merged commit 7c3408c into microsoft:main Oct 8, 2025
7 checks passed
@BillyONeal BillyONeal deleted the nuget-output branch October 8, 2025 23:21
vicroms pushed a commit to vicroms/vcpkg-tool that referenced this pull request Oct 9, 2025
Prior to this we were dropping NuGet console output on the floor requiring folks to use --debug.

There is a behavior change here in that if the exit code was 0 before but the console output included "Authentication may require manual action." we would print a warning anyway, now the warning is only printed on failure.

Drive by: Audit for 'Nuget' -> 'NuGet' for correct branding.
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.

3 participants