Skip to content

[release/8.0-rc1] Fix NullableAttribute illink test failures #90680

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.

Sign up for GitHub

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

Jump to bottom
Merged
Parent: [ppc64le] Performance improvements while branching during function calls
merged 1 commit into from
Aug 16, 2023
Merged

[release/8.0-rc1] Fix NullableAttribute illink test failures #90680

merged 1 commit into from
Aug 16, 2023

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Aug 16, 2023 •
edited by sbomer
Loading

Backport of #90449 to release/8.0-rc1

/cc @carlossanlop @sbomer

Customer Impact

No known impact. This fixes test bugs that were exposed by an update to a new SDK. The fix is relevant for internal testing purposes but does not reflect a customer scenario.

Testing

This fixes test issues that were fixed in main, but not in rc1.

Risk

Very low risk. No change to product behavior.

@sbomer
Fix NullableAttribute illink test failures
a1c06e9 
When we started building with preview 7 in
5549f72, NullableAttribute in
these testcases started to use the attribute definition from the
framework, instead of generating it into the code. This broke the
`--used-attrs-only` optimization because `skip` assemblies (the
default for the framework in these testcases) are treated as if
all types in them are kept, for the purposes of the
`--used-attrs-only` optimization. (The optimization removes
attribute instances unless the attribute type is preserved for
some other reason).

It's not clear what the intended behavior of `--used-attrs-only`
is for `skip` assemblies, and the discussion in
dotnet/linker#952 indicates that it's
considered experimental, so this fixes the failures by using the
`link` action. This represents a more realistic scenario since
`skip` is mainly used in testing to avoid linking the framework
in every testcase.
@github-actions github-actions bot requested a review from marek-safar as a code owner August 16, 2023 18:17
@ghost ghost added linkable-framework Issues associated with delivering a linker friendly framework needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 16, 2023
@ghost
Copy link

ghost commented Aug 16, 2023

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr, @marek-safar
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #90449 to release/8.0-rc1

/cc @carlossanlop @sbomer

Customer Impact

Testing

Risk

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

linkable-framework, needs-area-label
Milestone: -

@carlossanlop
Copy link
Contributor

This is currently blocking the arcade dependency flow PR for RC1: #90665

@carlossanlop carlossanlop added area-Tools-ILLink .NET linker development as well as trimming analyzers and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 16, 2023
@ghost
Copy link

ghost commented Aug 16, 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 #90449 to release/8.0-rc1

/cc @carlossanlop @sbomer

Customer Impact

Testing

Risk

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

linkable-framework, area-Tools-ILLink
Milestone: -

@carlossanlop carlossanlop added this to the 8.0.0 milestone Aug 16, 2023
@steveisok steveisok self-requested a review August 16, 2023 18:20
@carlossanlop carlossanlop added Servicing-consider Issue for next servicing release review Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Aug 16, 2023
@carlossanlop
Copy link
Contributor

Test-only change.

@carlossanlop carlossanlop merged commit dbb6f45 into release/8.0-rc1 Aug 16, 2023
@carlossanlop carlossanlop deleted the backport/pr-90449-to-release/8.0-rc1 branch August 16, 2023 18:44
@build-analysis build-analysis bot mentioned this pull request Aug 16, 2023
Closed
@radical radical mentioned this pull request Aug 28, 2023
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Sep 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@steveisok steveisok steveisok approved these changes

@marek-safar marek-safar Awaiting requested review from marek-safar

+1 more reviewer

@carlossanlop carlossanlop carlossanlop approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@sbomer sbomer

@carlossanlop carlossanlop

Labels

area-Tools-ILLink .NET linker development as well as trimming analyzers linkable-framework Issues associated with delivering a linker friendly framework Servicing-approved Approved for servicing release

Projects

None yet

Milestone

8.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@carlossanlop @steveisok @sbomer