Skip to content

Conversation

@rainersigwald
Copy link
Member

@rainersigwald rainersigwald commented Mar 5, 2019

Fixes #4210 by reverting to the old behavior of always using XmlTextReader. I didn't just revert #3584 because that would be a public API change, and it seems better to avoid that churn this close to release. With this PR, the read-only option is ignored.

@rainersigwald rainersigwald added this to the MSBuild 16.0 milestone Mar 5, 2019
Introduce an end-to-end test using `loadProjectReadOnly` and a multiline
`Exec` task.
@rainersigwald rainersigwald force-pushed the newlines-in-task-arguments branch from 5519e11 to 6e4048e Compare March 5, 2019 23:21
@rainersigwald rainersigwald changed the base branch from master to vs16.0 March 5, 2019 23:23
@rainersigwald rainersigwald marked this pull request as ready for review March 5, 2019 23:49
@rainersigwald rainersigwald requested a review from livarcocc March 5, 2019 23:49
@rainersigwald
Copy link
Member Author

fyi @jeffkl

@jeffkl
Copy link
Contributor

jeffkl commented Mar 5, 2019

☹️

Works around a behavior difference between XmlReader and XmlTextReader
that caused newlines in attributes to be ignored in command-line builds.

Fixes dotnet#4210.
@rainersigwald rainersigwald force-pushed the newlines-in-task-arguments branch from a4b500d to 873c127 Compare March 8, 2019 16:51
@rainersigwald rainersigwald merged commit 01bae62 into dotnet:vs16.0 Mar 8, 2019
@rainersigwald rainersigwald deleted the newlines-in-task-arguments branch March 8, 2019 17:21
ladipro added a commit that referenced this pull request Apr 27, 2021
Fixes #2576

### Context

The code already has support for parsing project files as read-only when we know that we're not going to be asked to write them back to disk. This flag is currently unused because the initial implementation in #3584 introduced a regression related to whitespace in attribute values (#4210).

### Changes Made

Trivially reverted part of #4213 that addressed the regression and added a hack to make `XmlReader` behave the same as `XmlTextReader`.

### Testing

Existing unit tests for correctness and ETW for performance. `XmlDocumentWithLocation.Load()` is ~26% faster with this change compared to baseline when building .NET Core projects. This translates to about 10 ms per one incremental CLI build of a Hello world application.
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