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
Dont trim line break characters from inputs #3025
Conversation
src/Agent.Worker/TaskRunner.cs
Outdated
@@ -190,7 +190,7 @@ public async Task RunAsync() | |||
string key = input?.Name?.Trim() ?? string.Empty; | |||
if (!string.IsNullOrEmpty(key)) | |||
{ | |||
inputs[key] = input.DefaultValue?.Trim() ?? string.Empty; | |||
inputs[key] = input.DefaultValue ?? string.Empty; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add tests?
Is this going to break anyone that isn't currently broken? I guess anyone who has spacing now on accident will start to break. Will it be clear to them why it's broken?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this going to break anyone that isn't currently broken? I guess anyone who has spacing now on accident will start to break. Will it be clear to them why it's broken?
To be a little more exact, anyone who has spacing and is relying on there not being spacing is going to break. It probably won't be immediately clear, I didn't have a great solution for that. In theory we could warn that they have trailing spaces for a while, it would probably flag a bunch of false positives though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought more about this - I think not trimming all whitespace scares me. I updated to still trim spaces, but this will leave linebreaks. It solves the ticket in question in what I think is a pretty safe way.
Because of how yaml is structured/the importance of whitespace, I think its not unreasonable to trim spaces.
This appears to have broken the MSBuild task when used as follows:
This task ran as expected yesterday on pipeline agent 2.171.1 but is now failing on a hosted agent running 2.172.0 with the following error:
Note the extra line break in the error message within the search pattern quotes. |
This reverts commit 5e79b33.
This should fix this ticket - https://developercommunity.visualstudio.com/content/problem/1000335/azure-pipelines-trims-leading-and-trailing-rn-from.html.
Basically, some tasks/scripts can rely on their inputs not getting trimmed, we should respect the inputs.
This could break some people, but I think its the right thing to do and that they'd have to be relying on a bug.