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

[release/8.0-staging] Don't use File.OpenWrite when trying to overwrite a file in illink/wasm build tasks #93854

Merged

Conversation

akoeplinger
Copy link
Member

@akoeplinger akoeplinger commented Oct 23, 2023

Backport of #93744

File.OpenWrite will open an existing file and not truncate existing data, this can lead to unexpected results if the new data is shorter than the existing data.

Customer Impact

illink has a feature to dump a dependency analysis xml during the build and when you run another build after making a change that'd reduce dependencies the xml would get corrupted since it wrote less data than the existing file had.

The fix is to use the FileMode.Create option in cases where the intention is to create a new file or overwrite an existing one.

I noticed another instance of this in the Wasm build tasks so fixed it there as well.

Testing

Verified an existing file gets overwritten as expected.

Risk

Low

@akoeplinger akoeplinger added Servicing-consider Issue for next servicing release review area-Tools-ILLink .NET linker development as well as trimming analyzers labels Oct 23, 2023
@akoeplinger akoeplinger added this to the 8.0.x milestone Oct 23, 2023
@ghost
Copy link

ghost commented Oct 23, 2023

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #93744

File.OpenWrite will open an existing file and not truncate existing data, this can lead the unexpected results if the new data is shorter than the existing data.

Customer Impact

illink has a feature to dump a dependency analysis xml during the build and when you run another build after making a change that'd reduce dependencies the xml would get corrupted since it wrote less data than the existing file had.

The fix is to use the FileMode.Create option in cases where the intention is to create a new file or overwrite an existing one.

I noticed another instance of this in the Wasm build tasks so fixed it there as well.

Testing

Verified an existing file gets overwritten as expected.

Risk

Low

Author: akoeplinger
Assignees: -
Labels:

Servicing-consider, area-Tools-ILLink

Milestone: 8.0.x

@carlossanlop
Copy link
Member

@akoeplinger we still have time to merge fixes into release/8.0, in case you want to retarget this PR to that branch. Otherwise, we'll ship this in servicing version 1.

@akoeplinger
Copy link
Member Author

Servicing version 1 is fine.

@leecow leecow modified the milestones: 8.0.x, 8.0.1 Oct 24, 2023
@akoeplinger
Copy link
Member Author

Approved by tactics via email.

@akoeplinger akoeplinger added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Oct 25, 2023
@steveisok steveisok self-requested a review October 25, 2023 14:29
@akoeplinger akoeplinger merged commit 01d208d into dotnet:release/8.0-staging Oct 25, 2023
186 of 195 checks passed
@akoeplinger akoeplinger deleted the backport-file-openwrite branch October 25, 2023 14:44
@ghost ghost locked as resolved and limited conversation to collaborators Nov 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants