Skip to content

Overwrite ILLinkTasksAssembly#1822

Merged
marek-safar merged 1 commit into
mainfrom
ViktorHofer-illink
Feb 10, 2021
Merged

Overwrite ILLinkTasksAssembly#1822
marek-safar merged 1 commit into
mainfrom
ViktorHofer-illink

Conversation

@ViktorHofer

Copy link
Copy Markdown
Member

If the Microsoft.NET.ILLink.Tasks package is package referenced directly, overwriting ILLinkTasksAssembly guarantees that the SDK uses the right ILLink assembly. Github org searches didn't show any consumer of the private properties, hence removing them for simplification.

dotnet/arcade#6936 benefits from this change.

If the Microsoft.NET.ILLink.Tasks package is package referenced directly, overwriting `ILLinkTasksAssembly` guarantees that the SDK uses the right ILLink assembly. Github org searches didn't show any consumer of the private properties, hence removing them for simplification.

dotnet/arcade#6936 benefits from this change.
@ViktorHofer
ViktorHofer requested a review from sbomer as a code owner February 10, 2021 16:00
@ViktorHofer ViktorHofer self-assigned this Feb 10, 2021
@marek-safar
marek-safar merged commit 7f47c0c into main Feb 10, 2021
@marek-safar
marek-safar deleted the ViktorHofer-illink branch February 10, 2021 16:30
@sbomer

sbomer commented Feb 10, 2021

Copy link
Copy Markdown
Member

We shouldn't get rid of the '$(ILLinkTasksAssembly)' == '' condition - that's useful for testing against a local build of the linker.

@ViktorHofer

ViktorHofer commented Feb 10, 2021

Copy link
Copy Markdown
Member Author

For local builds, ILLinkTasksAssembly can still be passed in globally which always takes precedence and overwrites any values set here, i.e. dotnet build /p:ILLinkTasksAssembly=x\ILLink.Tasks.dll

@sbomer

sbomer commented Feb 10, 2021

Copy link
Copy Markdown
Member

I typically like to set it in the project file rather than the command-line.

@ViktorHofer

ViktorHofer commented Feb 10, 2021

Copy link
Copy Markdown
Member Author

That still works as props are imported before the project file which means that the value in the project file overwrites what is in the props file (as long as the property isn't passed in globally).

@sbomer

sbomer commented Feb 10, 2021

Copy link
Copy Markdown
Member

@ViktorHofer

Copy link
Copy Markdown
Member Author

oh forget that. yes, we should definitely fix that as props should be imported before the project is evaluated.

@ViktorHofer

ViktorHofer commented Feb 10, 2021

Copy link
Copy Markdown
Member Author

Either via a ILLink.props file in the SDK (which then is imported by Microsoft.Net.Sdk.props) or directly in the Microsoft.Net.Sdk.props file.

@ViktorHofer

Copy link
Copy Markdown
Member Author

@sbomer should I take care of that or do you want to?

@sbomer

sbomer commented Feb 10, 2021

Copy link
Copy Markdown
Member

I will take care of it, thanks!

sbomer added a commit to sbomer/sdk that referenced this pull request Feb 10, 2021
They should be imported early per convention.
See dotnet/linker#1822
sbomer added a commit to sbomer/sdk that referenced this pull request Feb 10, 2021
They should be imported early per convention.
See dotnet/linker#1822
ViktorHofer pushed a commit to dotnet/sdk that referenced this pull request Feb 10, 2021
* Fix ILLink props import

They should be imported early per convention.
See dotnet/linker#1822
agocke pushed a commit to dotnet/runtime that referenced this pull request Nov 16, 2022
If the Microsoft.NET.ILLink.Tasks package is package referenced directly, overwriting `ILLinkTasksAssembly` guarantees that the SDK uses the right ILLink assembly. Github org searches didn't show any consumer of the private properties, hence removing them for simplification.

dotnet/arcade#6936 benefits from this change.

Commit migrated from dotnet/linker@7f47c0c
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