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

Backport removing dead pwd code #48544

Merged
merged 2 commits into from
Mar 10, 2021
Merged

Conversation

danmoseley
Copy link
Member

Backport #48241 as required to remove Credscan noise from all ship branches.

Test only change.

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@danmoseley
Copy link
Member Author

@ViktorHofer is this known?

Path = D:\workspace\_work\1\s\__download__\libraries_test_assets_Windows_NT_x64_Debug\libraries_test_assets_Windows_NT_x64_Debug.zip
Type = zip
ERRORS:
Unexpected end of archive
Physical Size = 155888025

I do'nt think it's me.

@ViktorHofer
Copy link
Member

I would have said that's #32805, but that only tracks Unix. @MattGal do we know about this happening on Windows as well?

@ViktorHofer
Copy link
Member

@danmoseley can you please link to the build where this happened?

@MattGal
Copy link
Member

MattGal commented Feb 22, 2021

I would have said that's #32805, but that only tracks Unix. @MattGal do we know about this happening on Windows as well?

While that issue mentions Unix, the actual code changes are written in typescript and don't seem to branch on OS at all. When it does roll out, it does seem that the checkDownloadedFiles property will need to be set true as it defaults false.

Links for context:
Issue: microsoft/azure-pipelines-tasks#13250
PR: microsoft/azure-pipelines-tasks#14065

Per their labels, the issue is still in the "Awaiting Deployment" phase, and in directly pinging since on your respective behalf it should roll out in early March.

@alexander-smolyakov if you'd like to comment about whether your fix should work on both linux and Windows, that'd be helpful here; there seems to be some confusion.

@Anipik Anipik added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Feb 23, 2021
@Anipik Anipik removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Mar 10, 2021
@Anipik Anipik merged commit 117f230 into dotnet:release/5.0 Mar 10, 2021
@alexander-smolyakov
Copy link

@alexander-smolyakov if you'd like to comment about whether your fix should work on both linux and Windows, that'd be helpful here; there seems to be some confusion.

The fix for the DownloadBuildArtifacts task designed to work on all supported OS. This means that this fix should work on both Linux and Windows systems.

@danmoseley danmoseley deleted the deadcode.50 branch March 15, 2021 15:21
@ghost ghost locked as resolved and limited conversation to collaborators Apr 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants