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

[vcpkg] proper errorcheck during files installation #12378

Merged
merged 2 commits into from
Jul 13, 2020

Conversation

Maximus5
Copy link
Contributor

@Maximus5 Maximus5 commented Jul 11, 2020

If during install_files_and_write_listfile an error happens in for (auto&& file : files) loop, every file in files is skipped due to "saved" error, even if fs.symlink_status(file, ec) succeeds.

This PR fixes the behavior, correct files aren't skipped.

Sample output of vcpkg install libressl[tools]:x64-windows

without fix - vcpkg complains every file after an error occurs

Building package libressl[core,tools]:x64-windows... done
Installing package libressl[core,tools]:x64-windows...
failed: C:\SRC\vcpkg\toolsrc\msbuild.x64.debug\installed\x64-windows\debug\lib\crypto.lib: A required privilege is not held by the client.
failed: C:\SRC\vcpkg\toolsrc\msbuild.x64.debug\packages\libressl_x64-windows\debug\lib\ssl-47.lib: A required privilege is not held by the client.
failed: C:\SRC\vcpkg\toolsrc\msbuild.x64.debug\packages\libressl_x64-windows\debug\lib\ssl.lib: A required privilege is not held by the client.
failed: C:\SRC\vcpkg\toolsrc\msbuild.x64.debug\packages\libressl_x64-windows\debug\lib\tls-19.lib: A required privilege is not held by the client.
failed: C:\SRC\vcpkg\toolsrc\msbuild.x64.debug\packages\libressl_x64-windows\debug\lib\tls.lib: A required privilege is not held by the client.
failed: C:\SRC\vcpkg\toolsrc\msbuild.x64.debug\packages\libressl_x64-windows\etc: A required privilege is not held by the client.
failed: C:\SRC\vcpkg\toolsrc\msbuild.x64.debug\packages\libressl_x64-windows\etc\ssl: A required privilege is not held by the client.
failed: C:\SRC\vcpkg\toolsrc\msbuild.x64.debug\packages\libressl_x64-windows\etc\ssl\cert.pem: A required privilege is not held by the client.
failed: C:\SRC\vcpkg\toolsrc\msbuild.x64.debug\packages\libressl_x64-windows\etc\ssl\openssl.cnf: A required privilege is not held by the client.
...
   and about 100 wrong files
...
Installing package libressl[core,tools]:x64-windows... done

with fix - only proper errors

Building package libressl[core,tools]:x64-windows... done
Installing package libressl[core,tools]:x64-windows...
failed: C:\SRC\vcpkg\toolsrc\msbuild.x64.debug\installed\x64-windows\debug\lib\crypto.lib: A required privilege is not held by the client.
failed: C:\SRC\vcpkg\toolsrc\msbuild.x64.debug\installed\x64-windows\debug\lib\ssl.lib: A required privilege is not held by the client.
failed: C:\SRC\vcpkg\toolsrc\msbuild.x64.debug\installed\x64-windows\debug\lib\tls.lib: A required privilege is not held by the client.
failed: C:\SRC\vcpkg\toolsrc\msbuild.x64.debug\installed\x64-windows\lib\crypto.lib: A required privilege is not held by the client.
failed: C:\SRC\vcpkg\toolsrc\msbuild.x64.debug\installed\x64-windows\lib\ssl.lib: A required privilege is not held by the client.
failed: C:\SRC\vcpkg\toolsrc\msbuild.x64.debug\installed\x64-windows\lib\tls.lib: A required privilege is not held by the client.
Installing package libressl[core,tools]:x64-windows... done

Copy link
Contributor

@ras0219 ras0219 left a comment

Choose a reason for hiding this comment

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

I believe that this should actually be moved into Files::<anonymous>::status_implementation(). The contract for APIs accepting error_code& is that they will clear the error code during a success path, therefore this is a bug in fs.symlink_status() (which transitively calls the above function).

@Maximus5
Copy link
Contributor Author

Moved, works for me

@LilyWangL LilyWangL self-assigned this Jul 13, 2020
@LilyWangL LilyWangL added category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) info:reviewed Pull Request changes follow basic guidelines labels Jul 13, 2020
@ras0219-msft ras0219-msft merged commit 713950c into microsoft:master Jul 13, 2020
@ras0219-msft
Copy link
Contributor

Thanks for the fix!

strega-nil pushed a commit to strega-nil/vcpkg that referenced this pull request May 5, 2021
* [vcpkg] proper errorcheck during files installation

* [vcpkg] move ec.clear to status_implementation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants